aaron.ballman 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);
----------------
thakis wrote:
> 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 :-(
I think we're getting to the crux of it, and I'm sorry for my being obtuse --
yes, I would like this to properly parse it as a list of attributes, rather
than this eat-and-pray approach. The first element must be an identifier. If we
get uuid, we process it as discussed. Any other identifier, we eat until we hit
a ',', ']', or eof, and continue (if not ]).
https://reviews.llvm.org/D23895
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
