[clang] [Clang] [Sema] Don't crash on unexpanded pack in invalid block literal (PR #110762)

via cfe-commits cfe-commits at lists.llvm.org
Fri Oct 11 11:03:03 PDT 2024


https://github.com/Sirraide updated https://github.com/llvm/llvm-project/pull/110762

>From 9c073cc3145bf6961b565516aea0e8d0c3fca0d7 Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Thu, 10 Oct 2024 13:08:20 +0200
Subject: [PATCH 1/5] Undo previous changes, keeping the release note and tests

---
 clang/docs/ReleaseNotes.rst                  |  2 +
 clang/lib/Sema/SemaExpr.cpp                  |  2 +
 clang/test/SemaCXX/block-unexpanded-pack.cpp | 48 ++++++++++++++++++++
 3 files changed, 52 insertions(+)
 create mode 100644 clang/test/SemaCXX/block-unexpanded-pack.cpp

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 34d2b584274a5f..ed4a7affcf6919 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -457,6 +457,8 @@ Bug Fixes to C++ Support
   containing outer unexpanded parameters were not correctly expanded. (#GH101754)
 - Fixed a bug in constraint expression comparison where the ``sizeof...`` expression was not handled properly
   in certain friend declarations. (#GH93099)
+- Clang no longer crashes when a lambda contains an invalid block declaration that contains an unexpanded
+  parameter pack. (#GH109148)
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 2db9d1fc69ed1e..6c3280e17f7eab 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -16379,6 +16379,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());
diff --git a/clang/test/SemaCXX/block-unexpanded-pack.cpp b/clang/test/SemaCXX/block-unexpanded-pack.cpp
new file mode 100644
index 00000000000000..ce88db236d437c
--- /dev/null
+++ b/clang/test/SemaCXX/block-unexpanded-pack.cpp
@@ -0,0 +1,48 @@
+// RUN: %clang_cc1 -fblocks -triple x86_64-apple-darwin -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fblocks -triple x86_64-apple-darwin -fsyntax-only -verify %s -frecovery-ast -frecovery-ast-type
+
+// This checks that when a block is discarded, the enclosing lambda’s
+// unexpanded parameter pack flag is reset to what it was before the
+// block is parsed so we don't crash when trying to diagnose unexpanded
+// parameter packs in the lambda.
+
+template <typename ...Ts>
+void gh109148() {
+  (^Ts); // expected-error {{expected expression}} expected-error {{unexpanded parameter pack 'Ts'}}
+
+  [] {
+    (^Ts); // expected-error {{expected expression}}
+    ^Ts;   // expected-error {{expected expression}}
+    ^(Ts); // expected-error {{expected expression}}
+    ^ Ts); // expected-error {{expected expression}}
+  };
+
+  ([] {
+    (^Ts); // expected-error {{expected expression}}
+    ^Ts;   // expected-error {{expected expression}}
+    ^(Ts); // expected-error {{expected expression}}
+    ^ Ts); // expected-error {{expected expression}}
+  }, ...); // expected-error {{pack expansion does not contain any unexpanded parameter packs}}
+
+  [] { // expected-error {{unexpanded parameter pack 'Ts'}}
+    ^ (Ts) {};
+  };
+
+  [] { // expected-error {{unexpanded parameter pack 'Ts'}}
+    (void) ^ { Ts x; };
+  };
+
+  [] { // expected-error {{unexpanded parameter pack 'Ts'}}
+    Ts s;
+    (^Ts); // expected-error {{expected expression}}
+  };
+
+  ([] {
+    Ts s;
+    (^Ts); // expected-error {{expected expression}}
+  }, ...);
+
+  [] { // expected-error {{unexpanded parameter pack 'Ts'}}
+    ^ { Ts s; return not_defined; }; // expected-error {{use of undeclared identifier 'not_defined'}}
+  };
+}

>From eac7b115096de7662add64e37fb5529778fbb9e4 Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Thu, 10 Oct 2024 14:09:59 +0200
Subject: [PATCH 2/5] Move unexpanded parameter handling into
 CapturingScopeInfo

