Skip to content

Commit

Permalink
[Clang] Fix Handling of Init Capture with Parameter Packs in LambdaSc…
Browse files Browse the repository at this point in the history
…opeForCallOperatorInstantiationRAII (#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
#61426
  • Loading branch information
LYP951018 authored Aug 9, 2024
1 parent 669d844 commit 52126dc
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 13 deletions.
1 change: 1 addition & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 4 additions & 0 deletions clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -14186,6 +14186,10 @@ class Sema final : public SemaBase {
std::optional<unsigned> getNumArgumentsInExpansion(
QualType T, const MultiLevelTemplateArgumentList &TemplateArgs);

std::optional<unsigned> getNumArgumentsInExpansionFromUnexpanded(
llvm::ArrayRef<UnexpandedParameterPack> Unexpanded,
const MultiLevelTemplateArgumentList &TemplateArgs);

/// Determine whether the given declarator contains any unexpanded
/// parameter packs.
///
Expand Down
18 changes: 13 additions & 5 deletions clang/lib/Sema/SemaConcept.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -716,22 +716,30 @@ 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()) {
if (!CapturePattern.capturesVariable()) {
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<unsigned> NumArgumentsInExpansion =
getNumArgumentsInExpansion(CapturedPattern->getType(), TemplateArgs);
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)
Expand Down
9 changes: 7 additions & 2 deletions clang/lib/Sema/SemaLambda.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::pair<FunctionDecl *, FunctionDecl *>, 4>
ParentInstantiations;
while (true) {
Expand All @@ -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);
}
17 changes: 11 additions & 6 deletions clang/lib/Sema/SemaTemplateVariadic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -825,12 +825,9 @@ bool Sema::CheckParameterPacksForExpansion(
return false;
}

std::optional<unsigned> Sema::getNumArgumentsInExpansion(
QualType T, const MultiLevelTemplateArgumentList &TemplateArgs) {
QualType Pattern = cast<PackExpansionType>(T)->getPattern();
SmallVector<UnexpandedParameterPack, 2> Unexpanded;
CollectUnexpandedParameterPacksVisitor(Unexpanded).TraverseType(Pattern);

std::optional<unsigned> Sema::getNumArgumentsInExpansionFromUnexpanded(
llvm::ArrayRef<UnexpandedParameterPack> Unexpanded,
const MultiLevelTemplateArgumentList &TemplateArgs) {
std::optional<unsigned> Result;
for (unsigned I = 0, N = Unexpanded.size(); I != N; ++I) {
// Compute the depth and index for this parameter pack.
Expand Down Expand Up @@ -878,6 +875,14 @@ std::optional<unsigned> Sema::getNumArgumentsInExpansion(
return Result;
}

std::optional<unsigned> Sema::getNumArgumentsInExpansion(
QualType T, const MultiLevelTemplateArgumentList &TemplateArgs) {
QualType Pattern = cast<PackExpansionType>(T)->getPattern();
SmallVector<UnexpandedParameterPack, 2> Unexpanded;
CollectUnexpandedParameterPacksVisitor(Unexpanded).TraverseType(Pattern);
return getNumArgumentsInExpansionFromUnexpanded(Unexpanded, TemplateArgs);
}

bool Sema::containsUnexpandedParameterPacks(Declarator &D) {
const DeclSpec &DS = D.getDeclSpec();
switch (DS.getTypeSpecType()) {
Expand Down
28 changes: 28 additions & 0 deletions clang/test/SemaTemplate/concepts-lambda.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -251,3 +251,31 @@ void dependent_param() {
L(0, 1)(1, 2)(1);
}
} // namespace dependent_param_concept

namespace init_captures {
template <int N> 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

0 comments on commit 52126dc

Please sign in to comment.