Implement -Woverloaded-virtual.

The difference with gcc is that it warns if you overload virtual methods only if
the method doesn't also override any method. This is to cut down on the number of warnings
and make it more useful like reported here: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20423.
If we want to warn that not all overloads are overriden we can have an additional
warning like -Wpartial-override.

-Woverloaded-virtual, unlike gcc, is added to -Wmost. Addresses rdar://8757630.

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

diff --git a/include/clang/Basic/DiagnosticGroups.td b/include/clang/Basic/DiagnosticGroups.td
index f5fa92449130cdc02215a86b43d7f3078408cb8d..b2ab86b0528283d396a0d5f8a1faeefa1060338d 100644 (file)
--- a/include/clang/Basic/DiagnosticGroups.td
+++ b/include/clang/Basic/DiagnosticGroups.td
@@ -86,7 +86,7 @@ def : DiagGroup<"old-style-definition">;
 def OutOfLineDeclaration : DiagGroup<"out-of-line-declaration">;
 def : DiagGroup<"overflow">;
 def OverlengthStrings : DiagGroup<"overlength-strings">;
-def : DiagGroup<"overloaded-virtual">;
+def OverloadedVirtual : DiagGroup<"overloaded-virtual">;
 def Packed : DiagGroup<"packed">;
 def Padded : DiagGroup<"padded">;
 def PointerArith : DiagGroup<"pointer-arith">;
@@ -228,7 +228,8 @@ def Most : DiagGroup<"most", [
     UnknownPragmas,
     Unused,
     VectorConversions,
-    VolatileRegisterVar
+    VolatileRegisterVar,
+    OverloadedVirtual
  ]>;
 
 // -Wall is -Wmost -Wparentheses

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 17fb942fc80a6be9db4655157b34eb0b6a7c11d4..2c8ccbdfb39d16ac6a5baeb63c885930ffbb5447 100644 (file)
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2768,6 +2768,11 @@ def note_previous_exception_handler : Note<"for type %0">;
 def warn_non_virtual_dtor : Warning<
   "%0 has virtual functions but non-virtual destructor">,
   InGroup<NonVirtualDtor>, DefaultIgnore;
+def warn_overloaded_virtual : Warning<
+  "%q0 hides overloaded virtual %select{function|functions}1">,
+  InGroup<OverloadedVirtual>, DefaultIgnore;
+def note_hidden_overloaded_virtual_declared_here : Note<
+  "hidden overloaded virtual function %q0 declared here">;
 
 def err_conditional_void_nonvoid : Error<
   "%select{left|right}1 operand to ? is void, but %select{right|left}1 operand "

diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index 9a2487d5af13fb4850358692d68a67d2bac7788f..4777bca4b86d44166e96b2aed9a398c8b61e11e6 100644 (file)
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -732,6 +732,7 @@ public:
                                      bool IsFunctionDefinition,
                                      bool &Redeclaration);
   bool AddOverriddenMethods(CXXRecordDecl *DC, CXXMethodDecl *MD);
+  void DiagnoseHiddenVirtualMethods(CXXRecordDecl *DC, CXXMethodDecl *MD);
   void CheckFunctionDeclaration(Scope *S,
                                 FunctionDecl *NewFD, LookupResult &Previous,
                                 bool IsExplicitSpecialization,

diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index 6341de996adeb8146fa75a15c7181ee0bef3b276..9c4fdc99f27d8e3326bab601ac06efebfddbb92e 100644 (file)
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -4276,7 +4276,7 @@ void Sema::CheckFunctionDeclaration(Scope *S, FunctionDecl *NewFD,
                    diag::note_overridden_virtual_function);
             }
           }
-        }        
+        }
       }
     }
 

diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp
index b4c375d638a61d5182d5e2d82b5a86f11cd6e594..5fbc0f6f26f53457d009cfbc5157a727d55e11c8 100644 (file)
--- a/lib/Sema/SemaDeclCXX.cpp
+++ b/lib/Sema/SemaDeclCXX.cpp
@@ -2777,6 +2777,108 @@ void Sema::CheckCompletedCXXClass(CXXRecordDecl *Record) {
       Diag(dtor ? dtor->getLocation() : Record->getLocation(),
            diag::warn_non_virtual_dtor) << Context.getRecordType(Record);
   }
