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
