[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
akyrtzi at gmail.com
Thu Feb 3 10:01:15 PST 2011
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();
+};
More information about the cfe-commits
mailing list