[Libreoffice-commits] core.git: accessibility/inc avmedia/source compilerplugins/clang framework/inc framework/source include/sot include/svx sot/source svx/inc svx/source toolkit/source vcl/inc xmloff/source

Noel Grandin noel at peralex.com
Tue Jul 12 08:45:36 UTC 2016


 accessibility/inc/extended/accessibleeditbrowseboxcell.hxx |    2 
 accessibility/inc/extended/accessiblelistboxentry.hxx      |    2 
 avmedia/source/gstreamer/gstplayer.hxx                     |    2 
 avmedia/source/opengl/oglwindow.hxx                        |    2 
 compilerplugins/clang/fragiledestructor.cxx                |  113 +++++++++++++
 framework/inc/services/layoutmanager.hxx                   |    2 
 framework/source/jobs/jobexecutor.cxx                      |    2 
 framework/source/services/autorecovery.cxx                 |    2 
 framework/source/services/pathsettings.cxx                 |    2 
 framework/source/uiconfiguration/moduleuicfgsupplier.cxx   |    2 
 framework/source/uifactory/uicontrollerfactory.cxx         |    2 
 include/sot/stg.hxx                                        |    8 
 include/svx/svdedxv.hxx                                    |    2 
 sot/source/unoolestorage/xolesimplestorage.hxx             |    2 
 svx/inc/sdr/contact/objectcontactofpageview.hxx            |    2 
 svx/source/inc/GraphCtlAccessibleContext.hxx               |    2 
 svx/source/sdr/contact/viewobjectcontactofpageobj.cxx      |    2 
 svx/source/svdraw/svdoedge.cxx                             |    4 
 toolkit/source/controls/grid/sortablegriddatamodel.cxx     |    2 
 vcl/inc/headless/svpbmp.hxx                                |    2 
 vcl/inc/opengl/salbmp.hxx                                  |    2 
 xmloff/source/forms/elementexport.cxx                      |    6 
 22 files changed, 140 insertions(+), 27 deletions(-)

New commits:
commit 52b91f3454394a1792dec018804bf2c969f564e5
Author: Noel Grandin <noel at peralex.com>
Date:   Wed Jun 22 12:08:45 2016 +0200

    new loplugin fragiledestructor
    
    fix up a small number of places that it finds
    
    Change-Id: Iedc91e141edfb28f727454f698cd2155a7fd5bf4
    Reviewed-on: https://gerrit.libreoffice.org/26566
    Tested-by: Jenkins <ci at libreoffice.org>
    Reviewed-by: Noel Grandin <noelgrandin at gmail.com>

diff --git a/accessibility/inc/extended/accessibleeditbrowseboxcell.hxx b/accessibility/inc/extended/accessibleeditbrowseboxcell.hxx
index c9be43d..04dcd7c 100644
--- a/accessibility/inc/extended/accessibleeditbrowseboxcell.hxx
+++ b/accessibility/inc/extended/accessibleeditbrowseboxcell.hxx
@@ -81,7 +81,7 @@ namespace accessibility
         virtual void SAL_CALL disposing() override;
 
         // XComponent/OComponentProxyAggregationHelper (needs to be disambiguated)
-        virtual void SAL_CALL dispose() throw( css::uno::RuntimeException, std::exception ) override;
+        virtual void SAL_CALL dispose() throw( css::uno::RuntimeException, std::exception ) final override;
 
         // OAccessibleContextWrapperHelper();
         void notifyTranslatedEvent( const css::accessibility::AccessibleEventObject& _rEvent ) throw (css::uno::RuntimeException) override;
diff --git a/accessibility/inc/extended/accessiblelistboxentry.hxx b/accessibility/inc/extended/accessiblelistboxentry.hxx
index 886c27e..a58ce0e 100644
--- a/accessibility/inc/extended/accessiblelistboxentry.hxx
+++ b/accessibility/inc/extended/accessiblelistboxentry.hxx
@@ -112,7 +112,7 @@ namespace accessibility
         virtual void SAL_CALL                   disposing() override;
 
         // ListBoxAccessible/XComponent
