[PATCH] D94092: [Clang] Remove unnecessary Attr.isArgIdent checks.
Aaron Ballman via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 5 09:09:33 PST 2021
aaron.ballman added a comment.
In D94092#2479684 <https://reviews.llvm.org/D94092#2479684>, @erichkeane wrote:
> I'm not sure how well Attr.td's constraints are enforced on type attributes, as these often happen before parsing is completely done.
Type (and statement) attributes do not have the same automatic error checking that declaration attributes enjoy (yet), so perhaps this was misguided error handling code.
> I'd imagine this code was put into place at least the 1st time for good reason, but I'm curious as to why we wouldn't have tests that cover that (or, as you assert, it could simply be that this is simply dead code).
I think it's dead code. In `ParseAttributeArgsCommon()` in `ParseDecl.cpp`, we check to see whether the attribute definition from `Attr.td` specifies an identifier argument or not. If it doesn't (which these don't), then it parses the identifier as an expression instead. So I don't think there's a way to hit these code paths, even in template cases.
> I'm generally OK with this (the asserts are unnecessary), but would like @aaron.ballman to double check my expectations here.
My expectation is that it's dead code, but I've been surprised before. :-D
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D94092/new/
https://reviews.llvm.org/D94092
More information about the cfe-commits
mailing list