Improve loplugin:doubleconvert

(The use of isa_and_nonnull<> instead of isa<> is necessary for cases like

  return (i_styleSettings.*i_getDefaultColor)();

in lcl_getEffectiveColor, svtools/source/table/gridtablerenderer.cxx.)

Change-Id: Iffc59b1146dd4ce13bbd3c8a6f46bd3c78a39344
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/109663
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>

4 files changed, 27 insertions, 14 deletions

diff --git a/UnoControls/source/inc/progressbar.hxx b/UnoControls/source/inc/progressbar.hxx
index f327b453b19b..ef5eabe34781 100644
--- a/UnoControls/source/inc/progressbar.hxx
+++ b/UnoControls/source/inc/progressbar.hxx
@@ -30,8 +30,8 @@ namespace unocontrols {
 #define PROGRESSBAR_FREESPACE               4
 #define PROGRESSBAR_DEFAULT_HORIZONTAL      true
 #define PROGRESSBAR_DEFAULT_BLOCKDIMENSION  Size(1,1)
-#define PROGRESSBAR_DEFAULT_BACKGROUNDCOLOR sal_Int32(Color( 0xC0, 0xC0, 0xC0 )) // lightgray
-#define PROGRESSBAR_DEFAULT_FOREGROUNDCOLOR sal_Int32(Color( 0x00, 0x00, 0x80 )) // blue
+#define PROGRESSBAR_DEFAULT_BACKGROUNDCOLOR Color( 0xC0, 0xC0, 0xC0 ) // lightgray
+#define PROGRESSBAR_DEFAULT_FOREGROUNDCOLOR Color( 0x00, 0x00, 0x80 ) // blue
 #define PROGRESSBAR_DEFAULT_MINRANGE        INT_MIN
 #define PROGRESSBAR_DEFAULT_MAXRANGE        INT_MAX
 #define PROGRESSBAR_DEFAULT_BLOCKVALUE      1
diff --git a/compilerplugins/clang/compat.hxx b/compilerplugins/clang/compat.hxx
index bf3abf60c941..ae7557b71956 100644
--- a/compilerplugins/clang/compat.hxx
+++ b/compilerplugins/clang/compat.hxx
@@ -20,12 +20,26 @@
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/Compiler.h"
 
 #include "config_clang.h"
 
 // Compatibility wrapper to abstract over (trivial) changes in the Clang API:
 namespace compat {
 
+// Copies code from LLVM's include/llvm/Support/Casting.h:
+template<typename... X, typename Y> LLVM_NODISCARD inline bool isa_and_nonnull(Y const & Val) {
+#if CLANG_VERSION >= 90000
+    return llvm::isa_and_nonnull<X...>(Val);
+#else
+    if (!Val) {
+        return false;
+    }
+    return llvm::isa<X...>(Val);
+#endif
+}
+
 inline clang::SourceLocation getBeginLoc(clang::Decl const * decl) {
 #if CLANG_VERSION >= 80000
     return decl->getBeginLoc();
diff --git a/compilerplugins/clang/doubleconvert.cxx b/compilerplugins/clang/doubleconvert.cxx
index 2dbff56ca30b..6a08ae376a95 100644
--- a/compilerplugins/clang/doubleconvert.cxx
+++ b/compilerplugins/clang/doubleconvert.cxx
@@ -28,7 +28,7 @@ public:
     }
     void run() override { TraverseDecl(compiler.getASTContext().getTranslationUnitDecl()); }
 
-    bool VisitMaterializeTemporaryExpr(MaterializeTemporaryExpr const*);
+    bool VisitCXXConstructExpr(CXXConstructExpr const*);
 };
 
 /**
@@ -48,13 +48,9 @@ public:
               `-ImplicitCastExpr 0x8e5b6e0 'const class Color' lvalue <NoOp>
                 `-DeclRefExpr 0x8e5b6b0 'class Color' lvalue Var 0x8e5b518 'col1' 'class Color'
 */
-bool DoubleConvert::VisitMaterializeTemporaryExpr(MaterializeTemporaryExpr const* materializetemp)
+bool DoubleConvert::VisitCXXConstructExpr(CXXConstructExpr const* cxxConstruct)
 {
-    if (ignoreLocation(materializetemp))
-        return true;
-    auto cxxConstruct
-        = dyn_cast<CXXConstructExpr>(compat::getSubExpr(materializetemp)->IgnoreParenCasts());
-    if (!cxxConstruct)
+    if (ignoreLocation(cxxConstruct))
         return true;
     if (cxxConstruct->getNumArgs() == 0)
         return true;
@@ -62,21 +58,21 @@ bool DoubleConvert::VisitMaterializeTemporaryExpr(MaterializeTemporaryExpr const
         = dyn_cast<CXXMemberCallExpr>(cxxConstruct->getArg(0)->IgnoreParenCasts());
     if (!cxxMemberCallExpr)
         return true;
-    if (!isa<CXXConversionDecl>(cxxMemberCallExpr->getMethodDecl()))
+    if (!compat::isa_and_nonnull<CXXConversionDecl>(cxxMemberCallExpr->getMethodDecl()))
         return true;
-    if (materializetemp->getType().getCanonicalType().getTypePtr()
+    if (cxxConstruct->getType().getCanonicalType().getTypePtr()
         != cxxMemberCallExpr->getImplicitObjectArgument()
                ->getType()
                .getCanonicalType()
                .getTypePtr())
         return true;
-    if (!loplugin::TypeCheck(materializetemp->getType().getCanonicalType())
+    if (!loplugin::TypeCheck(cxxConstruct->getType().getCanonicalType())
              .Class("Color")
              .GlobalNamespace())
         return true;
 
-    report(DiagnosticsEngine::Warning, "redundant double conversion", materializetemp->getExprLoc())
-        << materializetemp->getSourceRange();
+    report(DiagnosticsEngine::Warning, "redundant double conversion", cxxConstruct->getExprLoc())
+        << cxxConstruct->getSourceRange();
     return true;
 }
 
diff --git a/compilerplugins/clang/test/doubleconvert.cxx b/compilerplugins/clang/test/doubleconvert.cxx
index d0fd8b787bdb..17bd49aa4f6f 100644
--- a/compilerplugins/clang/test/doubleconvert.cxx
+++ b/compilerplugins/clang/test/doubleconvert.cxx
@@ -17,6 +17,9 @@ int main()
     col2 = sal_Int32(
         col1); // expected-error@-1 {{redundant double conversion [loplugin:doubleconvert]}}
     (void)col2;
+    // expected-error@+1 {{redundant double conversion [loplugin:doubleconvert]}}
+    Color col3 = sal_Int32(Color());
+    (void)col3;
 }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */