From 374e14be61ad4c105188d07e4e1320fb0bec4e34 Mon Sep 17 00:00:00 2001
From: Richard Trieu <rtrieu@google.com>
Date: Wed, 1 Mar 2017 03:07:55 +0000
Subject: [PATCH] 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
---
 include/clang/Basic/DiagnosticGroups.td       |  2 ++
 include/clang/Basic/DiagnosticSemaKinds.td    |  3 ++
 lib/Sema/SemaDeclCXX.cpp                      | 11 ++++---
 ...n-inconsistent-missing-destructor-override | 33 +++++++++++++++++++
 4 files changed, 44 insertions(+), 5 deletions(-)
 create mode 100644 test/SemaCXX/warn-inconsistent-missing-destructor-override

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'}}
+};
-- 
2.50.1