From: Richard Smith Date: Mon, 6 Aug 2012 03:25:17 +0000 (+0000) Subject: PR13499: Don't try to check whether 'override' has been validly applied until X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=a4b39658673954fb9f75673594b50028685fc665;p=clang PR13499: Don't try to check whether 'override' has been validly applied until we know whether the function is virtual. But check it as soon as we do know; in some cases we don't need to wait for an instantiation. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@161316 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h index c0de5f7a93..80e4692c06 100644 --- a/include/clang/Sema/Sema.h +++ b/include/clang/Sema/Sema.h @@ -4379,12 +4379,12 @@ public: bool CheckPureMethod(CXXMethodDecl *Method, SourceRange InitRange); - /// CheckOverrideControl - Check C++0x override control semantics. - void CheckOverrideControl(const Decl *D); + /// CheckOverrideControl - Check C++11 override control semantics. + void CheckOverrideControl(Decl *D); /// CheckForFunctionMarkedFinal - Checks whether a virtual member function /// overrides a virtual member function marked 'final', according to - /// C++0x [class.virtual]p3. + /// C++11 [class.virtual]p4. bool CheckIfOverriddenFunctionIsMarkedFinal(const CXXMethodDecl *New, const CXXMethodDecl *Old); diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp index 9933bd10e1..71c72da93d 100644 --- a/lib/Sema/SemaDeclCXX.cpp +++ b/lib/Sema/SemaDeclCXX.cpp @@ -1405,32 +1405,50 @@ bool Sema::ActOnAccessSpecifier(AccessSpecifier Access, return ProcessAccessDeclAttributeList(ASDecl, Attrs); } -/// CheckOverrideControl - Check C++0x override control semantics. -void Sema::CheckOverrideControl(const Decl *D) { +/// CheckOverrideControl - Check C++11 override control semantics. +void Sema::CheckOverrideControl(Decl *D) { const CXXMethodDecl *MD = dyn_cast(D); - if (!MD || !MD->isVirtual()) + + // Do we know which functions this declaration might be overriding? + bool OverridesAreKnown = !MD || + (!MD->getParent()->hasAnyDependentBases() && + !MD->getType()->isDependentType()); + + if (!MD || !MD->isVirtual()) { + if (OverridesAreKnown) { + if (OverrideAttr *OA = D->getAttr()) { + Diag(OA->getLocation(), + diag::override_keyword_only_allowed_on_virtual_member_functions) + << "override" << FixItHint::CreateRemoval(OA->getLocation()); + D->dropAttr(); + } + if (FinalAttr *FA = D->getAttr()) { + Diag(FA->getLocation(), + diag::override_keyword_only_allowed_on_virtual_member_functions) + << "final" << FixItHint::CreateRemoval(FA->getLocation()); + D->dropAttr(); + } + } return; + } - if (MD->isDependentContext()) + if (!OverridesAreKnown) return; - // C++0x [class.virtual]p3: - // If a virtual function is marked with the virt-specifier override and does - // not override a member function of a base class, - // the program is ill-formed. - bool HasOverriddenMethods = + // C++11 [class.virtual]p5: + // If a virtual function is marked with the virt-specifier override and + // does not override a member function of a base class, the program is + // ill-formed. + bool HasOverriddenMethods = MD->begin_overridden_methods() != MD->end_overridden_methods(); - if (MD->hasAttr() && !HasOverriddenMethods) { - Diag(MD->getLocation(), - diag::err_function_marked_override_not_overriding) + if (MD->hasAttr() && !HasOverriddenMethods) + Diag(MD->getLocation(), diag::err_function_marked_override_not_overriding) << MD->getDeclName(); - return; - } } -/// CheckIfOverriddenFunctionIsMarkedFinal - Checks whether a virtual member +/// CheckIfOverriddenFunctionIsMarkedFinal - Checks whether a virtual member /// function overrides a virtual member function marked 'final', according to -/// C++0x [class.virtual]p3. +/// C++11 [class.virtual]p4. bool Sema::CheckIfOverriddenFunctionIsMarkedFinal(const CXXMethodDecl *New, const CXXMethodDecl *Old) { if (!Old->hasAttr()) @@ -1609,31 +1627,17 @@ Sema::ActOnCXXMemberDeclarator(Scope *S, AccessSpecifier AS, Declarator &D, FunTmpl->getTemplatedDecl()->setAccess(AS); } - if (VS.isOverrideSpecified()) { - CXXMethodDecl *MD = dyn_cast(Member); - if (!MD || !MD->isVirtual()) { - Diag(Member->getLocStart(), - diag::override_keyword_only_allowed_on_virtual_member_functions) - << "override" << FixItHint::CreateRemoval(VS.getOverrideLoc()); - } else - MD->addAttr(new (Context) OverrideAttr(VS.getOverrideLoc(), Context)); - } - if (VS.isFinalSpecified()) { - CXXMethodDecl *MD = dyn_cast(Member); - if (!MD || !MD->isVirtual()) { - Diag(Member->getLocStart(), - diag::override_keyword_only_allowed_on_virtual_member_functions) - << "final" << FixItHint::CreateRemoval(VS.getFinalLoc()); - } else - MD->addAttr(new (Context) FinalAttr(VS.getFinalLoc(), Context)); - } + if (VS.isOverrideSpecified()) + Member->addAttr(new (Context) OverrideAttr(VS.getOverrideLoc(), Context)); + if (VS.isFinalSpecified()) + Member->addAttr(new (Context) FinalAttr(VS.getFinalLoc(), Context)); if (VS.getLastLocation().isValid()) { // Update the end location of a method that has a virt-specifiers. if (CXXMethodDecl *MD = dyn_cast_or_null(Member)) MD->setRangeEnd(VS.getLastLocation()); } - + CheckOverrideControl(Member); assert((Name || isInstField) && "No identifier for non-field ?"); diff --git a/test/CXX/class.derived/class.virtual/p3-0x.cpp b/test/CXX/class.derived/class.virtual/p3-0x.cpp index c4a401bb27..16f98280ed 100644 --- a/test/CXX/class.derived/class.virtual/p3-0x.cpp +++ b/test/CXX/class.derived/class.virtual/p3-0x.cpp @@ -20,9 +20,15 @@ struct A { template struct B : A { + // FIXME: Diagnose this. virtual void f(T) override; }; +template +struct C : A { + virtual void f(int) override; // expected-error {{does not override}} +}; + } namespace Test3 { @@ -51,3 +57,46 @@ struct D : B { }; } + +namespace PR13499 { + struct X { + virtual void f(); + virtual void h(); + }; + template struct A : X { + void f() override; + void h() final; + }; + template struct B : X { + void g() override; // expected-error {{only virtual member functions can be marked 'override'}} + void i() final; // expected-error {{only virtual member functions can be marked 'final'}} + }; + B b; // no-note + template struct C : T { + void g() override; + void i() final; + }; + template struct D : X { + virtual void g() override; // expected-error {{does not override}} + virtual void i() final; + }; + template struct E : X { + void f(T...) override; + void g(T...) override; // expected-error {{only virtual member functions can be marked 'override'}} + void h(T...) final; + void i(T...) final; // expected-error {{only virtual member functions can be marked 'final'}} + }; + // FIXME: Diagnose these in the template definition, not in the instantiation. + E<> e; // expected-note {{in instantiation of}} + + template struct Y : T { + void f() override; + void h() final; + }; + template struct Z : T { + void g() override; // expected-error {{only virtual member functions can be marked 'override'}} + void i() final; // expected-error {{only virtual member functions can be marked 'final'}} + }; + Y y; + Z z; // expected-note {{in instantiation of}} +}