From 799ef666685d6c97d64d1970a6f68bf7923360c2 Mon Sep 17 00:00:00 2001 From: Argyrios Kyrtzidis Date: Thu, 3 Feb 2011 18:01:15 +0000 Subject: [PATCH] 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 --- include/clang/Basic/DiagnosticGroups.td | 5 +- include/clang/Basic/DiagnosticSemaKinds.td | 5 + include/clang/Sema/Sema.h | 1 + lib/Sema/SemaDecl.cpp | 2 +- lib/Sema/SemaDeclCXX.cpp | 102 +++++++++++++++++++++ test/SemaCXX/warn-overloaded-virtual.cpp | 41 +++++++++ 6 files changed, 153 insertions(+), 3 deletions(-) create mode 100644 test/SemaCXX/warn-overloaded-virtual.cpp diff --git a/include/clang/Basic/DiagnosticGroups.td b/include/clang/Basic/DiagnosticGroups.td index f5fa924491..b2ab86b052 100644 --- 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 17fb942fc8..2c8ccbdfb3 100644 --- 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, DefaultIgnore; +def warn_overloaded_virtual : Warning< + "%q0 hides overloaded virtual %select{function|functions}1">, + InGroup, 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 9a2487d5af..4777bca4b8 100644 --- 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 6341de996a..9c4fdc99f2 100644 --- 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 b4c375d638..5fbc0f6f26 100644 --- 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 OverridenAndUsingBaseMethods; + llvm::SmallVector 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()->getDecl(); + + FindHiddenVirtualMethodData &Data + = *static_cast(UserData); + + DeclarationName Name = Data.Method->getDeclName(); + assert(Name.getNameKind() == DeclarationName::Identifier); + + bool foundSameNameMethod = false; + llvm::SmallVector 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(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(*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(*res.first)) + if (CXXMethodDecl *MD = dyn_cast(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 index 0000000000..7636722e18 --- /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(); +}; -- 2.40.0