erichkeane marked 2 inline comments as done.
erichkeane added inline comments.
================
Comment at: clang/lib/Sema/SemaConcept.cpp:150
+namespace {
+struct SatisfactionStackRAII {
+ Sema &SemaRef;
----------------
aaron.ballman wrote:
> 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).
Yikes! Thanks, good catch!
================
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);
----------------
aaron.ballman wrote:
> 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.
I'm somewhat worried there too, but I'd hope that an M*N < ~1000 would be a
small impact. We actually DO memoize this, at least on a per instantiation
basis, as we cache constraint evaluations.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136975/new/
https://reviews.llvm.org/D136975
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
