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 c0de5f7a93ce16df2f067c9b61152fa9efde3de1..80e4692c068101da213043a7fb05bc0480b72c29 100644 (file)
--- 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 9933bd10e1fc9c09cb803a7f86300088edb81021..71c72da93d09639856b82d955553fbc5c48af8b2 100644 (file)
--- 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<CXXMethodDecl>(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<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;
+  }
 
-  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<OverrideAttr>() && !HasOverriddenMethods) {
-    Diag(MD->getLocation(), 
-                 diag::err_function_marked_override_not_overriding)
+  if (MD->hasAttr<OverrideAttr>() && !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<FinalAttr>())
@@ -1609,31 +1627,17 @@ Sema::ActOnCXXMemberDeclarator(Scope *S, AccessSpecifier AS, Declarator &D,
       FunTmpl->getTemplatedDecl()->setAccess(AS);
   }
 
-  if (VS.isOverrideSpecified()) {
-    CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(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<CXXMethodDecl>(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<CXXMethodDecl>(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 c4a401bb27c5817f046fc9e96e52a94e27deb427..16f98280ed87c3d72b7debee1c5907de48691427 100644 (file)
--- 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<typename T>
 struct B : A {
+  // FIXME: Diagnose this.
   virtual void f(T) override;
 };
 
+template<typename T>
+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<typename T> struct A : X {
+    void f() override;
+    void h() final;
+  };
+  template<typename T> 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<int> b; // no-note
+  template<typename T> struct C : T {
+    void g() override;
+    void i() final;
+  };
+  template<typename T> struct D : X {
+    virtual void g() override; // expected-error {{does not override}}
+    virtual void i() final;
+  };
+  template<typename...T> 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<typename T> struct Y : T {
+    void f() override;
+    void h() final;
+  };
+  template<typename T> 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<X> y;
+  Z<X> z; // expected-note {{in instantiation of}}
+}