[PATCH] D126891: [clang-tidy] The check should ignore final classes

[Balázs Benics via Phabricator via cfe-commits]

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);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D126891.435054.patch
Type: text/x-patch
Size: 2373 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220608/74eec46f/attachment.bin>