loplugin:redundantfcast look for unnecessary temporaries

when calling methods that take a const&

Change-Id: Idf45dfd9fea0de6fae0b1f89550f2f7fc302aa15
Reviewed-on: https://gerrit.libreoffice.org/50970
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>

2 files changed, 66 insertions, 4 deletions

diff --git a/compilerplugins/clang/redundantfcast.cxx b/compilerplugins/clang/redundantfcast.cxx
index 87d656c6d237..811c6d48647c 100644
--- a/compilerplugins/clang/redundantfcast.cxx
+++ b/compilerplugins/clang/redundantfcast.cxx
@@ -21,12 +21,63 @@ public:
     {
     }
 
+    /* Check for the creation of unnecessary temporaries when calling a method that takes a param by const & */
+    bool VisitCallExpr(CallExpr const* callExpr)
+    {
+        if (ignoreLocation(callExpr))
+            return true;
+        const FunctionDecl* functionDecl;
+        if (isa<CXXMemberCallExpr>(callExpr))
+            functionDecl = dyn_cast<CXXMemberCallExpr>(callExpr)->getMethodDecl();
+        else
+            functionDecl = callExpr->getDirectCallee();
+        if (!functionDecl)
+            return true;
+
+        unsigned len = std::min(callExpr->getNumArgs(), functionDecl->getNumParams());
+        for (unsigned i = 0; i < len; ++i)
+        {
+            // check if param is const&
+            auto param = functionDecl->getParamDecl(i);
+            auto lvalueType = param->getType()->getAs<LValueReferenceType>();
+            if (!lvalueType)
+                continue;
+            if (!lvalueType->getPointeeType().isConstQualified())
+                continue;
+            auto paramClassOrStructType = lvalueType->getPointeeType()->getAs<RecordType>();
+            if (!paramClassOrStructType)
+                continue;
+            // check for temporary and functional cast in argument expression
+            auto arg = callExpr->getArg(i)->IgnoreImpCasts();
+            auto materializeTemporaryExpr = dyn_cast<MaterializeTemporaryExpr>(arg);
+            if (!materializeTemporaryExpr)
+                continue;
+            auto functionalCast = dyn_cast<CXXFunctionalCastExpr>(
+                materializeTemporaryExpr->GetTemporaryExpr()->IgnoreImpCasts());
+            if (!functionalCast)
+                continue;
+            auto const t1 = functionalCast->getTypeAsWritten();
+            auto const t2 = compat::getSubExprAsWritten(functionalCast)->getType();
+            if (t1.getCanonicalType().getTypePtr() != t2.getCanonicalType().getTypePtr())
+                continue;
+            // Check that the underlying expression is of the same class/struct type as the param i.e. that we are not instantiating
+            // something useful
+            if (t1.getCanonicalType().getTypePtr() != paramClassOrStructType)
+                continue;
+
+            report(DiagnosticsEngine::Warning, "redundant functional cast from %0 to %1",
+                   arg->getExprLoc())
+                << t2 << t1 << arg->getSourceRange();
+            report(DiagnosticsEngine::Note, "in call to method here", param->getLocation())
+                << param->getSourceRange();
+        }
+        return true;
+    }
+
     bool VisitCXXFunctionalCastExpr(CXXFunctionalCastExpr const* expr)
     {
         if (ignoreLocation(expr))
-        {
             return true;
-        }
         auto const t1 = expr->getTypeAsWritten();
         auto const t2 = compat::getSubExprAsWritten(expr)->getType();
         if (t1.getCanonicalType().getTypePtr() != t2.getCanonicalType().getTypePtr())
diff --git a/compilerplugins/clang/test/redundantfcast.cxx b/compilerplugins/clang/test/redundantfcast.cxx
index f642098ed00f..609b787347e1 100644
--- a/compilerplugins/clang/test/redundantfcast.cxx
+++ b/compilerplugins/clang/test/redundantfcast.cxx
@@ -14,7 +14,14 @@
 #include "rtl/ustring.hxx"
 #include "tools/color.hxx"
 
-void method1(OUString const&);
+void method1(OUString const&); // expected-note {{in call to method here [loplugin:redundantfcast]}}
+
+struct Foo
+{
+    Foo(int) {}
+};
+
+void func1(Foo const& f); // expected-note {{in call to method here [loplugin:redundantfcast]}}
 
 int main()
 {
@@ -34,7 +41,7 @@ int main()
     OUString s1;
     method1(OUString(
         s1)); // expected-error@-1 {{redundant functional cast from 'rtl::OUString' to 'rtl::OUString' [loplugin:redundantfcast]}}
-
+    // expected-error@-2 {{redundant functional cast from 'rtl::OUString' to 'rtl::OUString' [loplugin:redundantfcast]}}
     OUString s2;
     s2 = OUString(
         s1); // expected-error@-1 {{redundant functional cast from 'rtl::OUString' to 'rtl::OUString' [loplugin:redundantfcast]}}
@@ -43,6 +50,10 @@ int main()
     Color col2 = Color(
         col1); // expected-error@-1 {{redundant functional cast from 'Color' to 'Color' [loplugin:redundantfcast]}}
     (void)col2;
+
+    Foo foo(1);
+    func1(Foo(
+        foo)); // expected-error@-1 {{redundant functional cast from 'Foo' to 'Foo' [loplugin:redundantfcast]}}
 }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */