AaronBallman wrote:
> > Does this impact anything user-facing? e.g., should there be an additional
> > test somewhere in clang/test/Sema/ for this change?
>
> I don't think there is any user-visible issue with the one attribute that
> uses this.
Thanks!
> At least how the Parse code is written currently. The attribute has three
> EnumArguments followed by an IntArgument. The call to
> attributeHasStrictIdentifierArgAtIndex is only done if the token is an
> identifier and if the IntArgument is an identifier that same thing happens
> anyway.
>
> Looking at this code now, I am not convinced the call to
> attributeHasStrictIdentifierArgAtIndex does anything useful.
>
> ```
> if (Tok.is(tok::identifier) && attributeHasStrictIdentifierArgAtIndex(
> *AttrName, ArgExprs.size())) {
> ArgExprs.push_back(ParseIdentifierLoc());
> continue;
> }
>
> ExprResult ArgExpr;
> if (Tok.is(tok::identifier)) {
> ArgExprs.push_back(ParseIdentifierLoc());
> } else {
> ```
>
> So I think we can remove the whole function
> attributeHasStrictIdentifierArgAtIndex and the related tablegen. No tests
> fail without it.
>
> The function attributeHasStrictIdentifierArgs still seems necessary but not
> the AtIndex.
>
> What do you think?
That code was added very recently by
https://github.com/llvm/llvm-project/pull/94056 and seem to be specific to the
`ptrauth_vtable_pointer` attribute. Perhaps we're lacking test coverage if
nothing breaks without that code?
CC @ojhunt @ahmedbougacha
https://github.com/llvm/llvm-project/pull/99993
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
