[clang] Warning for non-final classes with final destructors

Marking a class' destructor final prevents the class from being inherited from. However, it is a subtle and awkward way to express that at best, and unintended at worst. It may also generate worse code (in other compilers) than marking the class itself final. For these reasons, this revision adds a warning for nonfinal classes with final destructors, with a note to suggest marking the class final to silence the warning.

See https://reviews.llvm.org/D66621 for more background.

Patch by logan-5 (Logan Smith)

Differential Revision: https://reviews.llvm.org/D66711

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@370594 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp
index 633a1667b2dc9c89c324dd44cadebff59b5139bf..a5b4eda43c5d306653cd5d3a70c9eef38b724d0f 100644 (file)
--- a/lib/Sema/SemaDeclCXX.cpp
+++ b/lib/Sema/SemaDeclCXX.cpp
@@ -6236,6 +6236,19 @@ void Sema::CheckCompletedCXXClass(CXXRecordDecl *Record) {
     }
   }
 
+  // Warn if the class has a final destructor but is not itself marked final.
+  if (!Record->hasAttr<FinalAttr>()) {
+    if (const CXXDestructorDecl *dtor = Record->getDestructor()) {
+      if (const FinalAttr *FA = dtor->getAttr<FinalAttr>()) {
+        Diag(FA->getLocation(), diag::warn_final_dtor_non_final_class)
+          << FA->isSpelledAsSealed();
+        Diag(Record->getLocation(), diag::note_final_dtor_non_final_class_silence)
+          << Context.getRecordType(Record)
+          << FA->isSpelledAsSealed();
+      }
+    }
+  }
+
   // See if trivial_abi has to be dropped.
   if (Record->hasAttr<TrivialABIAttr>())
     checkIllFormedTrivialABIStruct(*Record);

diff --git a/test/SemaCXX/MicrosoftExtensions.cpp b/test/SemaCXX/MicrosoftExtensions.cpp
index cc6ac96aca9839882c66700eda8e4f43d10b4de0..4dca1b613421568e726c9f8ad1c6abf34e2e1775 100644 (file)
--- a/test/SemaCXX/MicrosoftExtensions.cpp
+++ b/test/SemaCXX/MicrosoftExtensions.cpp
@@ -440,6 +440,11 @@ struct SealedType sealed : SomeBase {
 // expected-error@+1 {{base 'SealedType' is marked 'sealed'}}
 struct InheritFromSealed : SealedType {};
 
+class SealedDestructor { // expected-note {{mark 'SealedDestructor' as 'sealed' to silence this warning}}
+    // expected-warning@+1 {{'sealed' keyword is a Microsoft extension}}
+    virtual ~SealedDestructor() sealed; // expected-warning {{class with destructor marked 'sealed' cannot be inherited from}}
+};
+
 void AfterClassBody() {
   // expected-warning@+1 {{attribute 'deprecated' is ignored, place it after "struct" to apply attribute to type declaration}}
   struct D {} __declspec(deprecated);

diff --git a/test/SemaCXX/warn-final-dtor-non-final-class.cpp b/test/SemaCXX/warn-final-dtor-non-final-class.cpp
new file mode 100644 (file)
index 0000000..32961c2
--- /dev/null
+++ b/test/SemaCXX/warn-final-dtor-non-final-class.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify %s -Wfinal-dtor-non-final-class
+
+class A {
+    ~A();
+};
+
+class B { // expected-note {{mark 'B' as 'final' to silence this warning}}
+    virtual ~B() final; // expected-warning {{class with destructor marked 'final' cannot be inherited from}}
+};
+
+class C final {
+    virtual ~C() final;
+};