Hi Argyrios, I love this warning, it's very useful.
Nico On Thu, Feb 3, 2011 at 10:01 AM, Argyrios Kyrtzidis <[email protected]> wrote: > Author: akirtzidis > Date: Thu Feb 3 12:01:15 2011 > New Revision: 124805 > > URL: http://llvm.org/viewvc/llvm-project?rev=124805&view=rev > Log: > 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. > > Added: > cfe/trunk/test/SemaCXX/warn-overloaded-virtual.cpp > Modified: > cfe/trunk/include/clang/Basic/DiagnosticGroups.td > cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td > cfe/trunk/include/clang/Sema/Sema.h > cfe/trunk/lib/Sema/SemaDecl.cpp > cfe/trunk/lib/Sema/SemaDeclCXX.cpp > > Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=124805&r1=124804&r2=124805&view=diff > ============================================================================== > --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original) > +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Thu Feb 3 12:01:15 2011 > @@ -86,7 +86,7 @@ > 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 @@ > UnknownPragmas, > Unused, > VectorConversions, > - VolatileRegisterVar > + VolatileRegisterVar, > + OverloadedVirtual > ]>; > > // -Wall is -Wmost -Wparentheses > > Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=124805&r1=124804&r2=124805&view=diff > ============================================================================== > --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) > +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Feb 3 12:01:15 > 2011 > @@ -2768,6 +2768,11 @@ > 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 " > > Modified: cfe/trunk/include/clang/Sema/Sema.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=124805&r1=124804&r2=124805&view=diff > ============================================================================== > --- cfe/trunk/include/clang/Sema/Sema.h (original) > +++ cfe/trunk/include/clang/Sema/Sema.h Thu Feb 3 12:01:15 2011 > @@ -732,6 +732,7 @@ > 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, > > Modified: cfe/trunk/lib/Sema/SemaDecl.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=124805&r1=124804&r2=124805&view=diff > ============================================================================== > --- cfe/trunk/lib/Sema/SemaDecl.cpp (original) > +++ cfe/trunk/lib/Sema/SemaDecl.cpp Thu Feb 3 12:01:15 2011 > @@ -4276,7 +4276,7 @@ > diag::note_overridden_virtual_function); > } > } > - } > + } > } > } > > > Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=124805&r1=124804&r2=124805&view=diff > ============================================================================== > --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original) > +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Thu Feb 3 12:01:15 2011 > @@ -2777,6 +2777,108 @@ > 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, > > Added: cfe/trunk/test/SemaCXX/warn-overloaded-virtual.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-overloaded-virtual.cpp?rev=124805&view=auto > ============================================================================== > --- cfe/trunk/test/SemaCXX/warn-overloaded-virtual.cpp (added) > +++ cfe/trunk/test/SemaCXX/warn-overloaded-virtual.cpp Thu Feb 3 12:01:15 > 2011 > @@ -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(); > +}; > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
