1 files changed, 55 insertions, 138 deletions

diff --git a/compilerplugins/clang/stringstatic.cxx b/compilerplugins/clang/stringstatic.cxx
index a6b5b7d2e8fe..a7439608d556 100644
--- a/compilerplugins/clang/stringstatic.cxx
+++ b/compilerplugins/clang/stringstatic.cxx
@@ -10,17 +10,9 @@
 #ifndef LO_CLANG_SHARED_PLUGINS
 
 #include "check.hxx"
-#include "compat.hxx"
 #include "plugin.hxx"
 
-#include <unordered_set>
-
-/** Look for static O*String and O*String[], they can be more efficiently declared as:
-
-        static constexpr OUStringLiteral our_aLBEntryMap[] = {u" ", u", "};
-        static constexpr OUStringLiteral sName(u"name");
-
-    which is more efficient at startup time.
+/** Look for static O*String and O*String[] which can be constepxr.
  */
 namespace {
 
@@ -33,16 +25,7 @@ public:
         FilteringPlugin(rData) {}
 
     void run() override;
-    bool preRun() override;
-    void postRun() override;
     bool VisitVarDecl(VarDecl const*);
-    bool VisitReturnStmt(ReturnStmt const*);
-    bool VisitDeclRefExpr(DeclRefExpr const*);
-    bool VisitMemberExpr(MemberExpr const*);
-
-private:
-    std::unordered_set<VarDecl const *> potentialVars;
-    std::unordered_set<VarDecl const *> excludeVars;
 };
 
 void StringStatic::run()
@@ -52,143 +35,77 @@ void StringStatic::run()
             postRun();
 }
 
-bool StringStatic::preRun()
-{
-    StringRef fn(handler.getMainFileName());
-    // passing around pointers to global OUString
-    if (loplugin::hasPathnamePrefix(fn, SRCDIR "/filter/source/svg/"))
-         return false;
-    return true;
-}
-
-void StringStatic::postRun()
-{
-    for (auto const & pVarDecl : excludeVars) {
-        potentialVars.erase(pVarDecl);
-    }
-    for (auto const & varDecl : potentialVars) {
-        report(DiagnosticsEngine::Warning,
-                "rather declare this using OUStringLiteral/OStringLiteral/char[]",
-                varDecl->getLocation())
-            << varDecl->getSourceRange();
-    }
-}
-
 bool StringStatic::VisitVarDecl(VarDecl const* varDecl)
 {
     if (ignoreLocation(varDecl))
         return true;
     QualType qt = varDecl->getType();
-    if (!varDecl->hasGlobalStorage())
-        return true;
-    if (varDecl->hasGlobalStorage() && !varDecl->isStaticLocal()) {
-        //TODO: For a non-public static member variable from an included file, we could still
-        // examine it further if all its uses must be seen in that included file:
-        if (!compiler.getSourceManager().isInMainFile(varDecl->getLocation())) {
-            return true;
-        }
-    }
     if (!varDecl->isThisDeclarationADefinition()
-        || !qt.isConstQualified())
+        || !qt.isConstQualified() || varDecl->isConstexpr())
         return true;
-    if (qt->isArrayType())
-        qt = qt->getAsArrayTypeUnsafe()->getElementType();
 
