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