+
+  // See if a method overloads virtual methods in a base
+  /// class without overriding any.
+  if (!Record->isDependentType()) {
+    for (CXXRecordDecl::method_iterator M = Record->method_begin(),
+                                     MEnd = Record->method_end();
+         M != MEnd; ++M) {
+      DiagnoseHiddenVirtualMethods(Record, *M);
+    }
+  }
+}
+
+/// \brief Data used with FindHiddenVirtualMethod
+struct FindHiddenVirtualMethodData {
+  Sema *S;
+  CXXMethodDecl *Method;
+  llvm::SmallPtrSet<const CXXMethodDecl *, 8> OverridenAndUsingBaseMethods;
+  llvm::SmallVector<CXXMethodDecl *, 8> OverloadedMethods;
+};
+
+/// \brief Member lookup function that determines whether a given C++
+/// method overloads virtual methods in a base class without overriding any,
+/// to be used with CXXRecordDecl::lookupInBases().
+static bool FindHiddenVirtualMethod(const CXXBaseSpecifier *Specifier,
+                                    CXXBasePath &Path,
+                                    void *UserData) {
+  RecordDecl *BaseRecord = Specifier->getType()->getAs<RecordType>()->getDecl();
+
+  FindHiddenVirtualMethodData &Data
+    = *static_cast<FindHiddenVirtualMethodData*>(UserData);
+
+  DeclarationName Name = Data.Method->getDeclName();
+  assert(Name.getNameKind() == DeclarationName::Identifier);
+
+  bool foundSameNameMethod = false;
+  llvm::SmallVector<CXXMethodDecl *, 8> overloadedMethods;
+  for (Path.Decls = BaseRecord->lookup(Name);
+       Path.Decls.first != Path.Decls.second;
+       ++Path.Decls.first) {
+    NamedDecl *D = *Path.Decls.first;
+    if (CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(D)) {
+      foundSameNameMethod = true;
+      // Interested only in hidden virtual methods.
+      if (!MD->isVirtual())
+        continue;
+      // If the method we are checking overrides a method from its base
+      // don't warn about the other overloaded methods.
+      if (!Data.S->IsOverload(Data.Method, MD, false))
+        return true;
+      // Collect the overload only if its hidden.
+      if (!Data.OverridenAndUsingBaseMethods.count(MD))
+        overloadedMethods.push_back(MD);
+    }
+  }
+
+  if (foundSameNameMethod)
+    Data.OverloadedMethods.append(overloadedMethods.begin(),
+                                   overloadedMethods.end());
+  return foundSameNameMethod;
+}
+
+/// \brief See if a method overloads virtual methods in a base class without
+/// overriding any.
+void Sema::DiagnoseHiddenVirtualMethods(CXXRecordDecl *DC, CXXMethodDecl *MD) {
+  if (Diags.getDiagnosticLevel(diag::warn_overloaded_virtual,
+                               MD->getLocation()) == Diagnostic::Ignored)
+    return;
+  if (MD->getDeclName().getNameKind() != DeclarationName::Identifier)
+    return;
+
+  CXXBasePaths Paths(/*FindAmbiguities=*/true, // true to look in all bases.
+                     /*bool RecordPaths=*/false,
+                     /*bool DetectVirtual=*/false);
+  FindHiddenVirtualMethodData Data;
+  Data.Method = MD;
+  Data.S = this;
+
+  // Keep the base methods that were overriden or introduced in the subclass
+  // by 'using' in a set. A base method not in this set is hidden.
+  for (DeclContext::lookup_result res = DC->lookup(MD->getDeclName());
+       res.first != res.second; ++res.first) {
+    if (CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(*res.first))
+      for (CXXMethodDecl::method_iterator I = MD->begin_overridden_methods(),
+                                          E = MD->end_overridden_methods();
+           I != E; ++I)
+        Data.OverridenAndUsingBaseMethods.insert(*I);
+    if (UsingShadowDecl *shad = dyn_cast<UsingShadowDecl>(*res.first))
+      if (CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(shad->getTargetDecl()))
+        Data.OverridenAndUsingBaseMethods.insert(MD);
+  }
+
+  if (DC->lookupInBases(&FindHiddenVirtualMethod, &Data, Paths) &&
+      !Data.OverloadedMethods.empty()) {
+    Diag(MD->getLocation(), diag::warn_overloaded_virtual)
+      << MD << (Data.OverloadedMethods.size() > 1);
+
+    for (unsigned i = 0, e = Data.OverloadedMethods.size(); i != e; ++i) {
+      CXXMethodDecl *overloadedMD = Data.OverloadedMethods[i];
+      Diag(overloadedMD->getLocation(),
+           diag::note_hidden_overloaded_virtual_declared_here) << overloadedMD;
+    }
+  }
 }
 
 void Sema::ActOnFinishCXXMemberSpecification(Scope* S, SourceLocation RLoc,

diff --git a/test/SemaCXX/warn-overloaded-virtual.cpp b/test/SemaCXX/warn-overloaded-virtual.cpp
new file mode 100644 (file)
index 0000000..7636722
--- /dev/null
+++ b/test/SemaCXX/warn-overloaded-virtual.cpp
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -fsyntax-only -Woverloaded-virtual -verify %s
+
+struct B1 {
+  virtual void foo(int); // expected-note {{declared here}}
+  virtual void foo(); // expected-note {{declared here}}
+};
+
+struct S1 : public B1 {
+  void foo(float); // expected-warning {{hides overloaded virtual functions}}
+};
+
+struct S2 : public B1 {
+  void foo(); // expected-note {{declared here}}
+};
+
+struct B2 {
+  virtual void foo(void*); // expected-note {{declared here}}
+};
+
+struct MS1 : public S2, public B2 {
+   virtual void foo(int); // expected-warning {{hides overloaded virtual functions}}
+};
+
+struct B3 {
+  virtual void foo(int);
+  virtual void foo();
+};
+
+struct S3 : public B3 {
+  using B3::foo;
+  void foo(float);
+};
+
+struct B4 {
+  virtual void foo();
+};
+
+struct S4 : public B4 {
+  void foo(float);
+  void foo();
+};