thakis added inline comments.

================
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:
> thakis wrote:
> > aaron.ballman wrote:
> > > Silently ignoring seems like the wrong thing to do -- can we diagnose 
> > > (even if it's a less-than-ideal diagnostics with a fixme)?
> > We still skip the majority of [] contents. Maybe the token 'uuid' can 
> > appear in some other attribute, followed by something else. So we probably 
> > shouldn't diag on 'uuid' followed by not-'(' (right?). Do you want me to 
> > add a diagnostic for 'uuid' '{'? What about 'uuid' '['?
> The grammar for the uuid attribute shows it requires the parens (it also 
> shows that it only accepts a string literal, so take that with a grain of 
> salt), so I think we should diagnose in this case, especially since we're 
> manually parsing the args. So if you see "[uuid" followed by any token other 
> than "(", I think that's an error (and MSVC treats it as such).
Sure, but uuid could be preceded by other tokens, e.g.

[ someotherattrib(foobar...) ..., uuid {

someotherattrib might take an argument that's called uuid and it might be valid 
to have uuid followed by something not '(' there, say [identify_class_by(uuid), 
uuid("1-2-3")]. This is a made-up example; I don't know if this is actually the 
case, I'm just saying I don't know, so I think I shouldn't diag on 'uuid' 
followed by something not-'(' in general.

Do you want me to peephole-diag on '[' 'uuid' not-'(' in the case when uuid is 
the first attrib in the attrib list?


https://reviews.llvm.org/D23895



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to