[PATCH] D134128: Resubmit an implemention for constrained template template parameters [P0857R0 Part B]

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.


================
Comment at: clang/lib/Sema/SemaConcept.cpp:590
 
+// FIXME: may be incomplete
+static unsigned CalculateTemplateDepthForConstraints(const Expr *Constr) {
----------------
I'd like some sort of assert in the case where you don't know what to do here.  We need to collect all of these cases best we can in advance.

I'd suggest ALSO running libcxx tests against this once you have the assert in place, which should help.


================
Comment at: clang/lib/Sema/SemaConcept.cpp:706
+  std::tie(OldConstr, NewConstr) =
+      AdjustConstraintDepth::UnifyConstraintDepthFromDecl(*this, Old, OldConstr,
+                                                          New, NewConstr);
----------------
Unless I'm missing something, I don't think this idea of 'unify constraint depth' is correct.  We should be able, from the decl itself, to determine the depths?  What is the difference here that I'm not getting?


================
Comment at: clang/lib/Sema/SemaConcept.cpp:1340
+bool Sema::IsAtLeastAsConstrained(NamedDecl *D1,
+                                  MutableArrayRef<const Expr *> AC1,
+                                  NamedDecl *D2,
----------------
Can you explain why this is a MutableArrayRef now?  I believe this means it'll now modify the arrays that are passed into it, which we don't necessarily want, right?  A new 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134128