bridges/source/cpp_uno/shared/vtablefactory.cxx | 2
compilerplugins/clang/cstylecast.cxx | 87 +++++++++++++++++-------
compilerplugins/clang/test/cstylecast.cxx | 11 +++
pyuno/inc/pyuno.hxx | 2
sc/source/filter/xcl97/xcl97rec.cxx | 2
sdext/source/pdfimport/tree/style.hxx | 2
sfx2/source/sidebar/SidebarController.cxx | 2
sw/qa/extras/globalfilter/globalfilter.cxx | 2
sw/source/core/doc/doctxm.cxx | 12 +--
sw/source/core/doc/notxtfrm.cxx | 4 -
sw/source/core/layout/paintfrm.cxx | 2
sw/source/core/text/porfld.cxx | 2
vcl/source/app/svmain.cxx | 2
vcl/unx/gtk3/gtk3gtkinst.cxx | 8 +-
14 files changed, 95 insertions(+), 45 deletions(-)
New commits:
commit 1ebeacb20ad0165e399629fcfd7795ad0da3edf8
Author: Stephan Bergmann <sberg...@redhat.com>
AuthorDate: Thu Mar 26 08:34:37 2020 +0100
Commit: Stephan Bergmann <sberg...@redhat.com>
CommitDate: Thu Mar 26 09:13:55 2020 +0100
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 <sberg...@redhat.com>
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");
}
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
