Skip to content

[Clang] Handle instantiating captures in addInstantiatedCapturesToScope() #128478

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 25, 2025

Conversation

zyn0217
Copy link
Contributor

@zyn0217 zyn0217 commented Feb 24, 2025

addInstantiatedCapturesToScope() might be called when transforming a lambda body. In this situation, it would look into all the lambda's parents and figure out all the instantiated captures. However, the instantiated captures are not visible from lambda's class decl until the lambda is rebuilt (i.e. after the lambda body transform). So this patch corrects that by also examining the LambdaScopeInfo, serving as a workaround for not having deferred lambda body instantiation in Clang 20, to avoid regressing some real-world use cases.

Fixes #128175

…pe()

addInstantiatedCapturesToScope() might be called when transforming a
lambda body. In this situation, it would look into all the lambda's
parents and figure out all the instantiated captures. However, the
instantiated captures are not visible from lambda's class decl until
the lambda is rebuilt (i.e. after the lambda body transform). So this
patch corrects that by also examining the LambdaScopeInfo, serving as
a workaround for not having deferred lambda body instantiation in
Clang 20, to avoid regressing some real-world use cases.
@zyn0217 zyn0217 requested a review from cor3ntin February 24, 2025 07:39
@zyn0217 zyn0217 marked this pull request as ready for review February 24, 2025 08:14
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Feb 24, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 24, 2025

@llvm/pr-subscribers-clang

Author: Younan Zhang (zyn0217)

Changes

addInstantiatedCapturesToScope() might be called when transforming a lambda body. In this situation, it would look into all the lambda's parents and figure out all the instantiated captures. However, the instantiated captures are not visible from lambda's class decl until the lambda is rebuilt (i.e. after the lambda body transform). So this patch corrects that by also examining the LambdaScopeInfo, serving as a workaround for not having deferred lambda body instantiation in Clang 20, to avoid regressing some real-world use cases.

Fixes #128175


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

2 Files Affected:

  • (modified) clang/lib/Sema/SemaConcept.cpp (+24-1)
  • (modified) clang/test/SemaTemplate/concepts-lambda.cpp (+18)
diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp
index 8a77cbf8c9477..7fff29f5ca167 100644
--- a/clang/lib/Sema/SemaConcept.cpp
+++ b/clang/lib/Sema/SemaConcept.cpp
@@ -711,9 +711,32 @@ bool Sema::addInstantiatedCapturesToScope(
 
   unsigned Instantiated = 0;
 
+  // FIXME: This is a workaround for not having deferred lambda body
+  // instantiation.
+  // When transforming a lambda's body, if we encounter another call to an
+  // inline lambda that contains a constraint expression, we add all of the
+  // parent lambda's instantiated captures to the current instantiation scope to
+  // facilitate constraint evaluation. However, these captures don't appear in
+  // the CXXRecordDecl until after the lambda expression is rebuilt, so we
+  // pull them out from the corresponding LSI.
+  LambdaScopeInfo *InstantiatingScope = nullptr;
+  if (LambdaPattern->capture_size() && !LambdaClass->capture_size()) {
+    for (FunctionScopeInfo *Scope : llvm::reverse(FunctionScopes)) {
+      auto *LSI = dyn_cast<LambdaScopeInfo>(Scope);
+      if (!LSI ||
+          LSI->CallOperator->getTemplateInstantiationPattern() != PatternDecl)
+        continue;
+      InstantiatingScope = LSI;
+      break;
+    }
+    assert(InstantiatingScope);
+  }
+
   auto AddSingleCapture = [&](const ValueDecl *CapturedPattern,
                               unsigned Index) {
-    ValueDecl *CapturedVar = LambdaClass->getCapture(Index)->getCapturedVar();
+    ValueDecl *CapturedVar =
+        InstantiatingScope ? InstantiatingScope->Captures[Index].getVariable()
+                           : LambdaClass->getCapture(Index)->getCapturedVar();
     assert(CapturedVar->isInitCapture());
     Scope.InstantiatedLocal(CapturedPattern, CapturedVar);
   };
diff --git a/clang/test/SemaTemplate/concepts-lambda.cpp b/clang/test/SemaTemplate/concepts-lambda.cpp
index 306f86cfcb28f..dcb09c76d26b6 100644
--- a/clang/test/SemaTemplate/concepts-lambda.cpp
+++ b/clang/test/SemaTemplate/concepts-lambda.cpp
@@ -307,3 +307,21 @@ void test() {
 }
 
 }
+
+namespace GH128175 {
+
+template <class> void f() {
+  [i{0}] {
+    [&] {
+      [&] {
+        []()
+          requires true
+        {}();
+      }();
+    }();
+  }();
+}
+
+template void f<int>();
+
+}

Copy link
Contributor

@cor3ntin cor3ntin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No changelog because presumably you want to backport?

@cor3ntin cor3ntin requested a review from erichkeane February 24, 2025 20:46
@zyn0217 zyn0217 merged commit ecc7e6c into llvm:main Feb 25, 2025
11 checks passed
@zyn0217 zyn0217 added this to the LLVM 20.X Release milestone Feb 25, 2025
@zyn0217
Copy link
Contributor Author

zyn0217 commented Feb 25, 2025

No changelog because presumably you want to backport?

Exactly :)

/cherry-pick ecc7e6c

@llvmbot
Copy link
Member

llvmbot commented Feb 25, 2025

/pull-request #128639

swift-ci pushed a commit to swiftlang/llvm-project that referenced this pull request Feb 25, 2025
…pe() (llvm#128478)

addInstantiatedCapturesToScope() might be called when transforming a
lambda body. In this situation, it would look into all the lambda's
parents and figure out all the instantiated captures. However, the
instantiated captures are not visible from lambda's class decl until the
lambda is rebuilt (i.e. after the lambda body transform). So this patch
corrects that by also examining the LambdaScopeInfo, serving as a
workaround for not having deferred lambda body instantiation in Clang
20, to avoid regressing some real-world use cases.

Fixes llvm#128175

(cherry picked from commit ecc7e6c)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
Development

Successfully merging this pull request may close these issues.

[Clang] Crash on constraint for lambda in lambda with init-capture
3 participants