Author: xbolva00
Date: Sat Aug 31 11:31:19 2019
New Revision: 370594

URL: http://llvm.org/viewvc/llvm-project?rev=370594&view=rev
Log:
[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

Added:
    cfe/trunk/test/SemaCXX/warn-final-dtor-non-final-class.cpp
Modified:
    cfe/trunk/lib/Sema/SemaDeclCXX.cpp
    cfe/trunk/test/SemaCXX/MicrosoftExtensions.cpp

Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=370594&r1=370593&r2=370594&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Sat Aug 31 11:31:19 2019
@@ -6236,6 +6236,19 @@ void Sema::CheckCompletedCXXClass(CXXRec
     }
   }
 
+  // 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);

Modified: cfe/trunk/test/SemaCXX/MicrosoftExtensions.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/MicrosoftExtensions.cpp?rev=370594&r1=370593&r2=370594&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/MicrosoftExtensions.cpp (original)
+++ cfe/trunk/test/SemaCXX/MicrosoftExtensions.cpp Sat Aug 31 11:31:19 2019
@@ -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);

Added: cfe/trunk/test/SemaCXX/warn-final-dtor-non-final-class.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-final-dtor-non-final-class.cpp?rev=370594&view=auto
==============================================================================
--- cfe/trunk/test/SemaCXX/warn-final-dtor-non-final-class.cpp (added)
+++ cfe/trunk/test/SemaCXX/warn-final-dtor-non-final-class.cpp Sat Aug 31 
11:31:19 2019
@@ -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;
+};


_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to