[PATCH] D134542: [Concepts] Recover properly from a RecoveryExpr in a concept

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added a comment.

The patch looks good. Two high-level questions:

- Does the similar thing happen for class templates? Like a constraint expr of a partial specialization has an error. Maybe we don't assert in this case?
- I suppose the constraint error does not always affect the overload resolution results. When it does not, an alternative would be to assume the constraint is a satisfaction failure and the compilation continues. Do you see any value in this approach? Personally, I could go either way. Basically a trade-off between pessimistic and optimistic.



================
Comment at: clang/docs/ReleaseNotes.rst:213
   of FAM-like arrays.
+- Clang will now diagnose an overload set where an option has a constraint that
+  refers to an expression with a previous error as nothing viable, so that it
----------------



================
Comment at: clang/include/clang/Sema/Overload.h:931
 
+    bool ConstraintFailureBecauseCascadingError() const;
+
----------------
How about `ConstraintExprHasError`? `Cascading` sounds like more details than useful.


================
Comment at: clang/lib/Sema/SemaOverload.cpp:10123-10127
+  if (DeductionFailure.Result != Sema::TDK_ConstraintsNotSatisfied)
+    return false;
+
+  return static_cast<CNSInfo *>(DeductionFailure.Data)
+      ->Satisfaction.ContainsErrors;
----------------
nit: might be easier to read


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

https://reviews.llvm.org/D134542