[Concepts] Add Decls from the outer scope of the current lambda for conversion function constraints

[zyn0217]

This fixes the case shown by #64808 (comment).

Similar to f9caa12, we have some calls to constraint checking for a lambda's conversion function while determining the conversion sequence.

This patch addresses the problem where the requires-expression within such a lambda references to a Decl outside of the lambda by adding these Decls to the current instantiation scope.

I'm abusing the flag ForOverloadResolution of CheckFunctionConstraints, which is actually meant to consider the Decls from parent DeclContexts.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Younan Zhang (zyn0217)

Changes

This fixes the case shown by #64808 (comment).

Similar to f9caa12, we have some calls to constraint checking for a lambda's conversion function while determining the conversion sequence.

This patch addresses the problem where the requires-expression within such a lambda references to a Decl outside of the lambda by adding these Decls to the current instantiation scope.

I'm abusing the flag ForOverloadResolution of CheckFunctionConstraints, which is actually meant to consider the Decls from parent DeclContexts.

---

Full diff: https://github.com/llvm/llvm-project/pull/83420.diff

3 Files Affected:

• (modified) clang/docs/ReleaseNotes.rst (+3)
• (modified) clang/lib/Sema/SemaOverload.cpp (+2-1)
• (modified) clang/test/SemaTemplate/concepts.cpp (+29)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 7e16b9f0c67dbd..10f7e7129e97ac 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -290,6 +290,9 @@ Bug Fixes to C++ Support
   lookup searches the bases of an incomplete class.
 - Fix a crash when an unresolved overload set is encountered on the RHS of a ``.*`` operator.
   (`#53815 <https://github.com/llvm/llvm-project/issues/53815>`_)
+- Fixed a crash while checking constraints of a trailing requires-expression of a lambda, that the
+  expression references to an entity declared outside of the lambda. This is a reduction from
+  (`#64808 <https://github.com/llvm/llvm-project/issues/64808>`_).
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 7d38043890ca20..f6a2cfa60892f9 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -7976,7 +7976,8 @@ void Sema::AddConversionCandidate(
 
   if (Conversion->getTrailingRequiresClause()) {
     ConstraintSatisfaction Satisfaction;
-    if (CheckFunctionConstraints(Conversion, Satisfaction) ||
+    if (CheckFunctionConstraints(Conversion, Satisfaction, /*UsageLoc=*/{},
+                                 /*ShouldAddDeclsFromParentScope=*/true) ||
         !Satisfaction.IsSatisfied) {
       Candidate.Viable = false;
       Candidate.FailureKind = ovl_fail_constraints_not_satisfied;
diff --git a/clang/test/SemaTemplate/concepts.cpp b/clang/test/SemaTemplate/concepts.cpp
index bac209a28da912..b7ea0d003a52d7 100644
--- a/clang/test/SemaTemplate/concepts.cpp
+++ b/clang/test/SemaTemplate/concepts.cpp
@@ -1085,3 +1085,32 @@ template void Struct<void>::bar<>();
 template int Struct<void>::field<1, 2>;
 
 }
+
+namespace GH64808 {
+
+template <class T> struct basic_sender {
+  T func;
+  basic_sender(T) : func(T()) {}
+};
+
+auto a = basic_sender{[](auto... __captures) {
+  return []() // #note-a-1
+    requires((__captures, ...), false) // #note-a-2
+  {};
+}()};
+
+auto b = basic_sender{[](auto... __captures) {
+  return []()
+    requires([](int, double) { return true; }(decltype(__captures)()...))
+  {};
+}(1, 2.33)};
+
+void foo() {
+  a.func();
+  // expected-error@-1{{no matching function for call}}
+  // expected-note@#note-a-1{{constraints not satisfied}}
+  // expected-note@#note-a-2{{evaluated to false}}
+  b.func();
+}
+
+} // namespace GH64808

[erichkeane]

This isn't really specific to lambdas right? Doing this on a function seems like it would do the wrong thing, right?

[zyn0217]

Hmm... Yes, the flag would tell the LSI to look into its parents as well, although we don't have any coverage test cases for it yet. Maybe I think we can narrow it down to the inside of CheckFunctionConstraints.

[zyn0217]

@erichkeane : I've updated the patch to limit the scope to lambda-only, which is now more appropriate to me. PTAL, thanks!

[erichkeane]

I think this looks good, but I want @cor3ntin to do the final approval, he touched it last :)

[zyn0217]

@cor3ntin WDYT?

[cor3ntin]

LGTM modulo nitpicks