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@redhat.com>

1 files changed, 14 insertions, 30 deletions

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;
 }