[clang-tools-extra] c58c7a6 - [clang-tidy] cppcoreguidelines-virtual-base-class-destructor: a new check
via cfe-commits
cfe-commits at lists.llvm.org
Thu Sep 9 04:24:16 PDT 2021
Author: Marco Gartmann
Date: 2021-09-09T13:23:38+02:00
New Revision: c58c7a6ea0535a75e382752b37cb68ccea43cde3
URL: https://github.com/llvm/llvm-project/commit/c58c7a6ea0535a75e382752b37cb68ccea43cde3
DIFF: https://github.com/llvm/llvm-project/commit/c58c7a6ea0535a75e382752b37cb68ccea43cde3.diff
LOG: [clang-tidy] cppcoreguidelines-virtual-base-class-destructor: a new check
Finds base classes and structs whose destructor is neither public and
virtual nor protected and non-virtual.
A base class's destructor should be specified in one of these ways to
prevent undefined behaviour.
Fixes are available for user-declared and implicit destructors that are
either public and non-virtual or protected and virtual.
This check implements C.35 [1] from the CppCoreGuidelines.
Reviewed By: aaron.ballman, njames93
Differential Revision: http://reviews.llvm.org/D102325
[1]: http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rc-dtor-virtual
Added:
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.h
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-virtual-class-destructor.rst
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
Modified:
clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt b/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
index a9f5b3e0c15bc..745eeb8d49f7b 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
@@ -26,6 +26,7 @@ add_clang_library(clangTidyCppCoreGuidelinesModule
ProTypeVarargCheck.cpp
SlicingCheck.cpp
SpecialMemberFunctionsCheck.cpp
+ VirtualClassDestructorCheck.cpp
LINK_LIBS
clangTidy
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
index a16c2d53b56a2..c7b73dd675792 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
@@ -35,6 +35,7 @@
#include "ProTypeVarargCheck.h"
#include "SlicingCheck.h"
#include "SpecialMemberFunctionsCheck.h"
+#include "VirtualClassDestructorCheck.h"
namespace clang {
namespace tidy {
@@ -94,6 +95,8 @@ class CppCoreGuidelinesModule : public ClangTidyModule {
CheckFactories.registerCheck<SlicingCheck>("cppcoreguidelines-slicing");
CheckFactories.registerCheck<misc::UnconventionalAssignOperatorCheck>(
"cppcoreguidelines-c-copy-assignment-signature");
+ CheckFactories.registerCheck<VirtualClassDestructorCheck>(
+ "cppcoreguidelines-virtual-class-destructor");
}
ClangTidyOptions getModuleOptions() override {
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
new file mode 100644
index 0000000000000..d9c8ecf9d033a
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
@@ -0,0 +1,200 @@
+//===--- VirtualClassDestructorCheck.cpp - clang-tidy -----------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "VirtualClassDestructorCheck.h"
+#include "../utils/LexerUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+#include <string>
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+void VirtualClassDestructorCheck::registerMatchers(MatchFinder *Finder) {
+ ast_matchers::internal::Matcher<CXXRecordDecl> InheritsVirtualMethod =
+ hasAnyBase(hasType(cxxRecordDecl(has(cxxMethodDecl(isVirtual())))));
+
+ Finder->addMatcher(
+ cxxRecordDecl(
+ anyOf(has(cxxMethodDecl(isVirtual())), InheritsVirtualMethod),
+ unless(anyOf(
+ has(cxxDestructorDecl(isPublic(), isVirtual())),
+ has(cxxDestructorDecl(isProtected(), unless(isVirtual()))))))
+ .bind("ProblematicClassOrStruct"),
+ this);
+}
+
+static CharSourceRange
+getVirtualKeywordRange(const CXXDestructorDecl &Destructor,
+ const SourceManager &SM, const LangOptions &LangOpts) {
+ SourceLocation VirtualBeginLoc = Destructor.getBeginLoc();
+ SourceLocation VirtualEndLoc = VirtualBeginLoc.getLocWithOffset(
+ Lexer::MeasureTokenLength(VirtualBeginLoc, SM, LangOpts));
+
+ /// Range ends with \c StartOfNextToken so that any whitespace after \c
+ /// virtual is included.
+ SourceLocation StartOfNextToken =
+ Lexer::findNextToken(VirtualEndLoc, SM, LangOpts)
+ .getValue()
+ .getLocation();
+
+ return CharSourceRange::getCharRange(VirtualBeginLoc, StartOfNextToken);
+}
+
+static const AccessSpecDecl *
+getPublicASDecl(const CXXRecordDecl &StructOrClass) {
+ for (DeclContext::specific_decl_iterator<AccessSpecDecl>
+ AS{StructOrClass.decls_begin()},
+ ASEnd{StructOrClass.decls_end()};
+ AS != ASEnd; ++AS) {
+ AccessSpecDecl *ASDecl = *AS;
+ if (ASDecl->getAccess() == AccessSpecifier::AS_public)
+ return ASDecl;
+ }
+
+ return nullptr;
+}
+
+static FixItHint
+generateUserDeclaredDestructor(const CXXRecordDecl &StructOrClass,
+ const SourceManager &SourceManager) {
+ std::string DestructorString;
+ SourceLocation Loc;
+ bool AppendLineBreak = false;
+
+ const AccessSpecDecl *AccessSpecDecl = getPublicASDecl(StructOrClass);
+
+ if (!AccessSpecDecl) {
+ if (StructOrClass.isClass()) {
+ Loc = StructOrClass.getEndLoc();
+ DestructorString = "public:";
+ AppendLineBreak = true;
+ } else {
+ Loc = StructOrClass.getBraceRange().getBegin().getLocWithOffset(1);
+ }
+ } else {
+ Loc = AccessSpecDecl->getEndLoc().getLocWithOffset(1);
+ }
+
+ DestructorString = (llvm::Twine(DestructorString) + "\nvirtual ~" +
+ StructOrClass.getName().str() + "() = default;" +
+ (AppendLineBreak ? "\n" : ""))
+ .str();
+
+ return FixItHint::CreateInsertion(Loc, DestructorString);
+}
+
+static std::string getSourceText(const CXXDestructorDecl &Destructor) {
+ std::string SourceText;
+ llvm::raw_string_ostream DestructorStream(SourceText);
+ Destructor.print(DestructorStream);
+ return SourceText;
+}
+
+static std::string eraseKeyword(std::string &DestructorString,
+ const std::string &Keyword) {
+ size_t KeywordIndex = DestructorString.find(Keyword);
+ if (KeywordIndex != std::string::npos)
+ DestructorString.erase(KeywordIndex, Keyword.length());
+ return DestructorString;
+}
+
+static FixItHint changePrivateDestructorVisibilityTo(
+ const std::string &Visibility, const CXXDestructorDecl &Destructor,
+ const SourceManager &SM, const LangOptions &LangOpts) {
+ std::string DestructorString =
+ (llvm::Twine() + Visibility + ":\n" +
+ (Visibility == "public" && !Destructor.isVirtual() ? "virtual " : ""))
+ .str();
+
+ std::string OriginalDestructor = getSourceText(Destructor);
+ if (Visibility == "protected" && Destructor.isVirtualAsWritten())
+ OriginalDestructor = eraseKeyword(OriginalDestructor, "virtual ");
+
+ DestructorString =
+ (llvm::Twine(DestructorString) + OriginalDestructor +
+ (Destructor.isExplicitlyDefaulted() ? ";\n" : "") + "private:")
+ .str();
+
+ /// Semicolons ending an explicitly defaulted destructor have to be deleted.
+ /// Otherwise, the left-over semicolon trails the \c private: access
+ /// specifier.
+ SourceLocation EndLocation;
+ if (Destructor.isExplicitlyDefaulted())
+ EndLocation =
+ utils::lexer::findNextTerminator(Destructor.getEndLoc(), SM, LangOpts)
+ .getLocWithOffset(1);
+ else
+ EndLocation = Destructor.getEndLoc().getLocWithOffset(1);
+
+ auto OriginalDestructorRange =
+ CharSourceRange::getCharRange(Destructor.getBeginLoc(), EndLocation);
+ return FixItHint::CreateReplacement(OriginalDestructorRange,
+ DestructorString);
+}
+
+void VirtualClassDestructorCheck::check(
+ const MatchFinder::MatchResult &Result) {
+
+ const auto *MatchedClassOrStruct =
+ Result.Nodes.getNodeAs<CXXRecordDecl>("ProblematicClassOrStruct");
+
+ const CXXDestructorDecl *Destructor = MatchedClassOrStruct->getDestructor();
+ if (!Destructor)
+ return;
+
+ if (Destructor->getAccess() == AccessSpecifier::AS_private) {
+ diag(MatchedClassOrStruct->getLocation(),
+ "destructor of %0 is private and prevents using the type")
+ << MatchedClassOrStruct;
+ diag(MatchedClassOrStruct->getLocation(),
+ /*FixDescription=*/"make it public and virtual", DiagnosticIDs::Note)
+ << changePrivateDestructorVisibilityTo(
+ "public", *Destructor, *Result.SourceManager, getLangOpts());
+ diag(MatchedClassOrStruct->getLocation(),
+ /*FixDescription=*/"make it protected", DiagnosticIDs::Note)
+ << changePrivateDestructorVisibilityTo(
+ "protected", *Destructor, *Result.SourceManager, getLangOpts());
+
+ return;
+ }
+
+ // Implicit destructors are public and non-virtual for classes and structs.
+ bool ProtectedAndVirtual = false;
+ FixItHint Fix;
+
+ if (MatchedClassOrStruct->hasUserDeclaredDestructor()) {
+ if (Destructor->getAccess() == AccessSpecifier::AS_public) {
+ Fix = FixItHint::CreateInsertion(Destructor->getLocation(), "virtual ");
+ } else if (Destructor->getAccess() == AccessSpecifier::AS_protected) {
+ ProtectedAndVirtual = true;
+ Fix = FixItHint::CreateRemoval(getVirtualKeywordRange(
+ *Destructor, *Result.SourceManager, Result.Context->getLangOpts()));
+ }
+ } else {
+ Fix = generateUserDeclaredDestructor(*MatchedClassOrStruct,
+ *Result.SourceManager);
+ }
+
+ diag(MatchedClassOrStruct->getLocation(),
+ "destructor of %0 is %select{public and non-virtual|protected and "
+ "virtual}1")
+ << MatchedClassOrStruct << ProtectedAndVirtual;
+ diag(MatchedClassOrStruct->getLocation(),
+ "make it %select{public and virtual|protected and non-virtual}0",
+ DiagnosticIDs::Note)
+ << ProtectedAndVirtual << Fix;
+}
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.h
new file mode 100644
index 0000000000000..4772a8063859c
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.h
@@ -0,0 +1,41 @@
+//===--- VirtualClassDestructorCheck.h - clang-tidy -------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_VIRTUALCLASSDESTRUCTORCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_VIRTUALCLASSDESTRUCTORCHECK_H
+
+#include "../ClangTidyCheck.h"
+#include <string>
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+/// Finds base classes whose destructor is neither public and virtual
+/// nor protected and non-virtual.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-virtual-class-destructor.html
+class VirtualClassDestructorCheck : public ClangTidyCheck {
+public:
+ VirtualClassDestructorCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+ return LangOpts.CPlusPlus;
+ }
+
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_VIRTUALCLASSDESTRUCTORCHECK_H
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index c997b2b37a2f2..609064f7610b0 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -75,12 +75,21 @@ New checks
- New :doc:`bugprone-suspicious-memory-comparison
<clang-tidy/checks/bugprone-suspicious-memory-comparison>` check.
- Finds potentially incorrect calls to ``memcmp()`` based on properties of the arguments.
+ Finds potentially incorrect calls to ``memcmp()`` based on properties of the
+ arguments.
+
+- New :doc:`cppcoreguidelines-virtual-class-destructor
+ <clang-tidy/checks/cppcoreguidelines-virtual-class-destructor>` check.
+
+ Finds virtual classes whose destructor is neither public and virtual nor
+ protected and non-virtual.
- New :doc:`readability-identifier-length
<clang-tidy/checks/readability-identifier-length>` check.
- Reports identifiers whose names are too short. Currently checks local variables and function parameters only.
+ Reports identifiers whose names are too short. Currently checks local
+ variables and function parameters only.
+
New check aliases
^^^^^^^^^^^^^^^^^
diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-virtual-class-destructor.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-virtual-class-destructor.rst
new file mode 100644
index 0000000000000..48a04e54300c7
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-virtual-class-destructor.rst
@@ -0,0 +1,57 @@
+.. title:: clang-tidy - cppcoreguidelines-virtual-class-destructor
+
+cppcoreguidelines-virtual-class-destructor
+===============================================
+
+Finds virtual classes whose destructor is neither public and virtual
+nor protected and non-virtual. A virtual class's destructor should be specified
+in one of these ways to prevent undefined behaviour.
+
+This check implements
+`C.35 <http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rc-dtor-virtual>`_
+from the CppCoreGuidelines.
+
+Note that this check will diagnose a class with a virtual method regardless of
+whether the class is used as a base class or not.
+
+Fixes are available for user-declared and implicit destructors that are either
+public and non-virtual or protected and virtual. No fixes are offered for
+private destructors. There, the decision whether to make them private and
+virtual or protected and non-virtual depends on the use case and is thus left
+to the user.
+
+Example
+-------
+
+For example, the following classes/structs get flagged by the check since they
+violate guideline **C.35**:
+
+.. code-block:: c++
+
+ struct Foo { // NOK, protected destructor should not be virtual
+ virtual void f();
+ protected:
+ virtual ~Foo(){}
+ };
+
+ class Bar { // NOK, public destructor should be virtual
+ virtual void f();
+ public:
+ ~Bar(){}
+ };
+
+This would be rewritten to look like this:
+
+.. code-block:: c++
+
+ struct Foo { // OK, destructor is not virtual anymore
+ virtual void f();
+ protected:
+ ~Foo(){}
+ };
+
+ class Bar { // OK, destructor is now virtual
+ virtual void f();
+ public:
+ virtual ~Bar(){}
+ };
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 7ba7181c5db0b..862817a9b5f68 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -169,6 +169,7 @@ Clang-Tidy Checks
`cppcoreguidelines-pro-type-vararg <cppcoreguidelines-pro-type-vararg.html>`_,
`cppcoreguidelines-slicing <cppcoreguidelines-slicing.html>`_,
`cppcoreguidelines-special-member-functions <cppcoreguidelines-special-member-functions.html>`_,
+ `cppcoreguidelines-virtual-class-destructor <cppcoreguidelines-virtual-class-destructor.html>`_, "Yes"
`darwin-avoid-spinlock <darwin-avoid-spinlock.html>`_,
`darwin-dispatch-once-nonstatic <darwin-dispatch-once-nonstatic.html>`_, "Yes"
`fuchsia-default-arguments-calls <fuchsia-default-arguments-calls.html>`_,
diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
new file mode 100644
index 0000000000000..0aa5723b81afa
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
@@ -0,0 +1,204 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-virtual-class-destructor %t -- --fix-notes
+
+// CHECK-MESSAGES: :[[@LINE+4]]:8: warning: destructor of 'PrivateVirtualBaseStruct' is private and prevents using the type [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+3]]:8: note: make it public and virtual
+// CHECK-MESSAGES: :[[@LINE+2]]:8: note: make it protected
+// As we have 2 conflicting fixes in notes, no fix is applied.
+struct PrivateVirtualBaseStruct {
+ virtual void f();
+
+private:
+ virtual ~PrivateVirtualBaseStruct() {}
+};
+
+struct PublicVirtualBaseStruct { // OK
+ virtual void f();
+ virtual ~PublicVirtualBaseStruct() {}
+};
+
+// CHECK-MESSAGES: :[[@LINE+2]]:8: warning: destructor of 'ProtectedVirtualBaseStruct' is protected and virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+1]]:8: note: make it protected and non-virtual
+struct ProtectedVirtualBaseStruct {
+ virtual void f();
+
+protected:
+ virtual ~ProtectedVirtualBaseStruct() {}
+ // CHECK-FIXES: ~ProtectedVirtualBaseStruct() {}
+};
+
+// CHECK-MESSAGES: :[[@LINE+2]]:8: warning: destructor of 'ProtectedVirtualDefaultBaseStruct' is protected and virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+1]]:8: note: make it protected and non-virtual
+struct ProtectedVirtualDefaultBaseStruct {
+ virtual void f();
+
+protected:
+ virtual ~ProtectedVirtualDefaultBaseStruct() = default;
+ // CHECK-FIXES: ~ProtectedVirtualDefaultBaseStruct() = default;
+};
+
+// CHECK-MESSAGES: :[[@LINE+4]]:8: warning: destructor of 'PrivateNonVirtualBaseStruct' is private and prevents using the type [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+3]]:8: note: make it public and virtual
+// CHECK-MESSAGES: :[[@LINE+2]]:8: note: make it protected
+// As we have 2 conflicting fixes in notes, no fix is applied.
+struct PrivateNonVirtualBaseStruct {
+ virtual void f();
+
+private:
+ ~PrivateNonVirtualBaseStruct() {}
+};
+
+// CHECK-MESSAGES: :[[@LINE+2]]:8: warning: destructor of 'PublicNonVirtualBaseStruct' is public and non-virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+1]]:8: note: make it public and virtual
+struct PublicNonVirtualBaseStruct {
+ virtual void f();
+ ~PublicNonVirtualBaseStruct() {}
+ // CHECK-FIXES: virtual ~PublicNonVirtualBaseStruct() {}
+};
+
+struct PublicNonVirtualNonBaseStruct { // OK according to C.35, since this struct does not have any virtual methods.
+ void f();
+ ~PublicNonVirtualNonBaseStruct() {}
+};
+
+// CHECK-MESSAGES: :[[@LINE+4]]:8: warning: destructor of 'PublicImplicitNonVirtualBaseStruct' is public and non-virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+3]]:8: note: make it public and virtual
+// CHECK-FIXES: struct PublicImplicitNonVirtualBaseStruct {
+// CHECK-FIXES-NEXT: virtual ~PublicImplicitNonVirtualBaseStruct() = default;
+struct PublicImplicitNonVirtualBaseStruct {
+ virtual void f();
+};
+
+// CHECK-MESSAGES: :[[@LINE+5]]:8: warning: destructor of 'PublicASImplicitNonVirtualBaseStruct' is public and non-virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+4]]:8: note: make it public and virtual
+// CHECK-FIXES: struct PublicASImplicitNonVirtualBaseStruct {
+// CHECK-FIXES-NEXT: virtual ~PublicASImplicitNonVirtualBaseStruct() = default;
+// CHECK-FIXES-NEXT: private:
+struct PublicASImplicitNonVirtualBaseStruct {
+private:
+ virtual void f();
+};
+
+struct ProtectedNonVirtualBaseStruct { // OK
+ virtual void f();
+
+protected:
+ ~ProtectedNonVirtualBaseStruct() {}
+};
+
+// CHECK-MESSAGES: :[[@LINE+4]]:7: warning: destructor of 'PrivateVirtualBaseClass' is private and prevents using the type [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+3]]:7: note: make it public and virtual
+// CHECK-MESSAGES: :[[@LINE+2]]:7: note: make it protected
+// As we have 2 conflicting fixes in notes, no fix is applied.
+class PrivateVirtualBaseClass {
+ virtual void f();
+ virtual ~PrivateVirtualBaseClass() {}
+};
+
+class PublicVirtualBaseClass { // OK
+ virtual void f();
+
+public:
+ virtual ~PublicVirtualBaseClass() {}
+};
+
+// CHECK-MESSAGES: :[[@LINE+2]]:7: warning: destructor of 'ProtectedVirtualBaseClass' is protected and virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+1]]:7: note: make it protected and non-virtual
+class ProtectedVirtualBaseClass {
+ virtual void f();
+
+protected:
+ virtual ~ProtectedVirtualBaseClass() {}
+ // CHECK-FIXES: ~ProtectedVirtualBaseClass() {}
+};
+
+// CHECK-MESSAGES: :[[@LINE+5]]:7: warning: destructor of 'PublicImplicitNonVirtualBaseClass' is public and non-virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+4]]:7: note: make it public and virtual
+// CHECK-FIXES: public:
+// CHECK-FIXES-NEXT: virtual ~PublicImplicitNonVirtualBaseClass() = default;
+// CHECK-FIXES-NEXT: };
+class PublicImplicitNonVirtualBaseClass {
+ virtual void f();
+};
+
+// CHECK-MESSAGES: :[[@LINE+6]]:7: warning: destructor of 'PublicASImplicitNonVirtualBaseClass' is public and non-virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+5]]:7: note: make it public and virtual
+// CHECK-FIXES: public:
+// CHECK-FIXES-NEXT: virtual ~PublicASImplicitNonVirtualBaseClass() = default;
+// CHECK-FIXES-NEXT: int foo = 42;
+// CHECK-FIXES-NEXT: };
+class PublicASImplicitNonVirtualBaseClass {
+ virtual void f();
+
+public:
+ int foo = 42;
+};
+
+// CHECK-MESSAGES: :[[@LINE+2]]:7: warning: destructor of 'PublicNonVirtualBaseClass' is public and non-virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+1]]:7: note: make it public and virtual
+class PublicNonVirtualBaseClass {
+ virtual void f();
+
+public:
+ ~PublicNonVirtualBaseClass() {}
+ // CHECK-FIXES: virtual ~PublicNonVirtualBaseClass() {}
+};
+
+class PublicNonVirtualNonBaseClass { // OK accoring to C.35, since this class does not have any virtual methods.
+ void f();
+
+public:
+ ~PublicNonVirtualNonBaseClass() {}
+};
+
+class ProtectedNonVirtualClass { // OK
+public:
+ virtual void f();
+
+protected:
+ ~ProtectedNonVirtualClass() {}
+};
+
+// CHECK-MESSAGES: :[[@LINE+7]]:7: warning: destructor of 'OverridingDerivedClass' is public and non-virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+6]]:7: note: make it public and virtual
+// CHECK-FIXES: class OverridingDerivedClass : ProtectedNonVirtualClass {
+// CHECK-FIXES-NEXT: public:
+// CHECK-FIXES-NEXT: virtual ~OverridingDerivedClass() = default;
+// CHECK-FIXES-NEXT: void f() override;
+// CHECK-FIXES-NEXT: };
+class OverridingDerivedClass : ProtectedNonVirtualClass {
+public:
+ void f() override; // is implicitly virtual
+};
+
+// CHECK-MESSAGES: :[[@LINE+7]]:7: warning: destructor of 'NonOverridingDerivedClass' is public and non-virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+6]]:7: note: make it public and virtual
+// CHECK-FIXES: class NonOverridingDerivedClass : ProtectedNonVirtualClass {
+// CHECK-FIXES-NEXT: void m();
+// CHECK-FIXES-NEXT: public:
+// CHECK-FIXES-NEXT: virtual ~NonOverridingDerivedClass() = default;
+// CHECK-FIXES-NEXT: };
+class NonOverridingDerivedClass : ProtectedNonVirtualClass {
+ void m();
+};
+// inherits virtual method
+
+// CHECK-MESSAGES: :[[@LINE+6]]:8: warning: destructor of 'OverridingDerivedStruct' is public and non-virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+5]]:8: note: make it public and virtual
+// CHECK-FIXES: struct OverridingDerivedStruct : ProtectedNonVirtualBaseStruct {
+// CHECK-FIXES-NEXT: virtual ~OverridingDerivedStruct() = default;
+// CHECK-FIXES-NEXT: void f() override;
+// CHECK-FIXES-NEXT: };
+struct OverridingDerivedStruct : ProtectedNonVirtualBaseStruct {
+ void f() override; // is implicitly virtual
+};
+
+// CHECK-MESSAGES: :[[@LINE+6]]:8: warning: destructor of 'NonOverridingDerivedStruct' is public and non-virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+5]]:8: note: make it public and virtual
+// CHECK-FIXES: struct NonOverridingDerivedStruct : ProtectedNonVirtualBaseStruct {
+// CHECK-FIXES-NEXT: virtual ~NonOverridingDerivedStruct() = default;
+// CHECK-FIXES-NEXT: void m();
+// CHECK-FIXES-NEXT: };
+struct NonOverridingDerivedStruct : ProtectedNonVirtualBaseStruct {
+ void m();
+};
+// inherits virtual method
More information about the cfe-commits
mailing list