-        virtual void SAL_CALL dispose() throw ( css::uno::RuntimeException, std::exception ) override;
+        virtual void SAL_CALL dispose() throw ( css::uno::RuntimeException, std::exception ) final override;
 
         // OCommonAccessibleText
         virtual OUString                        implGetText() override;
diff --git a/avmedia/source/gstreamer/gstplayer.hxx b/avmedia/source/gstreamer/gstplayer.hxx
index f0f23b5..4bf2f8e 100644
--- a/avmedia/source/gstreamer/gstplayer.hxx
+++ b/avmedia/source/gstreamer/gstplayer.hxx
@@ -75,7 +75,7 @@ public:
     virtual css::uno::Sequence< OUString > SAL_CALL getSupportedServiceNames(  ) throw (css::uno::RuntimeException, std::exception) override;
 
     // ::cppu::OComponentHelper
-    virtual void SAL_CALL disposing() override;
+    virtual void SAL_CALL disposing() final override;
 
 protected:
     OUString                maURL;
diff --git a/avmedia/source/opengl/oglwindow.hxx b/avmedia/source/opengl/oglwindow.hxx
index 4b28657..f939037 100644
--- a/avmedia/source/opengl/oglwindow.hxx
+++ b/avmedia/source/opengl/oglwindow.hxx
@@ -39,7 +39,7 @@ public:
     virtual sal_Bool SAL_CALL supportsService( const OUString& rServiceName ) throw (css::uno::RuntimeException, std::exception) override;
     virtual css::uno::Sequence< OUString > SAL_CALL getSupportedServiceNames() throw (css::uno::RuntimeException, std::exception) override;
 
-    virtual void SAL_CALL dispose() throw (css::uno::RuntimeException, std::exception) override;
+    virtual void SAL_CALL dispose() throw (css::uno::RuntimeException, std::exception) final override;
     virtual void SAL_CALL addEventListener( const css::uno::Reference< css::lang::XEventListener >& xListener ) throw (css::uno::RuntimeException, std::exception) override;
     virtual void SAL_CALL removeEventListener( const css::uno::Reference< css::lang::XEventListener >& aListener ) throw (css::uno::RuntimeException, std::exception) override;
 
