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>

14 files changed, 95 insertions, 45 deletions

diff --git a/bridges/source/cpp_uno/shared/vtablefactory.cxx b/bridges/source/cpp_uno/shared/vtablefactory.cxx
index 70187cbd2fe7..018b808d89e6 100644
--- a/bridges/source/cpp_uno/shared/vtablefactory.cxx
+++ b/bridges/source/cpp_uno/shared/vtablefactory.cxx
@@ -336,7 +336,7 @@ sal_Int32 VtableFactory::createVtables(
                 code = addLocalFunctions(
                     &slots, code,
 #ifdef USE_DOUBLE_MMAP
-                    sal_uIntPtr(block.exec) - sal_uIntPtr(block.start),
+                    reinterpret_cast<sal_uIntPtr>(block.exec) - reinterpret_cast<sal_uIntPtr>(block.start),
 #endif
                     type2,
                     baseOffset.getFunctionOffset(type2->aBase.pTypeName),
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]}}
 
diff --git a/pyuno/inc/pyuno.hxx b/pyuno/inc/pyuno.hxx
index e50dde9bb119..c190cfc348c2 100644
--- a/pyuno/inc/pyuno.hxx
+++ b/pyuno/inc/pyuno.hxx
@@ -148,7 +148,7 @@ public:
 
     struct Hash
     {
-        sal_IntPtr operator () ( const PyRef &r) const { return sal_IntPtr( r.get() ); }
+        sal_IntPtr operator () ( const PyRef &r) const { return reinterpret_cast<sal_IntPtr>( r.get() ); }
     };
 };
 
diff --git a/sc/source/filter/xcl97/xcl97rec.cxx b/sc/source/filter/xcl97/xcl97rec.cxx
index 3d81d18edaa8..d61040b1b030 100644
--- a/sc/source/filter/xcl97/xcl97rec.cxx
+++ b/sc/source/filter/xcl97/xcl97rec.cxx
@@ -921,7 +921,7 @@ void XclObjOle::WriteSubRecs( XclExpStream& rStrm )
     OUString        aStorageName( "MBD" );
     char        aBuf[ sizeof(sal_uInt32) * 2 + 1 ];
     // FIXME Eeek! Is this just a way to get a unique id?
-    sal_uInt32          nPictureId = sal_uInt32(sal_uIntPtr(this) >> 2);
+    sal_uInt32          nPictureId = sal_uInt32(reinterpret_cast<sal_uIntPtr>(this) >> 2);
     sprintf( aBuf, "%08X", static_cast< unsigned int >( nPictureId ) );
     aStorageName += OUString::createFromAscii(aBuf);
     tools::SvRef<SotStorage>    xOleStg = pRootStorage->OpenSotStorage( aStorageName );
diff --git a/sdext/source/pdfimport/tree/style.hxx b/sdext/source/pdfimport/tree/style.hxx
index 8aafe555f728..ee7cafeabc93 100644
--- a/sdext/source/pdfimport/tree/style.hxx
+++ b/sdext/source/pdfimport/tree/style.hxx
@@ -71,7 +71,7 @@ namespace pdfi
                         return sum ^ size_t(rEntry.first.hashCode()) ^ size_t(rEntry.second.hashCode());
                     });
                 nRet ^= size_t(Contents.hashCode());
-                nRet ^= size_t(ContainedElement);
+                nRet ^= reinterpret_cast<size_t>(ContainedElement);
                 for( size_t n = 0; n < SubStyles.size(); ++n )
                      nRet ^= size_t(SubStyles[n]);
                 return nRet;
diff --git a/sfx2/source/sidebar/SidebarController.cxx b/sfx2/source/sidebar/SidebarController.cxx
index ec077be3b44b..2935e2943aa8 100644
--- a/sfx2/source/sidebar/SidebarController.cxx
+++ b/sfx2/source/sidebar/SidebarController.cxx
@@ -941,7 +941,7 @@ Reference<ui::XUIElement> SidebarController::CreateUIElement (
         aCreationArguments.put("ParentWindow", makeAny(rxWindow));
         SfxDockingWindow* pSfxDockingWindow = dynamic_cast<SfxDockingWindow*>(mpParentWindow.get());
         if (pSfxDockingWindow != nullptr)
-            aCreationArguments.put("SfxBindings", makeAny(sal_uInt64(&pSfxDockingWindow->GetBindings())));
+            aCreationArguments.put("SfxBindings", makeAny(reinterpret_cast<sal_uInt64>(&pSfxDockingWindow->GetBindings())));
         aCreationArguments.put("Theme", Theme::GetPropertySet());
         aCreationArguments.put("Sidebar", makeAny(Reference<ui::XSidebar>(static_cast<ui::XSidebar*>(this))));
         if (bWantsCanvas)
diff --git a/sw/qa/extras/globalfilter/globalfilter.cxx b/sw/qa/extras/globalfilter/globalfilter.cxx
index 4ec7563ff324..e69a7ef4634d 100644
--- a/sw/qa/extras/globalfilter/globalfilter.cxx
+++ b/sw/qa/extras/globalfilter/globalfilter.cxx
@@ -899,7 +899,7 @@ static auto verifyNestedFieldmark(OUString const& rTestName,
         + u"baz" + OUStringChar(CH_TXT_ATR_FIELDEND)), outerString);
 
     // must return innermost mark
