[PATCH] D50713: [AST] Pack the unsigned of SubstTemplateTypeParmPackType into Type

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In https://reviews.llvm.org/D50713#1200564, @riccibruno wrote:

> The thing is that I have no idea what is the minimum number
>  of bits required. It was originally an unsigned but I suspect that
>  32 bits are not needed. Looking at [implimits] I see
>
>   Template arguments in a template declaration [1 024].
>   Recursively nested template instantiations, including substitution during template argument deduc-
>   tion (17.8.2) [1 024].
>   
>
> But 1024 seems to be too small and easily exceeded by a doing a bit
>  of template meta-programming.


I suspect a minor rephrase of this comment would be more than sufficient for me:

"This field corresponds to the number of template arguments, which is expected to be at least 1024 according to [implimits].  However, as this limit is somewhat easy to hit with template metaprogramming we'd prefer to keep it as large as possible.  At the moment it has been left as a non-bitfield since this type safely fits in 64 bits as an unsigned, so there is no reason to introduce the performance impact of a bitfield."

I'd suspect you/someone will bikeshed that, but I think it gets the point across that I want to make sure is there.


Repository:
  rC Clang

https://reviews.llvm.org/D50713