steffenlarsen added inline comments.
================
Comment at: clang/lib/Parse/ParseDecl.cpp:386
ArgsVector ArgExprs;
if (Tok.is(tok::identifier)) {
// If this attribute wants an 'identifier' argument, make it so.
----------------
steffenlarsen wrote:
> erichkeane wrote:
> > So does this mean that our pack-parsing code is allowing identifiers? I
> > really expected this patch to enforce via the clang-attr-emitter that ONLY
> > the 'expr' types were allowed. So you'd have:
> >
> > if (attributeAcceptsExprPack(AttrName)) {
> > ... <where this ONLY accepts an expression pack>
> > } else if (Tok.is(tok::identifier)) {
> > ...
> > }...
> This will only consume the identifier if the argument is an identifier
> argument (variadic or non-variadic). As you say, clang-attr-emitter will make
> sure that no attribute with `AcceptsExprPack` has any identifier arguments,
> so if there is an identifier here it will not be able to consume it and then
> it will continue on to try and parse it as an expression and fail.
>
> That said, the second part of the following parsing branches should probably
> also check `attributeHasIdentifierArg(*AttrName)`, so I will add that shortly.
> That said, the second part of the following parsing branches should probably
> also check `attributeHasIdentifierArg(*AttrName)`, so I will add that shortly.
Correction: Parsing any identifiers after this assumes there is a variadic
identifier argument. Effectively it assumes that all non-variadic identifiers
in an attribute are at the start of an attribute arguments, in which case
they'll be parsed here. This is a little lackluster as it could simply allow
identifiers in other places if there are variadic identifier arguments in the
attribute, but I don't think fixing it should be in the scope of this patch.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D114439/new/
https://reviews.llvm.org/D114439
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
