steakhal updated this revision to Diff 435054.
steakhal marked an inline comment as done.
steakhal added a comment.
- rebase
- use double-backticks for the keyword `final` in the Release Notes.
polite ping
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D126891/new/
https://reviews.llvm.org/D126891
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
Index:
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
===================================================================
---
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
+++
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
@@ -320,3 +320,22 @@
#undef XMACRO
#undef CONCAT
} // namespace macro_tests
+
+namespace FinalClassCannotBeBaseClass {
+class Base {
+public:
+ Base() = default;
+ virtual void func() = 0;
+
+protected:
+ ~Base() = default;
+};
+
+// no-warning: 'MostDerived' cannot be a base class, since it's marked 'final'.
+class MostDerived final : public Base {
+public:
+ MostDerived() = default;
+ ~MostDerived() = default;
+ void func() final;
+};
+} // namespace FinalClassCannotBeBaseClass
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -165,6 +165,12 @@
Fixed an issue when there was already an initializer in the constructor and
the check would try to create another initializer for the same member.
+- Fixed a false positive in :doc:`cppcoreguidelines-virtual-class-destructor
+ <clang-tidy/checks/cppcoreguidelines-virtual-class-destructor>` involving
+ ``final`` classes. The check will not diagnose ``final`` marked classes,
since
+ those cannot be used as base classes, consequently they can not violate the
+ rule.
+
- Fixed a crash in :doc:`llvmlibc-callee-namespace
<clang-tidy/checks/llvmlibc-callee-namespace>` when executing for C++ code
that contain calls to advanced constructs, e.g. overloaded operators.
Index:
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
===================================================================
---
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
+++
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
@@ -41,6 +41,7 @@
Finder->addMatcher(
cxxRecordDecl(
anyOf(has(cxxMethodDecl(isVirtual())), InheritsVirtualMethod),
+ unless(isFinal()),
unless(hasPublicVirtualOrProtectedNonVirtualDestructor()))
.bind("ProblematicClassOrStruct"),
this);
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
@@ -320,3 +320,22 @@
#undef XMACRO
#undef CONCAT
} // namespace macro_tests
+
+namespace FinalClassCannotBeBaseClass {
+class Base {
+public:
+ Base() = default;
+ virtual void func() = 0;
+
+protected:
+ ~Base() = default;
+};
+
+// no-warning: 'MostDerived' cannot be a base class, since it's marked 'final'.
+class MostDerived final : public Base {
+public:
+ MostDerived() = default;
+ ~MostDerived() = default;
+ void func() final;
+};
+} // namespace FinalClassCannotBeBaseClass
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -165,6 +165,12 @@
Fixed an issue when there was already an initializer in the constructor and
the check would try to create another initializer for the same member.
+- Fixed a false positive in :doc:`cppcoreguidelines-virtual-class-destructor
+ <clang-tidy/checks/cppcoreguidelines-virtual-class-destructor>` involving
+ ``final`` classes. The check will not diagnose ``final`` marked classes, since
+ those cannot be used as base classes, consequently they can not violate the
+ rule.
+
- Fixed a crash in :doc:`llvmlibc-callee-namespace
<clang-tidy/checks/llvmlibc-callee-namespace>` when executing for C++ code
that contain calls to advanced constructs, e.g. overloaded operators.
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
@@ -41,6 +41,7 @@
Finder->addMatcher(
cxxRecordDecl(
anyOf(has(cxxMethodDecl(isVirtual())), InheritsVirtualMethod),
+ unless(isFinal()),
unless(hasPublicVirtualOrProtectedNonVirtualDestructor()))
.bind("ProblematicClassOrStruct"),
this);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
