On Thu, Apr 7, 2022 at 8:35 AM Corentin via cfe-commits <cfe-commits@lists.llvm.org> wrote: > > > > 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
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). _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
