<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Apr 7, 2022 at 2:11 PM Aaron Ballman via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">aaron.ballman added a comment.<br>
<br>
In D123298#3435796 <<a href="https://reviews.llvm.org/D123298#3435796" rel="noreferrer" target="_blank">https://reviews.llvm.org/D123298#3435796</a>>, @cor3ntin wrote:<br>
<br>
> In D123298#3435770 <<a href="https://reviews.llvm.org/D123298#3435770" rel="noreferrer" target="_blank">https://reviews.llvm.org/D123298#3435770</a>>, @aaron.ballman wrote:<br>
><br>
>> 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?<br>
><br>
> If we are going to do anything, it ought to be a diagnostic?<br>
<br>
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.)<br></blockquote><div><br></div><div>Fair enough - I suspect godbolt would die in that scenario though.</div><div>Note that i was not asking for a diagnostic, just saying that the assertion may not protect anyone</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> 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. <br>
> On the other hand, why not use 16 for both?<br>
<br>
I think people instantiate to deeper template depths than they typically have for template parameters, so having a deeper depth made sense to me.<br></blockquote><div><br></div><div>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</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
<br>
Repository:<br>
  rG LLVM Github Monorepo<br>
<br>
CHANGES SINCE LAST ACTION<br>
  <a href="https://reviews.llvm.org/D123298/new/" rel="noreferrer" target="_blank">https://reviews.llvm.org/D123298/new/</a><br>
<br>
<a href="https://reviews.llvm.org/D123298" rel="noreferrer" target="_blank">https://reviews.llvm.org/D123298</a><br>
<br>
</blockquote></div></div>