-    CPPUNIT_ASSERT_EQUAL(sal_uIntPtr(pInner), sal_uIntPtr(rIDMA.getFieldmarkFor(innerPos)));
+    CPPUNIT_ASSERT_EQUAL(reinterpret_cast<sal_uIntPtr>(pInner), reinterpret_cast<sal_uIntPtr>(rIDMA.getFieldmarkFor(innerPos)));
 }
 
 void Test::testNestedFieldmark()
diff --git a/sw/source/core/doc/doctxm.cxx b/sw/source/core/doc/doctxm.cxx
index eb8b7dcd95d7..524ceeb810c8 100644
--- a/sw/source/core/doc/doctxm.cxx
+++ b/sw/source/core/doc/doctxm.cxx
@@ -282,10 +282,10 @@ const SwTOXMark& SwDoc::GotoTOXMark( const SwTOXMark& rCurTOXMark,
             case TOX_PRV:
                 if ( (aAbsNew < aAbsIdx && aAbsNew > aPrevPos) ||
                      (aAbsIdx == aAbsNew &&
-                      (sal_uLong(&rCurTOXMark) > sal_uLong(pTOXMark) &&
-                       (!pNew || aPrevPos < aAbsIdx || sal_uLong(pNew) < sal_uLong(pTOXMark) ) )) ||
+                      (reinterpret_cast<sal_uLong>(&rCurTOXMark) > reinterpret_cast<sal_uLong>(pTOXMark) &&
+                       (!pNew || aPrevPos < aAbsIdx || reinterpret_cast<sal_uLong>(pNew) < reinterpret_cast<sal_uLong>(pTOXMark) ) )) ||
                      (aPrevPos == aAbsNew && aAbsIdx != aAbsNew &&
-                      sal_uLong(pTOXMark) > sal_uLong(pNew)) )
+                      reinterpret_cast<sal_uLong>(pTOXMark) > reinterpret_cast<sal_uLong>(pNew)) )
                 {
                     pNew = pTOXMark;
                     aPrevPos = aAbsNew;
@@ -304,10 +304,10 @@ const SwTOXMark& SwDoc::GotoTOXMark( const SwTOXMark& rCurTOXMark,
             case TOX_NXT:
                 if ( (aAbsNew > aAbsIdx && aAbsNew < aNextPos) ||
                      (aAbsIdx == aAbsNew &&
-                      (sal_uLong(&rCurTOXMark) < sal_uLong(pTOXMark) &&
-                       (!pNew || aNextPos > aAbsIdx || sal_uLong(pNew) > sal_uLong(pTOXMark)) )) ||
+                      (reinterpret_cast<sal_uLong>(&rCurTOXMark) < reinterpret_cast<sal_uLong>(pTOXMark) &&
+                       (!pNew || aNextPos > aAbsIdx || reinterpret_cast<sal_uLong>(pNew) > reinterpret_cast<sal_uLong>(pTOXMark)) )) ||
                      (aNextPos == aAbsNew && aAbsIdx != aAbsNew &&
-                      sal_uLong(pTOXMark) < sal_uLong(pNew)) )
+                      reinterpret_cast<sal_uLong>(pTOXMark) < reinterpret_cast<sal_uLong>(pNew)) )
                 {
                     pNew = pTOXMark;
                     aNextPos = aAbsNew;
diff --git a/sw/source/core/doc/notxtfrm.cxx b/sw/source/core/doc/notxtfrm.cxx
index 19b11315c1b8..8c77fd28c93f 100644
--- a/sw/source/core/doc/notxtfrm.cxx
+++ b/sw/source/core/doc/notxtfrm.cxx
@@ -1285,7 +1285,7 @@ void SwNoTextFrame::PaintPicture( vcl::RenderContext* pOut, const SwRect &rGrfAr
                             "pOut should not be a virtual device" );
 
                     pGrfNd->StartGraphicAnimation(pOut, aAlignedGrfArea.Pos(),
-                                        aAlignedGrfArea.SSize(), sal_IntPtr(this),
+                                        aAlignedGrfArea.SSize(), reinterpret_cast<sal_IntPtr>(this),
                                         pVout );
                 }
                 else
@@ -1536,7 +1536,7 @@ void SwNoTextFrame::StopAnimation( OutputDevice* pOut ) const
 
     if( pGrfNd && pGrfNd->IsAnimated() )
     {
-        const_cast< SwGrfNode* >(pGrfNd)->StopGraphicAnimation( pOut, sal_IntPtr(this) );
+        const_cast< SwGrfNode* >(pGrfNd)->StopGraphicAnimation( pOut, reinterpret_cast<sal_IntPtr>(this) );
     }
 }
 
diff --git a/sw/source/core/layout/paintfrm.cxx b/sw/source/core/layout/paintfrm.cxx
index f0e20a164c27..93c28c6503f3 100644
--- a/sw/source/core/layout/paintfrm.cxx
+++ b/sw/source/core/layout/paintfrm.cxx
@@ -3841,7 +3841,7 @@ bool SwFlyFrame::IsPaint( SdrObject *pObj, const SwViewShell *pSh )
             {
                 if ( !pAnch->isFrameAreaPositionValid() )
                     pAnch = nullptr;
-                else if ( sal_IntPtr(pSh->GetOut()) == sal_IntPtr(pSh->getIDocumentDeviceAccess().getPrinter( false )))
+                else if ( reinterpret_cast<sal_IntPtr>(pSh->GetOut()) == reinterpret_cast<sal_IntPtr>(pSh->getIDocumentDeviceAccess().getPrinter( false )))
                 {
                     //HACK: we have to omit some of the objects for printing,
                     //otherwise they would be printed twice.
diff --git a/sw/source/core/text/porfld.cxx b/sw/source/core/text/porfld.cxx
index 41700584368c..5c60bed70425 100644
--- a/sw/source/core/text/porfld.cxx
+++ b/sw/source/core/text/porfld.cxx
@@ -932,7 +932,7 @@ void SwGrfNumPortion::Paint( const SwTextPaintInfo &rInf ) const
         bDraw = !rInf.GetOpt().IsGraphic();
         if( !nId )
         {
-            SetId( sal_IntPtr( rInf.GetTextFrame() ) );
+            SetId( reinterpret_cast<sal_IntPtr>( rInf.GetTextFrame() ) );
             rInf.GetTextFrame()->SetAnimation();
         }
         if( aTmp.IsOver( rInf.GetPaintRect() ) && !bDraw )
diff --git a/vcl/source/app/svmain.cxx b/vcl/source/app/svmain.cxx
index ec8dbca95d0c..9d861a5cadc6 100644
--- a/vcl/source/app/svmain.cxx
+++ b/vcl/source/app/svmain.cxx
@@ -453,7 +453,7 @@ void DeInitVCL()
             aBuf.append( "\" type = \"" );
             aBuf.append( typeid(*pWin).name() );
             aBuf.append( "\", ptr = 0x" );
-            aBuf.append( sal_Int64( pWin ), 16 );
+            aBuf.append( reinterpret_cast<sal_Int64>( pWin ), 16 );
             aBuf.append( "\n" );
         }
     }
diff --git a/vcl/unx/gtk3/gtk3gtkinst.cxx b/vcl/unx/gtk3/gtk3gtkinst.cxx
index 46fa3b9cf03a..6436c23d71ab 100644
--- a/vcl/unx/gtk3/gtk3gtkinst.cxx
+++ b/vcl/unx/gtk3/gtk3gtkinst.cxx
@@ -8262,9 +8262,9 @@ private:
         if (bContinue && !sText.isEmpty())
         {
             OString sFinalText(OUStringToOString(sText, RTL_TEXTENCODING_UTF8));
-            g_signal_handlers_block_by_func(pEntry, gpointer(signalInsertText), this);
+            g_signal_handlers_block_by_func(pEntry, reinterpret_cast<gpointer>(signalInsertText), this);
             gtk_editable_insert_text(GTK_EDITABLE(pEntry), sFinalText.getStr(), sFinalText.getLength(), position);
-            g_signal_handlers_unblock_by_func(pEntry, gpointer(signalInsertText), this);
+            g_signal_handlers_unblock_by_func(pEntry, reinterpret_cast<gpointer>(signalInsertText), this);
         }
         g_signal_stop_emission_by_name(pEntry, "insert-text");
     }
@@ -12286,9 +12286,9 @@ private:
             if (bContinue && !sText.isEmpty())
             {
                 OString sFinalText(OUStringToOString(sText, RTL_TEXTENCODING_UTF8));
-                g_signal_handlers_block_by_func(pEntry, gpointer(signalEntryInsertText), this);
+                g_signal_handlers_block_by_func(pEntry, reinterpret_cast<gpointer>(signalEntryInsertText), this);
                 gtk_editable_insert_text(GTK_EDITABLE(pEntry), sFinalText.getStr(), sFinalText.getLength(), position);
-                g_signal_handlers_unblock_by_func(pEntry, gpointer(signalEntryInsertText), this);
+                g_signal_handlers_unblock_by_func(pEntry, reinterpret_cast<gpointer>(signalEntryInsertText), this);
             }
             g_signal_stop_emission_by_name(pEntry, "insert-text");
         }