---
 clang/include/clang/AST/ComputeDependence.h   |  3 +-
 clang/include/clang/AST/Expr.h                |  4 +-
 clang/include/clang/Sema/ScopeInfo.h          | 12 ++---
 clang/include/clang/Sema/Sema.h               |  8 ++--
 clang/include/clang/Sema/Template.h           | 16 +++----
 clang/lib/AST/ComputeDependence.cpp           |  5 +-
 clang/lib/Sema/Sema.cpp                       |  9 ++--
 clang/lib/Sema/SemaDecl.cpp                   |  4 +-
 clang/lib/Sema/SemaExpr.cpp                   |  3 +-
 clang/lib/Sema/SemaLambda.cpp                 |  5 +-
 clang/lib/Sema/SemaTemplate.cpp               | 10 ++--
 clang/lib/Sema/SemaTemplateInstantiate.cpp    | 12 +++--
 clang/lib/Sema/SemaTemplateVariadic.cpp       | 46 ++++++++++++-------
 .../expr/expr.prim/expr.prim.lambda/blocks.mm |  7 ++-
 ...ck-unexpanded-pack.cpp => block-packs.cpp} | 32 +++++++++----
 15 files changed, 110 insertions(+), 66 deletions(-)
 rename clang/test/SemaCXX/{block-unexpanded-pack.cpp => block-packs.cpp} (67%)

diff --git a/clang/include/clang/AST/ComputeDependence.h b/clang/include/clang/AST/ComputeDependence.h
index 6d3a51c379f9df..dba68f165199c9 100644
--- a/clang/include/clang/AST/ComputeDependence.h
+++ b/clang/include/clang/AST/ComputeDependence.h
@@ -132,7 +132,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);
diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index 66c746cc25040f..458b44d547581a 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -6369,9 +6369,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.
diff --git a/clang/include/clang/Sema/ScopeInfo.h b/clang/include/clang/Sema/ScopeInfo.h
index 700e361ef83f13..dd924ee3607b5e 100644
--- a/clang/include/clang/Sema/ScopeInfo.h
+++ b/clang/include/clang/Sema/ScopeInfo.h
@@ -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) {
@@ -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;
 
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index d616c3834c429d..40d63ff01bc5eb 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -750,10 +750,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
diff --git a/clang/include/clang/Sema/Template.h b/clang/include/clang/Sema/Template.h
index fe27290efdbfc5..6872d04cc4dfb9 100644
--- a/clang/include/clang/Sema/Template.h
+++ b/clang/include/clang/Sema/Template.h
@@ -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.
@@ -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;
     }
 
@@ -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
diff --git a/clang/lib/AST/ComputeDependence.cpp b/clang/lib/AST/ComputeDependence.cpp
index 6ef49790481aca..9f7429f2184609 100644
--- a/clang/lib/AST/ComputeDependence.cpp
+++ b/clang/lib/AST/ComputeDependence.cpp
@@ -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;
 }
 
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 4be7dfbc293927..86da24e9bd44f8 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -2382,10 +2382,11 @@ 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
@@ -2393,7 +2394,7 @@ LambdaScopeInfo *Sema::getEnclosingLambda() const {
         assert(!CodeSynthesisContexts.empty());
         return nullptr;
       }
-      return LSI;
+      return CSI;
     }
   }
   return nullptr;
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 0e536f71a2f70d..c42092e69ab867 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -15139,8 +15139,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())
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 6c3280e17f7eab..90d0d391419acc 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -16356,7 +16356,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:
diff --git a/clang/lib/Sema/SemaLambda.cpp b/clang/lib/Sema/SemaLambda.cpp
index 15ed8572e60844..7c2914dd1e5f3a 100644
--- a/clang/lib/Sema/SemaLambda.cpp
+++ b/clang/lib/Sema/SemaLambda.cpp
@@ -2350,7 +2350,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);
 
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index c7d48b81bc0347..490689321a9b55 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -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);
@@ -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(),
@@ -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
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index b36381422851f8..616c25c5af691b 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -1693,12 +1693,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;
@@ -2475,7 +2481,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.
   //
