r190109 - Improve error for "override" + non-virtual func.
Eli Friedman
eli.friedman at gmail.com
Thu Sep 5 16:51:03 PDT 2013
Author: efriedma
Date: Thu Sep 5 18:51:03 2013
New Revision: 190109
URL: http://llvm.org/viewvc/llvm-project?rev=190109&view=rev
Log:
Improve error for "override" + non-virtual func.
Consider something like the following:
struct X {
virtual void foo(float x);
};
struct Y : X {
void foo(double x) override;
};
The error is almost certainly that Y::foo() has the wrong signature,
rather than incorrect usage of the override keyword. This patch
adds an appropriate diagnostic for that case.
Fixes <rdar://problem/14785106>.
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/test/CXX/class.derived/class.virtual/p3-0x.cpp
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=190109&r1=190108&r2=190109&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Sep 5 18:51:03 2013
@@ -1538,6 +1538,9 @@ def err_auto_fn_virtual : Error<
// C++11 override control
def override_keyword_only_allowed_on_virtual_member_functions : Error<
"only virtual member functions can be marked '%0'">;
+def override_keyword_hides_virtual_member_function : Error<
+ "non-virtual member function marked '%0' hides virtual member "
+ "%select{function|functions}1">;
def err_function_marked_override_not_overriding : Error<
"%0 marked 'override' but does not override any member functions">;
def err_class_marked_final_used_as_base : Error<
Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=190109&r1=190108&r2=190109&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Thu Sep 5 18:51:03 2013
@@ -1490,7 +1490,11 @@ public:
bool CheckConstexprFunctionDecl(const FunctionDecl *FD);
bool CheckConstexprFunctionBody(const FunctionDecl *FD, Stmt *Body);
- void DiagnoseHiddenVirtualMethods(CXXRecordDecl *DC, CXXMethodDecl *MD);
+ void DiagnoseHiddenVirtualMethods(CXXMethodDecl *MD);
+ void FindHiddenVirtualMethods(CXXMethodDecl *MD,
+ SmallVectorImpl<CXXMethodDecl*> &OverloadedMethods);
+ void NoteHiddenVirtualMethods(CXXMethodDecl *MD,
+ SmallVectorImpl<CXXMethodDecl*> &OverloadedMethods);
// Returns true if the function declaration is a redeclaration
bool CheckFunctionDeclaration(Scope *S,
FunctionDecl *NewFD, LookupResult &Previous,
@@ -4807,7 +4811,7 @@ public:
bool CheckPureMethod(CXXMethodDecl *Method, SourceRange InitRange);
/// CheckOverrideControl - Check C++11 override control semantics.
- void CheckOverrideControl(Decl *D);
+ void CheckOverrideControl(NamedDecl *D);
/// CheckForFunctionMarkedFinal - Checks whether a virtual member function
/// overrides a virtual member function marked 'final', according to
Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=190109&r1=190108&r2=190109&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Thu Sep 5 18:51:03 2013
@@ -1700,37 +1700,61 @@ bool Sema::ActOnAccessSpecifier(AccessSp
}
/// CheckOverrideControl - Check C++11 override control semantics.
-void Sema::CheckOverrideControl(Decl *D) {
+void Sema::CheckOverrideControl(NamedDecl *D) {
if (D->isInvalidDecl())
return;
- const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(D);
+ // We only care about "override" and "final" declarations.
+ if (!D->hasAttr<OverrideAttr>() && !D->hasAttr<FinalAttr>())
+ return;
- // Do we know which functions this declaration might be overriding?
- bool OverridesAreKnown = !MD ||
- (!MD->getParent()->hasAnyDependentBases() &&
- !MD->getType()->isDependentType());
+ CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(D);
- if (!MD || !MD->isVirtual()) {
- if (OverridesAreKnown) {
+ // We can't check dependent instance methods.
+ if (MD && MD->isInstance() &&
+ (MD->getParent()->hasAnyDependentBases() ||
+ MD->getType()->isDependentType()))
+ return;
+
+ if (MD && !MD->isVirtual()) {
+ // If we have a non-virtual method, check if if hides a virtual method.
+ // (In that case, it's most likely the method has the wrong type.)
+ SmallVector<CXXMethodDecl *, 8> OverloadedMethods;
+ FindHiddenVirtualMethods(MD, OverloadedMethods);
+
+ if (!OverloadedMethods.empty()) {
if (OverrideAttr *OA = D->getAttr<OverrideAttr>()) {
Diag(OA->getLocation(),
- diag::override_keyword_only_allowed_on_virtual_member_functions)
- << "override" << FixItHint::CreateRemoval(OA->getLocation());
- D->dropAttr<OverrideAttr>();
- }
- if (FinalAttr *FA = D->getAttr<FinalAttr>()) {
+ diag::override_keyword_hides_virtual_member_function)
+ << "override" << (OverloadedMethods.size() > 1);
+ } else if (FinalAttr *FA = D->getAttr<FinalAttr>()) {
Diag(FA->getLocation(),
- diag::override_keyword_only_allowed_on_virtual_member_functions)
- << "final" << FixItHint::CreateRemoval(FA->getLocation());
- D->dropAttr<FinalAttr>();
+ diag::override_keyword_hides_virtual_member_function)
+ << "final" << (OverloadedMethods.size() > 1);
}
+ NoteHiddenVirtualMethods(MD, OverloadedMethods);
+ MD->setInvalidDecl();
+ return;
}
- return;
+ // Fall through into the general case diagnostic.
+ // FIXME: We might want to attempt typo correction here.
}
- if (!OverridesAreKnown)
+ if (!MD || !MD->isVirtual()) {
+ if (OverrideAttr *OA = D->getAttr<OverrideAttr>()) {
+ Diag(OA->getLocation(),
+ diag::override_keyword_only_allowed_on_virtual_member_functions)
+ << "override" << FixItHint::CreateRemoval(OA->getLocation());
+ D->dropAttr<OverrideAttr>();
+ }
+ if (FinalAttr *FA = D->getAttr<FinalAttr>()) {
+ Diag(FA->getLocation(),
+ diag::override_keyword_only_allowed_on_virtual_member_functions)
+ << "final" << FixItHint::CreateRemoval(FA->getLocation());
+ D->dropAttr<FinalAttr>();
+ }
return;
+ }
// C++11 [class.virtual]p5:
// If a virtual function is marked with the virt-specifier override and
@@ -4222,7 +4246,7 @@ void Sema::CheckCompletedCXXClass(CXXRec
// See if a method overloads virtual methods in a base
// class without overriding any.
if (!M->isStatic())
- DiagnoseHiddenVirtualMethods(Record, *M);
+ DiagnoseHiddenVirtualMethods(*M);
// Check whether the explicitly-defaulted special members are valid.
if (!M->isInvalidDecl() && M->isExplicitlyDefaulted())
@@ -5604,12 +5628,10 @@ static void AddMostOverridenMethods(cons
AddMostOverridenMethods(*I, Methods);
}
-/// \brief See if a method overloads virtual methods in a base class without
+/// \brief Check 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()) == DiagnosticsEngine::Ignored)
- return;
+void Sema::FindHiddenVirtualMethods(CXXMethodDecl *MD,
+ SmallVectorImpl<CXXMethodDecl*> &OverloadedMethods) {
if (!MD->getDeclName().isIdentifier())
return;
@@ -5622,6 +5644,7 @@ void Sema::DiagnoseHiddenVirtualMethods(
// 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.
+ CXXRecordDecl *DC = MD->getParent();
DeclContext::lookup_result R = DC->lookup(MD->getDeclName());
for (DeclContext::lookup_iterator I = R.begin(), E = R.end(); I != E; ++I) {
NamedDecl *ND = *I;
@@ -5631,18 +5654,38 @@ void Sema::DiagnoseHiddenVirtualMethods(
AddMostOverridenMethods(MD, Data.OverridenAndUsingBaseMethods);
}
- if (DC->lookupInBases(&FindHiddenVirtualMethod, &Data, Paths) &&
- !Data.OverloadedMethods.empty()) {
+ if (DC->lookupInBases(&FindHiddenVirtualMethod, &Data, Paths))
+ OverloadedMethods = Data.OverloadedMethods;
+}
+
+void Sema::NoteHiddenVirtualMethods(CXXMethodDecl *MD,
+ SmallVectorImpl<CXXMethodDecl*> &OverloadedMethods) {
+ for (unsigned i = 0, e = OverloadedMethods.size(); i != e; ++i) {
+ CXXMethodDecl *overloadedMD = OverloadedMethods[i];
+ PartialDiagnostic PD = PDiag(
+ diag::note_hidden_overloaded_virtual_declared_here) << overloadedMD;
+ HandleFunctionTypeMismatch(PD, MD->getType(), overloadedMD->getType());
+ Diag(overloadedMD->getLocation(), PD);
+ }
+}
+
+/// \brief Diagnose methods which overload virtual methods in a base class
+/// without overriding any.
+void Sema::DiagnoseHiddenVirtualMethods(CXXMethodDecl *MD) {
+ if (MD->isInvalidDecl())
+ return;
+
+ if (Diags.getDiagnosticLevel(diag::warn_overloaded_virtual,
+ MD->getLocation()) == DiagnosticsEngine::Ignored)
+ return;
+
+ SmallVector<CXXMethodDecl *, 8> OverloadedMethods;
+ FindHiddenVirtualMethods(MD, OverloadedMethods);
+ if (!OverloadedMethods.empty()) {
Diag(MD->getLocation(), diag::warn_overloaded_virtual)
- << MD << (Data.OverloadedMethods.size() > 1);
+ << MD << (OverloadedMethods.size() > 1);
- for (unsigned i = 0, e = Data.OverloadedMethods.size(); i != e; ++i) {
- CXXMethodDecl *overloadedMD = Data.OverloadedMethods[i];
- PartialDiagnostic PD = PDiag(
- diag::note_hidden_overloaded_virtual_declared_here) << overloadedMD;
- HandleFunctionTypeMismatch(PD, MD->getType(), overloadedMD->getType());
- Diag(overloadedMD->getLocation(), PD);
- }
+ NoteHiddenVirtualMethods(MD, OverloadedMethods);
}
}
Modified: cfe/trunk/test/CXX/class.derived/class.virtual/p3-0x.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/class.derived/class.virtual/p3-0x.cpp?rev=190109&r1=190108&r2=190109&view=diff
==============================================================================
--- cfe/trunk/test/CXX/class.derived/class.virtual/p3-0x.cpp (original)
+++ cfe/trunk/test/CXX/class.derived/class.virtual/p3-0x.cpp Thu Sep 5 18:51:03 2013
@@ -130,3 +130,23 @@ namespace MemberOfUnknownSpecialization
// expected-note at +1 {{in instantiation of}}
A<double>::C c3;
}
+
+namespace DiagnosticsQOI {
+ struct X {
+ virtual ~X();
+ virtual void foo(int x); // expected-note {{hidden overloaded virtual function}}
+ virtual void bar(int x); // expected-note 2 {{hidden overloaded virtual function}}
+ virtual void bar(float x); // expected-note 2 {{hidden overloaded virtual function}}
+ };
+
+ struct Y : X {
+ void foo(int x, int y) override; // expected-error {{non-virtual member function marked 'override' hides virtual member function}}
+ void bar(double) override; // expected-error {{non-virtual member function marked 'override' hides virtual member functions}}
+ void bar(long double) final; // expected-error {{non-virtual member function marked 'final' hides virtual member functions}}
+ };
+
+ template<typename T>
+ struct Z : T {
+ static void foo() override; // expected-error {{only virtual member functions can be marked 'override'}}
+ };
+}
More information about the cfe-commits
mailing list