[clang-tools-extra] [clang-tidy] Report virtual-class-destructor at the destructor location (PR #227198)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Sep 28 23:46:13 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tools-extra
Author: Daniel Petrovic (daniel-petrovic)
<details>
<summary>Changes</summary>
Fixes #<!-- -->224686
---
Full diff: https://github.com/llvm/llvm-project/pull/227198.diff
3 Files Affected:
- (modified) clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp (+13-7)
- (modified) clang-tools-extra/docs/ReleaseNotes.md (+7)
- (modified) clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/virtual-class-destructor.cpp (+31-31)
``````````diff
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
index e43a652ed9d0c..3b5564e3ae9a7 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
@@ -174,15 +174,21 @@ void VirtualClassDestructorCheck::check(
if (!Destructor)
return;
+ const bool HasUserDeclaredDtor =
+ MatchedClassOrStruct->hasUserDeclaredDestructor();
+
+ const SourceLocation DiagLoc = HasUserDeclaredDtor
+ ? Destructor->getLocation()
+ : MatchedClassOrStruct->getLocation();
+
if (Destructor->getAccess() == AccessSpecifier::AS_private) {
- diag(MatchedClassOrStruct->getLocation(),
- "destructor of %0 is private and prevents using the type")
+ diag(DiagLoc, "destructor of %0 is private and prevents using the type")
<< MatchedClassOrStruct;
- diag(MatchedClassOrStruct->getLocation(),
+ diag(DiagLoc,
/*Description=*/"make it public and virtual", DiagnosticIDs::Note)
<< changePrivateDestructorVisibilityTo(
"public", *Destructor, *Result.SourceManager, getLangOpts());
- diag(MatchedClassOrStruct->getLocation(),
+ diag(DiagLoc,
/*Description=*/"make it protected", DiagnosticIDs::Note)
<< changePrivateDestructorVisibilityTo(
"protected", *Destructor, *Result.SourceManager, getLangOpts());
@@ -194,7 +200,7 @@ void VirtualClassDestructorCheck::check(
bool ProtectedAndVirtual = false;
FixItHint Fix;
- if (MatchedClassOrStruct->hasUserDeclaredDestructor()) {
+ if (HasUserDeclaredDtor) {
if (Destructor->getAccess() == AccessSpecifier::AS_public) {
Fix = FixItHint::CreateInsertion(Destructor->getLocation(), "virtual ");
} else if (Destructor->getAccess() == AccessSpecifier::AS_protected) {
@@ -209,11 +215,11 @@ void VirtualClassDestructorCheck::check(
*Result.SourceManager);
}
- diag(MatchedClassOrStruct->getLocation(),
+ diag(DiagLoc,
"destructor of %0 is %select{public and non-virtual|protected and "
"virtual}1")
<< MatchedClassOrStruct << ProtectedAndVirtual;
- diag(MatchedClassOrStruct->getLocation(),
+ diag(DiagLoc,
"make it %select{public and virtual|protected and non-virtual}0",
DiagnosticIDs::Note)
<< ProtectedAndVirtual << Fix;
diff --git a/clang-tools-extra/docs/ReleaseNotes.md b/clang-tools-extra/docs/ReleaseNotes.md
index 833638a47abc6..3eb7ea7b4c0ba 100644
--- a/clang-tools-extra/docs/ReleaseNotes.md
+++ b/clang-tools-extra/docs/ReleaseNotes.md
@@ -200,6 +200,13 @@ infrastructure are described first, followed by tool-specific sections.
- Improved {doc}`cppcoreguidelines-use-enum-class
<clang-tidy/checks/cppcoreguidelines/use-enum-class>` check by omitting unnamed enums from the `enum class` requirement, as previously the check suggested users an ill-formed fix.
+- Improved {doc}`cppcoreguidelines-virtual-class-destructor
+ <clang-tidy/checks/cppcoreguidelines/virtual-class-destructor>` check by
+ emitting the diagnostic and its fix-it notes at the destructor's location
+ instead of the class name, whenever the destructor is user-declared. The
+ diagnostics are still emitted at the class name for implicitly declared
+ destructors.
+
- Improved {doc}`misc-const-correctness
<clang-tidy/checks/misc/const-correctness>` check:
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
index 725a7094a0f17..9cfde2e02b3d0 100644
--- 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
@@ -1,8 +1,8 @@
// 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
+// CHECK-MESSAGES: :[[@LINE+8]]:11: warning: destructor of 'PrivateVirtualBaseStruct' is private and prevents using the type [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+7]]:11: note: make it public and virtual
+// CHECK-MESSAGES: :[[@LINE+6]]:11: note: make it protected
// As we have 2 conflicting fixes in notes, no fix is applied.
struct PrivateVirtualBaseStruct {
virtual void f();
@@ -16,8 +16,8 @@ struct PublicVirtualBaseStruct { // OK
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
+// CHECK-MESSAGES: :[[@LINE+6]]:11: warning: destructor of 'ProtectedVirtualBaseStruct' is protected and virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+5]]:11: note: make it protected and non-virtual
struct ProtectedVirtualBaseStruct {
virtual void f();
@@ -26,8 +26,8 @@ struct 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
+// CHECK-MESSAGES: :[[@LINE+6]]:11: warning: destructor of 'ProtectedVirtualDefaultBaseStruct' is protected and virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+5]]:11: note: make it protected and non-virtual
struct ProtectedVirtualDefaultBaseStruct {
virtual void f();
@@ -36,9 +36,9 @@ struct ProtectedVirtualDefaultBaseStruct {
// 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
+// CHECK-MESSAGES: :[[@LINE+8]]:3: warning: destructor of 'PrivateNonVirtualBaseStruct' is private and prevents using the type [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+7]]:3: note: make it public and virtual
+// CHECK-MESSAGES: :[[@LINE+6]]:3: note: make it protected
// As we have 2 conflicting fixes in notes, no fix is applied.
struct PrivateNonVirtualBaseStruct {
virtual void f();
@@ -47,8 +47,8 @@ struct PrivateNonVirtualBaseStruct {
~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
+// CHECK-MESSAGES: :[[@LINE+4]]:3: warning: destructor of 'PublicNonVirtualBaseStruct' is public and non-virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+3]]:3: note: make it public and virtual
struct PublicNonVirtualBaseStruct {
virtual void f();
~PublicNonVirtualBaseStruct() {}
@@ -85,9 +85,9 @@ struct ProtectedNonVirtualBaseStruct { // OK
~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
+// CHECK-MESSAGES: :[[@LINE+6]]:11: warning: destructor of 'PrivateVirtualBaseClass' is private and prevents using the type [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+5]]:11: note: make it public and virtual
+// CHECK-MESSAGES: :[[@LINE+4]]:11: note: make it protected
// As we have 2 conflicting fixes in notes, no fix is applied.
class PrivateVirtualBaseClass {
virtual void f();
@@ -101,8 +101,8 @@ class PublicVirtualBaseClass { // OK
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
+// CHECK-MESSAGES: :[[@LINE+6]]:11: warning: destructor of 'ProtectedVirtualBaseClass' is protected and virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+5]]:11: note: make it protected and non-virtual
class ProtectedVirtualBaseClass {
virtual void f();
@@ -133,8 +133,8 @@ class PublicASImplicitNonVirtualBaseClass {
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
+// CHECK-MESSAGES: :[[@LINE+6]]:3: warning: destructor of 'PublicNonVirtualBaseClass' is public and non-virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+5]]:3: note: make it public and virtual
class PublicNonVirtualBaseClass {
virtual void f();
@@ -275,44 +275,44 @@ namespace macro_tests {
#define MY_VIRTUAL virtual
#define CONCAT(x, y) x##y
-// CHECK-MESSAGES: :[[@LINE+2]]:7: warning: destructor of 'FooBar1' is protected and virtual [cppcoreguidelines-virtual-class-destructor]
-// CHECK-MESSAGES: :[[@LINE+1]]:7: note: make it protected and non-virtual
+// CHECK-MESSAGES: :[[@LINE+4]]:28: warning: destructor of 'FooBar1' is protected and virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+3]]:28: note: make it protected and non-virtual
class FooBar1 {
protected:
CONCAT(vir, tual) CONCAT(~Foo, Bar1()); // no-fixit
};
-// CHECK-MESSAGES: :[[@LINE+2]]:7: warning: destructor of 'FooBar2' is protected and virtual [cppcoreguidelines-virtual-class-destructor]
-// CHECK-MESSAGES: :[[@LINE+1]]:7: note: make it protected and non-virtual
+// CHECK-MESSAGES: :[[@LINE+4]]:18: warning: destructor of 'FooBar2' is protected and virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+3]]:18: note: make it protected and non-virtual
class FooBar2 {
protected:
virtual CONCAT(~Foo, Bar2()); // FIXME: We should have a fixit for this.
};
-// CHECK-MESSAGES: :[[@LINE+2]]:7: warning: destructor of 'FooBar3' is protected and virtual [cppcoreguidelines-virtual-class-destructor]
-// CHECK-MESSAGES: :[[@LINE+1]]:7: note: make it protected and non-virtual
+// CHECK-MESSAGES: :[[@LINE+4]]:21: warning: destructor of 'FooBar3' is protected and virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+3]]:21: note: make it protected and non-virtual
class FooBar3 {
protected:
CONCAT(vir, tual) ~FooBar3(); // FIXME: We should have a fixit for this.
};
-// CHECK-MESSAGES: :[[@LINE+2]]:7: warning: destructor of 'FooBar4' is protected and virtual [cppcoreguidelines-virtual-class-destructor]
-// CHECK-MESSAGES: :[[@LINE+1]]:7: note: make it protected and non-virtual
+// CHECK-MESSAGES: :[[@LINE+4]]:21: warning: destructor of 'FooBar4' is protected and virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+3]]:21: note: make it protected and non-virtual
class FooBar4 {
protected:
CONCAT(vir, tual) ~CONCAT(Foo, Bar4()); // FIXME: We should have a fixit for this.
};
-// CHECK-MESSAGES: :[[@LINE+3]]:7: warning: destructor of 'FooBar5' is protected and virtual [cppcoreguidelines-virtual-class-destructor]
-// CHECK-MESSAGES: :[[@LINE+2]]:7: note: make it protected and non-virtual
+// CHECK-MESSAGES: :[[@LINE+5]]:29: warning: destructor of 'FooBar5' is protected and virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+4]]:29: note: make it protected and non-virtual
#define XMACRO(COLUMN1, COLUMN2) COLUMN1 COLUMN2
class FooBar5 {
protected:
XMACRO(CONCAT(vir, tual), ~CONCAT(Foo, Bar5());) // no-crash, no-fixit
};
-// CHECK-MESSAGES: :[[@LINE+2]]:7: warning: destructor of 'FooBar6' is protected and virtual [cppcoreguidelines-virtual-class-destructor]
-// CHECK-MESSAGES: :[[@LINE+1]]:7: note: make it protected and non-virtual
+// CHECK-MESSAGES: :[[@LINE+4]]:14: warning: destructor of 'FooBar6' is protected and virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+3]]:14: note: make it protected and non-virtual
class FooBar6 {
protected:
MY_VIRTUAL ~FooBar6(); // FIXME: We should have a fixit for this.
``````````
</details>
https://github.com/llvm/llvm-project/pull/227198
More information about the cfe-commits
mailing list