aaron.ballman added inline comments.

================
Comment at: clang/lib/Sema/SemaConcept.cpp:150
+namespace {
+struct SatisfactionStackRAII {
+  Sema &SemaRef;
----------------
erichkeane wrote:
> aaron.ballman wrote:
> > Er, it'd be nice for this not to shadow the name of the class from `Sema`, 
> > that's pretty confusing.
> What do you mean?  What name does it shadow?
Sema.h:7239 (code you added in this patch).


================
Comment at: clang/lib/Sema/SemaConcept.cpp:276-278
+  for (const auto &List : MLTAL)
+    for (const auto &TemplateArg : List.Args)
+      TemplateArg.Profile(ID, S.Context);
----------------
erichkeane wrote:
> aaron.ballman wrote:
> > What are the chances that this `O(N^2)` operation is going to come back to 
> > bite us in terms of compile time performance?
> I'd hope not too much?  This is just going through the whole list of template 
> arguments on this expression, so I think that makes this `O(M*N)`, where M 
> and N are limited by the number of template arguments we allow.
> 
Okay, I was mostly worried about STL headers where there may be a long list of 
template arguments and not a lot of recursion to worry about. We don't really 
memoize whether we've already determined a given constraint is not recursive so 
that we don't have to repeat this work over and over again, but if performance 
is a concern in practice, we could explore that as an option.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136975/new/

https://reviews.llvm.org/D136975

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to