[PATCH] D138914: Make evaluation of nested requirement consistent with requires expr.

Thu Dec 1 06:44:51 PST 2022

erichkeane accepted this revision.
erichkeane added a comment.
This revision is now accepted and ready to land.

Happy now, thanks!

================
Comment at: clang/include/clang/AST/ExprConcepts.h:428
+      : Requirement(RK_Nested,
+                    Constraint && Constraint->isInstantiationDependent(),
+                    Constraint && Constraint->containsUnexpandedParameterPack(),
----------------
usaxena95 wrote:
> erichkeane wrote:
> > erichkeane wrote:
> > > I'm a little concerned that we're changing to a situation where the constraint in a nested-requirement can be null like this.  The other cases it can be null we consider it a 'substitution failure' (see the 1st constructor).
> > > 
> > > It seems to me that we perhaps need a different state here for that.
> > I still have this concern, we need to make sure that NestedRequirement is written to consider this type of failure, it likely needs a separate constructor that contains information about the failure.  We probably want at least some sort of stored diagnostic here, sot hat we dont lose the output we have below.
> NestedRequirement will no more capture a substitution failure. I am planning to remove all references to SubstitutionDiagnostics for it. I have **temporarily** added unreachable tags to the references of SubstitutionDiagnostics. 
> 
> If this looks fine to you then I would move forward with refactoring it to remove all support for SubstitutionDiagnostics in NestedRequirement. WDYT ?
Ok, that works for me.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138914