[PATCH] D114439: [Annotation] Allow parameter pack expansions in annotate attribute

Fri Dec 3 02:30:50 PST 2021

steffenlarsen added a comment.

Thank you both for the reviews! Consensus seems to be having support for pack expansion at argument level for now and let more complicated logic be added when there is an actual need. I have applied these changes as I understood them and have added/adjusted the requested test cases. Please have a look and let me know what you think. 😄

> That's what I want to avoid. :-D I would prefer that the arguments have to opt into that behavior because the alternative could get ambiguous. I'd rather we do some extra work to explicitly support that situation once we have a specific attribute in mind for it.

I'm okay with that! I have made the changes to only allow it in the last argument if it is marked as supporting pack expansion. Note that it assumes that there are no other variadic parameters except the last one, as suggested.

> Hmm, let's make sure we're on the same page. The situations I think we should avoid are:
>
>   // Mixing variadics and packs.
>   let Args = [VariadicExprArgument<"VarArgs">, VariadicExprArgument<"Pack", /*AllowPack*/1>];
>   
>   // Multiple packs.
>   let Args = [VariadicExprArgument<"FirstPack", /*AllowPack*/1>, VariadicExprArgument<"SecondPack", /*AllowPack*/1>];

Oh, I see what you mean. Yeah I agree for sure. I think the only exceptions we currently have to this is `omp` attributes, but given that they are pragmas they should be nowhere close to this.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114439/new/

https://reviews.llvm.org/D114439