[Libreoffice-commits] core.git: compilerplugins/clang extensions/source include/basegfx shell/source xmlscript/source
Noel Grandin
noel.grandin at collabora.co.uk
Fri Jan 27 09:49:55 UTC 2017
compilerplugins/clang/test/unnecessaryoverride-dtor.cxx | 19 ++++-
compilerplugins/clang/test/unnecessaryoverride-dtor.hxx | 10 ++
compilerplugins/clang/unnecessaryoverride.cxx | 57 +++++++++++++---
extensions/source/propctrlr/enumrepresentation.hxx | 1
include/basegfx/raster/bpixelraster.hxx | 4 -
include/basegfx/raster/bzpixelraster.hxx | 4 -
shell/source/sessioninstall/SyncDbusSessionHelper.hxx | 1
xmlscript/source/xmldlg_imexp/imp_share.hxx | 3
8 files changed, 73 insertions(+), 26 deletions(-)
New commits:
commit 4511431fb665dac192008fa063e783d9e8d7ed15
Author: Noel Grandin <noel.grandin at collabora.co.uk>
Date: Thu Jan 12 16:27:14 2017 +0200
improve "unnecessary user-declared destructor" check
to look for inline&empty destructors, where we can just let
the compiler do it's thing
Change-Id: Ibde8800bdfed6b77649c30ebc19921167c33dec3
Reviewed-on: https://gerrit.libreoffice.org/32999
Tested-by: Jenkins <ci at libreoffice.org>
Reviewed-by: Noel Grandin <noel.grandin at collabora.co.uk>
diff --git a/compilerplugins/clang/test/unnecessaryoverride-dtor.cxx b/compilerplugins/clang/test/unnecessaryoverride-dtor.cxx
index 26b3a81..18f7aed 100644
--- a/compilerplugins/clang/test/unnecessaryoverride-dtor.cxx
+++ b/compilerplugins/clang/test/unnecessaryoverride-dtor.cxx
@@ -16,7 +16,7 @@
struct NonVirtualBase {};
struct NonVirtualDerived1: NonVirtualBase {
- ~NonVirtualDerived1() {}
+ ~NonVirtualDerived1() {} // expected-error {{unnecessary user-declared destructor [loplugin:unnecessaryoverride]}}
};
struct NonVirtualDerived2: NonVirtualBase {
@@ -41,6 +41,10 @@ IncludedDerived3::IncludedDerived3() {}
IncludedDerived3::~IncludedDerived3() {}
+// vmiklos likes these because he can quickly add a DEBUG or something similar without
+// massive recompile
+IncludedNotDerived::~IncludedNotDerived() {}
+
struct NoExSpecDerived: VirtualBase {
~NoExSpecDerived() override {} // expected-error {{unnecessary user-declared destructor [loplugin:unnecessaryoverride]}}
};
@@ -113,9 +117,20 @@ struct PureDerived: PureBase {
~PureDerived() override {} // expected-error {{unnecessary user-declared destructor [loplugin:unnecessaryoverride]}}
};
-// aovid loplugin:unreffun:
+struct CompleteBase {
+ ~CompleteBase() {} // expected-error {{unnecessary user-declared destructor [loplugin:unnecessaryoverride]}}
+};
+
+// <sberg> noelgrandin, there's one other corner case one can imagine:
+// a class defined in a .hxx with the dtor declared (but not defined) as inline in the .hxx,
+// and then defined in the cxx (making it effectively only callable from within the cxx);
+// removing the dtor declaration from the class definition would change the dtor to be callable from everywhere
+MarkedInlineButNotDefined::~MarkedInlineButNotDefined() {}
+
+// avoid loplugin:unreffun:
int main() {
(void) NonVirtualDerived1();
+ (void) CompleteBase();
}
/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
diff --git a/compilerplugins/clang/test/unnecessaryoverride-dtor.hxx b/compilerplugins/clang/test/unnecessaryoverride-dtor.hxx
index fe1f9cb..81733a9 100644
--- a/compilerplugins/clang/test/unnecessaryoverride-dtor.hxx
+++ b/compilerplugins/clang/test/unnecessaryoverride-dtor.hxx
@@ -19,13 +19,17 @@ struct VirtualBase {
};
struct IncludedDerived1: VirtualBase {
- ~IncludedDerived1() override {};
+ ~IncludedDerived1() override {}; // expected-error {{unnecessary user-declared destructor [loplugin:unnecessaryoverride]}}
};
struct IncludedDerived2: VirtualBase {
~IncludedDerived2() override;
};
+struct IncludedNotDerived {
+ ~IncludedNotDerived();
+};
+
struct Incomplete;
struct IncludedDerived3: VirtualBase {
IncludedDerived3();
@@ -38,6 +42,10 @@ private:
rtl::Reference<Incomplete> m;
};
+struct MarkedInlineButNotDefined {
+ inline ~MarkedInlineButNotDefined();
+};
+
#endif
/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
diff --git a/compilerplugins/clang/unnecessaryoverride.cxx b/compilerplugins/clang/unnecessaryoverride.cxx
index 98b77d5..8cf811b 100644
--- a/compilerplugins/clang/unnecessaryoverride.cxx
+++ b/compilerplugins/clang/unnecessaryoverride.cxx
@@ -78,14 +78,25 @@ bool UnnecessaryOverride::VisitCXXMethodDecl(const CXXMethodDecl* methodDecl)
return true;
}
- if (isa<CXXDestructorDecl>(methodDecl)) {
- // Warn about unnecessarily user-declared overriding virtual
- // destructors; such a destructor is deemed unnecessary if
+ StringRef aFileName = compiler.getSourceManager().getFilename(compiler.getSourceManager().getSpellingLoc(methodDecl->getLocStart()));
+
+ if (isa<CXXDestructorDecl>(methodDecl)
+ && !isInUnoIncludeFile(methodDecl))
+ {
+ // the code is this method is only __compiled__ if OSL_DEBUG_LEVEL > 1
+ if (aFileName == SRCDIR "/tools/source/stream/strmunx.cxx")
+ return true;
+
+ // Warn about unnecessarily user-declared destructors.
+ // A destructor is deemed unnecessary if:
// * it is public;
// * its class is only defined in the .cxx file (i.e., the virtual
- // destructor is neither used to controll the place of vtable
+ // destructor is neither used to control the place of vtable
// emission, nor is its definition depending on types that may still
// be incomplete);
+ // or
+ // the destructor is inline, the class definition is complete,
+ // and the class has no superclasses
// * it either does not have an explicit exception specification, or has
// a non-dependent explicit exception specification that is compatible
// with a non-dependent exception specification the destructor would
@@ -102,14 +113,42 @@ bool UnnecessaryOverride::VisitCXXMethodDecl(const CXXMethodDecl* methodDecl)
// implicit definition of a copy constructor and/or copy assignment
// operator to change from being an obsolete feature to being a standard
// feature. That difference is not taken into account here.
- if ((methodDecl->begin_overridden_methods()
- == methodDecl->end_overridden_methods())
- || methodDecl->getAccess() != AS_public)
+ auto cls = methodDecl->getParent();
+ if (methodDecl->isVirtual() && cls->getNumBases() == 0)
+ {
+ return true;
+ }
+ if (methodDecl->getAccess() != AS_public)
{
return true;
}
if (!compiler.getSourceManager().isInMainFile(
- methodDecl->getCanonicalDecl()->getLocation()))
+ methodDecl->getCanonicalDecl()->getLocation())
+ && !( methodDecl->isInlined()))
+ {
+ return true;
+ }
+ // if it's virtual, but it has a base-class with a non-virtual destructor
+ if (methodDecl->isVirtual())
+ {
+ for (auto baseSpecifier = cls->bases_begin(); baseSpecifier != cls->bases_end(); ++baseSpecifier)
+ {
+ const RecordType* baseRecordType = baseSpecifier->getType()->getAs<RecordType>();
+ if (baseRecordType)
+ {
+ const CXXRecordDecl* baseRecordDecl = dyn_cast<CXXRecordDecl>(baseRecordType->getDecl());
+ if (baseRecordDecl && baseRecordDecl->getDestructor()
+ && !baseRecordDecl->getDestructor()->isVirtual())
+ {
+ return true;
+ }
+ }
+ }
+ }
+ // corner case
+ if (methodDecl->isInlined()
+ && compiler.getSourceManager().isInMainFile(methodDecl->getLocation())
+ && !compiler.getSourceManager().isInMainFile(methodDecl->getCanonicalDecl()->getLocation()))
{
return true;
}
@@ -123,7 +162,6 @@ bool UnnecessaryOverride::VisitCXXMethodDecl(const CXXMethodDecl* methodDecl)
}
}
//TODO: exception specification
- auto cls = methodDecl->getParent();
if (!(cls->hasUserDeclaredCopyConstructor()
|| cls->hasUserDeclaredCopyAssignment()
|| cls->hasUserDeclaredMoveConstructor()
@@ -166,7 +204,6 @@ bool UnnecessaryOverride::VisitCXXMethodDecl(const CXXMethodDecl* methodDecl)
}
}
// sometimes the disambiguation happens in a base class
- StringRef aFileName = compiler.getSourceManager().getFilename(compiler.getSourceManager().getSpellingLoc(methodDecl->getLocStart()));
if (aFileName == SRCDIR "/comphelper/source/property/propertycontainer.cxx")
return true;
// not sure what is happening here
diff --git a/extensions/source/propctrlr/enumrepresentation.hxx b/extensions/source/propctrlr/enumrepresentation.hxx
index 309af3c..550d0db 100644
--- a/extensions/source/propctrlr/enumrepresentation.hxx
+++ b/extensions/source/propctrlr/enumrepresentation.hxx
@@ -55,7 +55,6 @@ namespace pcr
const css::uno::Any& _rEnumValue
) const = 0;
- virtual ~IPropertyEnumRepresentation() override { };
};
diff --git a/include/basegfx/raster/bpixelraster.hxx b/include/basegfx/raster/bpixelraster.hxx
index 46ef649..491f88e 100644
--- a/include/basegfx/raster/bpixelraster.hxx
+++ b/include/basegfx/raster/bpixelraster.hxx
@@ -57,10 +57,6 @@ namespace basegfx
reset();
}
- ~BPixelRaster()
- {
- }
-
// coordinate calcs between X/Y and span
sal_uInt32 getIndexFromXY(sal_uInt32 nX, sal_uInt32 nY) const { return (nX + (nY * mnWidth)); }
diff --git a/include/basegfx/raster/bzpixelraster.hxx b/include/basegfx/raster/bzpixelraster.hxx
index 3e0d51b..e48f32f 100644
--- a/include/basegfx/raster/bzpixelraster.hxx
+++ b/include/basegfx/raster/bzpixelraster.hxx
@@ -41,10 +41,6 @@ namespace basegfx
memset(mpZBuffer.get(), 0, sizeof(sal_uInt16) * mnCount);
}
- ~BZPixelRaster()
- {
- }
-
// data access read only
const sal_uInt16& getZ(sal_uInt32 nIndex) const
{
diff --git a/shell/source/sessioninstall/SyncDbusSessionHelper.hxx b/shell/source/sessioninstall/SyncDbusSessionHelper.hxx
index 2bb0ff7..731b649 100644
--- a/shell/source/sessioninstall/SyncDbusSessionHelper.hxx
+++ b/shell/source/sessioninstall/SyncDbusSessionHelper.hxx
@@ -21,7 +21,6 @@ namespace shell { namespace sessioninstall
{
public:
SyncDbusSessionHelper(css::uno::Reference< css::uno::XComponentContext> const&);
- virtual ~SyncDbusSessionHelper() override {}
// XModify Methods
virtual void SAL_CALL InstallPackageFiles( ::sal_uInt32 xid, const css::uno::Sequence< OUString >& files, const OUString& interaction ) override;
diff --git a/xmlscript/source/xmldlg_imexp/imp_share.hxx b/xmlscript/source/xmldlg_imexp/imp_share.hxx
index 1859889..c37174b 100644
--- a/xmlscript/source/xmldlg_imexp/imp_share.hxx
+++ b/xmlscript/source/xmldlg_imexp/imp_share.hxx
@@ -480,9 +480,6 @@ public:
xProps,
rControlName )
{}
- inline ~ControlImportContext()
- {
- }
/// @throws css::xml::sax::SAXException
/// @throws css::uno::RuntimeException
More information about the Libreoffice-commits
mailing list