Improve diagnostic on C++11 attribute specifiers that appear at wrong syntactic locations around class specifiers.

This change list implemented logic that explicitly detects several combinations of locations where C++11 attribute
specifiers might be incorrectly placed within a class specifier. Previously we emit generic diagnostics like 
"expected identifier" for such cases; now we emit specific diagnostic against the misplaced attributes, this also 
fixed a bug in old code where attributes appear at legitimate locations were incorrectly rejected.

Thanks to Richard Smith for reviewing!

llvm-svn: 168626

4 files changed, 117 insertions, 8 deletions

diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index c433344602c..4c9efc71bcf 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -1637,7 +1637,8 @@ private:
 
   bool ParseImplicitInt(DeclSpec &DS, CXXScopeSpec *SS,
                         const ParsedTemplateInfo &TemplateInfo,
-                        AccessSpecifier AS, DeclSpecContext DSC);
+                        AccessSpecifier AS, DeclSpecContext DSC, 
+                        ParsedAttributesWithRange &Attrs);
   DeclSpecContext getDeclSpecContextFromDeclaratorContext(unsigned Context);
   void ParseDeclarationSpecifiers(DeclSpec &DS,
                 const ParsedTemplateInfo &TemplateInfo = ParsedTemplateInfo(),
@@ -2098,7 +2099,8 @@ private:
   void ParseClassSpecifier(tok::TokenKind TagTokKind, SourceLocation TagLoc,
                            DeclSpec &DS, const ParsedTemplateInfo &TemplateInfo,
                            AccessSpecifier AS, bool EnteringContext,
-                           DeclSpecContext DSC);
+                           DeclSpecContext DSC, 
+                           ParsedAttributesWithRange &Attributes);
   void ParseCXXMemberSpecification(SourceLocation StartLoc, unsigned TagType,
                                    Decl *TagDecl);
   ExprResult ParseCXXMemberInitializer(Decl *D, bool IsFunction,
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index 7c5b1531407..23d1f40e9eb 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -1842,7 +1842,8 @@ static bool isValidAfterIdentifierInDeclarator(const Token &T) {
 ///
 bool Parser::ParseImplicitInt(DeclSpec &DS, CXXScopeSpec *SS,
                               const ParsedTemplateInfo &TemplateInfo,
-                              AccessSpecifier AS, DeclSpecContext DSC) {
+                              AccessSpecifier AS, DeclSpecContext DSC, 
+                              ParsedAttributesWithRange &Attrs) {
   assert(Tok.is(tok::identifier) && "should have identifier");
 
   SourceLocation Loc = Tok.getLocation();
@@ -1928,7 +1929,7 @@ bool Parser::ParseImplicitInt(DeclSpec &DS, CXXScopeSpec *SS,
         ParseEnumSpecifier(Loc, DS, TemplateInfo, AS, DSC_normal);
       else
         ParseClassSpecifier(TagKind, Loc, DS, TemplateInfo, AS,
-                            /*EnteringContext*/ false, DSC_normal);
+                            /*EnteringContext*/ false, DSC_normal, Attrs);
       return true;
     }
   }
@@ -2352,7 +2353,14 @@ void Parser::ParseDeclarationSpecifiers(DeclSpec &DS,
       // typename.
       if (TypeRep == 0) {
         ConsumeToken();   // Eat the scope spec so the identifier is current.
-        if (ParseImplicitInt(DS, &SS, TemplateInfo, AS, DSContext)) continue;
+        ParsedAttributesWithRange Attrs(AttrFactory);
+        if (ParseImplicitInt(DS, &SS, TemplateInfo, AS, DSContext, Attrs)) {
+          if (!Attrs.empty()) {
+            AttrsLastTime = true;
+            attrs.takeAllFrom(Attrs);
+          }
+          continue;
+        }
         goto DoneWithDeclSpec;
       }
 
@@ -2448,7 +2456,14 @@ void Parser::ParseDeclarationSpecifiers(DeclSpec &DS,
       // If this is not a typedef name, don't parse it as part of the declspec,
       // it must be an implicit int or an error.
       if (!TypeRep) {
-        if (ParseImplicitInt(DS, 0, TemplateInfo, AS, DSContext)) continue;
+        ParsedAttributesWithRange Attrs(AttrFactory);
+        if (ParseImplicitInt(DS, 0, TemplateInfo, AS, DSContext, Attrs)) {
+          if (!Attrs.empty()) {
+            AttrsLastTime = true;
+            attrs.takeAllFrom(Attrs);
+          }
+          continue;
+        }
         goto DoneWithDeclSpec;
       }
 
@@ -2749,8 +2764,20 @@ void Parser::ParseDeclarationSpecifiers(DeclSpec &DS,
     case tok::kw_union: {
       tok::TokenKind Kind = Tok.getKind();
       ConsumeToken();
+
+      // These are attributes following class specifiers.
+      // To produce better diagnostic, we parse them when
+      // parsing class specifier.
+      ParsedAttributesWithRange Attributes(AttrFactory);
       ParseClassSpecifier(Kind, Loc, DS, TemplateInfo, AS,
-                          EnteringContext, DSContext);
+                          EnteringContext, DSContext, Attributes);
+
+      // If there are attributes following class specifier,
+      // take them over and handle them here.
+      if (!Attributes.empty()) {
+        AttrsLastTime = true;
+        attrs.takeAllFrom(Attributes);
+      }
       continue;
     }
 
diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp
index b8ebd9b6a7e..700cf4e4afd 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -1052,7 +1052,8 @@ void Parser::ParseClassSpecifier(tok::TokenKind TagTokKind,
                                  SourceLocation StartLoc, DeclSpec &DS,
                                  const ParsedTemplateInfo &TemplateInfo,
                                  AccessSpecifier AS, 
-                                 bool EnteringContext, DeclSpecContext DSC) {
+                                 bool EnteringContext, DeclSpecContext DSC, 
+                                 ParsedAttributesWithRange &Attributes) {
   DeclSpec::TST TagType;
   if (TagTokKind == tok::kw_struct)
     TagType = DeclSpec::TST_struct;
@@ -1234,12 +1235,24 @@ void Parser::ParseClassSpecifier(tok::TokenKind TagTokKind,
   //  - If we have 'struct foo;', then this is either a forward declaration
   //    or a friend declaration, which have to be treated differently.
   //  - Otherwise we have something like 'struct foo xyz', a reference.
+  //
+  //  We also detect these erroneous cases to provide better diagnostic for
+  //  C++11 attributes parsing.
+  //  - attributes follow class name:
+  //    struct foo [[]] {};
+  //  - attributes appear before or after 'final':
+  //    struct foo [[]] final [[]] {};
+  //
   // However, in type-specifier-seq's, things look like declarations but are
   // just references, e.g.
   //   new struct s;
   // or
   //   &T::operator struct s;
   // For these, DSC is DSC_type_specifier.
+
+  // If there are attributes after class name, parse them.
+  MaybeParseCXX0XAttributes(Attributes);
+
   Sema::TagUseKind TUK;
   if (DSC == DSC_trailing)
     TUK = Sema::TUK_Reference;
@@ -1261,6 +1274,39 @@ void Parser::ParseClassSpecifier(tok::TokenKind TagTokKind,
       // Okay, this is a class definition.
       TUK = Sema::TUK_Definition;
     }
+  } else if (isCXX0XFinalKeyword() && (NextToken().is(tok::l_square) || 
+                                       NextToken().is(tok::kw_alignas) ||
+                                       NextToken().is(tok::kw__Alignas))) {
+    // We can't tell if this is a definition or reference
+    // until we skipped the 'final' and C++11 attribute specifiers.
+    TentativeParsingAction PA(*this);
+
+    // Skip the 'final' keyword.
+    ConsumeToken();
+
+    // Skip C++11 attribute specifiers.
+    while (true) {
+      if (Tok.is(tok::l_square) && NextToken().is(tok::l_square)) {
+        ConsumeBracket();
+        if (!SkipUntil(tok::r_square))
+          break;
+      } else if ((Tok.is(tok::kw_alignas) || Tok.is(tok::kw__Alignas)) &&
+                 NextToken().is(tok::l_paren)) {
+        ConsumeToken();
+        ConsumeParen();
+        if (!SkipUntil(tok::r_paren))
+          break;
+      } else {
+        break;
+      }
+    }
+
+    if (Tok.is(tok::l_brace) || Tok.is(tok::colon))
+      TUK = Sema::TUK_Definition;
+    else
+      TUK = Sema::TUK_Reference;
+
+    PA.Revert();
   } else if (DSC != DSC_type_specifier &&
              (Tok.is(tok::semi) ||
               (Tok.isAtStartOfLine() && !isValidAfterTypeSpecifier(false)))) {
@@ -1275,6 +1321,12 @@ void Parser::ParseClassSpecifier(tok::TokenKind TagTokKind,
   } else
     TUK = Sema::TUK_Reference;
 
+  // Forbid misplaced attributes. In cases of a reference, we pass attributes
+  // to caller to handle.
+  // FIXME: provide fix-it hints if we can.
+  if (TUK != Sema::TUK_Reference)
+    ProhibitAttributes(Attributes);
+
   // If this is an elaborated type specifier, and we delayed
   // diagnostics before, just merge them into the current pool.
   if (shouldDelayDiagsInTag) {
@@ -2326,6 +2378,11 @@ void Parser::ParseCXXMemberSpecification(SourceLocation RecordLoc,
            diag::warn_cxx98_compat_override_control_keyword :
            diag::ext_override_control_keyword) << "final";
     }
+
+    // Forbid C++11 attributes that appear here.
+    ParsedAttributesWithRange Attrs(AttrFactory);
+    MaybeParseCXX0XAttributes(Attrs);
+    ProhibitAttributes(Attrs);
   }
 
   if (Tok.is(tok::colon)) {
diff --git a/clang/test/Parser/cxx0x-attributes.cpp b/clang/test/Parser/cxx0x-attributes.cpp
index 58e42bffcff..54baaedfd39 100644
--- a/clang/test/Parser/cxx0x-attributes.cpp
+++ b/clang/test/Parser/cxx0x-attributes.cpp
@@ -62,6 +62,29 @@ struct MemberFnOrder {
 struct [[]] struct_attr;
 class [[]] class_attr {};
 union [[]] union_attr;
+
+// Checks attributes placed at wrong syntactic locations of class specifiers.
+// FIXME: provide fix-it hint.
+class [[]] [[]]
+  attr_after_class_name_decl [[]] [[]]; // expected-error {{an attribute list cannot appear here}}
+
+class [[]] [[]]
+ attr_after_class_name_definition [[]] [[]] [[]]{}; // expected-error {{an attribute list cannot appear here}}
+
+class [[]] c {};
+class c [[]] [[]] x;
+class c [[]] [[]] y [[]] [[]];
+class c final [(int){0}];
+
+class base {};
+class [[]] [[]] final_class 
+  alignas(float) [[]] final // expected-error {{an attribute list cannot appear here}}
+  alignas(float) [[]] [[]] alignas(float): base{}; // expected-error {{an attribute list cannot appear here}}
+
+class [[]] [[]] final_class_another 
+  [[]] [[]] alignas(16) final // expected-error {{an attribute list cannot appear here}}
+  [[]] [[]] alignas(16) [[]]{}; // expected-error {{an attribute list cannot appear here}}
+
 [[]] struct with_init_declarators {} init_declarator;
 [[]] struct no_init_declarators; // expected-error {{an attribute list cannot appear here}}
 [[]];