[Clang] Fix a lambda pattern comparison mismatch after ecc7e6ce4 #133863
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The latter version doesn't find the very original template pattern for a transformed lambda, while getPatternFunctionDecl() finds what we want for constraint instantiation.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Younan Zhang (zyn0217) ChangesIn ecc7e6c, we tried to inspect the This corrects that by moving It is also worth exploring to coalesce the implementation of Fixes #133719 (I split up the commits to make the review easier. Also note that in #133719, Shafik and I discovered additional cases where clang behaves incorrectly. However, since those aren't regressions, their fixes aren't being considered at this time.) Full diff: https://github.com/llvm/llvm-project/pull/133863.diff 3 Files Affected:
diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp
index e7e0b4cfb72a7..2d29af4258cd7 100644
--- a/clang/lib/Sema/SemaConcept.cpp
+++ b/clang/lib/Sema/SemaConcept.cpp
@@ -702,75 +702,6 @@ bool Sema::CheckConstraintSatisfaction(const Expr *ConstraintExpr,
.isInvalid();
}
-bool Sema::addInstantiatedCapturesToScope(
- FunctionDecl *Function, const FunctionDecl *PatternDecl,
- LocalInstantiationScope &Scope,
- const MultiLevelTemplateArgumentList &TemplateArgs) {
- const auto *LambdaClass = cast<CXXMethodDecl>(Function)->getParent();
- const auto *LambdaPattern = cast<CXXMethodDecl>(PatternDecl)->getParent();
-
- 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 a
- // nested lambda that contains a constraint expression, we add all of the
- // outer 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 =
- InstantiatingScope ? InstantiatingScope->Captures[Index].getVariable()
- : LambdaClass->getCapture(Index)->getCapturedVar();
- assert(CapturedVar->isInitCapture());
- Scope.InstantiatedLocal(CapturedPattern, CapturedVar);
- };
-
- for (const LambdaCapture &CapturePattern : LambdaPattern->captures()) {
- if (!CapturePattern.capturesVariable()) {
- Instantiated++;
- continue;
- }
- ValueDecl *CapturedPattern = CapturePattern.getCapturedVar();
-
- if (!CapturedPattern->isInitCapture()) {
- Instantiated++;
- continue;
- }
-
- if (!CapturedPattern->isParameterPack()) {
- AddSingleCapture(CapturedPattern, Instantiated++);
- } else {
- Scope.MakeInstantiatedLocalArgPack(CapturedPattern);
- SmallVector<UnexpandedParameterPack, 2> Unexpanded;
- SemaRef.collectUnexpandedParameterPacks(
- dyn_cast<VarDecl>(CapturedPattern)->getInit(), Unexpanded);
- auto NumArgumentsInExpansion =
- getNumArgumentsInExpansionFromUnexpanded(Unexpanded, TemplateArgs);
- if (!NumArgumentsInExpansion)
- continue;
- for (unsigned Arg = 0; Arg < *NumArgumentsInExpansion; ++Arg)
- AddSingleCapture(CapturedPattern, Instantiated++);
- }
- }
- return false;
-}
-
bool Sema::SetupConstraintScope(
FunctionDecl *FD, std::optional<ArrayRef<TemplateArgument>> TemplateArgs,
const MultiLevelTemplateArgumentList &MLTAL,
diff --git a/clang/lib/Sema/SemaLambda.cpp b/clang/lib/Sema/SemaLambda.cpp
index 292406f886362..6f114b71981fa 100644
--- a/clang/lib/Sema/SemaLambda.cpp
+++ b/clang/lib/Sema/SemaLambda.cpp
@@ -2412,6 +2412,74 @@ static FunctionDecl *getPatternFunctionDecl(FunctionDecl *FD) {
return FTD->getTemplatedDecl();
}
+bool Sema::addInstantiatedCapturesToScope(
+ FunctionDecl *Function, const FunctionDecl *PatternDecl,
+ LocalInstantiationScope &Scope,
+ const MultiLevelTemplateArgumentList &TemplateArgs) {
+ const auto *LambdaClass = cast<CXXMethodDecl>(Function)->getParent();
+ const auto *LambdaPattern = cast<CXXMethodDecl>(PatternDecl)->getParent();
+
+ 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 a
+ // nested lambda that contains a constraint expression, we add all of the
+ // outer 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 || getPatternFunctionDecl(LSI->CallOperator) != PatternDecl)
+ continue;
+ InstantiatingScope = LSI;
+ break;
+ }
+ assert(InstantiatingScope);
+ }
+
+ auto AddSingleCapture = [&](const ValueDecl *CapturedPattern,
+ unsigned Index) {
+ ValueDecl *CapturedVar =
+ InstantiatingScope ? InstantiatingScope->Captures[Index].getVariable()
+ : LambdaClass->getCapture(Index)->getCapturedVar();
+ assert(CapturedVar->isInitCapture());
+ Scope.InstantiatedLocal(CapturedPattern, CapturedVar);
+ };
+
+ for (const LambdaCapture &CapturePattern : LambdaPattern->captures()) {
+ if (!CapturePattern.capturesVariable()) {
+ Instantiated++;
+ continue;
+ }
+ ValueDecl *CapturedPattern = CapturePattern.getCapturedVar();
+
+ if (!CapturedPattern->isInitCapture()) {
+ Instantiated++;
+ continue;
+ }
+
+ if (!CapturedPattern->isParameterPack()) {
+ AddSingleCapture(CapturedPattern, Instantiated++);
+ } else {
+ Scope.MakeInstantiatedLocalArgPack(CapturedPattern);
+ SmallVector<UnexpandedParameterPack, 2> Unexpanded;
+ SemaRef.collectUnexpandedParameterPacks(
+ dyn_cast<VarDecl>(CapturedPattern)->getInit(), Unexpanded);
+ auto NumArgumentsInExpansion =
+ getNumArgumentsInExpansionFromUnexpanded(Unexpanded, TemplateArgs);
+ if (!NumArgumentsInExpansion)
+ continue;
+ for (unsigned Arg = 0; Arg < *NumArgumentsInExpansion; ++Arg)
+ AddSingleCapture(CapturedPattern, Instantiated++);
+ }
+ }
+ return false;
+}
+
Sema::LambdaScopeForCallOperatorInstantiationRAII::
LambdaScopeForCallOperatorInstantiationRAII(
Sema &SemaRef, FunctionDecl *FD, MultiLevelTemplateArgumentList MLTAL,
diff --git a/clang/test/SemaTemplate/concepts-lambda.cpp b/clang/test/SemaTemplate/concepts-lambda.cpp
index dcb09c76d26b6..1f67c2511e096 100644
--- a/clang/test/SemaTemplate/concepts-lambda.cpp
+++ b/clang/test/SemaTemplate/concepts-lambda.cpp
@@ -325,3 +325,18 @@ template <class> void f() {
template void f<int>();
}
+
+namespace GH133719 {
+
+template <class T>
+constexpr auto f{[] (auto arg) {
+ return [a{arg}] {
+ [] () requires true {}();
+ };
+}};
+
+void foo() {
+ f<int>(0);
+}
+
+}
|
erichkeane
reviewed
Apr 1, 2025
erichkeane
approved these changes
Apr 1, 2025
|
/cherry-pick dcc2182 |
Error: Command failed due to missing milestone. |
|
/cherry-pick dcc2182 |
|
/pull-request #134194 |
swift-ci
pushed a commit
to swiftlang/llvm-project
that referenced
this pull request
Apr 15, 2025
…133863) In ecc7e6c, we tried to inspect the `LambdaScopeInfo` on stack to recover the instantiating lambda captures. However, there was a mismatch in how we compared the pattern declarations of lambdas: the constraint instantiation used a tailored `getPatternFunctionDecl()` which is localized in SemaLambda that finds the very primal template declaration of a lambda, while `FunctionDecl::getTemplateInstantiationPattern` finds the latest template pattern of a lambda. This difference causes issues when lambdas are nested, as we always want the primary template declaration. This corrects that by moving `Sema::addInstantiatedCapturesToScope` from SemaConcept to SemaLambda, allowing it to use the localized version of `getPatternFunctionDecl`. It is also worth exploring to coalesce the implementation of `getPatternFunctionDecl` with `FunctionDecl::getTemplateInstantiationPattern`. But I’m leaving that for the future, as I’d like to backport this fix (ecc7e6c made the issue more visible in clang 20, sorry!), and changing Sema’s ABI would not be suitable in that regards. Hence, no release note. Fixes llvm#133719 (cherry picked from commit dcc2182)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In ecc7e6c, we tried to inspect the
LambdaScopeInfoon stack to recover the instantiating lambda captures. However, there was a mismatch in how we compared the pattern declarations of lambdas: the constraint instantiation used a tailoredgetPatternFunctionDecl()which is localized in SemaLambda that finds the very primal template declaration of a lambda, whileFunctionDecl::getTemplateInstantiationPatternfinds the latest template pattern of a lambda. This difference causes issues when lambdas are nested, as we always want the primary template declaration.This corrects that by moving
Sema::addInstantiatedCapturesToScopefrom SemaConcept to SemaLambda, allowing it to use the localized version ofgetPatternFunctionDecl.It is also worth exploring to coalesce the implementation of
getPatternFunctionDeclwithFunctionDecl::getTemplateInstantiationPattern. But I’m leaving that for the future, as I’d like to backport this fix (ecc7e6c made the issue more visible in clang 20, sorry!), and changing Sema’s ABI would not be suitable in that regards. Hence, no release note.Fixes #133719
(I split up the commits to make the review easier. Also note that in #133719, Shafik and I discovered additional cases where clang behaves incorrectly. However, since those aren't regressions, their fixes aren't being considered at this time.)