From 52126dc72c3f6f4d27e3835b0ad53e162af25e53 Mon Sep 17 00:00:00 2001 From: Yupei Liu Date: Fri, 9 Aug 2024 23:13:11 +0800 Subject: [PATCH] [Clang] Fix Handling of Init Capture with Parameter Packs in LambdaScopeForCallOperatorInstantiationRAII (#100766) This PR addresses issues related to the handling of `init capture` with parameter packs in Clang's `LambdaScopeForCallOperatorInstantiationRAII`. Previously, `addInstantiatedCapturesToScope` would add `init capture` containing packs to the scope using the type of the `init capture` to determine the expanded pack size. However, this approach resulted in a pack size of 0 because `getType()->containsUnexpandedParameterPack()` returns `false`. After extensive testing, it appears that the correct pack size can only be inferred from `getInit`. But `getInit` may reference parameters and `init capture` from an outer lambda, as shown in the following example: ```cpp auto L = [](auto... z) { return [... w = z](auto... y) { // ... }; }; ``` To address this, `addInstantiatedCapturesToScope` in `LambdaScopeForCallOperatorInstantiationRAII` should be called last. Additionally, `addInstantiatedCapturesToScope` has been modified to only add `init capture` to the scope. The previous implementation incorrectly called `MakeInstantiatedLocalArgPack` for other non-init captures containing packs, resulting in a pack size of 0. ### Impact This patch affects scenarios where `LambdaScopeForCallOperatorInstantiationRAII` is passed with `ShouldAddDeclsFromParentScope = false`, preventing the correct addition of the current lambda's `init capture` to the scope. There are two main scenarios for `ShouldAddDeclsFromParentScope = false`: 1. **Constraints**: Sometimes constraints are instantiated in place rather than delayed. In this case, `LambdaScopeForCallOperatorInstantiationRAII` does not need to add `init capture` to the scope. 2. **`noexcept` Expressions**: The expressions inside `noexcept` have already been transformed, and the packs referenced within have been expanded. Only `RebuildLambdaInfo` needs to add the expanded captures to the scope, without requiring `addInstantiatedCapturesToScope` from `LambdaScopeForCallOperatorInstantiationRAII`. ### Considerations An alternative approach could involve adding a data structure within the lambda to record the expanded size of the `init capture` pack. However, this would increase the lambda's size and require extensive modifications. This PR is a prerequisite for implmenting https://github.com/llvm/llvm-project/issues/61426 --- clang/docs/ReleaseNotes.rst | 1 + clang/include/clang/Sema/Sema.h | 4 +++ clang/lib/Sema/SemaConcept.cpp | 18 +++++++++---- clang/lib/Sema/SemaLambda.cpp | 9 +++++-- clang/lib/Sema/SemaTemplateVariadic.cpp | 17 ++++++++----- clang/test/SemaTemplate/concepts-lambda.cpp | 28 +++++++++++++++++++++ 6 files changed, 64 insertions(+), 13 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 7a05ccf3184111..351b41b1c0c588 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -202,6 +202,7 @@ Bug Fixes to C++ Support ^^^^^^^^^^^^^^^^^^^^^^^^ - Fixed a crash when an expression with a dependent ``__typeof__`` type is used as the operand of a unary operator. (#GH97646) +- Fixed incorrect pack expansion of init-capture references in requires expresssions. - Fixed a failed assertion when checking invalid delete operator declaration. (#GH96191) - Fix a crash when checking destructor reference with an invalid initializer. (#GH97230) - Clang now correctly parses potentially declarative nested-name-specifiers in pointer-to-member declarators. diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index b7bd6c2433efd6..25cb6c8fbf6104 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -14186,6 +14186,10 @@ class Sema final : public SemaBase { std::optional getNumArgumentsInExpansion( QualType T, const MultiLevelTemplateArgumentList &TemplateArgs); + std::optional getNumArgumentsInExpansionFromUnexpanded( + llvm::ArrayRef Unexpanded, + const MultiLevelTemplateArgumentList &TemplateArgs); + /// Determine whether the given declarator contains any unexpanded /// parameter packs. /// diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp index d1e62fb5cee623..de24bbe7eb99ce 100644 --- a/clang/lib/Sema/SemaConcept.cpp +++ b/clang/lib/Sema/SemaConcept.cpp @@ -716,8 +716,8 @@ bool Sema::addInstantiatedCapturesToScope( auto AddSingleCapture = [&](const ValueDecl *CapturedPattern, unsigned Index) { ValueDecl *CapturedVar = LambdaClass->getCapture(Index)->getCapturedVar(); - if (CapturedVar->isInitCapture()) - Scope.InstantiatedLocal(CapturedPattern, CapturedVar); + assert(CapturedVar->isInitCapture()); + Scope.InstantiatedLocal(CapturedPattern, CapturedVar); }; for (const LambdaCapture &CapturePattern : LambdaPattern->captures()) { @@ -725,13 +725,21 @@ bool Sema::addInstantiatedCapturesToScope( Instantiated++; continue; } - const ValueDecl *CapturedPattern = CapturePattern.getCapturedVar(); + ValueDecl *CapturedPattern = CapturePattern.getCapturedVar(); + + if (!CapturedPattern->isInitCapture()) { + continue; + } + if (!CapturedPattern->isParameterPack()) { AddSingleCapture(CapturedPattern, Instantiated++); } else { Scope.MakeInstantiatedLocalArgPack(CapturedPattern); - std::optional NumArgumentsInExpansion = - getNumArgumentsInExpansion(CapturedPattern->getType(), TemplateArgs); + SmallVector Unexpanded; + SemaRef.collectUnexpandedParameterPacks( + dyn_cast(CapturedPattern)->getInit(), Unexpanded); + auto NumArgumentsInExpansion = + getNumArgumentsInExpansionFromUnexpanded(Unexpanded, TemplateArgs); if (!NumArgumentsInExpansion) continue; for (unsigned Arg = 0; Arg < *NumArgumentsInExpansion; ++Arg) diff --git a/clang/lib/Sema/SemaLambda.cpp b/clang/lib/Sema/SemaLambda.cpp index b697918120806b..05d406a8630fbe 100644 --- a/clang/lib/Sema/SemaLambda.cpp +++ b/clang/lib/Sema/SemaLambda.cpp @@ -2393,11 +2393,10 @@ Sema::LambdaScopeForCallOperatorInstantiationRAII:: if (!FDPattern) return; - SemaRef.addInstantiatedCapturesToScope(FD, FDPattern, Scope, MLTAL); - if (!ShouldAddDeclsFromParentScope) return; + FunctionDecl *InnermostFD = FD, *InnermostFDPattern = FDPattern; llvm::SmallVector, 4> ParentInstantiations; while (true) { @@ -2421,5 +2420,11 @@ Sema::LambdaScopeForCallOperatorInstantiationRAII:: for (const auto &[FDPattern, FD] : llvm::reverse(ParentInstantiations)) { SemaRef.addInstantiatedParametersToScope(FD, FDPattern, Scope, MLTAL); SemaRef.addInstantiatedLocalVarsToScope(FD, FDPattern, Scope); + + if (isLambdaCallOperator(FD)) + SemaRef.addInstantiatedCapturesToScope(FD, FDPattern, Scope, MLTAL); } + + SemaRef.addInstantiatedCapturesToScope(InnermostFD, InnermostFDPattern, Scope, + MLTAL); } diff --git a/clang/lib/Sema/SemaTemplateVariadic.cpp b/clang/lib/Sema/SemaTemplateVariadic.cpp index c45384f9da4cd7..bcd31c98871e22 100644 --- a/clang/lib/Sema/SemaTemplateVariadic.cpp +++ b/clang/lib/Sema/SemaTemplateVariadic.cpp @@ -825,12 +825,9 @@ bool Sema::CheckParameterPacksForExpansion( return false; } -std::optional Sema::getNumArgumentsInExpansion( - QualType T, const MultiLevelTemplateArgumentList &TemplateArgs) { - QualType Pattern = cast(T)->getPattern(); - SmallVector Unexpanded; - CollectUnexpandedParameterPacksVisitor(Unexpanded).TraverseType(Pattern); - +std::optional Sema::getNumArgumentsInExpansionFromUnexpanded( + llvm::ArrayRef Unexpanded, + const MultiLevelTemplateArgumentList &TemplateArgs) { std::optional Result; for (unsigned I = 0, N = Unexpanded.size(); I != N; ++I) { // Compute the depth and index for this parameter pack. @@ -878,6 +875,14 @@ std::optional Sema::getNumArgumentsInExpansion( return Result; } +std::optional Sema::getNumArgumentsInExpansion( + QualType T, const MultiLevelTemplateArgumentList &TemplateArgs) { + QualType Pattern = cast(T)->getPattern(); + SmallVector Unexpanded; + CollectUnexpandedParameterPacksVisitor(Unexpanded).TraverseType(Pattern); + return getNumArgumentsInExpansionFromUnexpanded(Unexpanded, TemplateArgs); +} + bool Sema::containsUnexpandedParameterPacks(Declarator &D) { const DeclSpec &DS = D.getDeclSpec(); switch (DS.getTypeSpecType()) { diff --git a/clang/test/SemaTemplate/concepts-lambda.cpp b/clang/test/SemaTemplate/concepts-lambda.cpp index 252ef08549a486..9c5807bbabdcbf 100644 --- a/clang/test/SemaTemplate/concepts-lambda.cpp +++ b/clang/test/SemaTemplate/concepts-lambda.cpp @@ -251,3 +251,31 @@ void dependent_param() { L(0, 1)(1, 2)(1); } } // namespace dependent_param_concept + +namespace init_captures { +template struct V {}; + +void sink(V<0>, V<1>, V<2>, V<3>, V<4>) {} + +void init_capture_pack() { + auto L = [](auto... z) { + return [=](auto... y) { + return [... w = z, y...](auto) + requires requires { sink(w..., y...); } + {}; + }; + }; + L(V<0>{}, V<1>{}, V<2>{})(V<3>{}, V<4>{})(1); +} + +void dependent_capture_packs() { + auto L = [](auto... z) { + return [... w = z](auto... y) { + return [... c = w](auto) + requires requires { sink(c..., y...); } + {}; + }; + }; + L(V<0>{}, V<1>{}, V<2>{})(V<3>{}, V<4>{})(1); +} +} // namespace init_captures