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:
> > > 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?
> I'm a bit lost. You've processed the open square bracket, so you know you're
> in an attribute list. Then you ignore everything until you get an identifier.
> If that identifier is uuid, you do special processing. Right now, you require
> uuid( to do the special processing. What I am saying is that if, when
> processing an attribute-token within an attribute list, you find uuid
> followed by anything other than a (, it should be diagnosed.
>
> So, I expect [identify_class_by(uuid), uuid("1-2-3")] to be fine, but
> [uuid["1-2-3-"], identify_by_class(uuid), uuid*] to diagnose the
> uuid["1-2-3"] and uuid* as being malformed (though I'd be fine if we skipped
> everything past the first malformed attribute if it's better for error
> recovery).
I too feel a bit lost :-)
Right now, the parsing code doesn't look at attribute lists, only at tokens, so
`[identify_class_by(uuid),` skips tokens until we hit 'uuid', and if I were to
diag on not-'(' after 'uuid', then I'd diag here, right? Do you want me to do
some more parsing to look for comma-separated entities in the attribute list
(and count parens brackets braces and ignore commas there) to fix this?
Sorry, I feel we're talking past each other :-(
https://reviews.llvm.org/D23895
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
