r193731 - Factor out custom parsing for iboutletcollection and vec_type_hint attributes

Richard Smith richard-llvm at metafoo.co.uk
Wed Oct 30 18:56:19 PDT 2013


Author: rsmith
Date: Wed Oct 30 20:56:18 2013
New Revision: 193731

URL: http://llvm.org/viewvc/llvm-project?rev=193731&view=rev
Log:
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.

Modified:
    cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
    cfe/trunk/include/clang/Parse/Parser.h
    cfe/trunk/include/clang/Sema/AttributeList.h
    cfe/trunk/lib/Parse/ParseDecl.cpp
    cfe/trunk/lib/Sema/SemaDeclAttr.cpp
    cfe/trunk/test/Parser/attributes.c
    cfe/trunk/test/Parser/cxx-attributes.cpp
    cfe/trunk/test/SemaObjC/iboutletcollection-attr.m
    cfe/trunk/tools/libclang/CIndex.cpp
    cfe/trunk/tools/libclang/IndexingContext.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=193731&r1=193730&r2=193731&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Wed Oct 30 20:56:18 2013
@@ -169,8 +169,6 @@ def err_expected_member_name_or_semi : E
   "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++">;

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=193731&r1=193730&r2=193731&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Oct 30 20:56:18 2013
@@ -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>;

Modified: cfe/trunk/include/clang/Parse/Parser.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=193731&r1=193730&r2=193731&view=diff
==============================================================================
--- cfe/trunk/include/clang/Parse/Parser.h (original)
+++ cfe/trunk/include/clang/Parse/Parser.h Wed Oct 30 20:56:18 2013
@@ -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,

Modified: cfe/trunk/include/clang/Sema/AttributeList.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/AttributeList.h?rev=193731&r1=193730&r2=193731&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/AttributeList.h (original)
+++ cfe/trunk/include/clang/Sema/AttributeList.h Wed Oct 30 20:56:18 2013
@@ -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();
   }
 

Modified: cfe/trunk/lib/Parse/ParseDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=193731&r1=193730&r2=193731&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/ParseDecl.cpp (original)
+++ cfe/trunk/lib/Parse/ParseDecl.cpp Wed Oct 30 20:56:18 2013
@@ -200,6 +200,32 @@ IdentifierLoc *Parser::ParseIdentifierLo
   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(Ident
   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(Ident
     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(Ident
         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);
   }
 }
 

Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=193731&r1=193730&r2=193731&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Wed Oct 30 20:56:18 2013
@@ -1339,33 +1339,37 @@ static void handleIBOutletCollection(Sem
   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()));
 }
 

Modified: cfe/trunk/test/Parser/attributes.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/attributes.c?rev=193731&r1=193730&r2=193731&view=diff
==============================================================================
--- cfe/trunk/test/Parser/attributes.c (original)
+++ cfe/trunk/test/Parser/attributes.c Wed Oct 30 20:56:18 2013
@@ -58,7 +58,7 @@ void d2(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 ')'}}
 

Modified: cfe/trunk/test/Parser/cxx-attributes.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/cxx-attributes.cpp?rev=193731&r1=193730&r2=193731&view=diff
==============================================================================
--- cfe/trunk/test/Parser/cxx-attributes.cpp (original)
+++ cfe/trunk/test/Parser/cxx-attributes.cpp Wed Oct 30 20:56:18 2013
@@ -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}}
 }

Modified: cfe/trunk/test/SemaObjC/iboutletcollection-attr.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/iboutletcollection-attr.m?rev=193731&r1=193730&r2=193731&view=diff
==============================================================================
--- cfe/trunk/test/SemaObjC/iboutletcollection-attr.m (original)
+++ cfe/trunk/test/SemaObjC/iboutletcollection-attr.m Wed Oct 30 20:56:18 2013
@@ -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}}
 }
- at property (nonatomic, retain) __attribute__((iboutletcollection(I,2,3))) id prop1; // expected-error {{'iboutletcollection' attribute takes one argument}}
- at property (nonatomic, retain) __attribute__((iboutletcollection(B))) id prop2; // expected-error {{invalid type 'B' as argument of iboutletcollection attribute}}
+ at property (nonatomic, retain) __attribute__((iboutletcollection(I,2,3))) id prop1; // expected-error {{expected ')'}} expected-note {{to match}}
+ at 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

Modified: cfe/trunk/tools/libclang/CIndex.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/CIndex.cpp?rev=193731&r1=193730&r2=193731&view=diff
==============================================================================
--- cfe/trunk/tools/libclang/CIndex.cpp (original)
+++ cfe/trunk/tools/libclang/CIndex.cpp Wed Oct 30 20:56:18 2013
@@ -532,8 +532,8 @@ bool CursorVisitor::VisitChildren(CXCurs
   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));
   }
 

Modified: cfe/trunk/tools/libclang/IndexingContext.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/IndexingContext.cpp?rev=193731&r1=193730&r2=193731&view=diff
==============================================================================
--- cfe/trunk/tools/libclang/IndexingContext.cpp (original)
+++ cfe/trunk/tools/libclang/IndexingContext.cpp Wed Oct 30 20:56:18 2013
@@ -99,8 +99,8 @@ AttrListInfo::AttrListInfo(const Decl *D
     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,





More information about the cfe-commits mailing list