logan-5 created this revision.
logan-5 added reviewers: rsmith, yamauchi.
logan-5 added a project: clang.
Herald added a subscriber: cfe-commits.

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.


Repository:
  rC Clang

https://reviews.llvm.org/D66711

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/SemaCXX/warn-final-dtor-nonfinal-class.cpp


Index: clang/test/SemaCXX/warn-final-dtor-nonfinal-class.cpp
===================================================================
--- /dev/null
+++ clang/test/SemaCXX/warn-final-dtor-nonfinal-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;
+};
Index: clang/lib/Sema/SemaDeclCXX.cpp
===================================================================
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -6235,6 +6235,19 @@
     }
   }
 
+  // Warn if the class has a final destructor but is not itself marked final.
+  if (!Record->hasAttr<FinalAttr>()) {
+    if (CXXDestructorDecl *dtor = Record->getDestructor()) {
+      if (FinalAttr *FA = dtor->getAttr<FinalAttr>()) {
+        Diag(FA->getLocation(), diag::warn_final_destructor_nonfinal_class)
+          << FA->isSpelledAsSealed();
+        Diag(Record->getLocation(), 
diag::note_final_destructor_nonfinal_class_silence)
+          << Context.getRecordType(Record)
+          << FA->isSpelledAsSealed();
+      }
+    }
+  }
+
   // See if trivial_abi has to be dropped.
   if (Record->hasAttr<TrivialABIAttr>())
     checkIllFormedTrivialABIStruct(*Record);
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2201,6 +2201,11 @@
   "base %0 is marked '%select{final|sealed}1'">;
 def warn_abstract_final_class : Warning<
   "abstract class is marked '%select{final|sealed}0'">, 
InGroup<AbstractFinalClass>;
+def warn_final_destructor_nonfinal_class : Warning<
+  "class with destructor marked '%select{final|sealed}0' cannot be inherited 
from">,
+  InGroup<FinalDtorNonFinalClass>;
+def note_final_destructor_nonfinal_class_silence : Note<
+  "mark %0 as '%select{final|sealed}1' to silence this warning">;
 
 // C++11 attributes
 def err_repeat_attribute : Error<"%0 attribute cannot be repeated">;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===================================================================
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -114,6 +114,7 @@
                                      [DeleteNonAbstractNonVirtualDtor,
                                       DeleteAbstractNonVirtualDtor]>;
 def AbstractFinalClass : DiagGroup<"abstract-final-class">;
+def FinalDtorNonFinalClass : DiagGroup<"final-dtor-non-final-class">;
 
 def CXX11CompatDeprecatedWritableStr :
   DiagGroup<"c++11-compat-deprecated-writable-strings">;


Index: clang/test/SemaCXX/warn-final-dtor-nonfinal-class.cpp
===================================================================
--- /dev/null
+++ clang/test/SemaCXX/warn-final-dtor-nonfinal-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;
+};
Index: clang/lib/Sema/SemaDeclCXX.cpp
===================================================================
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -6235,6 +6235,19 @@
     }
   }
 
+  // Warn if the class has a final destructor but is not itself marked final.
+  if (!Record->hasAttr<FinalAttr>()) {
+    if (CXXDestructorDecl *dtor = Record->getDestructor()) {
+      if (FinalAttr *FA = dtor->getAttr<FinalAttr>()) {
+        Diag(FA->getLocation(), diag::warn_final_destructor_nonfinal_class)
+          << FA->isSpelledAsSealed();
+        Diag(Record->getLocation(), diag::note_final_destructor_nonfinal_class_silence)
+          << Context.getRecordType(Record)
+          << FA->isSpelledAsSealed();
+      }
+    }
+  }
+
   // See if trivial_abi has to be dropped.
   if (Record->hasAttr<TrivialABIAttr>())
     checkIllFormedTrivialABIStruct(*Record);
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2201,6 +2201,11 @@
   "base %0 is marked '%select{final|sealed}1'">;
 def warn_abstract_final_class : Warning<
   "abstract class is marked '%select{final|sealed}0'">, InGroup<AbstractFinalClass>;
+def warn_final_destructor_nonfinal_class : Warning<
+  "class with destructor marked '%select{final|sealed}0' cannot be inherited from">,
+  InGroup<FinalDtorNonFinalClass>;
+def note_final_destructor_nonfinal_class_silence : Note<
+  "mark %0 as '%select{final|sealed}1' to silence this warning">;
 
 // C++11 attributes
 def err_repeat_attribute : Error<"%0 attribute cannot be repeated">;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===================================================================
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -114,6 +114,7 @@
                                      [DeleteNonAbstractNonVirtualDtor,
                                       DeleteAbstractNonVirtualDtor]>;
 def AbstractFinalClass : DiagGroup<"abstract-final-class">;
+def FinalDtorNonFinalClass : DiagGroup<"final-dtor-non-final-class">;
 
 def CXX11CompatDeprecatedWritableStr :
   DiagGroup<"c++11-compat-deprecated-writable-strings">;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to