thakis added inline comments.
================
Comment at: include/clang/Parse/Parser.h:2217
@@ -2215,1 +2216,3 @@
+ SourceLocation UuidLoc,
+ ParsedAttributes &attrs);
void ParseMicrosoftAttributes(ParsedAttributes &attrs,
----------------
aaron.ballman wrote:
> `Attrs` instead of `attrs`
It's spelled `attrs` in the function above and below this one…
================
Comment at: lib/Parse/ParseDeclCXX.cpp:3947
@@ +3946,3 @@
+
+ SmallString<42> StrBuffer; // 2 "", 36 bytes UUID, 2 optional {}, 1 nul
+ StrBuffer += "\"";
----------------
aaron.ballman wrote:
> Should we use Twine for building this string up (since this is used in a lot
> of header files)?
I thought Twine is a rvalue type…yes, from its docs:
```
/// A Twine is not intended for use directly and should not be stored, its
/// implementation relies on the ability to store pointers to temporary stack
/// objects which may be deallocated at the end of a statement. Twines should
/// only be used accepted as const references in arguments, when an API wishes
/// to accept possibly-concatenated strings.
```
I think this is should be pretty fast as-is.
================
Comment at: lib/Parse/ParseDeclCXX.cpp:3993
@@ +3992,3 @@
+ StringLiteral *UuidString =
+ cast<StringLiteral>(Actions.ActOnStringLiteral(Toks, nullptr).get());
+ ArgExprs.push_back(UuidString);
----------------
aaron.ballman wrote:
> 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.
This is rejected, "" is returned as a string token which becomes `""`
(including quotes) after getSpelling. Added a test case with this.
================
Comment at: lib/Parse/ParseDeclCXX.cpp:4031
@@ +4030,3 @@
+ ConsumeToken();
+ if (Name->getName() == "uuid" && Tok.is(tok::l_paren))
+ ParseMicrosoftUuidAttributeArgs(Name, Loc, attrs);
----------------
aaron.ballman wrote:
> 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.)
That's currently silently ignored since ParseMicrosoftAttributes() looks for
'uuid' '(' to call the uuid parser.
================
Comment at: test/Parser/ms-square-bracket-attributes.mm:12
@@ +11,3 @@
+}
+
+
----------------
aaron.ballman wrote:
> 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 {};`
The first is a nice testcase, added, thanks.
We don't currently reject multiple uuids (also not with declspec). Fixing this
is on my list of follow-ups, but since the fix will live in sema land and be
independent of the attribute spelling, I didn't put it in this change. (Fun
fact: cl.exe doesn't warn if you have one []-uuid and one declspec-uuid. We'll
get that right once we diag on it.)
https://reviews.llvm.org/D23895
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
