compilerplugins/clang/staticdynamic.cxx | 64 +++++++++++++++---- compilerplugins/clang/test/staticdynamic.cxx | 8 ++ sd/source/ui/accessibility/AccessibleOutlineView.cxx | 3 sd/source/ui/unoidl/unoobj.cxx | 5 - sd/source/ui/view/drviews6.cxx | 9 -- sfx2/source/control/unoctitm.cxx | 6 - sfx2/source/doc/objxtor.cxx | 5 + sfx2/source/toolbox/tbxitem.cxx | 7 +- svx/source/svdraw/svdedtv.cxx | 14 +--- svx/source/svdraw/svdedtv2.cxx | 5 + sw/source/core/access/accmap.cxx | 6 - sw/source/core/doc/doclay.cxx | 16 ++-- sw/source/core/draw/dview.cxx | 3 sw/source/core/frmedt/fews.cxx | 5 - 14 files changed, 102 insertions(+), 54 deletions(-)
New commits: commit bd37588605f7773d41b5388b18952e5c90f12214 Author: Noel <noel.gran...@collabora.co.uk> AuthorDate: Fri Mar 5 08:37:41 2021 +0200 Commit: Noel Grandin <noel.gran...@collabora.co.uk> CommitDate: Sat Mar 6 13:08:26 2021 +0100 loplugin:staticdynamic look for static after dynamic Change-Id: Ic3066d9a9441e369370cc6aa0fbffb9a321bc928 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/111985 Tested-by: Jenkins Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk> diff --git a/compilerplugins/clang/staticdynamic.cxx b/compilerplugins/clang/staticdynamic.cxx index b2383413b287..b104b0333fcd 100644 --- a/compilerplugins/clang/staticdynamic.cxx +++ b/compilerplugins/clang/staticdynamic.cxx @@ -45,22 +45,26 @@ public: private: // the key is the pair of VarDecl and the type being cast to. - typedef std::map<std::pair<VarDecl const*, clang::Type const*>, SourceLocation> MapType; - MapType staticCastVars; + struct BlockState + { + std::map<std::pair<VarDecl const*, clang::Type const*>, SourceLocation> staticCastVars; + std::map<std::pair<VarDecl const*, clang::Type const*>, SourceLocation> dynamicCastVars; + }; // only maintain state inside a single basic block, we're not trying to analyse // cross-block interactions. - std::vector<MapType> blockStack; + std::vector<BlockState> blockStack; + BlockState blockState; }; bool StaticDynamic::PreTraverseCompoundStmt(CompoundStmt*) { - blockStack.push_back(std::move(staticCastVars)); + blockStack.push_back(std::move(blockState)); return true; } bool StaticDynamic::PostTraverseCompoundStmt(CompoundStmt*, bool) { - staticCastVars = std::move(blockStack.back()); + blockState = std::move(blockStack.back()); blockStack.pop_back(); return true; } @@ -86,8 +90,28 @@ bool StaticDynamic::VisitCXXStaticCastExpr(CXXStaticCastExpr const* staticCastEx auto varDecl = dyn_cast_or_null<VarDecl>(subExprDecl->getDecl()); if (!varDecl) return true; - staticCastVars.insert({ { varDecl, staticCastExpr->getTypeAsWritten().getTypePtr() }, - compat::getBeginLoc(staticCastExpr) }); + auto it = blockState.dynamicCastVars.find( + { varDecl, staticCastExpr->getTypeAsWritten().getTypePtr() }); + if (it != blockState.dynamicCastVars.end()) + { + StringRef fn = getFilenameOfLocation( + compiler.getSourceManager().getSpellingLoc(compat::getBeginLoc(staticCastExpr))); + // loop + if (loplugin::isSamePathname(fn, SRCDIR "/basctl/source/basicide/basobj3.cxx")) + return true; + if (loplugin::isSamePathname(fn, SRCDIR "/sw/source/core/doc/swserv.cxx")) + return true; + if (loplugin::isSamePathname(fn, SRCDIR "/sw/source/core/text/txtfly.cxx")) + return true; + + report(DiagnosticsEngine::Warning, "static_cast after dynamic_cast", + compat::getBeginLoc(staticCastExpr)) + << staticCastExpr->getSourceRange(); + report(DiagnosticsEngine::Note, "dynamic_cast here", it->second); + return true; + } + blockState.staticCastVars.insert({ { varDecl, staticCastExpr->getTypeAsWritten().getTypePtr() }, + compat::getBeginLoc(staticCastExpr) }); return true; } @@ -102,13 +126,27 @@ bool StaticDynamic::VisitCXXDynamicCastExpr(CXXDynamicCastExpr const* dynamicCas auto varDecl = dyn_cast_or_null<VarDecl>(subExprDecl->getDecl()); if (!varDecl) return true; - auto it = staticCastVars.find({ varDecl, dynamicCastExpr->getTypeAsWritten().getTypePtr() }); - if (it == staticCastVars.end()) + auto it = blockState.staticCastVars.find( + { varDecl, dynamicCastExpr->getTypeAsWritten().getTypePtr() }); + if (it != blockState.staticCastVars.end()) + { + report(DiagnosticsEngine::Warning, "dynamic_cast after static_cast", + compat::getBeginLoc(dynamicCastExpr)) + << dynamicCastExpr->getSourceRange(); + report(DiagnosticsEngine::Note, "static_cast here", it->second); return true; - report(DiagnosticsEngine::Warning, "dynamic_cast after static_cast", - compat::getBeginLoc(dynamicCastExpr)) - << dynamicCastExpr->getSourceRange(); - report(DiagnosticsEngine::Note, "static_cast here", it->second); + } + auto loc = compat::getBeginLoc(dynamicCastExpr); + if (compiler.getSourceManager().isMacroArgExpansion(loc) + && (Lexer::getImmediateMacroNameForDiagnostics(loc, compiler.getSourceManager(), + compiler.getLangOpts()) + == "assert")) + { + return true; + } + blockState.dynamicCastVars.insert( + { { varDecl, dynamicCastExpr->getTypeAsWritten().getTypePtr() }, + compat::getBeginLoc(dynamicCastExpr) }); return true; } diff --git a/compilerplugins/clang/test/staticdynamic.cxx b/compilerplugins/clang/test/staticdynamic.cxx index af4ea94c754a..d700ea06c435 100644 --- a/compilerplugins/clang/test/staticdynamic.cxx +++ b/compilerplugins/clang/test/staticdynamic.cxx @@ -25,4 +25,12 @@ void f1(ClassA* p1) dynamic_cast<ClassB*>(p1)->foo(); }; +void f2(ClassA* p1) +{ + // expected-note@+1 {{dynamic_cast here [loplugin:staticdynamic]}} + dynamic_cast<ClassB*>(p1)->foo(); + // expected-error@+1 {{static_cast after dynamic_cast [loplugin:staticdynamic]}} + static_cast<ClassB*>(p1)->foo(); +}; + /* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */ diff --git a/sd/source/ui/accessibility/AccessibleOutlineView.cxx b/sd/source/ui/accessibility/AccessibleOutlineView.cxx index 60edcdadf92e..963a9ba3c5ac 100644 --- a/sd/source/ui/accessibility/AccessibleOutlineView.cxx +++ b/sd/source/ui/accessibility/AccessibleOutlineView.cxx @@ -63,8 +63,7 @@ AccessibleOutlineView::AccessibleOutlineView ( return; OutlinerView* pOutlineView = pShellView->GetViewByWindow( pSdWindow ); - SdrOutliner& rOutliner = - static_cast< ::sd::OutlineView*>(pView)->GetOutliner(); + SdrOutliner& rOutliner = pShellView->GetOutliner(); if( pOutlineView ) { diff --git a/sd/source/ui/unoidl/unoobj.cxx b/sd/source/ui/unoidl/unoobj.cxx index 4b27bd429af0..fdedfb6dcc35 100644 --- a/sd/source/ui/unoidl/unoobj.cxx +++ b/sd/source/ui/unoidl/unoobj.cxx @@ -898,8 +898,9 @@ void SdXShape::SetEmptyPresObj(bool bEmpty) // really delete SdrOutlinerObj at pObj pObj->NbcSetOutlinerParaObject(nullptr); - if( bVertical && dynamic_cast<SdrTextObj*>( pObj ) ) - static_cast<SdrTextObj*>(pObj)->SetVerticalWriting( true ); + if( bVertical ) + if (auto pTextObj = dynamic_cast<SdrTextObj*>( pObj ) ) + pTextObj->SetVerticalWriting( true ); SdrGrafObj* pGraphicObj = dynamic_cast<SdrGrafObj*>( pObj ); if( pGraphicObj ) diff --git a/sd/source/ui/view/drviews6.cxx b/sd/source/ui/view/drviews6.cxx index 6d96270be42a..7c34a560ccad 100644 --- a/sd/source/ui/view/drviews6.cxx +++ b/sd/source/ui/view/drviews6.cxx @@ -326,12 +326,9 @@ void DrawViewShell::GetBmpMaskState( SfxItemSet& rSet ) pObj = rMarkList.GetMark(0)->GetMarkedSdrObj(); // valid graphic object? - if( dynamic_cast< const SdrGrafObj *>( pObj ) && - !static_cast<const SdrGrafObj*>(pObj)->IsEPS() && - !mpDrawView->IsTextEdit() ) - { - bEnable = true; - } + if( auto pGrafObj = dynamic_cast< const SdrGrafObj *>( pObj ) ) + if (!pGrafObj->IsEPS() && !mpDrawView->IsTextEdit() ) + bEnable = true; // put value rSet.Put( SfxBoolItem( SID_BMPMASK_EXEC, bEnable ) ); diff --git a/sfx2/source/control/unoctitm.cxx b/sfx2/source/control/unoctitm.cxx index ddccd885ab15..3b5a8e16de75 100644 --- a/sfx2/source/control/unoctitm.cxx +++ b/sfx2/source/control/unoctitm.cxx @@ -894,7 +894,9 @@ void SfxDispatchController_Impl::StateChanged( sal_uInt16 nSID, SfxItemState eSt bool bNotify = true; if ( pState && !IsInvalidItem( pState ) ) { - if ( dynamic_cast< const SfxVisibilityItem *>( pState ) == nullptr ) + if ( auto pVisibilityItem = dynamic_cast< const SfxVisibilityItem *>( pState ) ) + bVisible = pVisibilityItem->GetValue(); + else { if (pLastState && !IsInvalidItem(pLastState)) { @@ -904,8 +906,6 @@ void SfxDispatchController_Impl::StateChanged( sal_uInt16 nSID, SfxItemState eSt pLastState = !IsInvalidItem(pState) ? pState->Clone() : pState; bVisible = true; } - else - bVisible = static_cast<const SfxVisibilityItem *>(pState)->GetValue(); } else { diff --git a/sfx2/source/doc/objxtor.cxx b/sfx2/source/doc/objxtor.cxx index f079afdea704..7dc904433d1c 100644 --- a/sfx2/source/doc/objxtor.cxx +++ b/sfx2/source/doc/objxtor.cxx @@ -580,8 +580,11 @@ bool SfxObjectShell::PrepareClose pPoolItem = pFrame->GetBindings().ExecuteSynchron( SID_SAVEDOC, ppArgs ); } - if ( !pPoolItem || pPoolItem->IsVoidItem() || ( dynamic_cast< const SfxBoolItem *>( pPoolItem ) != nullptr && !static_cast<const SfxBoolItem*>( pPoolItem )->GetValue() ) ) + if ( !pPoolItem || pPoolItem->IsVoidItem() ) return false; + if ( auto pBoolItem = dynamic_cast< const SfxBoolItem *>( pPoolItem ) ) + if ( !pBoolItem->GetValue() ) + return false; } else if ( RET_CANCEL == nRet ) // Cancelled diff --git a/sfx2/source/toolbox/tbxitem.cxx b/sfx2/source/toolbox/tbxitem.cxx index efdd50a2ecd7..2a76b6534068 100644 --- a/sfx2/source/toolbox/tbxitem.cxx +++ b/sfx2/source/toolbox/tbxitem.cxx @@ -507,8 +507,11 @@ void SfxToolBoxControl::StateChanged nItemBits |= ToolBoxItemBits::CHECKABLE; } } - else if ( pImpl->bShowString && dynamic_cast< const SfxStringItem *>( pState ) != nullptr ) - pImpl->pBox->SetItemText(nId, static_cast<const SfxStringItem*>(pState)->GetValue() ); + else if ( pImpl->bShowString ) + { + if (auto pStringItem = dynamic_cast< const SfxStringItem *>( pState ) ) + pImpl->pBox->SetItemText(nId, pStringItem->GetValue() ); + } } break; diff --git a/svx/source/svdraw/svdedtv.cxx b/svx/source/svdraw/svdedtv.cxx index 2fd71bee4092..56cd41c700b1 100644 --- a/svx/source/svdraw/svdedtv.cxx +++ b/svx/source/svdraw/svdedtv.cxx @@ -1014,15 +1014,13 @@ void SdrEditView::ReplaceObjectAtView(SdrObject* pOldObj, SdrPageView& rPV, SdrO if(IsTextEdit()) { #ifdef DBG_UTIL - if(dynamic_cast< SdrTextObj* >(pOldObj) && static_cast< SdrTextObj* >(pOldObj)->IsTextEditActive()) - { - OSL_ENSURE(false, "OldObject is in TextEdit mode, this has to be ended before replacing it using SdrEndTextEdit (!)"); - } + if(auto pTextObj = dynamic_cast< SdrTextObj* >(pOldObj)) + if (pTextObj->IsTextEditActive()) + OSL_ENSURE(false, "OldObject is in TextEdit mode, this has to be ended before replacing it using SdrEndTextEdit (!)"); - if(dynamic_cast< SdrTextObj* >(pNewObj) && static_cast< SdrTextObj* >(pNewObj)->IsTextEditActive()) - { - OSL_ENSURE(false, "NewObject is in TextEdit mode, this has to be ended before replacing it using SdrEndTextEdit (!)"); - } + if(auto pTextObj = dynamic_cast< SdrTextObj* >(pNewObj)) + if (pTextObj->IsTextEditActive()) + OSL_ENSURE(false, "NewObject is in TextEdit mode, this has to be ended before replacing it using SdrEndTextEdit (!)"); #endif // #i123468# emergency repair situation, needs to cast up to a class derived from diff --git a/svx/source/svdraw/svdedtv2.cxx b/svx/source/svdraw/svdedtv2.cxx index 06cf4cdc06d3..17ced3685bff 100644 --- a/svx/source/svdraw/svdedtv2.cxx +++ b/svx/source/svdraw/svdedtv2.cxx @@ -1423,7 +1423,10 @@ void SdrEditView::CombineMarkedObjects(bool bNoPolyPoly) const drawing::FillStyle eFillStyle = pAttrObj->GetMergedItem(XATTR_FILLSTYLE).GetValue(); // Take fill style/closed state of pAttrObj in account when deciding to change the line style - bool bIsClosedPathObj(dynamic_cast<const SdrPathObj*>( pAttrObj) != nullptr && static_cast<const SdrPathObj*>(pAttrObj)->IsClosed()); + bool bIsClosedPathObj = false; + if (auto pPathObj = dynamic_cast<const SdrPathObj*>(pAttrObj)) + if (pPathObj->IsClosed()) + bIsClosedPathObj = true; if(drawing::LineStyle_NONE == eLineStyle && (drawing::FillStyle_NONE == eFillStyle || !bIsClosedPathObj)) { diff --git a/sw/source/core/access/accmap.cxx b/sw/source/core/access/accmap.cxx index cfb598f6cd32..95c1891057f6 100644 --- a/sw/source/core/access/accmap.cxx +++ b/sw/source/core/access/accmap.cxx @@ -1164,8 +1164,7 @@ void SwAccessibleMap::InvalidateShapeInParaSelection() size_t nShapes = 0; const SwViewShell *pVSh = GetShell(); - const SwFEShell *pFESh = dynamic_cast<const SwFEShell*>( pVSh) != nullptr ? - static_cast< const SwFEShell * >( pVSh ) : nullptr; + const SwFEShell *pFESh = dynamic_cast<const SwFEShell*>(pVSh); SwPaM* pCursor = pFESh ? pFESh->GetCursor( false /* ??? */ ) : nullptr; //const size_t nSelShapes = pFESh ? pFESh->IsObjSelected() : 0; @@ -1501,8 +1500,7 @@ void SwAccessibleMap::DoInvalidateShapeSelection(bool bInvalidateFocusMode /*=fa size_t nShapes = 0; const SwViewShell *pVSh = GetShell(); - const SwFEShell *pFESh = dynamic_cast<const SwFEShell*>( pVSh) != nullptr ? - static_cast< const SwFEShell * >( pVSh ) : nullptr; + const SwFEShell *pFESh = dynamic_cast<const SwFEShell*>(pVSh); const size_t nSelShapes = pFESh ? pFESh->IsObjSelected() : 0; //when InvalidateFocus Call this function ,and the current selected shape count is not 1 , diff --git a/sw/source/core/doc/doclay.cxx b/sw/source/core/doc/doclay.cxx index b586536403c1..bd0b40063a8e 100644 --- a/sw/source/core/doc/doclay.cxx +++ b/sw/source/core/doc/doclay.cxx @@ -714,12 +714,12 @@ lcl_InsertLabel(SwDoc & rDoc, SwTextFormatColls *const pTextFormatCollTable, // #i115719# // <title> and <description> attributes are lost when calling <DelFrames()>. // Thus, keep them and restore them after the calling <MakeFrames()> - const bool bIsSwFlyFrameFormatInstance( dynamic_cast<SwFlyFrameFormat*>(pOldFormat) != nullptr ); - const OUString sTitle( bIsSwFlyFrameFormatInstance - ? static_cast<SwFlyFrameFormat*>(pOldFormat)->GetObjTitle() + auto pOldFlyFrameFormat = dynamic_cast<SwFlyFrameFormat*>(pOldFormat); + const OUString sTitle( pOldFlyFrameFormat + ? pOldFlyFrameFormat->GetObjTitle() : OUString() ); - const OUString sDescription( bIsSwFlyFrameFormatInstance - ? static_cast<SwFlyFrameFormat*>(pOldFormat)->GetObjDescription() + const OUString sDescription( pOldFlyFrameFormat + ? pOldFlyFrameFormat->GetObjDescription() : OUString() ); pOldFormat->DelFrames(); @@ -867,10 +867,10 @@ lcl_InsertLabel(SwDoc & rDoc, SwTextFormatColls *const pTextFormatCollTable, // We leave this to established methods (especially for InCntFlys). pNewFormat->MakeFrames(); // #i115719# - if ( bIsSwFlyFrameFormatInstance ) + if ( pOldFlyFrameFormat ) { - static_cast<SwFlyFrameFormat*>(pOldFormat)->SetObjTitle( sTitle ); - static_cast<SwFlyFrameFormat*>(pOldFormat)->SetObjDescription( sDescription ); + pOldFlyFrameFormat->SetObjTitle( sTitle ); + pOldFlyFrameFormat->SetObjDescription( sDescription ); } } break; diff --git a/sw/source/core/draw/dview.cxx b/sw/source/core/draw/dview.cxx index 4512543cadfb..5ae8d93b0778 100644 --- a/sw/source/core/draw/dview.cxx +++ b/sw/source/core/draw/dview.cxx @@ -79,8 +79,7 @@ bool SwSdrHdl::IsFocusHdl() const static const SwFrame *lcl_FindAnchor( const SdrObject *pObj, bool bAll ) { - const SwVirtFlyDrawObj *pVirt = dynamic_cast< const SwVirtFlyDrawObj *>( pObj ) != nullptr ? - static_cast<const SwVirtFlyDrawObj*>(pObj) : nullptr; + const SwVirtFlyDrawObj *pVirt = dynamic_cast< const SwVirtFlyDrawObj *>( pObj ); if ( pVirt ) { if ( bAll || !pVirt->GetFlyFrame()->IsFlyInContentFrame() ) diff --git a/sw/source/core/frmedt/fews.cxx b/sw/source/core/frmedt/fews.cxx index fd1d137b15f4..0d6519dba776 100644 --- a/sw/source/core/frmedt/fews.cxx +++ b/sw/source/core/frmedt/fews.cxx @@ -1299,8 +1299,9 @@ bool SwFEShell::IsFrameVertical(const bool bEnvironment, bool& bRTL, bool& bVert return bVert; } - if ( dynamic_cast<const SwVirtFlyDrawObj*>( pObj) != nullptr && !bEnvironment ) - pRef = static_cast<const SwVirtFlyDrawObj*>(pObj)->GetFlyFrame(); + if ( !bEnvironment ) + if ( auto pVirtFly = dynamic_cast<const SwVirtFlyDrawObj*>( pObj) ) + pRef = pVirtFly->GetFlyFrame(); bVert = pRef->IsVertical(); bRTL = pRef->IsRightToLeft(); _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits