Factor out custom parsing for iboutletcollection and vec_type_hint attributes

into a separate "parse an attribute that takes a type argument" codepath. This
results in both codepaths being a lot cleaner and simpler, and fixes some bugs
where the type argument handling bled into the expression argument handling and
caused us to both accept invalid and reject valid attribute arguments.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@193731 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Basic/DiagnosticParseKinds.td b/include/clang/Basic/DiagnosticParseKinds.td
index a45e89e3980cdb2e596cfbe3315e5268a969d9de..865200b17b79e4b5735abbfb2e5487b851c2e00d 100644 (file)
--- a/include/clang/Basic/DiagnosticParseKinds.td
+++ b/include/clang/Basic/DiagnosticParseKinds.td
@@ -169,8 +169,6 @@ def err_expected_member_name_or_semi : Error<
   "expected member name or ';' after declaration specifiers">;
 def err_function_declared_typedef : Error<
   "function definition declared 'typedef'">;
-def err_iboutletcollection_builtintype : Error<
-  "type argument of iboutletcollection attribute cannot be a builtin type">;
 def err_iboutletcollection_with_protocol : Error<
   "invalid argument of iboutletcollection attribute">;
 def err_at_defs_cxx : Error<"@defs is not supported in Objective-C++">;

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index c90b54600a4c26da95e13822505b6991533f16ca..fea3fe996de351b9981202660fcd899cee6cabc3 100644 (file)
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2399,6 +2399,8 @@ def warn_attribute_ibaction: Warning<
   InGroup<IgnoredAttributes>;
 def err_iboutletcollection_type : Error<
   "invalid type %0 as argument of iboutletcollection attribute">;
+def err_iboutletcollection_builtintype : Error<
+  "type argument of iboutletcollection attribute cannot be a builtin type">;
 def warn_iboutlet_object_type : Warning<
   "%select{instance variable|property}2 with %0 attribute must "
   "be an object type (invalid %1)">, InGroup<ObjCInvalidIBOutletProperty>;

diff --git a/include/clang/Parse/Parser.h b/include/clang/Parse/Parser.h
index 9ff5bdfb456cf7b061f0d53897fe22dc37659e59..d314087ddffd3b9231c8c0ee7532e354ca2146f6 100644 (file)
--- a/include/clang/Parse/Parser.h
+++ b/include/clang/Parse/Parser.h
@@ -1980,6 +1980,11 @@ private:
                                         ParsedAttributes &Attrs,
                                         SourceLocation *EndLoc);
 
+  void ParseAttributeWithTypeArg(IdentifierInfo &AttrName,
+                                 SourceLocation AttrNameLoc,
+                                 ParsedAttributes &Attrs,
+                                 SourceLocation *EndLoc);
+
   void ParseTypeofSpecifier(DeclSpec &DS);
   SourceLocation ParseDecltypeSpecifier(DeclSpec &DS);
   void AnnotateExistingDecltypeSpecifier(const DeclSpec &DS,

diff --git a/include/clang/Sema/AttributeList.h b/include/clang/Sema/AttributeList.h
index 583f61933b3eafbd6820bd9ecdf10db58ae4594c..508064def2caf04246fded12e688563f366e985d 100644 (file)
--- a/include/clang/Sema/AttributeList.h
+++ b/include/clang/Sema/AttributeList.h
@@ -450,7 +450,7 @@ public:
   }
 
   const ParsedType &getTypeArg() const {
-    assert(getKind() == AT_VecTypeHint && "Not a type attribute");
+    assert(HasParsedType && "Not a type attribute");
     return getTypeBuffer();
   }
 

diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp
index 524754bd9079e7fe5711a3d7aa3665d4c040cb0d..27b4919b3426de59bae8da624534e44bf4ded4b9 100644 (file)
--- a/lib/Parse/ParseDecl.cpp
+++ b/lib/Parse/ParseDecl.cpp
@@ -200,6 +200,32 @@ IdentifierLoc *Parser::ParseIdentifierLoc() {
   return IL;
 }
 
