thakis updated this revision to Diff 69312.
thakis marked an inline comment as done.
thakis added a comment.

comments round 1


https://reviews.llvm.org/D23895

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/Attributes.h
  include/clang/Parse/Parser.h
  include/clang/Sema/AttributeList.h
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseDeclCXX.cpp
  lib/Parse/ParseObjc.cpp
  lib/Parse/ParseOpenMP.cpp
  lib/Parse/Parser.cpp
  test/Parser/MicrosoftExtensions.cpp
  test/Sema/MicrosoftExtensions.c

Index: test/Sema/MicrosoftExtensions.c
===================================================================
--- test/Sema/MicrosoftExtensions.c
+++ test/Sema/MicrosoftExtensions.c
@@ -28,6 +28,8 @@
 
 struct __declspec(uuid("00000000-0000-0000-C000-000000000046")) IUnknown {}; /* expected-error {{'uuid' attribute is not supported in C}} */
 
+[uuid("00000000-0000-0000-C000-000000000046")] struct IUnknown2 {}; /* expected-error {{'uuid' attribute is not supported in C}} */
+
 typedef struct notnested {
   long bad1;
   long bad2;
Index: test/Parser/MicrosoftExtensions.cpp
===================================================================
--- test/Parser/MicrosoftExtensions.cpp
+++ test/Parser/MicrosoftExtensions.cpp
@@ -49,6 +49,7 @@
 struct __declspec(uuid("0000000-0000-0000-1234-0000500000047")) uuid_attr_bad3 { };// expected-error {{uuid attribute contains a malformed GUID}}
 struct __declspec(uuid("0000000-0000-0000-Z234-000000000047")) uuid_attr_bad4 { };// expected-error {{uuid attribute contains a malformed GUID}}
 struct __declspec(uuid("000000000000-0000-1234-000000000047")) uuid_attr_bad5 { };// expected-error {{uuid attribute contains a malformed GUID}}
+[uuid("000000000000-0000-1234-000000000047")] struct uuid_attr_bad6 { };// expected-error {{uuid attribute contains a malformed GUID}}
 
 __declspec(uuid("000000A0-0000-0000-C000-000000000046")) int i; // expected-warning {{'uuid' attribute only applies to classes}}
 
@@ -59,6 +60,8 @@
 struct __declspec(uuid("000000A0-0000-0000-C000-000000000049"))
 struct_with_uuid2;
 
+[uuid("000000A0-0000-0000-C000-000000000049")] struct struct_with_uuid3;
+
 struct
 struct_with_uuid2 {} ;
 
@@ -69,6 +72,7 @@
 
    __uuidof(struct_with_uuid);
    __uuidof(struct_with_uuid2);
+   __uuidof(struct_with_uuid3);
    __uuidof(struct_without_uuid); // expected-error {{cannot call operator __uuidof on a type with no GUID}}
    __uuidof(struct_with_uuid*);
    __uuidof(struct_without_uuid*); // expected-error {{cannot call operator __uuidof on a type with no GUID}}
Index: lib/Parse/Parser.cpp
===================================================================
--- lib/Parse/Parser.cpp
+++ lib/Parse/Parser.cpp
@@ -608,7 +608,6 @@
 
   ParsedAttributesWithRange attrs(AttrFactory);
   MaybeParseCXX11Attributes(attrs);
-  MaybeParseMicrosoftAttributes(attrs);
 
   Result = ParseExternalDeclaration(attrs);
   return false;
@@ -871,11 +870,10 @@
          Tok.is(tok::kw_try);          // X() try { ... }
 }
 
-/// ParseDeclarationOrFunctionDefinition - Parse either a function-definition or
-/// a declaration.  We can't tell which we have until we read up to the
-/// compound-statement in function-definition. TemplateParams, if
-/// non-NULL, provides the template parameters when we're parsing a
-/// C++ template-declaration.
+/// Parse either a function-definition or a declaration.  We can't tell which
+/// we have until we read up to the compound-statement in function-definition.
+/// TemplateParams, if non-NULL, provides the template parameters when we're
+/// parsing a C++ template-declaration.
 ///
 ///       function-definition: [C99 6.9.1]
 ///         decl-specs      declarator declaration-list[opt] compound-statement
@@ -891,6 +889,7 @@
 Parser::ParseDeclOrFunctionDefInternal(ParsedAttributesWithRange &attrs,
                                        ParsingDeclSpec &DS,
                                        AccessSpecifier AS) {
+  MaybeParseMicrosoftAttributes(DS.getAttributes());
   // Parse the common declaration-specifiers piece.
   ParseDeclarationSpecifiers(DS, ParsedTemplateInfo(), AS, DSC_top_level);
 
@@ -970,7 +969,7 @@
     // parsing c constructs and re-enter objc container scope
     // afterwards.
     ObjCDeclContextSwitch ObjCDC(*this);
-      
+
     return ParseDeclOrFunctionDefInternal(attrs, PDS, AS);
   }
 }
@@ -2006,7 +2005,6 @@
   while (Tok.isNot(tok::r_brace) && !isEofOrEom()) {
     ParsedAttributesWithRange attrs(AttrFactory);
     MaybeParseCXX11Attributes(attrs);
-    MaybeParseMicrosoftAttributes(attrs);
     DeclGroupPtrTy Result = ParseExternalDeclaration(attrs);
     if (Result && !getCurScope()->getParent())
       Actions.getASTConsumer().HandleTopLevelDecl(Result.get());
Index: lib/Parse/ParseOpenMP.cpp
===================================================================
--- lib/Parse/ParseOpenMP.cpp
+++ lib/Parse/ParseOpenMP.cpp
@@ -610,7 +610,6 @@
       if (AS == AS_none) {
         assert(TagType == DeclSpec::TST_unspecified);
         MaybeParseCXX11Attributes(Attrs);
-        MaybeParseMicrosoftAttributes(Attrs);
         ParsingDeclSpec PDS(*this);
         Ptr = ParseExternalDeclaration(Attrs, &PDS);
       } else {
@@ -672,7 +671,6 @@
            Tok.isNot(tok::eof) && Tok.isNot(tok::r_brace)) {
       ParsedAttributesWithRange attrs(AttrFactory);
       MaybeParseCXX11Attributes(attrs);
-      MaybeParseMicrosoftAttributes(attrs);
       ParseExternalDeclaration(attrs);
       if (Tok.isAnnotation() && Tok.is(tok::annot_pragma_openmp)) {
         TentativeParsingAction TPA(*this);
Index: lib/Parse/ParseObjc.cpp
===================================================================
--- lib/Parse/ParseObjc.cpp
+++ lib/Parse/ParseObjc.cpp
@@ -2238,7 +2238,6 @@
     while (!ObjCImplParsing.isFinished() && !isEofOrEom()) {
       ParsedAttributesWithRange attrs(AttrFactory);
       MaybeParseCXX11Attributes(attrs);
-      MaybeParseMicrosoftAttributes(attrs);
       if (DeclGroupPtrTy DGP = ParseExternalDeclaration(attrs)) {
         DeclGroupRef DG = DGP.get();
         DeclsInGroup.append(DG.begin(), DG.end());
Index: lib/Parse/ParseDeclCXX.cpp
===================================================================
--- lib/Parse/ParseDeclCXX.cpp
+++ lib/Parse/ParseDeclCXX.cpp
@@ -217,7 +217,6 @@
            Tok.isNot(tok::eof)) {
       ParsedAttributesWithRange attrs(AttrFactory);
       MaybeParseCXX11Attributes(attrs);
-      MaybeParseMicrosoftAttributes(attrs);
       ParseExternalDeclaration(attrs);
     }
 
@@ -310,7 +309,6 @@
 
   ParsedAttributesWithRange attrs(AttrFactory);
   MaybeParseCXX11Attributes(attrs);
-  MaybeParseMicrosoftAttributes(attrs);
 
   if (Tok.isNot(tok::l_brace)) {
     // Reset the source range in DS, as the leading "extern"
@@ -361,7 +359,6 @@
     default:
       ParsedAttributesWithRange attrs(AttrFactory);
       MaybeParseCXX11Attributes(attrs);
-      MaybeParseMicrosoftAttributes(attrs);
       ParseExternalDeclaration(attrs);
       continue;
     }
@@ -1742,7 +1739,7 @@
       TParams =
         MultiTemplateParamsArg(&(*TemplateParams)[0], TemplateParams->size());
 
-    handleDeclspecAlignBeforeClassKey(attrs, DS, TUK);
+    stripTypeAttribsOffDeclSpec(attrs, DS, TUK);
 
     // Declaration or definition of a class type
     TagOrTempResult = Actions.ActOnTag(getCurScope(), TagType, TUK, StartLoc,
@@ -3941,7 +3938,19 @@
     // FIXME: If this is actually a C++11 attribute, parse it as one.
     BalancedDelimiterTracker T(*this, tok::l_square);
     T.consumeOpen();
-    SkipUntil(tok::r_square, StopAtSemi | StopBeforeMatch);
+
+    // Skip most ms attributes except for a whitelist.
+    while (true) {
+      SkipUntil(tok::r_square, tok::identifier, StopAtSemi | StopBeforeMatch);
+      if (Tok.isNot(tok::identifier))  // ']', but also eof
+        break;
+      IdentifierInfo *Name = Tok.getIdentifierInfo();
+      SourceLocation Loc = Tok.getLocation();
+      ConsumeToken();
+      if (Name->getName() == "uuid")
+        ParseMicrosoftDeclSpecArgs(Name, Loc, attrs, AttributeList::AS_MS);
+    }
+
     T.consumeClose();
     if (endLoc)
       *endLoc = T.getCloseLocation();
Index: lib/Parse/ParseDecl.cpp
===================================================================
--- lib/Parse/ParseDecl.cpp
+++ lib/Parse/ParseDecl.cpp
@@ -385,7 +385,8 @@
 
 bool Parser::ParseMicrosoftDeclSpecArgs(IdentifierInfo *AttrName,
                                         SourceLocation AttrNameLoc,
-                                        ParsedAttributes &Attrs) {
+                                        ParsedAttributes &Attrs,
+                                        AttributeList::Syntax AS) {
   // If the attribute isn't known, we will not attempt to parse any
   // arguments.
   if (!hasAttribute(AttrSyntax::Declspec, nullptr, AttrName,
@@ -507,14 +508,13 @@
     if (!HasInvalidAccessor)
       Attrs.addNewPropertyAttr(AttrName, AttrNameLoc, nullptr, SourceLocation(),
                                AccessorNames[AK_Get], AccessorNames[AK_Put],
-                               AttributeList::AS_Declspec);
+                               AS);
     T.skipToEnd();
     return !HasInvalidAccessor;
   }
 
-  unsigned NumArgs =
-      ParseAttributeArgsCommon(AttrName, AttrNameLoc, Attrs, nullptr, nullptr,
-                               SourceLocation(), AttributeList::AS_Declspec);
+  unsigned NumArgs = ParseAttributeArgsCommon(
+      AttrName, AttrNameLoc, Attrs, nullptr, nullptr, SourceLocation(), AS);
 
   // If this attribute's args were parsed, and it was expected to have
   // arguments but none were provided, emit a diagnostic.
@@ -582,7 +582,8 @@
 
       // Parse attribute arguments.
       if (Tok.is(tok::l_paren))
-        AttrHandled = ParseMicrosoftDeclSpecArgs(AttrName, AttrNameLoc, Attrs);
+        AttrHandled = ParseMicrosoftDeclSpecArgs(AttrName, AttrNameLoc, Attrs,
+                                                 AttributeList::AS_Declspec);
       else if (AttrName->getName() == "property")
         // The property attribute must have an argument list.
         Diag(Tok.getLocation(), diag::err_expected_lparen_after)
@@ -1423,11 +1424,14 @@
   }
 }
 
+// Usually, `__attribute__((attrib)) class Foo {} var` means that attrib applies
+// to var, not the type Foo.
 // As an exception to the rule, __declspec(align(...)) before the
 // class-key affects the type instead of the variable.
-void Parser::handleDeclspecAlignBeforeClassKey(ParsedAttributesWithRange &Attrs,
-                                               DeclSpec &DS,
-                                               Sema::TagUseKind TUK) {
+// Also, Microsoft-style [attributes] affect the type instead of the variable.
+// This function moves attributes that should apply to the type off DS to Attrs.
+void Parser::stripTypeAttribsOffDeclSpec(ParsedAttributesWithRange &Attrs,
+                                         DeclSpec &DS, Sema::TagUseKind TUK) {
   if (TUK == Sema::TUK_Reference)
     return;
 
@@ -1437,10 +1441,9 @@
   while (AL) {
     AttributeList *Next = AL->getNext();
 
-    // We only consider attributes using the appropriate '__declspec' spelling.
-    // This behavior doesn't extend to any other spellings.
-    if (AL->getKind() == AttributeList::AT_Aligned &&
-        AL->isDeclspecAttribute()) {
+    if ((AL->getKind() == AttributeList::AT_Aligned &&
+         AL->isDeclspecAttribute()) ||
+        AL->isMSAttribute()) {
       // Stitch the attribute into the tag's attribute list.
       AL->setNext(nullptr);
       Attrs.add(AL);
@@ -4071,7 +4074,7 @@
     return;
   }
 
-  handleDeclspecAlignBeforeClassKey(attrs, DS, TUK);
+  stripTypeAttribsOffDeclSpec(attrs, DS, TUK);
 
   Sema::SkipBodyInfo SkipBody;
   if (!Name && TUK == Sema::TUK_Definition && Tok.is(tok::l_brace) &&
Index: include/clang/Sema/AttributeList.h
===================================================================
--- include/clang/Sema/AttributeList.h
+++ include/clang/Sema/AttributeList.h
@@ -101,12 +101,14 @@
     AS_CXX11,
     /// __declspec(...)
     AS_Declspec,
+    /// [uuid("...")] class Foo
+    AS_MS,
     /// __ptr16, alignas(...), etc.
     AS_Keyword,
     /// Context-sensitive version of a keyword attribute.
     AS_ContextSensitiveKeyword,
     /// #pragma ...
-    AS_Pragma
+    AS_Pragma,
   };
 
 private:
@@ -369,6 +371,7 @@
   }
 
   bool isDeclspecAttribute() const { return SyntaxUsed == AS_Declspec; }
+  bool isMSAttribute() const { return SyntaxUsed == AS_MS; }
   bool isCXX11Attribute() const {
     return SyntaxUsed == AS_CXX11 || isAlignasAttribute();
   }
Index: include/clang/Parse/Parser.h
===================================================================
--- include/clang/Parse/Parser.h
+++ include/clang/Parse/Parser.h
@@ -2106,8 +2106,8 @@
   void DiagnoseMisplacedCXX11Attribute(ParsedAttributesWithRange &Attrs,
                                        SourceLocation CorrectLocation);
 
-  void handleDeclspecAlignBeforeClassKey(ParsedAttributesWithRange &Attrs,
-                                         DeclSpec &DS, Sema::TagUseKind TUK);
+  void stripTypeAttribsOffDeclSpec(ParsedAttributesWithRange &Attrs,
+                                   DeclSpec &DS, Sema::TagUseKind TUK);
 
   void ProhibitAttributes(ParsedAttributesWithRange &attrs) {
     if (!attrs.Range.isValid()) return;
@@ -2224,7 +2224,8 @@
                                SourceLocation *End = nullptr);
   bool ParseMicrosoftDeclSpecArgs(IdentifierInfo *AttrName,
                                   SourceLocation AttrNameLoc,
-                                  ParsedAttributes &Attrs);
+                                  ParsedAttributes &Attrs,
+                                  AttributeList::Syntax AS);
   void ParseMicrosoftTypeAttributes(ParsedAttributes &attrs);
   void DiagnoseAndSkipExtendedMicrosoftTypeAttributes();
   SourceLocation SkipExtendedMicrosoftTypeAttributes();
Index: include/clang/Basic/Attributes.h
===================================================================
--- include/clang/Basic/Attributes.h
+++ include/clang/Basic/Attributes.h
@@ -22,6 +22,8 @@
   GNU,
   /// Is the identifier known as a __declspec-style attribute?
   Declspec,
+  /// Is the identifier known as a [] MS-style attribute?
+  MS,
   // Is the identifier known as a C++-style attribute?
   CXX,
   // Is the identifier known as a pragma attribute?
Index: include/clang/Basic/Attr.td
===================================================================
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -198,6 +198,7 @@
 
 class GNU<string name> : Spelling<name, "GNU">;
 class Declspec<string name> : Spelling<name, "Declspec">;
+class MS<string name> : Spelling<name, "MS">;
 class CXX11<string namespace, string name, int version = 1>
     : Spelling<name, "CXX11"> {
   string Namespace = namespace;
@@ -1574,7 +1575,7 @@
 }
 
 def Uuid : InheritableAttr {
-  let Spellings = [Declspec<"uuid">];
+  let Spellings = [Declspec<"uuid">, MS<"uuid">];
   let Args = [StringArgument<"Guid">];
 //  let Subjects = SubjectList<[CXXRecord]>;
   let LangOpts = [MicrosoftExt, Borland];
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to