Skip to content

Commit

Permalink
[Clang] [Sema] Don't crash on unexpanded pack in invalid block literal (
Browse files Browse the repository at this point in the history
#110762)

Consider #109148:
```c++
template <typename ...Ts>
void f() {
    [] {
        (^Ts);
    };
}
```

When we encounter `^Ts`, we try to parse a block and subsequently call
`DiagnoseUnexpandedParameterPack()` (in `ActOnBlockArguments()`), which
sees `Ts` and sets `ContainsUnexpandedParameterPack` to `true` in the
`LambdaScopeInfo` of the enclosing lambda. However, the entire block is
subsequently discarded entirely because it isn’t even syntactically
well-formed. As a result, `ContainsUnexpandedParameterPack` is `true`
despite the lambda’s body no longer containing any unexpanded packs,
which causes an assertion the next time
`DiagnoseUnexpandedParameterPack()` is called.

This pr moves handling of unexpanded parameter packs into
`CapturingScopeInfo` instead so that the same logic is used for both
blocks and lambdas. This fixes this issue since the
`ContainsUnexpandedParameterPack` flag is now part of the block (and
before that, its `CapturingScopeInfo`) and no longer affects the
surrounding lambda directly when the block is parsed. Moreover, this
change makes blocks actually usable with pack expansion.

This fixes #109148.
  • Loading branch information
