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@libreoffice.org>
Reviewed-by: Noel Grandin <noelgrandin@gmail.com>

22 files changed, 140 insertions, 27 deletions

diff --git a/accessibility/inc/extended/accessibleeditbrowseboxcell.hxx b/accessibility/inc/extended/accessibleeditbrowseboxcell.hxx
index c9be43d09481..04dcd7c0dc30 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 886c27e7b7aa..a58ce0e7d1db 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 f0f23b56f6f5..4bf2f8edfef0 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 4b28657ed7ce..f93903776534 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 000000000000..ebce1d677f8b
--- /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 873f067b52ff..a6ba5c516c0d 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 9ab48255f65d..ecf5b9134e07 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 cf97b6d981dd..40238c69105d 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 68dda4effb78..5b22b36b8aab 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 ebdf8277dcc1..2c707dc18325 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 7d4809bcdcf6..fb6cdb48fb45 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 8d4479e90511..ada5c7e75319 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 78ed1e0fcdd7..4aac4ffed86b 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 4c0905800500..dfe034d1d4b6 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 5f8728662b2a..9de11ad95123 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 a87d6f3ea50a..c8c5c439245b 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 15ebcfb63d4a..43c860ed53a1 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 6a88a875e5b5..a1f15f63796a 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 9bb92367af24..1138c18d2e3b 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 85c4e13c5a5a..f0fce62f09e6 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 5a9270edd61c..771c4aa88413 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 688c6e8427fb..1de42c284ade 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()