[PATCH] D123298: [NFC] [AST] Reduce the size of TemplateParmPosition

[Aaron Ballman via cfe-commits]

On Thu, Apr 7, 2022 at 8:35 AM Corentin via cfe-commits
<cfe-commits at lists.llvm.org> wrote:
>
>
>
> On Thu, Apr 7, 2022 at 2:11 PM Aaron Ballman via Phabricator <reviews at reviews.llvm.org> wrote:
>>
>> aaron.ballman added a comment.
>>
>> In D123298#3435796 <https://reviews.llvm.org/D123298#3435796>, @cor3ntin wrote:
>>
>> > In D123298#3435770 <https://reviews.llvm.org/D123298#3435770>, @aaron.ballman wrote:
>> >
>> >> Changes LGTM, I also don't think we should hit these limits. Perhaps we should add some assertions to the ctor and the setter functions just to be sure though?
>> >
>> > If we are going to do anything, it ought to be a diagnostic?
>>
>> Doing a diagnostic would mean finding all the places where we form a `TemplateParmPosition` and ensure we have enough source location information to issue the diagnostic. Given that we don't expect users to ever hit it, having the assertion gives a wee bit of coverage (godbolt exposes an assertions build, for example) but without as much implementation burden. That said, if it's easy enough to give diagnostics, that's a more user-friendly approach if we think anyone would hit that limit. (I suspect template instantiation depth would be hit before bumping up against these limits, though.)
>
>
> Fair enough - I suspect godbolt would die in that scenario though.
> Note that i was not asking for a diagnostic, just saying that the assertion may not protect anyone

Agreed, it's a pretty paltry protection. :-) My thinking is: we likely
have torture test suite cases that may tell us if we've guessed wrong
(either in tree, or in some downstream).

>> > I can't imagine a scenario in which someone would hit these limits and have assertions enabled. But i agree with you that the limit themselves should not be hit.
>> > On the other hand, why not use 16 for both?
>>
>> I think people instantiate to deeper template depths than they typically have for template parameters, so having a deeper depth made sense to me.
>
> Sure, but there is a huge imbalance there. 1048576 vs 4096 - I think it's still better than msvc and it's conforming - the standard sets the minimum at 1024 for both afaik

That's a fair point. I don't have a strong opinion on what bit widths
we use so long as they are "sensible" (for some definition of the
term).