steffenlarsen added inline comments.
================
Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:724
+ "attribute %0 does not support pack expansion in the last argument">;
+def err_attribute_parm_pack_last_argument_only : Error<
+ "pack expansion in attributes is restricted to only the last argument">;
----------------
erichkeane wrote:
> I don't really see why this is required? I would think the 722 would be all
> you would need.
Intention was to make a distinction between the two cases, since we had the
granularity to do so. I.e. `clang::annotate` would never use 722 as it
explicitly states that the attribute doesn't support pack expansion (in the
last argument) which is untrue for `clang::annotate`, but if a user was to do
pack expansion on the first argument of `clang::annotate` they would get this
error telling them that the given argument cannot accept the pack expansion.
================
Comment at: clang/lib/Parse/ParseDecl.cpp:450
+ // Pack expansion is only allowed in the last attribute argument.
+ if (ArgExprs.size() + 1 < attributeNumberOfArguments(*AttrName)) {
+ Diag(Tok.getLocation(),
----------------
erichkeane wrote:
> I don't think this should be diagnosed here, and I don't think it is 'right'.
> I think the ClangAttrEmitter should ensure that the VariadicExprArgument
> needs to be the 'last' thing, but I think that REALLY means "supports a pack
> anywhere inside of it".
>
> See my test examples below, I don't think this parsing is sufficient for that.
That is also the intention here. All it checks is that we are in or beyond the
last argument of the attribute. For example, `clang::annotate` will return 2
from `attributeNumberOfArguments` as `VariadicExprArgument` only counts as a
single argument. Thus, any pack expansion expressions parsed after the first
will be accepted. I do agree though that the error message may be a little
confusing for users.
I will add the suggested tests and rethink the diagnostics.
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
