[cfe-commits] r124805 - in /cfe/trunk: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td include/clang/Sema/Sema.h lib/Sema/SemaDecl.cpp lib/Sema/SemaDeclCXX.cpp test/SemaCXX/warn-overloaded-virtual.cpp
Argyrios Kyrtzidis
kyrtzidis at apple.com
Tue Mar 8 12:06:04 PST 2011
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 <akyrtzi at gmail.com> 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
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
More information about the cfe-commits
mailing list