On Mar 8, 2011, at 10:16 AM, Nico Weber wrote: > Hi Argyrios, > > I love this warning, it's very useful.
Heh, nice to see warnings getting some lovin' :-) > > 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
