[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
Nico Weber
thakis at chromium.org
Tue Mar 8 10:16:58 PST 2011
Hi Argyrios,
I love this warning, it's very useful.
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