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

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 7 05:11:40 PDT 2022


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.)

> 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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123298



More information about the cfe-commits mailing list