Sirraide authored Oct 11, 2024
1 parent 2aa1dbf commit 48bda00
Show file tree
Hide file tree
Showing 16 changed files with 151 additions and 69 deletions.
2 changes: 2 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,8 @@ Bug Fixes to C++ Support
- Fix a crash when parsing a pseudo destructor involving an invalid type. (#GH111460)
- Fixed an assertion failure when invoking recovery call expressions with explicit attributes
and undeclared templates. (#GH107047, #GH49093)
- Clang no longer crashes when a lambda contains an invalid block declaration that contains an unexpanded
parameter pack. (#GH109148)

Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
3 changes: 2 additions & 1 deletion clang/include/clang/AST/ComputeDependence.h
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,8 @@ ExprDependence computeDependence(ArrayInitLoopExpr *E);
ExprDependence computeDependence(ImplicitValueInitExpr *E);
ExprDependence computeDependence(InitListExpr *E);
ExprDependence computeDependence(ExtVectorElementExpr *E);
ExprDependence computeDependence(BlockExpr *E);
ExprDependence computeDependence(BlockExpr *E,
bool ContainsUnexpandedParameterPack);
ExprDependence computeDependence(AsTypeExpr *E);
ExprDependence computeDependence(DeclRefExpr *E, const ASTContext &Ctx);
ExprDependence computeDependence(RecoveryExpr *E);
Expand Down
4 changes: 2 additions & 2 deletions clang/include/clang/AST/Expr.h
Original file line number Diff line number Diff line change
Expand Up @@ -6413,9 +6413,9 @@ class BlockExpr : public Expr {
protected:
BlockDecl *TheBlock;
public:
BlockExpr(BlockDecl *BD, QualType ty)
BlockExpr(BlockDecl *BD, QualType ty, bool ContainsUnexpandedParameterPack)
: Expr(BlockExprClass, ty, VK_PRValue, OK_Ordinary), TheBlock(BD) {
setDependence(computeDependence(this));
setDependence(computeDependence(this, ContainsUnexpandedParameterPack));
}

/// Build an empty block expression.
Expand Down
12 changes: 6 additions & 6 deletions clang/include/clang/Sema/ScopeInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -724,10 +724,16 @@ class CapturingScopeInfo : public FunctionScopeInfo {
/// is deduced (e.g. a lambda or block with omitted return type).
bool HasImplicitReturnType = false;

/// Whether this contains an unexpanded parameter pack.
bool ContainsUnexpandedParameterPack = false;

/// ReturnType - The target type of return statements in this context,
/// or null if unknown.
QualType ReturnType;

/// Packs introduced by this, if any.
SmallVector<NamedDecl *, 4> LocalPacks;

void addCapture(ValueDecl *Var, bool isBlock, bool isByref, bool isNested,
SourceLocation Loc, SourceLocation EllipsisLoc,
QualType CaptureType, bool Invalid) {
Expand Down Expand Up @@ -895,12 +901,6 @@ class LambdaScopeInfo final :
/// Whether any of the capture expressions requires cleanups.
CleanupInfo Cleanup;

/// Whether the lambda contains an unexpanded parameter pack.
bool ContainsUnexpandedParameterPack = false;

/// Packs introduced by this lambda, if any.
SmallVector<NamedDecl*, 4> LocalPacks;

/// Source range covering the explicit template parameter list (if it exists).
SourceRange ExplicitTemplateParamsRange;

Expand Down
8 changes: 4 additions & 4 deletions clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -702,10 +702,10 @@ class Sema final : public SemaBase {
/// Retrieve the current block, if any.
sema::BlockScopeInfo *getCurBlock();

/// Get the innermost lambda enclosing the current location, if any. This
/// looks through intervening non-lambda scopes such as local functions and
/// blocks.
sema::LambdaScopeInfo *getEnclosingLambda() const;
/// Get the innermost lambda or block enclosing the current location, if any.
/// This looks through intervening non-lambda, non-block scopes such as local
/// functions.
sema::CapturingScopeInfo *getEnclosingLambdaOrBlock() const;

/// Retrieve the current lambda scope info, if any.
/// \param IgnoreNonLambdaCapturingScope true if should find the top-most
Expand Down
16 changes: 8 additions & 8 deletions clang/include/clang/Sema/Template.h
Original file line number Diff line number Diff line change
Expand Up @@ -411,10 +411,10 @@ enum class TemplateSubstitutionKind : char {
/// lookup will search our outer scope.
bool CombineWithOuterScope;

/// Whether this scope is being used to instantiate a lambda expression,
/// in which case it should be reused for instantiating the lambda's
/// FunctionProtoType.
bool InstantiatingLambda = false;
/// Whether this scope is being used to instantiate a lambda or block
/// expression, in which case it should be reused for instantiating the
/// lambda's FunctionProtoType.
bool InstantiatingLambdaOrBlock = false;

/// If non-NULL, the template parameter pack that has been
/// partially substituted per C++0x [temp.arg.explicit]p9.
Expand All @@ -431,10 +431,10 @@ enum class TemplateSubstitutionKind : char {

public:
LocalInstantiationScope(Sema &SemaRef, bool CombineWithOuterScope = false,
bool InstantiatingLambda = false)
bool InstantiatingLambdaOrBlock = false)
: SemaRef(SemaRef), Outer(SemaRef.CurrentInstantiationScope),
CombineWithOuterScope(CombineWithOuterScope),
InstantiatingLambda(InstantiatingLambda) {
InstantiatingLambdaOrBlock(InstantiatingLambdaOrBlock) {
SemaRef.CurrentInstantiationScope = this;
}

Expand Down Expand Up @@ -561,8 +561,8 @@ enum class TemplateSubstitutionKind : char {
/// Determine whether D is a pack expansion created in this scope.
bool isLocalPackExpansion(const Decl *D);

/// Determine whether this scope is for instantiating a lambda.
bool isLambda() const { return InstantiatingLambda; }
/// Determine whether this scope is for instantiating a lambda or block.
bool isLambdaOrBlock() const { return InstantiatingLambdaOrBlock; }
};

class TemplateDeclInstantiator
Expand Down
5 changes: 4 additions & 1 deletion clang/lib/AST/ComputeDependence.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -252,10 +252,13 @@ ExprDependence clang::computeDependence(ExtVectorElementExpr *E) {
return E->getBase()->getDependence();
}

ExprDependence clang::computeDependence(BlockExpr *E) {
ExprDependence clang::computeDependence(BlockExpr *E,
bool ContainsUnexpandedParameterPack) {
auto D = toExprDependenceForImpliedType(E->getType()->getDependence());
if (E->getBlockDecl()->isDependentContext())
D |= ExprDependence::Instantiation;
if (ContainsUnexpandedParameterPack)
D |= ExprDependence::UnexpandedPack;
return D;
}

Expand Down
9 changes: 5 additions & 4 deletions clang/lib/Sema/Sema.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2385,18 +2385,19 @@ FunctionScopeInfo *Sema::getEnclosingFunction() const {
return nullptr;
}

LambdaScopeInfo *Sema::getEnclosingLambda() const {
CapturingScopeInfo *Sema::getEnclosingLambdaOrBlock() const {
for (auto *Scope : llvm::reverse(FunctionScopes)) {
if (auto *LSI = dyn_cast<sema::LambdaScopeInfo>(Scope)) {
if (LSI->Lambda && !LSI->Lambda->Encloses(CurContext) &&
if (auto *CSI = dyn_cast<CapturingScopeInfo>(Scope)) {
auto *LSI = dyn_cast<LambdaScopeInfo>(CSI);
if (LSI && LSI->Lambda && !LSI->Lambda->Encloses(CurContext) &&
LSI->AfterParameterList) {
// We have switched contexts due to template instantiation.
// FIXME: We should swap out the FunctionScopes during code synthesis
// so that we don't need to check for this.
assert(!CodeSynthesisContexts.empty());
return nullptr;
}
return LSI;
return CSI;
}
}
return nullptr;
Expand Down
4 changes: 2 additions & 2 deletions clang/lib/Sema/SemaDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15165,8 +15165,8 @@ ParmVarDecl *Sema::CheckParameter(DeclContext *DC, SourceLocation StartLoc,
// we know that references to that pack must also be expanded within the
// lambda scope.
if (New->isParameterPack())
if (auto *LSI = getEnclosingLambda())
LSI->LocalPacks.push_back(New);
if (auto *CSI = getEnclosingLambdaOrBlock())
CSI->LocalPacks.push_back(New);

if (New->getType().hasNonTrivialToPrimitiveDestructCUnion() ||
New->getType().hasNonTrivialToPrimitiveCopyCUnion())
Expand Down
17 changes: 5 additions & 12 deletions clang/lib/Sema/SemaExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16087,17 +16087,7 @@ void Sema::ActOnBlockArguments(SourceLocation CaretLoc, Declarator &ParamInfo,

TypeSourceInfo *Sig = GetTypeForDeclarator(ParamInfo);
QualType T = Sig->getType();

// FIXME: We should allow unexpanded parameter packs here, but that would,
// in turn, make the block expression contain unexpanded parameter packs.
if (DiagnoseUnexpandedParameterPack(CaretLoc, Sig, UPPC_Block)) {
// Drop the parameters.
FunctionProtoType::ExtProtoInfo EPI;
EPI.HasTrailingReturn = false;
EPI.TypeQuals.addConst();
T = Context.getFunctionType(Context.DependentTy, std::nullopt, EPI);
Sig = Context.getTrivialTypeSourceInfo(T);
}
DiagnoseUnexpandedParameterPack(CaretLoc, Sig, UPPC_Block);

// GetTypeForDeclarator always produces a function type for a block
// literal signature. Furthermore, it is always a FunctionProtoType
Expand Down Expand Up @@ -16369,7 +16359,8 @@ ExprResult Sema::ActOnBlockStmtExpr(SourceLocation CaretLoc,
AnalysisBasedWarnings::Policy WP = AnalysisWarnings.getDefaultPolicy();
PoppedFunctionScopePtr ScopeRAII = PopFunctionScopeInfo(&WP, BD, BlockTy);

BlockExpr *Result = new (Context) BlockExpr(BD, BlockTy);
BlockExpr *Result = new (Context)
BlockExpr(BD, BlockTy, BSI->ContainsUnexpandedParameterPack);

// If the block isn't obviously global, i.e. it captures anything at
// all, then we need to do a few things in the surrounding context:
Expand All @@ -16392,6 +16383,8 @@ ExprResult Sema::ActOnBlockStmtExpr(SourceLocation CaretLoc,
if (getCurFunction())
getCurFunction()->addBlock(BD);

// This can happen if the block's return type is deduced, but
// the return expression is invalid.
if (BD->isInvalidDecl())
return CreateRecoveryExpr(Result->getBeginLoc(), Result->getEndLoc(),
{Result}, Result->getType());
Expand Down
5 changes: 4 additions & 1 deletion clang/lib/Sema/SemaLambda.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2353,7 +2353,10 @@ ExprResult Sema::BuildBlockForLambdaConversion(SourceLocation CurrentLocation,
Block->setBody(new (Context) CompoundStmt(ConvLocation));

// Create the block literal expression.
Expr *BuildBlock = new (Context) BlockExpr(Block, Conv->getConversionType());
// TODO: Do we ever get here if we have unexpanded packs in the lambda???
Expr *BuildBlock =
new (Context) BlockExpr(Block, Conv->getConversionType(),
/*ContainsUnexpandedParameterPack=*/false);
ExprCleanupObjects.push_back(Block);
Cleanup.setExprNeedsCleanups(true);

Expand Down
10 changes: 5 additions & 5 deletions clang/lib/Sema/SemaTemplate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1010,8 +1010,8 @@ NamedDecl *Sema::ActOnTypeParameter(Scope *S, bool Typename,
Param->setAccess(AS_public);

if (Param->isParameterPack())
if (auto *LSI = getEnclosingLambda())
LSI->LocalPacks.push_back(Param);
if (auto *CSI = getEnclosingLambdaOrBlock())
CSI->LocalPacks.push_back(Param);

if (ParamName) {
maybeDiagnoseTemplateParameterShadow(*this, S, ParamNameLoc, ParamName);
Expand Down Expand Up @@ -1542,8 +1542,8 @@ NamedDecl *Sema::ActOnNonTypeTemplateParameter(Scope *S, Declarator &D,
Param->setInvalidDecl();

if (Param->isParameterPack())
if (auto *LSI = getEnclosingLambda())
LSI->LocalPacks.push_back(Param);
if (auto *CSI = getEnclosingLambdaOrBlock())
CSI->LocalPacks.push_back(Param);

if (ParamName) {
maybeDiagnoseTemplateParameterShadow(*this, S, D.getIdentifierLoc(),
Expand Down Expand Up @@ -1593,7 +1593,7 @@ NamedDecl *Sema::ActOnTemplateTemplateParameter(
Param->setAccess(AS_public);

if (Param->isParameterPack())
if (auto *LSI = getEnclosingLambda())
if (auto *LSI = getEnclosingLambdaOrBlock())
LSI->LocalPacks.push_back(Param);

// If the template template parameter has a name, then link the identifier
Expand Down
12 changes: 9 additions & 3 deletions clang/lib/Sema/SemaTemplateInstantiate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1694,12 +1694,18 @@ namespace {
if (SemaRef.RebuildingImmediateInvocation)
return E;
LocalInstantiationScope Scope(SemaRef, /*CombineWithOuterScope=*/true,
/*InstantiatingLambda=*/true);
/*InstantiatingLambdaOrBlock=*/true);
Sema::ConstraintEvalRAII<TemplateInstantiator> RAII(*this);

return inherited::TransformLambdaExpr(E);
}

ExprResult TransformBlockExpr(BlockExpr *E) {
LocalInstantiationScope Scope(SemaRef, /*CombineWithOuterScope=*/true,
/*InstantiatingLambdaOrBlock=*/true);
return inherited::TransformBlockExpr(E);
}

ExprResult RebuildLambdaExpr(SourceLocation StartLoc, SourceLocation EndLoc,
LambdaScopeInfo *LSI) {
CXXMethodDecl *MD = LSI->CallOperator;
Expand Down Expand Up @@ -2488,7 +2494,7 @@ QualType TemplateInstantiator::TransformFunctionProtoType(TypeLocBuilder &TLB,
CXXRecordDecl *ThisContext,
Qualifiers ThisTypeQuals,
Fn TransformExceptionSpec) {
// If this is a lambda, the transformation MUST be done in the
// If this is a lambda or block, the transformation MUST be done in the
// CurrentInstantiationScope since it introduces a mapping of
// the original to the newly created transformed parameters.
//
Expand All @@ -2497,7 +2503,7 @@ QualType TemplateInstantiator::TransformFunctionProtoType(TypeLocBuilder &TLB,
// a second one.
LocalInstantiationScope *Current = getSema().CurrentInstantiationScope;
std::optional<LocalInstantiationScope> Scope;
if (!Current || !Current->isLambda())
if (!Current || !Current->isLambdaOrBlock())
Scope.emplace(SemaRef, /*CombineWithOuterScope=*/true);

return inherited::TransformFunctionProtoType(
Expand Down
Loading

0 comments on commit 48bda00

Please sign in to comment.