[PATCH] D50360: [Concepts] Requires Expressions

[Saar Raz via Phabricator via cfe-commits]

saar.raz added inline comments.


================
Comment at: lib/Sema/SemaTemplateInstantiate.cpp:1021-1052
+    ExprResult TransformRequiresExpr(RequiresExpr *E) {
+      LocalInstantiationScope Scope(SemaRef, /*CombineWithOuterScope=*/true);
+      return TreeTransform<TemplateInstantiator>::TransformRequiresExpr(E);
+    }
+
+    bool TransformRequiresExprRequirements(ArrayRef<Requirement *> Reqs,
+        SmallVectorImpl<Requirement *> &Transformed) {
----------------
rsmith wrote:
> Do we really need to duplicate the `TreeTransform` code here, and add a representation for a requirement whose substitution failed, and so on?
> 
> Possible alternative: create a `SFINAETrap` for the entire requires-expression, perform a `TreeTransform` of the whole thing, and if it fails, capture the diagnostic and build a `FailedRequiresExpr` node that stores the diagnostic and evaluates to `false`. (Under this model, a non-value-dependent `RequiresExpr` would always evaluate to `true`.) That should simplify this code, and the representations of `RequiresExpr` and `Requirements` (since you don't need to model "substitution failed" any more, nor conditionally store a diagnostic on a requirement).
> 
> Are there cases where we want to retain more information than that? (I don't think there are, since you don't actually retain any useful information from the requirement that failed, other than the diagnostic itself, but maybe I've missed something.) The results of a successful substitution into some parts of a failed `RequiresExpr` don't really seem interesting.
Wouldn't it be interesting to store the successful requirements prior to the failed ones? I imagine some tools might want this information


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D50360