[Libreoffice-commits] core.git: compilerplugins/clang

[Stephan Bergmann (via logerrit)]

 compilerplugins/clang/stringadd.cxx      |   44 +++++++++----------------------
 compilerplugins/clang/test/stringadd.cxx |    6 ++--
 2 files changed, 17 insertions(+), 33 deletions(-)

New commits:
commit 2d64e046e8d11451834816fade63a5ec0890f99b
Author:     Stephan Bergmann <sbergman at redhat.com>
AuthorDate: Tue Oct 15 12:11:34 2019 +0200
Commit:     Stephan Bergmann <sbergman at redhat.com>
CommitDate: Tue Oct 15 16:13:54 2019 +0200

    Improve loplugin:stringadd
    
    ...after 9b5dad13b56bdde7c40970351af3da3a2c3c9350 "loplugin:stringadd look for
    unnecessary temporaries".  There was no reason to check for implicit
    MaterializeTemporaryExpr instead of explicitly written CXXFunctionalCastExpr,
    and checking for the latter makes it easier to report the casted-from type,
    which gives useful information how to change code that exhibits the warning.
    See the comments at <https://gerrit.libreoffice.org/#/c/80724/>
    "loplugin:stringadd look for unnecessary temporaries" for details.
    
    (And while at it, remove some commented-out debug code that is probably no
    longer relevant now.)
    
    Change-Id: I7d4cab85432885d617dd7114c75163c1eb376fc2
    Reviewed-on: https://gerrit.libreoffice.org/80823
    Tested-by: Jenkins
    Reviewed-by: Stephan Bergmann <sbergman at redhat.com>

diff --git a/compilerplugins/clang/stringadd.cxx b/compilerplugins/clang/stringadd.cxx
index 69251411ae70..764ef25adecc 100644
--- a/compilerplugins/clang/stringadd.cxx
+++ b/compilerplugins/clang/stringadd.cxx
@@ -193,46 +193,30 @@ bool StringAdd::VisitCXXOperatorCallExpr(CXXOperatorCallExpr const* operatorCall
         && !tc.Class("OString").Namespace("rtl").GlobalNamespace())
         return true;
 
-    auto check = [/* operatorCall, */ this, &tc](const MaterializeTemporaryExpr* matTempExpr) {
-        auto tc3 = loplugin::TypeCheck(matTempExpr->getType());
+    auto check = [this, &tc](const Expr* expr) {
+        auto const e = dyn_cast<CXXFunctionalCastExpr>(expr->IgnoreParenImpCasts());
+        if (e == nullptr)
+            return;
+        auto tc3 = loplugin::TypeCheck(e->getType());
         if (!tc3.Class("OUString").Namespace("rtl").GlobalNamespace()
             && !tc3.Class("OString").Namespace("rtl").GlobalNamespace())
             return;
-        if (auto bindTemp
-            = dyn_cast<CXXBindTemporaryExpr>(matTempExpr->GetTemporaryExpr()->IgnoreCasts()))
-        {
-            // ignore temporaries returned from function calls
-            if (isa<CallExpr>(bindTemp->getSubExpr()))
-                return;
-            // we don't have OStringLiteral1, so char needs to generate a temporary
-            if (tc.Class("OString").Namespace("rtl").GlobalNamespace()
-                || tc.Struct("OStringConcat").Namespace("rtl").GlobalNamespace())
+        // we don't have OStringLiteral1, so char needs to generate a temporary
+        if (tc.Class("OString").Namespace("rtl").GlobalNamespace()
+            || tc.Struct("OStringConcat").Namespace("rtl").GlobalNamespace())
+            if (auto bindTemp = dyn_cast<CXXBindTemporaryExpr>(e->getSubExpr()))
                 if (auto cxxConstruct = dyn_cast<CXXConstructExpr>(bindTemp->getSubExpr()))
                     if (loplugin::TypeCheck(
                             cxxConstruct->getConstructor()->getParamDecl(0)->getType())
                             .Char())
                         return;
-            // calls where we pass in an explicit character encoding
-            if (auto cxxTemp = dyn_cast<CXXTemporaryObjectExpr>(bindTemp->getSubExpr()))
-                if (cxxTemp->getNumArgs() > 1)
-                    return;
-        }
-        // conditional operators ( a ? b : c ) will result in temporaries
-        if (isa<ConditionalOperator>(
-                matTempExpr->GetTemporaryExpr()->IgnoreCasts()->IgnoreParens()))
-            return;
-        report(DiagnosticsEngine::Warning, "avoid constructing temporary copies during +",
-               compat::getBeginLoc(matTempExpr))
-            << matTempExpr->getSourceRange();
-        //        operatorCall->dump();
-        //        matTempExpr->getType()->dump();
-        //        operatorCall->getType()->getUnqualifiedDesugaredType()->dump();
+        report(DiagnosticsEngine::Warning, "avoid constructing temporary object from %0 during +",
+               compat::getBeginLoc(e))
+            << e->getSubExprAsWritten()->getType() << e->getSourceRange();
     };
 
-    if (auto matTempExpr = dyn_cast<MaterializeTemporaryExpr>(operatorCall->getArg(0)))
-        check(matTempExpr);
-    if (auto matTempExpr = dyn_cast<MaterializeTemporaryExpr>(operatorCall->getArg(1)))
-        check(matTempExpr);
+    check(operatorCall->getArg(0));
+    check(operatorCall->getArg(1));
     return true;
 }
 
diff --git a/compilerplugins/clang/test/stringadd.cxx b/compilerplugins/clang/test/stringadd.cxx
index 0e0986258254..fc06e5e33b5c 100644
--- a/compilerplugins/clang/test/stringadd.cxx
+++ b/compilerplugins/clang/test/stringadd.cxx
@@ -163,9 +163,9 @@ void f1(OUString s, OUString t, int i, const char* pChar)
 {
     // no warning expected
     t = t + "xxx";
-    // expected-error at +1 {{avoid constructing temporary copies during + [loplugin:stringadd]}}
+    // expected-error at +1 {{avoid constructing temporary object from 'const char [4]' during + [loplugin:stringadd]}}
     s = s + OUString("xxx");
-    // expected-error at +1 {{avoid constructing temporary copies during + [loplugin:stringadd]}}
+    // expected-error at +1 {{avoid constructing temporary object from 'const rtl::OUString' during + [loplugin:stringadd]}}
     s = s + OUString(getByRef());
 
     // no warning expected
@@ -183,7 +183,7 @@ void f1(OUString s, OUString t, int i, const char* pChar)
 void f2(char ch)
 {
     OString s;
-    // expected-error at +1 {{avoid constructing temporary copies during + [loplugin:stringadd]}}
+    // expected-error at +1 {{avoid constructing temporary object from 'const char [4]' during + [loplugin:stringadd]}}
     s = s + OString("xxx");
     // no warning expected, no OStringLiteral1
     s = s + OString(ch);