@@ -2484,7 +2490,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(
diff --git a/clang/lib/Sema/SemaTemplateVariadic.cpp b/clang/lib/Sema/SemaTemplateVariadic.cpp
index 40522a07f6339c..3bb3f1687731b6 100644
--- a/clang/lib/Sema/SemaTemplateVariadic.cpp
+++ b/clang/lib/Sema/SemaTemplateVariadic.cpp
@@ -36,7 +36,7 @@ namespace {
 
     SmallVectorImpl<UnexpandedParameterPack> &Unexpanded;
 
-    bool InLambda = false;
+    bool InLambdaOrBlock = false;
     unsigned DepthLimit = (unsigned)-1;
 
 #ifndef NDEBUG
@@ -140,7 +140,7 @@ namespace {
     /// do not contain unexpanded parameter packs.
     bool TraverseStmt(Stmt *S) {
       Expr *E = dyn_cast_or_null<Expr>(S);
-      if ((E && E->containsUnexpandedParameterPack()) || InLambda)
+      if ((E && E->containsUnexpandedParameterPack()) || InLambdaOrBlock)
         return inherited::TraverseStmt(S);
 
       return true;
@@ -149,7 +149,7 @@ namespace {
     /// Suppress traversal into types that do not contain
     /// unexpanded parameter packs.
     bool TraverseType(QualType T) {
-      if ((!T.isNull() && T->containsUnexpandedParameterPack()) || InLambda)
+      if ((!T.isNull() && T->containsUnexpandedParameterPack()) || InLambdaOrBlock)
         return inherited::TraverseType(T);
 
       return true;
@@ -160,7 +160,7 @@ namespace {
     bool TraverseTypeLoc(TypeLoc TL) {
       if ((!TL.getType().isNull() &&
            TL.getType()->containsUnexpandedParameterPack()) ||
-          InLambda)
+          InLambdaOrBlock)
         return inherited::TraverseTypeLoc(TL);
 
       return true;
@@ -262,20 +262,34 @@ namespace {
       if (!Lambda->containsUnexpandedParameterPack())
         return true;
 
-      bool WasInLambda = InLambda;
+      bool WasInLambdaOrBlock = InLambdaOrBlock;
       unsigned OldDepthLimit = DepthLimit;
 
-      InLambda = true;
+      InLambdaOrBlock = true;
       if (auto *TPL = Lambda->getTemplateParameterList())
         DepthLimit = TPL->getDepth();
 
       inherited::TraverseLambdaExpr(Lambda);
 
-      InLambda = WasInLambda;
+      InLambdaOrBlock = WasInLambdaOrBlock;
       DepthLimit = OldDepthLimit;
       return true;
     }
 
+    /// Analogously for blocks.
+    bool TraverseBlockExpr(BlockExpr* Block) {
+      if (!Block->containsUnexpandedParameterPack())
+        return true;
+
+      bool WasInLambdaOrBlock = InLambdaOrBlock;
+      InLambdaOrBlock = true;
+
+      inherited::TraverseBlockExpr(Block);
+
+      InLambdaOrBlock = WasInLambdaOrBlock;
+      return true;
+    }
+
     /// Suppress traversal within pack expansions in lambda captures.
     bool TraverseLambdaCapture(LambdaExpr *Lambda, const LambdaCapture *C,
                                Expr *Init) {
@@ -323,11 +337,11 @@ Sema::DiagnoseUnexpandedParameterPacks(SourceLocation Loc,
 
   // If we are within a lambda expression and referencing a pack that is not
   // declared within the lambda itself, that lambda contains an unexpanded
-  // parameter pack, and we are done.
+  // parameter pack, and we are done. Analogously for blocks.
   // FIXME: Store 'Unexpanded' on the lambda so we don't need to recompute it
   // later.
-  SmallVector<UnexpandedParameterPack, 4> LambdaParamPackReferences;
-  if (auto *LSI = getEnclosingLambda()) {
+  SmallVector<UnexpandedParameterPack, 4> ParamPackReferences;
+  if (sema::CapturingScopeInfo *CSI = getEnclosingLambdaOrBlock()) {
     for (auto &Pack : Unexpanded) {
       auto DeclaresThisPack = [&](NamedDecl *LocalPack) {
         if (auto *TTPT = Pack.first.dyn_cast<const TemplateTypeParmType *>()) {
@@ -336,11 +350,11 @@ Sema::DiagnoseUnexpandedParameterPacks(SourceLocation Loc,
         }
         return declaresSameEntity(Pack.first.get<NamedDecl *>(), LocalPack);
       };
-      if (llvm::any_of(LSI->LocalPacks, DeclaresThisPack))
-        LambdaParamPackReferences.push_back(Pack);
+      if (llvm::any_of(CSI->LocalPacks, DeclaresThisPack))
+        ParamPackReferences.push_back(Pack);
     }
 
-    if (LambdaParamPackReferences.empty()) {
+    if (ParamPackReferences.empty()) {
       // Construct in lambda only references packs declared outside the lambda.
       // That's OK for now, but the lambda itself is considered to contain an
       // unexpanded pack in this case, which will require expansion outside the
@@ -363,16 +377,16 @@ Sema::DiagnoseUnexpandedParameterPacks(SourceLocation Loc,
         }
         // Coumpound-statements outside the lambda are OK for now; we'll check
         // for those when we finish handling the lambda.
-        if (Func == LSI)
+        if (Func == CSI)
           break;
       }
 
       if (!EnclosingStmtExpr) {
-        LSI->ContainsUnexpandedParameterPack = true;
+        CSI->ContainsUnexpandedParameterPack = true;
         return false;
       }
     } else {
-      Unexpanded = LambdaParamPackReferences;
+      Unexpanded = ParamPackReferences;
     }
   }
 
diff --git a/clang/test/CXX/expr/expr.prim/expr.prim.lambda/blocks.mm b/clang/test/CXX/expr/expr.prim/expr.prim.lambda/blocks.mm
index e93c37f3b9ae12..b644e0fdfe3c0f 100644
--- a/clang/test/CXX/expr/expr.prim/expr.prim.lambda/blocks.mm
+++ b/clang/test/CXX/expr/expr.prim/expr.prim.lambda/blocks.mm
@@ -119,17 +119,16 @@ void call_with_lambda() {
 
     template<typename ... Args> static void f1()
     {
-      (void)^(Args args) { // expected-error{{block contains unexpanded parameter pack 'Args'}}
+      (void)^(Args args) { // expected-error{{expression contains unexpanded parameter pack 'Args'}}
       };
     }
 
     template<typename ... Args> static void f2()
     {
-      // FIXME: Allow this.
       f(
-        ^(Args args) // expected-error{{block contains unexpanded parameter pack 'Args'}}
+        ^(Args args)
         { }
-        ... // expected-error{{pack expansion does not contain any unexpanded parameter packs}}
+        ...
       );
     }
 
diff --git a/clang/test/SemaCXX/block-unexpanded-pack.cpp b/clang/test/SemaCXX/block-packs.cpp
similarity index 67%
rename from clang/test/SemaCXX/block-unexpanded-pack.cpp
rename to clang/test/SemaCXX/block-packs.cpp
index ce88db236d437c..4f3d68a727b7c9 100644
--- a/clang/test/SemaCXX/block-unexpanded-pack.cpp
+++ b/clang/test/SemaCXX/block-packs.cpp
@@ -1,14 +1,20 @@
-// RUN: %clang_cc1 -fblocks -triple x86_64-apple-darwin -fsyntax-only -verify %s
-// RUN: %clang_cc1 -fblocks -triple x86_64-apple-darwin -fsyntax-only -verify %s -frecovery-ast -frecovery-ast-type
+// RUN: %clang_cc1 -fblocks -triple x86_64-apple-darwin -fsyntax-only -verify -Wno-unused %s
+// RUN: %clang_cc1 -fblocks -triple x86_64-apple-darwin -fsyntax-only -verify -Wno-unused %s -frecovery-ast -frecovery-ast-type
 
-// This checks that when a block is discarded, the enclosing lambda’s
-// unexpanded parameter pack flag is reset to what it was before the
-// block is parsed so we don't crash when trying to diagnose unexpanded
-// parameter packs in the lambda.
+template <typename ...Ts>
+void f() {
+  ((^ { Ts t; }), ...);
+  ((^ (Ts t) {}), ...);
+  ((^ Ts () {}), ...);
+
+  ^ { Ts t; }; // expected-error {{unexpanded parameter pack 'Ts'}}
+  ^ (Ts t) {}; // expected-error {{unexpanded parameter pack 'Ts'}}
+  ^ Ts () {};  // expected-error {{unexpanded parameter pack 'Ts'}}
+}
 
 template <typename ...Ts>
 void gh109148() {
-  (^Ts); // expected-error {{expected expression}} expected-error {{unexpanded parameter pack 'Ts'}}
+  (^Ts); // expected-error {{expected expression}}
 
   [] {
     (^Ts); // expected-error {{expected expression}}
@@ -29,7 +35,7 @@ void gh109148() {
   };
 
   [] { // expected-error {{unexpanded parameter pack 'Ts'}}
-    (void) ^ { Ts x; };
+    ^ { Ts x; };
   };
 
   [] { // expected-error {{unexpanded parameter pack 'Ts'}}
@@ -46,3 +52,13 @@ void gh109148() {
     ^ { Ts s; return not_defined; }; // expected-error {{use of undeclared identifier 'not_defined'}}
   };
 }
+
+void g() {
+  f<>();
+  f<int>();
+  f<long, float>();
+
+  gh109148<>();
+  gh109148<int>();
+  gh109148<long, float>();
+}

>From 0425fe364b962e70c5680bdf62a267c4c65c50de Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Thu, 10 Oct 2024 14:10:29 +0200
Subject: [PATCH 3/5] clang-format

---
 clang/include/clang/Sema/ScopeInfo.h    | 2 +-
 clang/lib/Sema/SemaTemplateVariadic.cpp | 5 +++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/clang/include/clang/Sema/ScopeInfo.h b/clang/include/clang/Sema/ScopeInfo.h
index dd924ee3607b5e..958d65055fa9b2 100644
--- a/clang/include/clang/Sema/ScopeInfo.h
+++ b/clang/include/clang/Sema/ScopeInfo.h
@@ -732,7 +732,7 @@ class CapturingScopeInfo : public FunctionScopeInfo {
   QualType ReturnType;
 
   /// Packs introduced by this, if any.
-  SmallVector<NamedDecl*, 4> LocalPacks;
+  SmallVector<NamedDecl *, 4> LocalPacks;
 
   void addCapture(ValueDecl *Var, bool isBlock, bool isByref, bool isNested,
                   SourceLocation Loc, SourceLocation EllipsisLoc,
diff --git a/clang/lib/Sema/SemaTemplateVariadic.cpp b/clang/lib/Sema/SemaTemplateVariadic.cpp
index 3bb3f1687731b6..718bffa02f30d4 100644
--- a/clang/lib/Sema/SemaTemplateVariadic.cpp
+++ b/clang/lib/Sema/SemaTemplateVariadic.cpp
@@ -149,7 +149,8 @@ namespace {
     /// Suppress traversal into types that do not contain
     /// unexpanded parameter packs.
     bool TraverseType(QualType T) {
-      if ((!T.isNull() && T->containsUnexpandedParameterPack()) || InLambdaOrBlock)
+      if ((!T.isNull() && T->containsUnexpandedParameterPack()) ||
+          InLambdaOrBlock)
         return inherited::TraverseType(T);
 
       return true;
@@ -277,7 +278,7 @@ namespace {
     }
 
     /// Analogously for blocks.
-    bool TraverseBlockExpr(BlockExpr* Block) {
+    bool TraverseBlockExpr(BlockExpr *Block) {
       if (!Block->containsUnexpandedParameterPack())
         return true;
 

>From 6c1b4619fd6e30c0893d4c849a480029e7b825e6 Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Thu, 10 Oct 2024 14:28:34 +0200
Subject: [PATCH 4/5] No longer throw away the block type if it contains
 unexpanded packs

---
 clang/lib/Sema/SemaExpr.cpp | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 90d0d391419acc..8a65b3573fb4b7 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -16076,17 +16076,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

>From 5de8c4f9cb3e37ea63c46c05710be84879be03f8 Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Thu, 10 Oct 2024 14:45:05 +0200
Subject: [PATCH 5/5] [NFC] Use SaveAndRestore

---
 clang/lib/Sema/SemaTemplateVariadic.cpp | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/clang/lib/Sema/SemaTemplateVariadic.cpp b/clang/lib/Sema/SemaTemplateVariadic.cpp
index 718bffa02f30d4..19bd4547665835 100644
--- a/clang/lib/Sema/SemaTemplateVariadic.cpp
+++ b/clang/lib/Sema/SemaTemplateVariadic.cpp
@@ -18,6 +18,7 @@
 #include "clang/Sema/ScopeInfo.h"
 #include "clang/Sema/SemaInternal.h"
 #include "clang/Sema/Template.h"
+#include "llvm/Support/SaveAndRestore.h"
 #include <optional>
 
 using namespace clang;
@@ -263,16 +264,14 @@ namespace {
       if (!Lambda->containsUnexpandedParameterPack())
         return true;
 
-      bool WasInLambdaOrBlock = InLambdaOrBlock;
+      SaveAndRestore _(InLambdaOrBlock, true);
       unsigned OldDepthLimit = DepthLimit;
 
-      InLambdaOrBlock = true;
       if (auto *TPL = Lambda->getTemplateParameterList())
         DepthLimit = TPL->getDepth();
 
       inherited::TraverseLambdaExpr(Lambda);
 
-      InLambdaOrBlock = WasInLambdaOrBlock;
       DepthLimit = OldDepthLimit;
       return true;
     }
@@ -282,12 +281,8 @@ namespace {
       if (!Block->containsUnexpandedParameterPack())
         return true;
 
-      bool WasInLambdaOrBlock = InLambdaOrBlock;
-      InLambdaOrBlock = true;
-
+      SaveAndRestore _(InLambdaOrBlock, true);
       inherited::TraverseBlockExpr(Block);
-
-      InLambdaOrBlock = WasInLambdaOrBlock;
       return true;
     }
 



More information about the cfe-commits mailing list