+void Parser::ParseAttributeWithTypeArg(IdentifierInfo &AttrName,
+                                       SourceLocation AttrNameLoc,
+                                       ParsedAttributes &Attrs,
+                                       SourceLocation *EndLoc) {
+  BalancedDelimiterTracker Parens(*this, tok::l_paren);
+  Parens.consumeOpen();
+
+  TypeResult T;
+  if (Tok.isNot(tok::r_paren))
+    T = ParseTypeName();
+
+  if (Parens.consumeClose())
+    return;
+
+  if (T.isInvalid())
+    return;
+
+  if (T.isUsable())
+    Attrs.addNewTypeAttr(&AttrName,
+                         SourceRange(AttrNameLoc, Parens.getCloseLocation()), 0,
+                         AttrNameLoc, T.get(), AttributeList::AS_GNU);
+  else
+    Attrs.addNew(&AttrName, SourceRange(AttrNameLoc, Parens.getCloseLocation()),
+                 0, AttrNameLoc, 0, 0, AttributeList::AS_GNU);
+}
+
 /// Parse the arguments to a parameterized GNU attribute or
 /// a C++11 attribute in "gnu" namespace.
 void Parser::ParseGNUAttributeArgs(IdentifierInfo *AttrName,
@@ -213,15 +239,18 @@ void Parser::ParseGNUAttributeArgs(IdentifierInfo *AttrName,
   assert(Tok.is(tok::l_paren) && "Attribute arg list not starting with '('");
 
   AttributeList::Kind AttrKind =
-      AttributeList::getKind(AttrName, ScopeName, AttributeList::AS_GNU);
+      AttributeList::getKind(AttrName, ScopeName, Syntax);
 
   // Availability attributes have their own grammar.
+  // FIXME: All these cases fail to pass in the syntax and scope, and might be
+  // written as C++11 gnu:: attributes.
   if (AttrKind == AttributeList::AT_Availability) {
     ParseAvailabilityAttribute(*AttrName, AttrNameLoc, Attrs, EndLoc);
     return;
   }
-  // Thread safety attributes fit into the FIXME case above, so we
-  // just parse the arguments as a list of expressions
+  // Thread safety attributes are parsed in an unevaluated context.
+  // FIXME: Share the bulk of the parsing code here and just pull out
+  // the unevaluated context.
   if (IsThreadSafetyAttribute(AttrName->getName())) {
     ParseThreadSafetyAttribute(*AttrName, AttrNameLoc, Attrs, EndLoc);
     return;
@@ -231,74 +260,34 @@ void Parser::ParseGNUAttributeArgs(IdentifierInfo *AttrName,
     ParseTypeTagForDatatypeAttribute(*AttrName, AttrNameLoc, Attrs, EndLoc);
     return;
   }
+  // __attribute__((vec_type_hint)) and iboutletcollection expect a type arg.
+  if (AttrKind == AttributeList::AT_VecTypeHint ||
+      AttrKind == AttributeList::AT_IBOutletCollection) {
+    ParseAttributeWithTypeArg(*AttrName, AttrNameLoc, Attrs, EndLoc);
+    return;
+  }
 
   // Ignore the left paren location for now.
   ConsumeParen();
 
-  bool BuiltinType = false;
   ArgsVector ArgExprs;
 
-  TypeResult T;
-  SourceRange TypeRange;
-  bool TypeParsed = false;
-
-  switch (Tok.getKind()) {
-  case tok::kw_char:
-  case tok::kw_wchar_t:
-  case tok::kw_char16_t:
-  case tok::kw_char32_t:
-  case tok::kw_bool:
-  case tok::kw_short:
-  case tok::kw_int:
-  case tok::kw_long:
-  case tok::kw___int64:
-  case tok::kw___int128:
-  case tok::kw_signed:
-  case tok::kw_unsigned:
-  case tok::kw_float:
-  case tok::kw_double:
-  case tok::kw_void:
-  case tok::kw_typeof:
-    // __attribute__(( vec_type_hint(char) ))
-    BuiltinType = true;
-    T = ParseTypeName(&TypeRange);
-    TypeParsed = true;
-    break;
-
-  case tok::identifier: {
-    if (AttrKind == AttributeList::AT_VecTypeHint) {
-      T = ParseTypeName(&TypeRange);
-      TypeParsed = true;
-      break;
-    }
-
+  if (Tok.is(tok::identifier)) {
     // If this attribute wants an 'identifier' argument, make it so.
-    if (attributeHasIdentifierArg(*AttrName))
-      ArgExprs.push_back(ParseIdentifierLoc());
-
-    // __attribute__((iboutletcollection)) expects an identifier then
-    // some other (ignored) things.
-    if (AttrKind == AttributeList::AT_IBOutletCollection)
-      ArgExprs.push_back(ParseIdentifierLoc());
+    bool IsIdentifierArg = attributeHasIdentifierArg(*AttrName);
 
     // If we don't know how to parse this attribute, but this is the only
     // token in this argument, assume it's meant to be an identifier.
     if (AttrKind == AttributeList::UnknownAttribute) {
       const Token &Next = NextToken();
-      if (Next.is(tok::r_paren) || Next.is(tok::comma))
-        ArgExprs.push_back(ParseIdentifierLoc());
+      IsIdentifierArg = Next.is(tok::r_paren) || Next.is(tok::comma);
     }
-  } break;
 
-  default:
-    break;
+    if (IsIdentifierArg)
+      ArgExprs.push_back(ParseIdentifierLoc());
   }
 
-  bool isInvalid = false;
-  bool isParmType = false;
-
-  if (!BuiltinType && AttrKind != AttributeList::AT_VecTypeHint &&
-      (!ArgExprs.empty() ? Tok.is(tok::comma) : Tok.isNot(tok::r_paren))) {
+  if (!ArgExprs.empty() ? Tok.is(tok::comma) : Tok.isNot(tok::r_paren)) {
     // Eat the comma.
     if (!ArgExprs.empty())
       ConsumeToken();
@@ -315,49 +304,13 @@ void Parser::ParseGNUAttributeArgs(IdentifierInfo *AttrName,
         break;
       ConsumeToken(); // Eat the comma, move to the next argument
     }
-  } else if (Tok.is(tok::less) &&
-             AttrKind == AttributeList::AT_IBOutletCollection) {
-    if (!ExpectAndConsume(tok::less, diag::err_expected_less_after, "<",
-                          tok::greater)) {
-      while (Tok.is(tok::identifier)) {
-        ConsumeToken();
-        if (Tok.is(tok::greater))
-          break;
-        if (Tok.is(tok::comma)) {
-          ConsumeToken();
-          continue;
-        }
-      }
-      if (Tok.isNot(tok::greater))
-        Diag(Tok, diag::err_iboutletcollection_with_protocol);
-      SkipUntil(tok::r_paren, false, true); // skip until ')'
-    }
-  } else if (AttrKind == AttributeList::AT_VecTypeHint) {
-    if (T.get() && !T.isInvalid())
-      isParmType = true;
-    else {
-      if (Tok.is(tok::identifier))
-        ConsumeToken();
-      if (TypeParsed)
-        isInvalid = true;
-    }
   }
 
   SourceLocation RParen = Tok.getLocation();
-  if (!ExpectAndConsume(tok::r_paren, diag::err_expected_rparen) &&
-      !isInvalid) {
+  if (!ExpectAndConsume(tok::r_paren, diag::err_expected_rparen)) {
     SourceLocation AttrLoc = ScopeLoc.isValid() ? ScopeLoc : AttrNameLoc;
-    if (isParmType) {
-      Attrs.addNewTypeAttr(AttrName, SourceRange(AttrLoc, RParen), ScopeName,
-                           ScopeLoc, T.get(), Syntax);
-    } else {
-      AttributeList *attr = Attrs.addNew(
-          AttrName, SourceRange(AttrLoc, RParen), ScopeName, ScopeLoc,
-          ArgExprs.data(), ArgExprs.size(), Syntax);
-      if (BuiltinType &&
-          attr->getKind() == AttributeList::AT_IBOutletCollection)
-        Diag(Tok, diag::err_iboutletcollection_builtintype);
-    }
+    Attrs.addNew(AttrName, SourceRange(AttrLoc, RParen), ScopeName, ScopeLoc,
+                 ArgExprs.data(), ArgExprs.size(), Syntax);
   }
 }
 

diff --git a/lib/Sema/SemaDeclAttr.cpp b/lib/Sema/SemaDeclAttr.cpp
index a279abbbd8ebea539662f8115a3cee6ac7d7dc63..bfdf26aabd4bdfcfef9d9bc267468ca33d259898 100644 (file)
--- a/lib/Sema/SemaDeclAttr.cpp
+++ b/lib/Sema/SemaDeclAttr.cpp
@@ -1339,33 +1339,37 @@ static void handleIBOutletCollection(Sema &S, Decl *D,
   if (!checkIBOutletCommon(S, D, Attr))
     return;
 
-  IdentifierLoc *IL = Attr.isArgIdent(0) ? Attr.getArgAsIdent(0) : 0;
-  IdentifierInfo *II;
-  SourceLocation ILS;
-  if (IL) {
-    II = IL->Ident;
-    ILS = IL->Loc;
-  } else {
-    II = &S.Context.Idents.get("NSObject");
-  }
-  
-  ParsedType TypeRep = S.getTypeName(*II, Attr.getLoc(), 
-                        S.getScopeForContext(D->getDeclContext()->getParent()));
-  if (!TypeRep) {
-    S.Diag(Attr.getLoc(), diag::err_iboutletcollection_type) << II;
-    return;
+  ParsedType PT;
+
+  if (Attr.hasParsedType())
+    PT = Attr.getTypeArg();
+  else {
+    PT = S.getTypeName(S.Context.Idents.get("NSObject"), Attr.getLoc(),
+                       S.getScopeForContext(D->getDeclContext()->getParent()));
+    if (!PT) {
+      S.Diag(Attr.getLoc(), diag::err_iboutletcollection_type) << "NSObject";
+      return;
+    }
   }
-  QualType QT = TypeRep.get();
+
+  TypeSourceInfo *TSI = 0;
+  QualType QT = S.GetTypeFromParser(PT, &TSI);
+  SourceLocation QTLoc =
+      TSI ? TSI->getTypeLoc().getLocStart() : SourceLocation();
+
   // Diagnose use of non-object type in iboutletcollection attribute.
   // FIXME. Gnu attribute extension ignores use of builtin types in
   // attributes. So, __attribute__((iboutletcollection(char))) will be
   // treated as __attribute__((iboutletcollection())).
   if (!QT->isObjCIdType() && !QT->isObjCObjectType()) {
-    S.Diag(Attr.getLoc(), diag::err_iboutletcollection_type) << II;
+    S.Diag(Attr.getLoc(),
+           QT->isBuiltinType() ? diag::err_iboutletcollection_builtintype
+                               : diag::err_iboutletcollection_type) << QT;
     return;
   }
+
   D->addAttr(::new (S.Context)
-             IBOutletCollectionAttr(Attr.getRange(), S.Context, QT, ILS,
+             IBOutletCollectionAttr(Attr.getRange(), S.Context, QT, QTLoc,
                                     Attr.getAttributeSpellingListIndex()));
 }
 

diff --git a/test/Parser/attributes.c b/test/Parser/attributes.c
index 347cb9c1bfbf3600d9236fe563baaed23368596a..376ed2e7d2c9c5af2937bd6e61300c7a2b36c55a 100644 (file)
--- a/test/Parser/attributes.c
+++ b/test/Parser/attributes.c
@@ -58,7 +58,7 @@ void d2(void) __attribute__((noreturn)), d3(void) __attribute__((noreturn));
 void __attribute__((returns_twice)) returns_twice_test();
 
 int aligned(int);
-int __attribute__((vec_type_hint(char, aligned(16) )) missing_rparen_1; // expected-error {{expected ')'}}
+int __attribute__((vec_type_hint(char, aligned(16) )) missing_rparen_1; // expected-error 2{{expected ')'}} expected-note {{to match}} expected-warning {{does not declare anything}}
 int __attribute__((mode(x aligned(16) )) missing_rparen_2; // expected-error {{expected ')'}}
 int __attribute__((format(printf, 0 aligned(16) )) missing_rparen_3; // expected-error {{expected ')'}}
 

diff --git a/test/Parser/cxx-attributes.cpp b/test/Parser/cxx-attributes.cpp
index 3c99ccdb3a78f412aa19504710a50144c24b0df4..6fd7f4f68a465adec87252164df41534e55c3342 100644 (file)
--- a/test/Parser/cxx-attributes.cpp
+++ b/test/Parser/cxx-attributes.cpp
@@ -16,4 +16,7 @@ namespace PR17666 {
   const int A = 1;
   typedef int __attribute__((__aligned__(A))) T1;
   int check1[__alignof__(T1) == 1 ? 1 : -1];
+
+  typedef int __attribute__((aligned(int(1)))) T1;
+  typedef int __attribute__((aligned(int))) T2; // expected-error {{expected '(' for function-style cast}}
 }

diff --git a/test/SemaObjC/iboutletcollection-attr.m b/test/SemaObjC/iboutletcollection-attr.m
index 3dea1337a8bc00a07273dd452090368676a08956..f088ca32b6adf90a54dddd1a735b4d1539746d79 100644 (file)
--- a/test/SemaObjC/iboutletcollection-attr.m
+++ b/test/SemaObjC/iboutletcollection-attr.m
@@ -18,15 +18,15 @@
 
 typedef void *PV;
 @interface BAD {
-    __attribute__((iboutletcollection(I, 1))) id ivar1; // expected-error {{'iboutletcollection' attribute takes one argument}}
-    __attribute__((iboutletcollection(B))) id ivar2; // expected-error {{invalid type 'B' as argument of iboutletcollection attribute}}
-    __attribute__((iboutletcollection(PV))) id ivar3; // expected-error {{invalid type 'PV' as argument of iboutletcollection attribute}}
+    __attribute__((iboutletcollection(I, 1))) id ivar1; // expected-error {{expected ')'}} expected-note {{to match}}
+    __attribute__((iboutletcollection(B))) id ivar2; // expected-error {{unknown type name 'B'}}
+    __attribute__((iboutletcollection(PV))) id ivar3; // expected-error {{invalid type 'PV' (aka 'void *') as argument of iboutletcollection attribute}}
     __attribute__((iboutletcollection(PV))) void *ivar4; // expected-warning {{instance variable with 'iboutletcollection' attribute must be an object type (invalid 'void *')}}
     __attribute__((iboutletcollection(int))) id ivar5; // expected-error {{type argument of iboutletcollection attribute cannot be a builtin type}}
     __attribute__((iboutlet)) int ivar6;  // expected-warning {{instance variable with 'iboutlet' attribute must be an object type}}
 }
-@property (nonatomic, retain) __attribute__((iboutletcollection(I,2,3))) id prop1; // expected-error {{'iboutletcollection' attribute takes one argument}}
-@property (nonatomic, retain) __attribute__((iboutletcollection(B))) id prop2; // expected-error {{invalid type 'B' as argument of iboutletcollection attribute}}
+@property (nonatomic, retain) __attribute__((iboutletcollection(I,2,3))) id prop1; // expected-error {{expected ')'}} expected-note {{to match}}
+@property (nonatomic, retain) __attribute__((iboutletcollection(B))) id prop2; // expected-error {{unknown type name 'B'}}
 
 @property __attribute__((iboutletcollection(BAD))) int prop3; // expected-warning {{property with 'iboutletcollection' attribute must be an object type (invalid 'int')}}
 @end

diff --git a/tools/libclang/CIndex.cpp b/tools/libclang/CIndex.cpp
index 7288402963badcc1a674353db4c0689a8fb9bdd1..48c2ee35363b0f38324f05dfa06eaefbc39fbd45 100644 (file)
--- a/tools/libclang/CIndex.cpp
+++ b/tools/libclang/CIndex.cpp
@@ -532,8 +532,8 @@ bool CursorVisitor::VisitChildren(CXCursor Cursor) {
   if (Cursor.kind == CXCursor_IBOutletCollectionAttr) {
     const IBOutletCollectionAttr *A =
       cast<IBOutletCollectionAttr>(cxcursor::getCursorAttr(Cursor));
-    if (const ObjCInterfaceType *InterT = A->getInterface()->getAs<ObjCInterfaceType>())
-      return Visit(cxcursor::MakeCursorObjCClassRef(InterT->getInterface(),
+    if (const ObjCObjectType *ObjT = A->getInterface()->getAs<ObjCObjectType>())
+      return Visit(cxcursor::MakeCursorObjCClassRef(ObjT->getInterface(),
                                                     A->getInterfaceLoc(), TU));
   }
 

diff --git a/tools/libclang/IndexingContext.cpp b/tools/libclang/IndexingContext.cpp
index cfa6b851737eba0e90e2d6e0294abca133846f5e..6dadd7291f7785b072db0028b07f6f1be19d4daf 100644 (file)
--- a/tools/libclang/IndexingContext.cpp
+++ b/tools/libclang/IndexingContext.cpp
@@ -99,8 +99,8 @@ AttrListInfo::AttrListInfo(const Decl *D, IndexingContext &IdxCtx)
     IBInfo.IBCollInfo.objcClass = 0;
     IBInfo.IBCollInfo.classCursor = clang_getNullCursor();
     QualType Ty = IBAttr->getInterface();
-    if (const ObjCInterfaceType *InterTy = Ty->getAs<ObjCInterfaceType>()) {
-      if (const ObjCInterfaceDecl *InterD = InterTy->getInterface()) {
+    if (const ObjCObjectType *ObjectTy = Ty->getAs<ObjCObjectType>()) {
+      if (const ObjCInterfaceDecl *InterD = ObjectTy->getInterface()) {
         IdxCtx.getEntityInfo(InterD, IBInfo.ClassInfo, SA);
         IBInfo.IBCollInfo.objcClass = &IBInfo.ClassInfo;
         IBInfo.IBCollInfo.classCursor = MakeCursorObjCClassRef(InterD,