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 <[email protected]> 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 <[email protected]> Reviewed-by: Noel Grandin <[email protected]> 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() _______________________________________________ Libreoffice-commits mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
