summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorStephan Bergmann <sbergman@redhat.com>2016-12-05 09:06:14 +0100
committerStephan Bergmann <sbergman@redhat.com>2016-12-05 09:06:14 +0100
commit03c215ab07c3dc91fd1fd08a9f3ea627fbdea002 (patch)
treea5ebc15eca690b9a1641ad12c782454bc215d97b
parentd12c8a0f2cedfb6a15742b51854492ec3829dc36 (diff)
loplugin:unnecessaryoverride (dtors)
Change-Id: Ia38672028668959bf3d5541fe4ddb9fb72848617
-rw-r--r--compilerplugins/clang/test/unnecessaryoverride-dtor.cxx121
-rw-r--r--compilerplugins/clang/test/unnecessaryoverride-dtor.hxx43
-rw-r--r--compilerplugins/clang/unnecessaryoverride.cxx84
-rw-r--r--solenv/CompilerTest_compilerplugins_clang.mk1
4 files changed, 247 insertions, 2 deletions
diff --git a/compilerplugins/clang/test/unnecessaryoverride-dtor.cxx b/compilerplugins/clang/test/unnecessaryoverride-dtor.cxx
new file mode 100644
index 000000000000..26b3a81abd68
--- /dev/null
+++ b/compilerplugins/clang/test/unnecessaryoverride-dtor.cxx
@@ -0,0 +1,121 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4; fill-column: 100 -*- */
+/*
+ * 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 <sal/config.h>
+
+#include <salhelper/simplereferenceobject.hxx>
+
+#include <unnecessaryoverride-dtor.hxx>
+
+struct NonVirtualBase {};
+
+struct NonVirtualDerived1: NonVirtualBase {
+ ~NonVirtualDerived1() {}
+};
+
+struct NonVirtualDerived2: NonVirtualBase {
+ virtual ~NonVirtualDerived2() {}
+};
+
+struct PrivateDerived: VirtualBase {
+private:
+ ~PrivateDerived() override {}
+};
+
+struct ProtectedDerived: VirtualBase {
+protected:
+ ~ProtectedDerived() override {}
+};
+
+IncludedDerived2::~IncludedDerived2() {}
+
+struct Incomplete: salhelper::SimpleReferenceObject {};
+
+IncludedDerived3::IncludedDerived3() {}
+
+IncludedDerived3::~IncludedDerived3() {}
+
+struct NoExSpecDerived: VirtualBase {
+ ~NoExSpecDerived() override {} // expected-error {{unnecessary user-declared destructor [loplugin:unnecessaryoverride]}}
+};
+
+struct NoThrowDerived: VirtualBase {
+ ~NoThrowDerived() throw () override {} // expected-error {{unnecessary user-declared destructor [loplugin:unnecessaryoverride]}}
+};
+
+struct NoexceptDerived: VirtualBase {
+ ~NoexceptDerived() noexcept override {} // expected-error {{unnecessary user-declared destructor [loplugin:unnecessaryoverride]}}
+};
+
+struct NoexceptTrueDerived: VirtualBase {
+ ~NoexceptTrueDerived() noexcept(true) override {} // expected-error {{unnecessary user-declared destructor [loplugin:unnecessaryoverride]}}
+};
+
+#if 0
+struct NoexceptFalseBase {
+ virtual ~NoexceptFalseBase() noexcept(false) {}
+};
+
+struct NoexceptFalseDerived: NoexceptFalseBase {
+ ~NoexceptFalseDerived() noexcept(false) override {}
+};
+#endif
+
+struct NoDtorDerived: VirtualBase {};
+
+struct DefaultDerived1: VirtualBase {
+ ~DefaultDerived1() override = default; // expected-error {{unnecessary user-declared destructor [loplugin:unnecessaryoverride]}}
+};
+
+struct DefaultDerived2: VirtualBase {
+ ~DefaultDerived2() override; // expected-note {{declared here [loplugin:unnecessaryoverride]}}
+};
+
+DefaultDerived2::~DefaultDerived2() = default; // expected-error {{unnecessary user-declared destructor [loplugin:unnecessaryoverride]}}
+
+struct EmptyDerived1: VirtualBase {
+ ~EmptyDerived1() override {}; // expected-error {{unnecessary user-declared destructor [loplugin:unnecessaryoverride]}}
+};
+
+struct EmptyDerived2: VirtualBase {
+ ~EmptyDerived2() override; // expected-note {{declared here [loplugin:unnecessaryoverride]}}
+};
+
+EmptyDerived2::~EmptyDerived2() {} // expected-error {{unnecessary user-declared destructor [loplugin:unnecessaryoverride]}}
+
+struct NonEmptyDerived: VirtualBase {
+ ~NonEmptyDerived() override { (void) 0; }
+};
+
+struct CatchDerived: VirtualBase {
+ ~CatchDerived() override try {} catch (...) {}
+};
+
+struct DeleteBase {
+ virtual ~DeleteBase() = delete;
+};
+
+struct DeleteDerived: DeleteBase {
+ ~DeleteDerived() override = delete;
+};
+
+struct PureBase {
+ virtual ~PureBase() = 0;
+};
+
+struct PureDerived: PureBase {
+ ~PureDerived() override {} // expected-error {{unnecessary user-declared destructor [loplugin:unnecessaryoverride]}}
+};
+
+// aovid loplugin:unreffun:
+int main() {
+ (void) NonVirtualDerived1();
+}
+
+/* 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
new file mode 100644
index 000000000000..fe1f9cb180c3
--- /dev/null
+++ b/compilerplugins/clang/test/unnecessaryoverride-dtor.hxx
@@ -0,0 +1,43 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4; fill-column: 100 -*- */
+/*
+ * 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/.
+ */
+
+#ifndef INCLUDED_COMPILERPLUGINS_CLANG_TEST_UNNECESSARYOVERRIDE_DTOR_HXX
+#define INCLUDED_COMPILERPLUGINS_CLANG_TEST_UNNECESSARYOVERRIDE_DTOR_HXX
+
+#include <sal/config.h>
+
+#include <rtl/ref.hxx>
+
+struct VirtualBase {
+ virtual ~VirtualBase() {}
+};
+
+struct IncludedDerived1: VirtualBase {
+ ~IncludedDerived1() override {};
+};
+
+struct IncludedDerived2: VirtualBase {
+ ~IncludedDerived2() override;
+};
+
+struct Incomplete;
+struct IncludedDerived3: VirtualBase {
+ IncludedDerived3();
+ ~IncludedDerived3() override;
+
+private:
+ IncludedDerived3(IncludedDerived3 &) = delete;
+ void operator =(IncludedDerived3) = delete;
+
+ rtl::Reference<Incomplete> m;
+};
+
+#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 64239a62746f..98b77d5a7b7a 100644
--- a/compilerplugins/clang/unnecessaryoverride.cxx
+++ b/compilerplugins/clang/unnecessaryoverride.cxx
@@ -71,10 +71,90 @@ private:
bool UnnecessaryOverride::VisitCXXMethodDecl(const CXXMethodDecl* methodDecl)
{
- if (ignoreLocation(methodDecl->getCanonicalDecl()) || !methodDecl->doesThisDeclarationHaveABody()) {
+ if (ignoreLocation(methodDecl->getCanonicalDecl())) {
return true;
}
- if (isa<CXXConstructorDecl>(methodDecl) || isa<CXXDestructorDecl>(methodDecl)) {
+ if (isa<CXXConstructorDecl>(methodDecl)) {
+ return true;
+ }
+
+ if (isa<CXXDestructorDecl>(methodDecl)) {
+ // Warn about unnecessarily user-declared overriding virtual
+ // destructors; such 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
+ // emission, nor is its definition depending on types that may still
+ // be incomplete);
+ // * 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
+ // have if it did not have an explicit one (TODO);
+ // * it is either defined as defaulted or with an empty body.
+ // Removing the user-declared destructor may cause the class to get an
+ // implicitly declared move constructor and/or move assignment operator;
+ // that is considered acceptable: If any subobject cannot be moved, the
+ // implicitly declared function will be defined as deleted (which is in
+ // practice not much different from not having it declared), and
+ // otherwise offering movability is likely even an improvement over not
+ // offering it due to a "pointless" user-declared destructor.
+ // Similarly, removing the user-declared destructor may cause the
+ // 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)
+ {
+ return true;
+ }
+ if (!compiler.getSourceManager().isInMainFile(
+ methodDecl->getCanonicalDecl()->getLocation()))
+ {
+ return true;
+ }
+ if (!methodDecl->isExplicitlyDefaulted()) {
+ if (!methodDecl->doesThisDeclarationHaveABody()) {
+ return true;
+ }
+ auto stmt = dyn_cast<CompoundStmt>(methodDecl->getBody());
+ if (stmt == nullptr || stmt->size() != 0) {
+ return true;
+ }
+ }
+ //TODO: exception specification
+ auto cls = methodDecl->getParent();
+ if (!(cls->hasUserDeclaredCopyConstructor()
+ || cls->hasUserDeclaredCopyAssignment()
+ || cls->hasUserDeclaredMoveConstructor()
+ || cls->hasUserDeclaredMoveAssignment()))
+ {
+ }
+ if ((cls->needsImplicitMoveConstructor()
+ && !(cls->hasUserDeclaredCopyConstructor()
+ || cls->hasUserDeclaredCopyAssignment()
+ || cls->hasUserDeclaredMoveAssignment()))
+ || (cls->needsImplicitMoveAssignment()
+ && !(cls->hasUserDeclaredCopyConstructor()
+ || cls->hasUserDeclaredCopyAssignment()
+ || cls->hasUserDeclaredMoveConstructor())))
+ {
+ report(DiagnosticsEngine::Fatal, "TODO", methodDecl->getLocation());
+ return true;
+ }
+ report(
+ DiagnosticsEngine::Warning, "unnecessary user-declared destructor",
+ methodDecl->getLocation())
+ << methodDecl->getSourceRange();
+ auto cd = methodDecl->getCanonicalDecl();
+ if (cd->getLocation() != methodDecl->getLocation()) {
+ report(DiagnosticsEngine::Note, "declared here", cd->getLocation())
+ << cd->getSourceRange();
+ }
+ return true;
+ }
+
+ if (!methodDecl->doesThisDeclarationHaveABody()) {
return true;
}
diff --git a/solenv/CompilerTest_compilerplugins_clang.mk b/solenv/CompilerTest_compilerplugins_clang.mk
index dc3a67ef7801..a4c0a15e6b8e 100644
--- a/solenv/CompilerTest_compilerplugins_clang.mk
+++ b/solenv/CompilerTest_compilerplugins_clang.mk
@@ -17,6 +17,7 @@ $(eval $(call gb_CompilerTest_add_exception_objects,compilerplugins_clang, \
compilerplugins/clang/test/oslendian-2 \
compilerplugins/clang/test/oslendian-3 \
compilerplugins/clang/test/salbool \
+ compilerplugins/clang/test/unnecessaryoverride-dtor \
compilerplugins/clang/test/vclwidgets \
))