On Thu, Apr 7, 2022 at 2:11 PM Aaron Ballman via Phabricator < revi...@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 > >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