-    auto tc = loplugin::TypeCheck(qt);
-    if (!tc.Class("OUString").Namespace("rtl").GlobalNamespace()
-        && !tc.Class("OString").Namespace("rtl").GlobalNamespace())
-        return true;
-    if (varDecl->hasInit())
+    if (varDecl->hasGlobalStorage())
     {
-        Expr const * expr = varDecl->getInit();
-        while (true) {
-            if (ExprWithCleanups const * exprWithCleanups = dyn_cast<ExprWithCleanups>(expr)) {
-                expr = exprWithCleanups->getSubExpr();
-            }
-            else if (CastExpr const * castExpr = dyn_cast<CastExpr>(expr)) {
-                expr = castExpr->getSubExpr();
-            }
-            else if (MaterializeTemporaryExpr const * materializeExpr = dyn_cast<MaterializeTemporaryExpr>(expr)) {
-                expr = compat::getSubExpr(materializeExpr);
-            }
-            else if (CXXBindTemporaryExpr const * bindExpr = dyn_cast<CXXBindTemporaryExpr>(expr)) {
-                expr = bindExpr->getSubExpr();
-            }
-            else if (CXXConstructExpr const * constructExpr = dyn_cast<CXXConstructExpr>(expr)) {
-                if (constructExpr->getNumArgs() == 0) {
+        if (qt->isArrayType())
+            qt = qt->getAsArrayTypeUnsafe()->getElementType();
+
+        auto tc = loplugin::TypeCheck(qt);
+        if (!tc.Class("OUString").Namespace("rtl").GlobalNamespace()
+            && !tc.Class("OString").Namespace("rtl").GlobalNamespace())
+            return true;
+        if (varDecl->hasInit())
+        {
+            Expr const * expr = varDecl->getInit();
+            while (true) {
+                if (ExprWithCleanups const * exprWithCleanups = dyn_cast<ExprWithCleanups>(expr)) {
+                    expr = exprWithCleanups->getSubExpr();
+                }
+                else if (CastExpr const * castExpr = dyn_cast<CastExpr>(expr)) {
+                    expr = castExpr->getSubExpr();
+                }
+                else if (MaterializeTemporaryExpr const * materializeExpr = dyn_cast<MaterializeTemporaryExpr>(expr)) {
+                    expr = materializeExpr->getSubExpr();
+                }
+                else if (CXXBindTemporaryExpr const * bindExpr = dyn_cast<CXXBindTemporaryExpr>(expr)) {
+                    expr = bindExpr->getSubExpr();
+                }
+                else if (CXXConstructExpr const * constructExpr = dyn_cast<CXXConstructExpr>(expr)) {
+                    if (constructExpr->getNumArgs() == 0) {
+                        return true;
+                    }
+                    expr = constructExpr->getArg(0);
+                } else if (isa<CallExpr>(expr)) {
                     return true;
+                } else {
+                    break;
                 }
-                expr = constructExpr->getArg(0);
-            } else if (isa<CallExpr>(expr)) {
-                return true;
-            } else {
-                break;
             }
         }
     }
-    potentialVars.insert(varDecl);
-
-    return true;
-}
-
-bool StringStatic::VisitReturnStmt(ReturnStmt const * returnStmt)
-{
-    if (ignoreLocation(returnStmt)) {
-        return true;
-    }
-    if (!returnStmt->getRetValue()) {
-        return true;
-    }
-    DeclRefExpr const * declRef = dyn_cast<DeclRefExpr>(returnStmt->getRetValue());
-    if (!declRef) {
-        return true;
-    }
-    VarDecl const * varDecl = dyn_cast<VarDecl>(declRef->getDecl());
-    if (varDecl) {
-        excludeVars.insert(varDecl);
-    }
-    return true;
-}
-
-bool StringStatic::VisitDeclRefExpr(DeclRefExpr const * declRef)
-{
-    if (ignoreLocation(declRef))
-        return true;
-    VarDecl const * varDecl = dyn_cast<VarDecl>(declRef->getDecl());
-    if (!varDecl)
-        return true;
-    if (potentialVars.count(varDecl) == 0)
-        return true;
-    // ignore globals that are used in CPPUNIT_ASSERT expressions, otherwise we can end up
-    // trying to compare an OUStringLiteral and an OUString, and CPPUNIT can't handle that
-    auto loc = compat::getBeginLoc(declRef);
-    if (compiler.getSourceManager().isMacroArgExpansion(loc))
+    else
     {
-        StringRef name { Lexer::getImmediateMacroName(loc, compiler.getSourceManager(), compiler.getLangOpts()) };
-        if (name.startswith("CPPUNIT_ASSERT"))
-            excludeVars.insert(varDecl);
+        if (isa<ParmVarDecl>(varDecl))
+            return true;
+        loplugin::TypeCheck const tc(varDecl->getType());
+        if (!(tc.Class("OString").Namespace("rtl").GlobalNamespace()
+              || tc.Class("OUString").Namespace("rtl").GlobalNamespace()))
+        {
+            return true;
+        }
+        if (varDecl->hasInit())
+        {
+            auto cxxConstruct = dyn_cast<CXXConstructExpr>(varDecl->getInit()->IgnoreImplicit());
+            if (!cxxConstruct || cxxConstruct->getNumArgs() == 0)
+                return true;
+            if (!isa<clang::StringLiteral>(cxxConstruct->getArg(0)))
+                return true;
+        }
     }
-    return true;
-}
+    report(DiagnosticsEngine::Warning,
+           "rather declare this as constexpr",
+           varDecl->getLocation())
+        << varDecl->getSourceRange();
 
-bool StringStatic::VisitMemberExpr(MemberExpr const * expr)
-{
-    if (ignoreLocation(expr))
-        return true;
-    auto const declRef = dyn_cast<DeclRefExpr>(expr->getBase());
-    if (declRef == nullptr) {
-        return true;
-    }
-    VarDecl const * varDecl = dyn_cast<VarDecl>(declRef->getDecl());
-    if (!varDecl)
-        return true;
-    if (potentialVars.count(varDecl) == 0)
-        return true;
-    auto const id = expr->getMemberDecl()->getIdentifier();
-    if (id == nullptr || id->getName() != "pData") {
-        return true;
-    }
-    excludeVars.insert(varDecl);
     return true;
 }