[Libreoffice-commits] core.git: compilerplugins/clang sd/source sfx2/source svx/source sw/source

Noel (via logerrit) logerrit at kemper.freedesktop.org
Sat Mar 6 12:09:17 UTC 2021


 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.grandin at collabora.co.uk>
AuthorDate: Fri Mar 5 08:37:41 2021 +0200
Commit:     Noel Grandin <noel.grandin at 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.grandin at 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 at +1 {{dynamic_cast here [loplugin:staticdynamic]}}
+    dynamic_cast<ClassB*>(p1)->foo();
+    // expected-error at +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();


More information about the Libreoffice-commits mailing list