loplugin:redundantfcast check for std::function cast

noticed by mike kaganski

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

compilerplugins/clang/redundantfcast.cxx

@@ -13,6 +13,7 @@
 #include "plugin.hxx"
 #include <iostream>
 #include <fstream>
+#include <unordered_set>
 
 namespace
 {
@@ -53,9 +54,10 @@ public:
         {
             return true;
         }
-        report(DiagnosticsEngine::Warning, "redundant functional cast from %0 to %1",
+        if (m_Seen.insert(cxxFunctionalCastExpr->getExprLoc()).second)
-               cxxFunctionalCastExpr->getExprLoc())
+            report(DiagnosticsEngine::Warning, "redundant functional cast from %0 to %1",
-            << t2 << t1 << cxxFunctionalCastExpr->getSourceRange();
+                   cxxFunctionalCastExpr->getExprLoc())
+                << t2 << t1 << cxxFunctionalCastExpr->getSourceRange();
         return true;
     }
 
@@ -103,11 +105,14 @@ public:
             if (t1.getCanonicalType().getTypePtr() != paramClassOrStructType)
                 continue;
 
-            report(DiagnosticsEngine::Warning, "redundant functional cast from %0 to %1",
+            if (m_Seen.insert(arg->getExprLoc()).second)
-                   arg->getExprLoc())
+            {
-                << t2 << t1 << arg->getSourceRange();
+                report(DiagnosticsEngine::Warning, "redundant functional cast from %0 to %1",
-            report(DiagnosticsEngine::Note, "in call to method here", param->getLocation())
+                       arg->getExprLoc())
-                << param->getSourceRange();
+                    << t2 << t1 << arg->getSourceRange();
+                report(DiagnosticsEngine::Note, "in call to method here", param->getLocation())
+                    << param->getSourceRange();
+            }
         }
         return true;
     }
@@ -146,15 +151,28 @@ public:
             if (t1.getCanonicalType().getTypePtr() != paramClassOrStructType)
                 continue;
 
-            report(DiagnosticsEngine::Warning, "redundant functional cast from %0 to %1",
+            if (m_Seen.insert(arg->getExprLoc()).second)
-                   arg->getExprLoc())
+            {
-                << t2 << t1 << arg->getSourceRange();
+                report(DiagnosticsEngine::Warning, "redundant functional cast from %0 to %1",
-            report(DiagnosticsEngine::Note, "in call to method here", param->getLocation())
+                       arg->getExprLoc())
-                << param->getSourceRange();
+                    << t2 << t1 << arg->getSourceRange();
+                report(DiagnosticsEngine::Note, "in call to method here", param->getLocation())
+                    << param->getSourceRange();
+            }
         }
         return true;
     }
 
+    // Find redundant cast to std::function, where clang reports
+    // two different types for the inner and outer
+    static bool isRedundantStdFunctionCast(CXXFunctionalCastExpr const* expr)
+    {
+        auto cxxConstruct = dyn_cast<CXXConstructExpr>(compat::IgnoreImplicit(expr->getSubExpr()));
+        if (!cxxConstruct)
+            return false;
+        return isa<LambdaExpr>(cxxConstruct->getArg(0));
+    }
+
     bool VisitCXXFunctionalCastExpr(CXXFunctionalCastExpr const* expr)
     {
         if (ignoreLocation(expr))
@@ -166,7 +184,8 @@ public:
             return true;
         auto const t1 = expr->getTypeAsWritten();
         auto const t2 = compat::getSubExprAsWritten(expr)->getType();
-        if (t1.getCanonicalType().getTypePtr() != t2.getCanonicalType().getTypePtr())
+        if (!(t1.getCanonicalType().getTypePtr() == t2.getCanonicalType().getTypePtr()
+              || isRedundantStdFunctionCast(expr)))
         {
             return true;
         }
@@ -191,9 +210,10 @@ public:
         if (tc.Typedef("sal_Int32").GlobalNamespace())
             return true;
 
-        report(DiagnosticsEngine::Warning, "redundant functional cast from %0 to %1",
+        if (m_Seen.insert(expr->getExprLoc()).second)
-               expr->getExprLoc())
+            report(DiagnosticsEngine::Warning, "redundant functional cast from %0 to %1",
-            << t2 << t1 << expr->getSourceRange();
+                   expr->getExprLoc())
+                << t2 << t1 << expr->getSourceRange();
         //getParentStmt(expr)->dump();
         return true;
     }
@@ -218,6 +238,8 @@ public:
         if (preRun())
             TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
     }
+
+    std::unordered_set<SourceLocation> m_Seen;
 };
 
 static loplugin::Plugin::Registration<RedundantFCast> redundantfcast("redundantfcast");

compilerplugins/clang/test/redundantfcast.cxx

@@ -9,11 +9,12 @@
 
 #include "sal/config.h"
 
-#include <memory>
-
 #include "rtl/ustring.hxx"
 #include "tools/color.hxx"
 
+#include <functional>
+#include <memory>
+
 void method1(OUString const&); // expected-note {{in call to method here [loplugin:redundantfcast]}}
 
 struct Foo
@@ -56,7 +57,6 @@ 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]}}
@@ -68,11 +68,11 @@ int main()
 
     Foo foo(1);
     func1(Foo(
-        foo)); // expected-error@-1 {{redundant functional cast from 'Foo' to 'Foo' [loplugin:redundantfcast]}} expected-error@-1 {{redundant functional cast from 'Foo' to 'Foo' [loplugin:redundantfcast]}}
+        foo)); // expected-error@-1 {{redundant functional cast from 'Foo' to 'Foo' [loplugin:redundantfcast]}}
 
     const tools::Polygon aPolygon;
     ImplWritePolyPolygonRecord(tools::PolyPolygon(tools::Polygon(
-        aPolygon))); // expected-error@-1 {{redundant functional cast from 'const tools::Polygon' to 'tools::Polygon' [loplugin:redundantfcast]}} expected-error@-1 {{redundant functional cast from 'const tools::Polygon' to 'tools::Polygon' [loplugin:redundantfcast]}}
+        aPolygon))); // expected-error@-1 {{redundant functional cast from 'const tools::Polygon' to 'tools::Polygon' [loplugin:redundantfcast]}}
 }
 
 class Class1
@@ -81,7 +81,19 @@ class Class1
     Foo func2()
     {
         return Foo(
-            foo); // expected-error@-1 {{redundant functional cast from 'Foo' to 'Foo' [loplugin:redundantfcast]}} expected-error@-1 {{redundant functional cast from 'Foo' to 'Foo' [loplugin:redundantfcast]}}
+            foo); // expected-error@-1 {{redundant functional cast from 'Foo' to 'Foo' [loplugin:redundantfcast]}}
     }
 };
+
+// casting of lambdas
+namespace test5
+{
+void f1(std::function<void()> x);
+void f2()
+{
+    // expected-error-re@+1 {{redundant functional cast {{.+}} [loplugin:redundantfcast]}}
+    f1(std::function([&]() {}));
+}
+};
+
 /* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */

vcl/qt5/Qt5Instance.cxx

@@ -413,7 +413,7 @@ Qt5Instance::createPicker(css::uno::Reference<css::uno::XComponentContext> const
     {
         SolarMutexGuard g;
         Qt5FilePicker* pPicker;
-        RunInMainThread(std::function([&, this]() { pPicker = createPicker(context, eMode); }));
+        RunInMainThread([&, this]() { pPicker = createPicker(context, eMode); });
         assert(pPicker);
         return pPicker;
     }

vcl/unx/kf5/KF5SalInstance.cxx

@@ -43,9 +43,9 @@ KF5SalInstance::KF5SalInstance(std::unique_ptr<QApplication>& pQApp)
 SalFrame* KF5SalInstance::CreateFrame(SalFrame* pParent, SalFrameStyleFlags nState)
 {
     SalFrame* pRet(nullptr);
-    RunInMainThread(std::function([&pRet, pParent, nState]() {
+    RunInMainThread([&pRet, pParent, nState]() {
         pRet = new KF5SalFrame(static_cast<KF5SalFrame*>(pParent), nState, true);
-    }));
+    });
     assert(pRet);
     return pRet;
 }
@@ -65,7 +65,7 @@ KF5SalInstance::createPicker(css::uno::Reference<css::uno::XComponentContext> co
     {
         SolarMutexGuard g;
         Qt5FilePicker* pPicker;
-        RunInMainThread(std::function([&, this]() { pPicker = createPicker(context, eMode); }));
+        RunInMainThread([&, this]() { pPicker = createPicker(context, eMode); });
         assert(pPicker);
         return pPicker;
     }