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

[Corentin via cfe-commits]

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


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


>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D123298/new/
>
> https://reviews.llvm.org/D123298
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220407/2a4982f0/attachment.html>