From 81fd3bb4d76a0e57a4e8464cb1c5813115ea28f3 Mon Sep 17 00:00:00 2001 From: Noel Grandin Date: Sat, 7 Sep 2019 14:28:59 +0200 Subject: [PATCH] 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 --- compilerplugins/clang/redundantfcast.cxx | 56 +++++++++++++------ compilerplugins/clang/test/redundantfcast.cxx | 24 ++++++-- vcl/qt5/Qt5Instance.cxx | 2 +- vcl/unx/kf5/KF5SalInstance.cxx | 6 +- 4 files changed, 61 insertions(+), 27 deletions(-) diff --git a/compilerplugins/clang/redundantfcast.cxx b/compilerplugins/clang/redundantfcast.cxx index a53c42d73add..8879a386d621 100644 --- a/compilerplugins/clang/redundantfcast.cxx +++ b/compilerplugins/clang/redundantfcast.cxx @@ -13,6 +13,7 @@ #include "plugin.hxx" #include #include +#include namespace { @@ -53,9 +54,10 @@ public: { return true; } - report(DiagnosticsEngine::Warning, "redundant functional cast from %0 to %1", - cxxFunctionalCastExpr->getExprLoc()) - << t2 << t1 << cxxFunctionalCastExpr->getSourceRange(); + if (m_Seen.insert(cxxFunctionalCastExpr->getExprLoc()).second) + report(DiagnosticsEngine::Warning, "redundant functional cast from %0 to %1", + 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", - arg->getExprLoc()) - << t2 << t1 << arg->getSourceRange(); - report(DiagnosticsEngine::Note, "in call to method here", param->getLocation()) - << param->getSourceRange(); + if (m_Seen.insert(arg->getExprLoc()).second) + { + 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; } @@ -146,15 +151,28 @@ public: 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(); + if (m_Seen.insert(arg->getExprLoc()).second) + { + 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; } + // 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(compat::IgnoreImplicit(expr->getSubExpr())); + if (!cxxConstruct) + return false; + return isa(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", - expr->getExprLoc()) - << t2 << t1 << expr->getSourceRange(); + if (m_Seen.insert(expr->getExprLoc()).second) + report(DiagnosticsEngine::Warning, "redundant functional cast from %0 to %1", + 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 m_Seen; }; static loplugin::Plugin::Registration redundantfcast("redundantfcast"); diff --git a/compilerplugins/clang/test/redundantfcast.cxx b/compilerplugins/clang/test/redundantfcast.cxx index 13e09b5e05b6..9b377c97d395 100644 --- a/compilerplugins/clang/test/redundantfcast.cxx +++ b/compilerplugins/clang/test/redundantfcast.cxx @@ -9,11 +9,12 @@ #include "sal/config.h" -#include - #include "rtl/ustring.hxx" #include "tools/color.hxx" +#include +#include + 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 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: */ diff --git a/vcl/qt5/Qt5Instance.cxx b/vcl/qt5/Qt5Instance.cxx index 86c94ba792de..64481b64108e 100644 --- a/vcl/qt5/Qt5Instance.cxx +++ b/vcl/qt5/Qt5Instance.cxx @@ -413,7 +413,7 @@ Qt5Instance::createPicker(css::uno::Reference const { SolarMutexGuard g; Qt5FilePicker* pPicker; - RunInMainThread(std::function([&, this]() { pPicker = createPicker(context, eMode); })); + RunInMainThread([&, this]() { pPicker = createPicker(context, eMode); }); assert(pPicker); return pPicker; } diff --git a/vcl/unx/kf5/KF5SalInstance.cxx b/vcl/unx/kf5/KF5SalInstance.cxx index da4906a70a49..5b95ff8df572 100644 --- a/vcl/unx/kf5/KF5SalInstance.cxx +++ b/vcl/unx/kf5/KF5SalInstance.cxx @@ -43,9 +43,9 @@ KF5SalInstance::KF5SalInstance(std::unique_ptr& 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(pParent), nState, true); - })); + }); assert(pRet); return pRet; } @@ -65,7 +65,7 @@ KF5SalInstance::createPicker(css::uno::Reference co { SolarMutexGuard g; Qt5FilePicker* pPicker; - RunInMainThread(std::function([&, this]() { pPicker = createPicker(context, eMode); })); + RunInMainThread([&, this]() { pPicker = createPicker(context, eMode); }); assert(pPicker); return pPicker; }