Extend loplugin:cstylecast to certain function-style casts

...that are equivalent to const_cast or reinterpret_cast, and should arguably
better be written that way for clarity.  Drawing inspiration from
<https://reviews.llvm.org/D76572> "Replace `T(x)` with `reinterpret_cast<T>(x)`
everywhere it means reinterpret_cast. No functional change".

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

2 files changed, 74 insertions, 24 deletions

diff --git a/compilerplugins/clang/cstylecast.cxx b/compilerplugins/clang/cstylecast.cxx
index fe3b2a19c561..7fa2ce0ae1b0 100644
--- a/compilerplugins/clang/cstylecast.cxx
+++ b/compilerplugins/clang/cstylecast.cxx
@@ -17,7 +17,9 @@
 #include "plugin.hxx"
 
 //
-// We don't like using C-style casts in C++ code
+// We don't like using C-style casts in C++ code.  Similarly, warn about function-style casts (which
+// are semantically equivalent to C-style casts) that are not semantically equivalent to static_cast
+// and should rather be written as const_cast or reinterpret_cast.
 //
 
 namespace {
@@ -187,6 +189,8 @@ public:
 
     bool VisitCStyleCastExpr(const CStyleCastExpr * expr);
 
+    bool VisitCXXFunctionalCastExpr(CXXFunctionalCastExpr const * expr);
+
 private:
     bool isConstCast(QualType from, QualType to);
 
@@ -199,6 +203,8 @@ private:
 
     bool rewriteArithmeticCast(CStyleCastExpr const * expr, char const ** replacement);
 
+    void reportCast(ExplicitCastExpr const * expr, char const * performsHint);
+
     unsigned int externCContexts_ = 0;
     std::set<SourceLocation> rewritten_;
         // needed when rewriting in macros, in general to avoid "double code replacement, possible
@@ -258,32 +264,35 @@ bool CStyleCast::VisitCStyleCastExpr(const CStyleCastExpr * expr) {
             perf = "const_cast";
         }
     }
-    std::string incompFrom;
-    std::string incompTo;
-    if( expr->getCastKind() == CK_BitCast ) {
-        if (resolvePointers(expr->getSubExprAsWritten()->getType())
-            ->isIncompleteType())
+    reportCast(expr, perf);
+    return true;
+}
+
+bool CStyleCast::VisitCXXFunctionalCastExpr(CXXFunctionalCastExpr const * expr) {
+    if (ignoreLocation(expr)) {
+        return true;
+    }
+    char const * perf = nullptr;
+    switch (expr->getCastKind()) {
+    case CK_ConstructorConversion:
+    case CK_Dependent: //TODO: really filter out all of these?
+    case CK_IntegralCast:
+    case CK_IntegralToBoolean:
+    case CK_ToVoid:
+        return true;
+    case CK_NoOp:
+        if (isConstCast(
+                expr->getSubExprAsWritten()->getType(),
+                expr->getTypeAsWritten()))
         {
-            incompFrom = "incomplete ";
-        }
-        if (resolvePointers(expr->getType())->isIncompleteType()) {
-            incompTo = "incomplete ";
+            perf = "const_cast";
+            break;
         }
+        return true; //TODO: really filter out all of these?
+    default:
+        break;
     }
-    if (perf == nullptr) {
-        perf = recommendedFix(expr->getCastKind());
-    }
-    std::string performs;
-    if (perf != nullptr) {
-        performs = std::string(" (performs: ") + perf + ")";
-    }
-    report(
-        DiagnosticsEngine::Warning, "C-style cast from %0%1 to %2%3%4 (%5)",
-        expr->getSourceRange().getBegin())
-      << incompFrom << expr->getSubExprAsWritten()->getType()
-      << incompTo << expr->getTypeAsWritten() << performs
-      << expr->getCastKindName()
-      << expr->getSourceRange();
+    reportCast(expr, perf);
     return true;
 }
 
@@ -661,6 +670,36 @@ bool CStyleCast::rewriteArithmeticCast(CStyleCastExpr const * expr, char const *
     return true;
 }
 
+void CStyleCast::reportCast(ExplicitCastExpr const * expr, char const * performsHint) {
+    std::string incompFrom;
+    std::string incompTo;
+    if( expr->getCastKind() == CK_BitCast ) {
+        if (resolvePointers(expr->getSubExprAsWritten()->getType())
+            ->isIncompleteType())
+        {
+            incompFrom = "incomplete ";
+        }
+        if (resolvePointers(expr->getType())->isIncompleteType()) {
+            incompTo = "incomplete ";
+        }
+    }
+    if (performsHint == nullptr) {
+        performsHint = recommendedFix(expr->getCastKind());
+    }
+    std::string performs;
+    if (performsHint != nullptr) {
+        performs = std::string(" (performs: ") + performsHint + ")";
+    }
+    report(
+        DiagnosticsEngine::Warning, "%select{C|Function}0-style cast from %1%2 to %3%4%5 (%6)",
+        expr->getSourceRange().getBegin())
+      << isa<CXXFunctionalCastExpr>(expr)
+      << incompFrom << expr->getSubExprAsWritten()->getType()
+      << incompTo << expr->getTypeAsWritten() << performs
+      << expr->getCastKindName()
+      << expr->getSourceRange();
+}
+
 loplugin::Plugin::Registration< CStyleCast > X("cstylecast", true);
 
 }
diff --git a/compilerplugins/clang/test/cstylecast.cxx b/compilerplugins/clang/test/cstylecast.cxx
index c272005650ff..916896d54db9 100644
--- a/compilerplugins/clang/test/cstylecast.cxx
+++ b/compilerplugins/clang/test/cstylecast.cxx
@@ -7,6 +7,10 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
 
+#include <sal/config.h>
+
+#include <sal/types.h>
+
 namespace N
 {
 enum E
@@ -17,6 +21,13 @@ enum E
 using T = unsigned int;
 }
 
+void FunctionalCast(void* p)
+{
+    // expected-error@+1 {{Function-style cast from 'void *' to 'sal_IntPtr' (aka 'long') (performs: reinterpret_cast) (PointerToIntegral) [loplugin:cstylecast]}}
+    auto n = sal_IntPtr(p);
+    (void(n)); // no warning expected (outer parens to disambiguate expr vs. decl)
+}
+
 static const int C
     = (int)0; // expected-error {{C-style cast from 'int' to 'int' (performs: functional cast) (NoOp) [loplugin:cstylecast]}}