[Concepts] Traverse the instantiation chain for parameter injection inside a constraint scope

[zyn0217]

We preserve the trailing requires-expression during the lambda
expression transformation. In order to get those referenced parameters
inside a requires-expression properly resolved to the instantiated
decls, we intended to inject these 'original' ParmVarDecls to the
current instantiaion scope, at Sema::SetupConstraintScope.

The previous approach seems to overlook nested instantiation chains,
leading to the crash within a nested lambda followed by a requires clause.

This fixes #73418.

[zyn0217]

In passing, the current strategy of preserving trailing requires expressions in TransformLambdaExpr confuses me:

llvm-project/clang/lib/Sema/TreeTransform.h

Lines 13686 to 13692 in f13aac6

	getSema().CompleteLambdaCallOperator(
	NewCallOperator, E->getCallOperator()->getLocation(),
	E->getCallOperator()->getInnerLocStart(),
	E->getCallOperator()->getTrailingRequiresClause(), NewCallOpTSI,
	E->getCallOperator()->getConstexprKind(),
	E->getCallOperator()->getStorageClass(), Params,
	E->hasExplicitResultType());

Is there a reason why we decided not to perform the requires-expression transformation before this line? Could it be due to some "dependency" issues during the requires-expression evaluation?

Interestingly, this bug could also be resolved by performing additional transformation for such requirements, which is the case since those variables (e.g. params in the reproducer) inside the requires-clause get redirected to the current instantiated lambda Decl that way. But it makes two tests fail, and therefore I doubt if it's that I've missed some hacks, or just because I'm on the wrong direction?

[llvmbot]

@llvm/pr-subscribers-clang

Author: Younan Zhang (zyn0217)

Changes

We preserve the trailing requires-expression during the lambda
expression transformation. In order to get those referenced parameters
inside a requires-expression properly resolved to the instantiated
decls, we intended to inject these 'original' ParmVarDecls to the
current instantiaion scope, at Sema::SetupConstraintScope.

The previous approach seems to overlook nested instantiation chains,
leading to the crash within a nested lambda followed by a requires clause.

This fixes #73418.

---

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

3 Files Affected:

• (modified) clang/docs/ReleaseNotes.rst (+3)
• (modified) clang/lib/Sema/SemaConcept.cpp (+6-2)
• (modified) clang/test/SemaTemplate/concepts-lambda.cpp (+18)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 392f694065a242..63b04982629b09 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -1025,6 +1025,9 @@ Bug Fixes to AST Handling
 - Fixed a bug where Template Instantiation failed to handle Lambda Expressions
   with certain types of Attributes.
   (`#76521 <https://github.com/llvm/llvm-project/issues/76521>`_)
+- Fixed a bug where variables referenced by requires-clauses inside
+  nested generic lambdas were not properly injected into the constraint scope.
+  (`#73418 <https://github.com/llvm/llvm-project/issues/73418>`_)
 
 Miscellaneous Bug Fixes
 ^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp
index acfc00f4125407..88fc846c89e42c 100644
--- a/clang/lib/Sema/SemaConcept.cpp
+++ b/clang/lib/Sema/SemaConcept.cpp
@@ -612,8 +612,12 @@ bool Sema::SetupConstraintScope(
 
     // If this is a member function, make sure we get the parameters that
     // reference the original primary template.
-    if (const auto *FromMemTempl =
-            PrimaryTemplate->getInstantiatedFromMemberTemplate()) {
+    // We walk up the instantiated template chain so that nested lambdas get
+    // handled properly.
+    for (FunctionTemplateDecl *FromMemTempl =
+             PrimaryTemplate->getInstantiatedFromMemberTemplate();
+         FromMemTempl;
+         FromMemTempl = FromMemTempl->getInstantiatedFromMemberTemplate()) {
       if (addInstantiatedParametersToScope(FD, FromMemTempl->getTemplatedDecl(),
                                            Scope, MLTAL))
         return true;
diff --git a/clang/test/SemaTemplate/concepts-lambda.cpp b/clang/test/SemaTemplate/concepts-lambda.cpp
index 7e431529427dff..0b7580f91043c7 100644
--- a/clang/test/SemaTemplate/concepts-lambda.cpp
+++ b/clang/test/SemaTemplate/concepts-lambda.cpp
@@ -149,3 +149,21 @@ void foo() {
   auto caller = make_caller.operator()<&S1::f1>();
 }
 } // namespace ReturnTypeRequirementInLambda
+
+namespace GH73418 {
+void foo() {
+  int x;
+  [&x](auto) {
+    return [](auto y) {
+      return [](auto obj, auto... params)
+        requires requires {
+          sizeof...(params);
+          [](auto... pack) {
+            return sizeof...(pack);
+          }(params...);
+        }
+      { return false; }(y);
+    }(x);
+  }(x);
+}
+} // namespace GH73418

[erichkeane]

This makes sense to me.

[zyn0217]

The failing libcxx test libcxx/gdb/gdb_pretty_printer_test.sh.cpp doesn't appear to relate to this patch, so I'm landing it anyway.