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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
