Change ParseOptionalCXX0XVirtSpecifierSeq to take a VirtSpecifiers struct.

Enforce C++[class.mem]p8:
A virt-specifier-seq shall contain at most one of each virt-specifier.

llvm-svn: 123611

clang/include/clang/Basic/DiagnosticParseKinds.td

@@ -389,6 +389,10 @@ def err_friend_decl_defines_class : Error<
 def warn_deleted_function_accepted_as_extension: ExtWarn<
   "deleted function definition accepted as a C++0x extension">, InGroup<CXX0x>;
 
+// C++0x override control
+def err_duplicate_virt_specifier : Error<
+  "member function already marked '%0'">;
+
 def err_scoped_enum_missing_identifier : Error<
   "scoped enumeration requires a name">;
 

clang/include/clang/Parse/Parser.h

@@ -1525,8 +1525,8 @@ private:
   
   ExprResult ParseCXX0XAlignArgument(SourceLocation Start);
 
-  bool isCXX0XVirtSpecifier() const;
-  void ParseOptionalCXX0XVirtSpecifierSeq();
+  VirtSpecifiers::VirtSpecifier isCXX0XVirtSpecifier() const;
+  void ParseOptionalCXX0XVirtSpecifierSeq(VirtSpecifiers &VS);
 
   /// DeclaratorScopeObj - RAII object used in Parser::ParseDirectDeclarator to
   /// enter a new C++ declarator scope and exit it when the function is

clang/include/clang/Sema/DeclSpec.h

@@ -1467,7 +1467,31 @@ struct FieldDeclarator {
     BitfieldSize = 0;
   }
 };
-  
+
+/// 
+class VirtSpecifiers {
+public:
+  enum VirtSpecifier {
+    VS_None = 0,
+    VS_Override = 1,
+    VS_Final = 2,
+    VS_New = 4
+  };
+
+  VirtSpecifiers() : Specifiers(0) { }
+
+  bool SetVirtSpecifier(VirtSpecifier VS, SourceLocation Loc, 
+                        const char *&PrevSpec);
+
+private:
+  unsigned Specifiers;
+
+  SourceLocation VS_overrideLoc, VS_finalLoc, VS_newLoc;
+
+  static const char *getSpecifierName(VirtSpecifier VS);
+
+};
+
 } // end namespace clang
 
 #endif

clang/lib/Parse/ParseDeclCXX.cpp

@@ -1268,15 +1268,21 @@ void Parser::HandleMemberFunctionDefaultArgs(Declarator& DeclaratorInfo,
 ///         override
 ///         final
 ///         new
-bool Parser::isCXX0XVirtSpecifier() const {
+VirtSpecifiers::VirtSpecifier Parser::isCXX0XVirtSpecifier() const {
   if (Tok.is(tok::kw_new))
-    return true;
+    return VirtSpecifiers::VS_New;
 
-  if (Tok.isNot(tok::identifier))
-    return false;
+  if (Tok.is(tok::identifier)) {
+    IdentifierInfo *II = Tok.getIdentifierInfo();
 
-  const IdentifierInfo *II = Tok.getIdentifierInfo();
-  return II == Ident_override || II == Ident_final;
+    if (II == Ident_override)
+      return VirtSpecifiers::VS_Override;
+
+    if (II == Ident_final)
+      return VirtSpecifiers::VS_Final;
+  }
+
+  return VirtSpecifiers::VS_None;
 }
 
 /// ParseOptionalCXX0XVirtSpecifierSeq - Parse a virt-specifier-seq.
@@ -1284,10 +1290,26 @@ bool Parser::isCXX0XVirtSpecifier() const {
 ///       virt-specifier-seq:
 ///         virt-specifier
 ///         virt-specifier-seq virt-specifier
-void Parser::ParseOptionalCXX0XVirtSpecifierSeq() {
+void Parser::ParseOptionalCXX0XVirtSpecifierSeq(VirtSpecifiers &VS) {
   if (!getLang().CPlusPlus0x)
     return;
 
+  while (true) {
+    VirtSpecifiers::VirtSpecifier Specifier = isCXX0XVirtSpecifier();
+    if (Specifier == VirtSpecifiers::VS_None)
+      return;
+
+    // C++ [class.mem]p8:
+    //   A virt-specifier-seq shall contain at most one of each virt-specifier.
+    const char* PrevSpec = 0;
+    if (VS.SetVirtSpecifier(Specifier, Tok.getLocation(), PrevSpec))
+      Diag(Tok.getLocation(), diag::err_duplicate_virt_specifier)
+        << PrevSpec
+        << FixItHint::CreateRemoval(Tok.getLocation());
+
+    ConsumeToken();
+  }
+
   while (isCXX0XVirtSpecifier()) {
     // FIXME: Actually do something with the specifier.
     ConsumeToken();
@@ -1512,7 +1534,8 @@ void Parser::ParseCXXClassMemberDeclaration(AccessSpecifier AS,
         SkipUntil(tok::comma, true, true);
     }
 
-    ParseOptionalCXX0XVirtSpecifierSeq();
+    VirtSpecifiers VS;
+    ParseOptionalCXX0XVirtSpecifierSeq(VS);
 
     // pure-specifier:
     //   '= 0'

clang/lib/Sema/DeclSpec.cpp

@@ -668,3 +668,31 @@ void UnqualifiedId::setOperatorFunctionId(SourceLocation OperatorLoc,
       EndLocation = SymbolLocations[I];
   }
 }
+
+const char *VirtSpecifiers::getSpecifierName(VirtSpecifier VS) {
+  switch (VS) {
+  default: assert(0 && "Unknown specifier");
+  case VS_Override: return "override";
+  case VS_Final: return "final";
+  case VS_New: return "new";
+  }
+}
+
+bool VirtSpecifiers::SetVirtSpecifier(VirtSpecifier VS, SourceLocation Loc,
+                                      const char *&PrevSpec) {
+  if (Specifiers & VS) {
+    PrevSpec = getSpecifierName(VS);
+    return true;
+  }
+
+  Specifiers |= VS;
+
+  switch (VS) {
+  default: assert(0 && "Unknown specifier!");
+  case VS_Override: VS_overrideLoc = Loc; break;
+  case VS_Final:    VS_finalLoc = Loc; break;
+  case VS_New:      VS_newLoc = Loc; break;
+  }
+  return false;
+}
+

clang/test/CXX/class/class.mem/p8-0x.cpp

@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++0x -verify %s 
+struct A {
+  virtual void f() new new; // expected-error {{member function already marked 'new'}}
+  virtual void g() override override; // expected-error {{member function already marked 'override'}}
+  virtual void h() final final; // expected-error {{member function already marked 'final'}}
+};