From: Richard Trieu <rtrieu@google.com>
Date: Wed, 1 Mar 2017 03:07:55 +0000 (+0000)
Subject: Add warning for inconsistent overrides on destructor.
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=374e14be61ad4c105188d07e4e1320fb0bec4e34;p=clang

Add warning for inconsistent overrides on destructor.

The exisiting warning for inconsistent overrides does not include the destructor
as it was noted in review that it was too noisy.  Instead, add to a separate
warning group that is off by default for users who want consistent warnings
between methods and destructors.


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

diff --git a/include/clang/Basic/DiagnosticGroups.td b/include/clang/Basic/DiagnosticGroups.td
index 1317e49bc7..6153b8e7ed 100644
--- a/include/clang/Basic/DiagnosticGroups.td
+++ b/include/clang/Basic/DiagnosticGroups.td
@@ -175,6 +175,8 @@ def CXX98CompatPedantic : DiagGroup<"c++98-compat-pedantic",
 
 def CXX11Narrowing : DiagGroup<"c++11-narrowing">;
 
+def CXX11WarnOverrideDestructor :
+  DiagGroup<"inconsistent-missing-destructor-override">;
 def CXX11WarnOverrideMethod : DiagGroup<"inconsistent-missing-override">;
 
 // Original name of this warning in Clang
diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 04451c992a..a2269a1026 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2039,6 +2039,9 @@ def override_keyword_hides_virtual_member_function : Error<
   "%select{function|functions}1">;
 def err_function_marked_override_not_overriding : Error<
   "%0 marked 'override' but does not override any member functions">;
+def warn_destructor_marked_not_override_overriding : Warning <
+  "%0 overrides a destructor but is not marked 'override'">,
+  InGroup<CXX11WarnOverrideDestructor>, DefaultIgnore;
 def warn_function_marked_not_override_overriding : Warning <
   "%0 overrides a member function but is not marked 'override'">,
   InGroup<CXX11WarnOverrideMethod>;
diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp
index d3d5edfdce..668d4b2de3 100644
--- a/lib/Sema/SemaDeclCXX.cpp
+++ b/lib/Sema/SemaDeclCXX.cpp
@@ -2716,8 +2716,7 @@ void Sema::DiagnoseAbsenceOfOverrideControl(NamedDecl *D) {
   if (D->isInvalidDecl() || D->hasAttr<OverrideAttr>())
     return;
   CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(D);
-  if (!MD || MD->isImplicit() || MD->hasAttr<FinalAttr>() ||
-      isa<CXXDestructorDecl>(MD))
+  if (!MD || MD->isImplicit() || MD->hasAttr<FinalAttr>())
     return;
 
   SourceLocation Loc = MD->getLocation();
@@ -2727,10 +2726,12 @@ void Sema::DiagnoseAbsenceOfOverrideControl(NamedDecl *D) {
   SpellingLoc = getSourceManager().getSpellingLoc(SpellingLoc);
   if (SpellingLoc.isValid() && getSourceManager().isInSystemHeader(SpellingLoc))
       return;
-    
+
   if (MD->size_overridden_methods() > 0) {
-    Diag(MD->getLocation(), diag::warn_function_marked_not_override_overriding)
-      << MD->getDeclName();
+    unsigned DiagID = isa<CXXDestructorDecl>(MD)
+                          ? diag::warn_destructor_marked_not_override_overriding
+                          : diag::warn_function_marked_not_override_overriding;
+    Diag(MD->getLocation(), DiagID) << MD->getDeclName();
     const CXXMethodDecl *OMD = *MD->begin_overridden_methods();
     Diag(OMD->getLocation(), diag::note_overridden_virtual_function);
   }
diff --git a/test/SemaCXX/warn-inconsistent-missing-destructor-override b/test/SemaCXX/warn-inconsistent-missing-destructor-override
new file mode 100644
index 0000000000..75e9ba8d14
--- /dev/null
+++ b/test/SemaCXX/warn-inconsistent-missing-destructor-override
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 %s -verify -Winconsistent-missing-destructor-override
+
+class A {
+ public:
+  ~A() {}
+  void virtual run() {}
+};
+
+class B : public A {
+ public:
+  void run() override {}
+  ~B() {}
+};
+
+class C {
+ public:
+  virtual void run() {}
+  virtual ~C() {}  // expected-note 2{{overridden virtual function is here}}
+};
+
+class D : public C {
+ public:
+  void run() override {}
+  ~D() {}
+  // expected-warning@-1 {{'~D' overrides a destructor but is not marked 'override'}}
+};
+
+class E : public C {
+ public:
+  void run() override {}
+  virtual ~E() {}
+  // expected-warning@-1 {{'~E' overrides a destructor but is not marked 'override'}}
+};