diff --git a/compilerplugins/clang/fragiledestructor.cxx b/compilerplugins/clang/fragiledestructor.cxx
new file mode 100644
index 0000000..ebce1d6
--- /dev/null
+++ b/compilerplugins/clang/fragiledestructor.cxx
@@ -0,0 +1,113 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+#include <string>
+#include <iostream>
+
+#include "plugin.hxx"
+#include "compat.hxx"
+#include "clang/AST/CXXInheritance.h"
+
+
+// Check for calls to virtual methods from destructors. These are dangerous because intention might be to call
+// a method on a subclass, while in actual fact, it only calls the method on the current or super class.
+//
+
+namespace {
+
+class FragileDestructor:
+    public RecursiveASTVisitor<FragileDestructor>, public loplugin::Plugin
+{
+public:
+    explicit FragileDestructor(InstantiationData const & data): Plugin(data) {}
+
+    virtual void run() override { TraverseDecl(compiler.getASTContext().getTranslationUnitDecl()); }
+
+    bool VisitCXXDestructorDecl(const CXXDestructorDecl *);
+
+    bool VisitCXXMemberCallExpr(const CXXMemberCallExpr *);
+
+private:
+    bool mbChecking = false;
+};
+
+bool FragileDestructor::VisitCXXDestructorDecl(const CXXDestructorDecl* pCXXDestructorDecl)
+{
+    if (ignoreLocation(pCXXDestructorDecl)) {
+        return true;
+    }
+    if (!pCXXDestructorDecl->isThisDeclarationADefinition()) {
+        return true;
+    }
+    // ignore this for now, too tricky for me to work out
+    StringRef aFileName = compiler.getSourceManager().getFilename(
+            compiler.getSourceManager().getSpellingLoc(pCXXDestructorDecl->getLocStart()));
+    if (aFileName.startswith(SRCDIR "/include/comphelper/")
+        || aFileName.startswith(SRCDIR "/include/cppuhelper/")
+        || aFileName.startswith(SRCDIR "/cppuhelper/")
+        || aFileName.startswith(SRCDIR "/comphelper/")
+        // dont know how to detect this in clang - it is making an explicit call to it's own method, so presumably OK
+        || aFileName == SRCDIR "/basic/source/sbx/sbxvalue.cxx"
+       )
+        return true;
+    mbChecking = true;
+    Stmt * pStmt = pCXXDestructorDecl->getBody();
+    TraverseStmt(pStmt);
+    mbChecking = false;
+    return true;
+}
+
+bool FragileDestructor::VisitCXXMemberCallExpr(const CXXMemberCallExpr* callExpr)
+{
+    if (!mbChecking || ignoreLocation(callExpr)) {
+        return true;
+    }
+    const CXXMethodDecl* methodDecl = callExpr->getMethodDecl();
+    if (!methodDecl->isVirtual() || methodDecl->hasAttr<FinalAttr>()) {
+        return true;
+    }
+    const CXXRecordDecl* parentRecordDecl = methodDecl->getParent();
+    if (parentRecordDecl->hasAttr<FinalAttr>()) {
+        return true;
+    }
+    if (!callExpr->getImplicitObjectArgument()->IgnoreImpCasts()->isImplicitCXXThis()) {
+        return true;
+    }
+    // if we see an explicit call to it's own method, thats OK
+    auto s1 = compiler.getSourceManager().getCharacterData(callExpr->getLocStart());
+    auto s2 = compiler.getSourceManager().getCharacterData(callExpr->getLocEnd());
+    std::string tok(s1, s2-s1);
+    if (tok.find("::") != std::string::npos) {
+        return true;
+    }
+    // e.g. osl/thread.hxx and cppuhelper/compbase.hxx
+    StringRef aFileName = compiler.getSourceManager().getFilename(compiler.getSourceManager().getSpellingLoc(methodDecl->getLocStart()));
+    if (aFileName.startswith(SRCDIR "/include/osl/")
+        || aFileName.startswith(SRCDIR "/include/comphelper/")
+        || aFileName.startswith(SRCDIR "/include/cppuhelper/"))
+        return true;
+    report(
+        DiagnosticsEngine::Warning,
+        "calling virtual method from destructor, either make the virtual method SAL_FINAL, or make this class SAL_FINAL",
+        callExpr->getLocStart())
+      << callExpr->getSourceRange();
+    report(
+        DiagnosticsEngine::Note,
+        "callee method here",
+        methodDecl->getLocStart())
+      << methodDecl->getSourceRange();
+    return true;
+}
+
+
+loplugin::Plugin::Registration< FragileDestructor > X("fragiledestructor", false);
+
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/framework/inc/services/layoutmanager.hxx b/framework/inc/services/layoutmanager.hxx
index 873f067..a6ba5c5 100644
--- a/framework/inc/services/layoutmanager.hxx
+++ b/framework/inc/services/layoutmanager.hxx
@@ -110,7 +110,7 @@ namespace framework
             virtual void SAL_CALL reset() throw (css::uno::RuntimeException, std::exception) override;
             virtual css::awt::Rectangle SAL_CALL getCurrentDockingArea(  ) throw (css::uno::RuntimeException, std::exception) override;
             virtual css::uno::Reference< css::ui::XDockingAreaAcceptor > SAL_CALL getDockingAreaAcceptor() throw (css::uno::RuntimeException, std::exception) override;
-            virtual void SAL_CALL setDockingAreaAcceptor( const css::uno::Reference< css::ui::XDockingAreaAcceptor >& xDockingAreaAcceptor ) throw (css::uno::RuntimeException, std::exception) override;
+            virtual void SAL_CALL setDockingAreaAcceptor( const css::uno::Reference< css::ui::XDockingAreaAcceptor >& xDockingAreaAcceptor ) throw (css::uno::RuntimeException, std::exception) final override;
             virtual void SAL_CALL createElement( const OUString& aName ) throw (css::uno::RuntimeException, std::exception) override;
             virtual void SAL_CALL destroyElement( const OUString& aName ) throw (css::uno::RuntimeException, std::exception) override;
             virtual sal_Bool SAL_CALL requestElement( const OUString& ResourceURL ) throw (css::uno::RuntimeException, std::exception) override;
diff --git a/framework/source/jobs/jobexecutor.cxx b/framework/source/jobs/jobexecutor.cxx
index 9ab4825..ecf5b91 100644
--- a/framework/source/jobs/jobexecutor.cxx
+++ b/framework/source/jobs/jobexecutor.cxx
@@ -79,7 +79,7 @@ private:
     /** helper to allow us listen to the configuration without a cyclic dependency */
     css::uno::Reference<css::container::XContainerListener> m_xConfigListener;
 
-    virtual void SAL_CALL disposing() override;
+    virtual void SAL_CALL disposing() final override;
 
 public:
 
diff --git a/framework/source/services/autorecovery.cxx b/framework/source/services/autorecovery.cxx
index cf97b6d..40238c6 100644
--- a/framework/source/services/autorecovery.cxx
+++ b/framework/source/services/autorecovery.cxx
@@ -533,7 +533,7 @@ protected:
         throw(css::uno::RuntimeException, std::exception) override;
 
 private:
-    virtual void SAL_CALL disposing() override;
+    virtual void SAL_CALL disposing() final override;
 
     /** @short  open the underlying configuration.
 
diff --git a/framework/source/services/pathsettings.cxx b/framework/source/services/pathsettings.cxx
index 68dda4e..5b22b36 100644
--- a/framework/source/services/pathsettings.cxx
+++ b/framework/source/services/pathsettings.cxx
@@ -354,7 +354,7 @@ public:
     void impl_readAll();
 
 private:
-    virtual void SAL_CALL disposing() override;
+    virtual void SAL_CALL disposing() final override;
 
     OUString getStringProperty(const OUString& p1)
         throw(css::uno::RuntimeException);
diff --git a/framework/source/uiconfiguration/moduleuicfgsupplier.cxx b/framework/source/uiconfiguration/moduleuicfgsupplier.cxx
index ebdf827..2c707dc 100644
--- a/framework/source/uiconfiguration/moduleuicfgsupplier.cxx
+++ b/framework/source/uiconfiguration/moduleuicfgsupplier.cxx
@@ -89,7 +89,7 @@ public:
         throw (css::container::NoSuchElementException, css::uno::RuntimeException, std::exception) override;
 
 private:
-    virtual void SAL_CALL disposing() override;
+    virtual void SAL_CALL disposing() final override;
 
     typedef std::unordered_map< OUString, css::uno::Reference< css::ui::XModuleUIConfigurationManager2 >, OUStringHash > ModuleToModuleCfgMgr;
 
diff --git a/framework/source/uifactory/uicontrollerfactory.cxx b/framework/source/uifactory/uicontrollerfactory.cxx
index 7d4809b..fb6cdb4 100644
--- a/framework/source/uifactory/uicontrollerfactory.cxx
+++ b/framework/source/uifactory/uicontrollerfactory.cxx
@@ -66,7 +66,7 @@ protected:
     rtl::Reference<ConfigurationAccess_ControllerFactory>    m_pConfigAccess;
 
 private:
-    virtual void SAL_CALL disposing() override;
+    virtual void SAL_CALL disposing() final override;
 };
 
 UIControllerFactory::UIControllerFactory(
diff --git a/include/sot/stg.hxx b/include/sot/stg.hxx
index 8d4479e..ada5c7e 100644
--- a/include/sot/stg.hxx
+++ b/include/sot/stg.hxx
@@ -148,7 +148,7 @@ public:
     virtual bool        SetSize( sal_uLong nNewSize ) override;
     virtual sal_uLong   GetSize() const override;
     virtual void        CopyTo( BaseStorageStream * pDestStm ) override;
-    virtual bool        Commit() override;
+    virtual bool        Commit() final override;
     virtual bool        Validate( bool=false ) const override;
     virtual bool        ValidateMode( StreamMode ) const override;
     virtual bool        Equals( const BaseStorageStream& rStream ) const override;
@@ -172,7 +172,7 @@ public:
     static bool                 IsStorageFile( const OUString & rFileName );
     static bool                 IsStorageFile( SvStream* );
 
-    virtual const OUString&     GetName() const override;
+    virtual const OUString&     GetName() const final override;
     virtual bool                IsRoot() const override { return bIsRoot; }
     virtual void                SetClassId( const ClsId& ) override;
     virtual const ClsId&        GetClassId() const override;
@@ -185,7 +185,7 @@ public:
     virtual OUString            GetUserName() override;
     virtual void                FillInfoList( SvStorageInfoList* ) const override;
     virtual bool                CopyTo( BaseStorage* pDestStg ) const override;
-    virtual bool                Commit() override;
+    virtual bool                Commit() final override;
     virtual bool                Revert() override;
     virtual BaseStorageStream*  OpenStream( const OUString & rEleName,
                                             StreamMode = STREAM_STD_READWRITE,
@@ -288,7 +288,7 @@ public:
     virtual OUString            GetUserName() override;
     virtual void                FillInfoList( SvStorageInfoList* ) const override;
     virtual bool                CopyTo( BaseStorage* pDestStg ) const override;
-    virtual bool                Commit() override;
+    virtual bool                Commit() final override;
     virtual bool                Revert() override;
     virtual BaseStorageStream*  OpenStream( const OUString & rEleName,
                                             StreamMode = STREAM_STD_READWRITE,
diff --git a/include/svx/svdedxv.hxx b/include/svx/svdedxv.hxx
index 78ed1e0..4aac4ff 100644
--- a/include/svx/svdedxv.hxx
+++ b/include/svx/svdedxv.hxx
@@ -186,7 +186,7 @@ public:
     // (in place of SDRENDTEXTEDIT_BEDELETED), which says, the obj should be
     // deleted.
     virtual SdrEndTextEditKind SdrEndTextEdit(bool bDontDeleteReally = false);
-    virtual bool IsTextEdit() const override;
+    virtual bool IsTextEdit() const final override;
 
     // This method returns sal_True, if the point rHit is inside the
     // objectspace or the OutlinerView.
diff --git a/sot/source/unoolestorage/xolesimplestorage.hxx b/sot/source/unoolestorage/xolesimplestorage.hxx
index 4c09058..dfe034d 100644
--- a/sot/source/unoolestorage/xolesimplestorage.hxx
+++ b/sot/source/unoolestorage/xolesimplestorage.hxx
@@ -106,7 +106,7 @@ public:
     //  XComponent
 
     virtual void SAL_CALL dispose()
-        throw ( css::uno::RuntimeException, std::exception ) override;
+        throw ( css::uno::RuntimeException, std::exception ) final override;
 
     virtual void SAL_CALL addEventListener(
             const css::uno::Reference< css::lang::XEventListener >& xListener )
diff --git a/svx/inc/sdr/contact/objectcontactofpageview.hxx b/svx/inc/sdr/contact/objectcontactofpageview.hxx
index 5f87286..9de11ad 100644
--- a/svx/inc/sdr/contact/objectcontactofpageview.hxx
+++ b/svx/inc/sdr/contact/objectcontactofpageview.hxx
@@ -61,7 +61,7 @@ namespace sdr
             virtual void PrepareProcessDisplay() override;
 
             // From baseclass Timer, the timeout call triggered by the LazyInvalidate mechanism
-            virtual void Invoke() override;
+            virtual void Invoke() final override;
 
             // Process the whole displaying
             virtual void ProcessDisplay(DisplayInfo& rDisplayInfo) override;
diff --git a/svx/source/inc/GraphCtlAccessibleContext.hxx b/svx/source/inc/GraphCtlAccessibleContext.hxx
index a87d6f3..c8c5c43 100644
--- a/svx/source/inc/GraphCtlAccessibleContext.hxx
+++ b/svx/source/inc/GraphCtlAccessibleContext.hxx
@@ -185,7 +185,7 @@ protected:
     /// Return the object's current bounding box relative to the parent object.
     Rectangle GetBoundingBox() throw (css::uno::RuntimeException);
 
-    virtual void SAL_CALL disposing() override;
+    virtual void SAL_CALL disposing() final override;
 
 private:
     SdrObject* getSdrObject( sal_Int32 nIndex )
diff --git a/svx/source/sdr/contact/viewobjectcontactofpageobj.cxx b/svx/source/sdr/contact/viewobjectcontactofpageobj.cxx
index 15ebcfb..43c860e 100644
--- a/svx/source/sdr/contact/viewobjectcontactofpageobj.cxx
+++ b/svx/source/sdr/contact/viewobjectcontactofpageobj.cxx
@@ -54,7 +54,7 @@ public:
     virtual void setLazyInvalidate(ViewObjectContact& rVOC) override;
 
     // From baseclass Timer, the timeout call triggered by the LazyInvalidate mechanism
-    virtual void Invoke() override;
+    virtual void Invoke() final override;
 
     // get primitive visualization
     drawinglayer::primitive2d::Primitive2DContainer createPrimitive2DSequenceForPage(const DisplayInfo& rDisplayInfo);
diff --git a/svx/source/svdraw/svdoedge.cxx b/svx/source/svdraw/svdoedge.cxx
index 6a88a87..a1f15f6 100644
--- a/svx/source/svdraw/svdoedge.cxx
+++ b/svx/source/svdraw/svdoedge.cxx
@@ -177,8 +177,8 @@ SdrEdgeObj::SdrEdgeObj()
 
 SdrEdgeObj::~SdrEdgeObj()
 {
-    DisconnectFromNode(true);
-    DisconnectFromNode(false);
+    SdrEdgeObj::DisconnectFromNode(true);
+    SdrEdgeObj::DisconnectFromNode(false);
     delete pEdgeTrack;
 }
 
diff --git a/toolkit/source/controls/grid/sortablegriddatamodel.cxx b/toolkit/source/controls/grid/sortablegriddatamodel.cxx
index 9bb9236..1138c18 100644
--- a/toolkit/source/controls/grid/sortablegriddatamodel.cxx
+++ b/toolkit/source/controls/grid/sortablegriddatamodel.cxx
@@ -125,7 +125,7 @@ public:
 
     // XInterface
     virtual css::uno::Any SAL_CALL queryInterface( const css::uno::Type& aType ) throw (css::uno::RuntimeException, std::exception) override;
-    virtual void SAL_CALL acquire(  ) throw () override;
+    virtual void SAL_CALL acquire(  ) throw () final override;
     virtual void SAL_CALL release(  ) throw () override;
 
     // XTypeProvider
diff --git a/vcl/inc/headless/svpbmp.hxx b/vcl/inc/headless/svpbmp.hxx
index 85c4e13..f0fce62 100644
--- a/vcl/inc/headless/svpbmp.hxx
+++ b/vcl/inc/headless/svpbmp.hxx
@@ -49,7 +49,7 @@ public:
     {
         return mpDIB;
     }
-    virtual void            Destroy() override;
+    virtual void            Destroy() final override;
     virtual Size            GetSize() const override;
     virtual sal_uInt16      GetBitCount() const override;
 
diff --git a/vcl/inc/opengl/salbmp.hxx b/vcl/inc/opengl/salbmp.hxx
index 5a9270e..771c4aa 100644
--- a/vcl/inc/opengl/salbmp.hxx
+++ b/vcl/inc/opengl/salbmp.hxx
@@ -66,7 +66,7 @@ public:
                             Size& rSize,
                             bool bMask = false ) override;
 
-    void            Destroy() override;
+    void            Destroy() final override;
 
     Size            GetSize() const override;
     sal_uInt16      GetBitCount() const override;
diff --git a/xmloff/source/forms/elementexport.cxx b/xmloff/source/forms/elementexport.cxx
index 688c6e8..1de42c2 100644
--- a/xmloff/source/forms/elementexport.cxx
+++ b/xmloff/source/forms/elementexport.cxx
@@ -98,7 +98,7 @@ namespace xmloff
 
     OElementExport::~OElementExport()
     {
-        implEndElement();
+        delete m_pXMLElement;
     }
 
     void OElementExport::doExport()
@@ -246,7 +246,8 @@ namespace xmloff
 
     OControlExport::~OControlExport()
     {
-        implEndElement();
+        // end the outer element if it exists
+        delete m_pOuterElement;
     }
 
     void OControlExport::exportOuterAttributes()
@@ -1971,7 +1972,6 @@ namespace xmloff
 
     OColumnExport::~OColumnExport()
     {
-        implEndElement();
     }
 
     void OColumnExport::exportServiceNameAttribute()


More information about the Libreoffice-commits mailing list