aaron.ballman added inline comments.
================
Comment at: include/clang/Parse/Parser.h:2217
@@ -2215,1 +2216,3 @@
+ SourceLocation UuidLoc,
+ ParsedAttributes &attrs);
void ParseMicrosoftAttributes(ParsedAttributes &attrs,
----------------
`Attrs` instead of `attrs`
================
Comment at: lib/Parse/ParseDeclCXX.cpp:3947
@@ +3946,3 @@
+
+ SmallString<42> StrBuffer; // 2 "", 36 bytes UUID, 2 optional {}, 1 nul
+ StrBuffer += "\"";
----------------
Should we use Twine for building this string up (since this is used in a lot of
header files)?
================
Comment at: lib/Parse/ParseDeclCXX.cpp:3993
@@ +3992,3 @@
+ StringLiteral *UuidString =
+ cast<StringLiteral>(Actions.ActOnStringLiteral(Toks, nullptr).get());
+ ArgExprs.push_back(UuidString);
----------------
Will this properly handle an abomination involving string literal
concatenation? I shudder to type this, but:
`[uuid({000000A0-0000-""0000-C000-000000000049})] struct S {};` MSVC rejects,
but I think this code will silently accept it.
================
Comment at: lib/Parse/ParseDeclCXX.cpp:4031
@@ +4030,3 @@
+ ConsumeToken();
+ if (Name->getName() == "uuid" && Tok.is(tok::l_paren))
+ ParseMicrosoftUuidAttributeArgs(Name, Loc, attrs);
----------------
How well does this diagnose `[uuid{"000000A0-0000-0000-C000-000000000049"}]
struct S {};`? (Note the use of curly braces in lieu of parens for the
argument.)
================
Comment at: test/Parser/ms-square-bracket-attributes.mm:12
@@ +11,3 @@
+}
+
+
----------------
Can we also get a test case showing the behavior of multiple arguments in the
same uuid attribute? e.g., `[uuid("{000000A0-0000-0000-C000-000000000049}",
"1")] struct S {};`
And a test showing that we reject multiple uuid attributes on the same entity?
e.g., `[uuid("{000000A0-0000-0000-C000-000000000049}"),
uuid("{000000A0-0000-0000-C000-000000000050}")] struct S {};`
https://reviews.llvm.org/D23895
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
