thakis marked an inline comment as done. thakis added a comment. Thanks for the fast review and the good comments! I'll leave it to y'all to agree on some name if you don't like the one I picked.
> Also, doesn't this introduce ambiguities into the grammar? Something like > this: > > void useit(int); > int main() { > int uuid = 42; > [uuid]() { > useit(uuid); > }(); > } > > > Will we keep parsing that as a lambda after this change or not? This change shouldn't change how things are parsed. It moved calls of MaybeParseMicrosoftAttributes(attrs) that precede ParseExternalDeclaration(attrs) into ParseExternalDeclaration(). As far as I can tell, every call of ParseExternalDeclaration() was preceded by a call to MaybeParseMicrosoftAttributes(), so this should be behavior-preserving. In your example, the lamda is not an external declaration, so it's not affected. (That means [uuid(...)] won't work for local classes, but that's probably ok.) Now that I looked at this more, behavior changes slightly for empty external declarations. Before, [foob]; didn't produce any output, now it produces "declaration doesn't declare anything". I don't know if that's important. ================ Comment at: include/clang/Basic/Attr.td:201 @@ -200,2 +200,3 @@ class Declspec<string name> : Spelling<name, "Declspec">; +class MS<string name> : Spelling<name, "MS">; class CXX11<string namespace, string name, int version = 1> ---------------- rsmith wrote: > Is there some better description for this than MS? `__declspec` is also an MS > attribute. Is it fair to call this IDL or MSIDL or similar? There are many IDL compilers. MSIDL works for me, so does anything else you 3 agree on :-) ================ Comment at: include/clang/Parse/Parser.h:2109-2110 @@ -2108,4 +2108,4 @@ - void handleDeclspecAlignBeforeClassKey(ParsedAttributesWithRange &Attrs, - DeclSpec &DS, Sema::TagUseKind TUK); + void stripSomeAttribsOffDeclSpec(ParsedAttributesWithRange &Attrs, + DeclSpec &DS, Sema::TagUseKind TUK); ---------------- rsmith wrote: > Can you give this a less vague-sounding name? :) Changed to stripTypeAttribsOffDeclSpec ================ Comment at: lib/Parse/ParseDeclCXX.cpp:3950 @@ +3949,3 @@ + ConsumeToken(); + if (Name->getName() == "uuid") + ParseMicrosoftDeclSpecArgs(Name, Loc, attrs, AttributeList::AS_MS); ---------------- rsmith wrote: > Is anything known to fail if you accept arbitrary attributes here, or is this > just the only one you /know/ you need right now? (If we can ditch the > whitelist, that would seem preferable.) These attributes can be different from decl spec args. For example, we don't have anything that parses `threading("apartment")`, `module(name="MyLib")` at the moment (I suppose ParseAttributeArgsCommon does that at a token level, but it won't know all these attributes that are allowed here). https://reviews.llvm.org/D23895 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits