[clang] [Clang] Add captures to the instantiation scope for noexcept specifiers (PR #97166)

Younan Zhang via cfe-commits cfe-commits at lists.llvm.org
Thu Jul 4 19:12:02 PDT 2024


https://github.com/zyn0217 updated https://github.com/llvm/llvm-project/pull/97166

>From 91a28dcde9dc569b955df92db91c858c550f6ad3 Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Sat, 29 Jun 2024 21:32:01 +0800
Subject: [PATCH 1/3] [Clang] Add captures to the instantiation scope for
 noexcept specifiers

---
 clang/docs/ReleaseNotes.rst                   |  1 +
 .../lib/Sema/SemaTemplateInstantiateDecl.cpp  |  6 +++++
 clang/test/SemaTemplate/generic-lambda.cpp    | 23 +++++++++++++++++++
 3 files changed, 30 insertions(+)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index da967fcdda8089..8c5bd989c60cea 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -935,6 +935,7 @@ Bug Fixes to C++ Support
 - Fix an assertion failure caused by parsing a lambda used as a default argument for the value of a
   forward-declared class. (#GH93512).
 - Fixed a bug in access checking inside return-type-requirement of compound requirements. (#GH93788).
+- Fixed a bug where references to lambda capture inside a ``noexcept`` specifier were not correctly instantiated. (#GH95735).
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 0681520764d9a0..d33db60d784487 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4719,6 +4719,12 @@ void Sema::InstantiateExceptionSpec(SourceLocation PointOfInstantiation,
     return;
   }
 
+  // The noexcept specification could reference any lambda captures. Ensure
+  // those are added to the LocalInstantiationScope.
+  LambdaScopeForCallOperatorInstantiationRAII PushLambdaCaptures(
+      *this, Decl, TemplateArgs, Scope,
+      /*ShouldAddDeclsFromParentScope=*/false);
+
   SubstExceptionSpec(Decl, Template->getType()->castAs<FunctionProtoType>(),
                      TemplateArgs);
 }
diff --git a/clang/test/SemaTemplate/generic-lambda.cpp b/clang/test/SemaTemplate/generic-lambda.cpp
index fb5fa09ebcc1fd..804eeaa29d6a1d 100644
--- a/clang/test/SemaTemplate/generic-lambda.cpp
+++ b/clang/test/SemaTemplate/generic-lambda.cpp
@@ -60,3 +60,26 @@ template<class T1> C1<X<X<T1>>> auto t3() {
 template C1<X<X<int>>> auto t3<int>();
 static_assert(is_same<decltype(t3<int>()), X<X<X<int>>>>);
 #endif
+
+namespace GH95735 {
+
+int g(int fn) {
+  return [f = fn](auto tpl) noexcept(noexcept(f)) { return f; }(0);
+}
+
+int foo(auto... fn) {
+  // FIXME: This one hits the assertion "if the exception specification is dependent,
+  // then the noexcept expression should be value-dependent" in the constructor of
+  // FunctionProtoType.
+  // One possible solution is to update Sema::canThrow() to consider expressions
+  // (e.g. DeclRefExpr/FunctionParmPackExpr) involving unexpanded parameters as Dependent.
+  // This would effectively add an extra value-dependent flag to the noexcept expression.
+  // However, I'm afraid that would also cause ABI breakage.
+  // [...f = fn](auto tpl) noexcept(noexcept(f)) { return 0; }(0);
+  [...f = fn](auto tpl) noexcept(noexcept(g(fn...))) { return 0; }(0);
+  return [...f = fn](auto tpl) noexcept(noexcept(g(f...))) { return 0; }(0);
+}
+
+int v = foo(42);
+
+} // namespace GH95735

>From 72fc2fcf472066a78c1496119aabf23a93c06ca0 Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Wed, 3 Jul 2024 13:36:17 +0800
Subject: [PATCH 2/3] [clang] PackIndexingExpr

---
 clang/include/clang/AST/ExprCXX.h          | 27 ++++++++++++++++++++--
 clang/lib/AST/ComputeDependence.cpp        |  2 +-
 clang/lib/AST/ExprCXX.cpp                  | 21 +++++++++++------
 clang/lib/Sema/SemaTemplateVariadic.cpp    | 17 ++++++++++++--
 clang/lib/Sema/TreeTransform.h             |  7 +++---
 clang/test/SemaCXX/cxx2c-pack-indexing.cpp | 19 +++++++++++++++
 6 files changed, 77 insertions(+), 16 deletions(-)

diff --git a/clang/include/clang/AST/ExprCXX.h b/clang/include/clang/AST/ExprCXX.h
index c2feac525c1ea6..aebf4843417fe2 100644
--- a/clang/include/clang/AST/ExprCXX.h
+++ b/clang/include/clang/AST/ExprCXX.h
@@ -4417,8 +4417,9 @@ class PackIndexingExpr final
 public:
   static PackIndexingExpr *Create(ASTContext &Context,
                                   SourceLocation EllipsisLoc,
-                                  SourceLocation RSquareLoc, Expr *PackIdExpr,
-                                  Expr *IndexExpr, std::optional<int64_t> Index,
+                                  SourceLocation RSquareLoc, QualType Type,
+                                  Expr *PackIdExpr, Expr *IndexExpr,
+                                  std::optional<int64_t> Index,
                                   ArrayRef<Expr *> SubstitutedExprs = {},
                                   bool ExpandedToEmptyPack = false);
   static PackIndexingExpr *CreateDeserialized(ASTContext &Context,
@@ -4465,6 +4466,28 @@ class PackIndexingExpr final
     return {getTrailingObjects<Expr *>(), TransformedExpressions};
   }
 
+  /// Returns the number of expansion, if known.
+  /// Note we have four states for pack expansion,
+  ///
+  /// 1) The PackIndexingExpr has never been expanded. In this case,
+  /// \code expandsToEmptyPack() == false && getNumExpansions() == nullopt \endcode
+  ///
+  /// 2) The PackIndexingExpr has been expanded, but it expands to empty.
+  /// \code expandsToEmptyPack() == true && getNumExpansions() == 0 \endcode
+  ///
+  /// 3) The PackIndexingExpr has been expanded, but the expansion should be
+  /// held off because the expansion size is unknown.
+  /// \code
+  /// expandsToEmptyPack() == false && getNumExpansions() == nullopt
+  /// \endcode
+  ///
+  /// 4) The PackIndexingExpr has been expanded to non-dependent expressions.
+  /// \code
+  /// expandsToEmptyPack() == false && getNumExpansions() ==
+  /// getExpressions().size()
+  /// \endcode
+  std::optional<unsigned> getNumExpansions() const;
+
   static bool classof(const Stmt *T) {
     return T->getStmtClass() == PackIndexingExprClass;
   }
diff --git a/clang/lib/AST/ComputeDependence.cpp b/clang/lib/AST/ComputeDependence.cpp
index 62ca15ea398f5c..38b2c885adc1f5 100644
--- a/clang/lib/AST/ComputeDependence.cpp
+++ b/clang/lib/AST/ComputeDependence.cpp
@@ -385,7 +385,7 @@ ExprDependence clang::computeDependence(PackIndexingExpr *E) {
          ExprDependence::Instantiation;
 
   ArrayRef<Expr *> Exprs = E->getExpressions();
-  if (Exprs.empty())
+  if (!E->getNumExpansions())
     D |= PatternDep | ExprDependence::Instantiation;
 
   else if (!E->getIndexExpr()->isInstantiationDependent()) {
diff --git a/clang/lib/AST/ExprCXX.cpp b/clang/lib/AST/ExprCXX.cpp
index 8d2a1b5611ccc6..9ed13358ab7d1d 100644
--- a/clang/lib/AST/ExprCXX.cpp
+++ b/clang/lib/AST/ExprCXX.cpp
@@ -1714,13 +1714,9 @@ NonTypeTemplateParmDecl *SubstNonTypeTemplateParmExpr::getParameter() const {
 
 PackIndexingExpr *PackIndexingExpr::Create(
     ASTContext &Context, SourceLocation EllipsisLoc, SourceLocation RSquareLoc,
-    Expr *PackIdExpr, Expr *IndexExpr, std::optional<int64_t> Index,
-    ArrayRef<Expr *> SubstitutedExprs, bool ExpandedToEmptyPack) {
-  QualType Type;
-  if (Index && !SubstitutedExprs.empty())
-    Type = SubstitutedExprs[*Index]->getType();
-  else
-    Type = Context.DependentTy;
+    QualType Type, Expr *PackIdExpr, Expr *IndexExpr,
+    std::optional<int64_t> Index, ArrayRef<Expr *> SubstitutedExprs,
+    bool ExpandedToEmptyPack) {
 
   void *Storage =
       Context.Allocate(totalSizeToAlloc<Expr *>(SubstitutedExprs.size()));
@@ -1729,6 +1725,17 @@ PackIndexingExpr *PackIndexingExpr::Create(
                        SubstitutedExprs, ExpandedToEmptyPack);
 }
 
+std::optional<unsigned> PackIndexingExpr::getNumExpansions() const {
+  if (!TransformedExpressions && !expandsToEmptyPack())
+    return std::nullopt;
+  if (llvm::any_of(getExpressions(), [](Expr *E) {
+        return isa<PackExpansionExpr>(E) &&
+               !cast<PackExpansionExpr>(E)->getNumExpansions();
+      }))
+    return std::nullopt;
+  return TransformedExpressions;
+}
+
 NamedDecl *PackIndexingExpr::getPackDecl() const {
   if (auto *D = dyn_cast<DeclRefExpr>(getPackIdExpression()); D) {
     NamedDecl *ND = dyn_cast<NamedDecl>(D->getDecl());
diff --git a/clang/lib/Sema/SemaTemplateVariadic.cpp b/clang/lib/Sema/SemaTemplateVariadic.cpp
index 7a44b978aacdb7..3021668554f041 100644
--- a/clang/lib/Sema/SemaTemplateVariadic.cpp
+++ b/clang/lib/Sema/SemaTemplateVariadic.cpp
@@ -1121,7 +1121,14 @@ Sema::BuildPackIndexingExpr(Expr *PackExpression, SourceLocation EllipsisLoc,
     IndexExpr = Res.get();
   }
 
-  if (Index && (!ExpandedExprs.empty() || EmptyPack)) {
+  bool ContainsUnexpandedParameterPack =
+      llvm::any_of(ExpandedExprs, [](Expr *E) {
+        return isa<PackExpansionExpr>(E) &&
+               !cast<PackExpansionExpr>(E)->getNumExpansions();
+      });
+
+  if (!ContainsUnexpandedParameterPack && Index &&
+      (!ExpandedExprs.empty() || EmptyPack)) {
     if (*Index < 0 || EmptyPack || *Index >= int64_t(ExpandedExprs.size())) {
       Diag(PackExpression->getBeginLoc(), diag::err_pack_index_out_of_bound)
           << *Index << PackExpression << ExpandedExprs.size();
@@ -1129,8 +1136,14 @@ Sema::BuildPackIndexingExpr(Expr *PackExpression, SourceLocation EllipsisLoc,
     }
   }
 
+  QualType Type;
+  if (!ContainsUnexpandedParameterPack && Index && !ExpandedExprs.empty())
+    Type = ExpandedExprs[*Index]->getType();
+  else
+    Type = Context.DependentTy;
+
   return PackIndexingExpr::Create(getASTContext(), EllipsisLoc, RSquareLoc,
-                                  PackExpression, IndexExpr, Index,
+                                  Type, PackExpression, IndexExpr, Index,
                                   ExpandedExprs, EmptyPack);
 }
 
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index ec678a55b11b7a..8dc375e8317d6e 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -15113,8 +15113,7 @@ TreeTransform<Derived>::TransformPackIndexingExpr(PackIndexingExpr *E) {
     // be expanded.
     bool ShouldExpand = true;
     bool RetainExpansion = false;
-    std::optional<unsigned> OrigNumExpansions;
-    std::optional<unsigned> NumExpansions = OrigNumExpansions;
+    std::optional<unsigned> NumExpansions;
     if (getDerived().TryExpandParameterPacks(
             E->getEllipsisLoc(), Pattern->getSourceRange(), Unexpanded,
             ShouldExpand, RetainExpansion, NumExpansions))
@@ -15135,7 +15134,7 @@ TreeTransform<Derived>::TransformPackIndexingExpr(PackIndexingExpr *E) {
         return true;
       if (Out.get()->containsUnexpandedParameterPack()) {
         Out = getDerived().RebuildPackExpansion(Out.get(), E->getEllipsisLoc(),
-                                                OrigNumExpansions);
+                                                /*NumExpansions=*/std::nullopt);
         if (Out.isInvalid())
           return true;
       }
@@ -15151,7 +15150,7 @@ TreeTransform<Derived>::TransformPackIndexingExpr(PackIndexingExpr *E) {
         return true;
 
       Out = getDerived().RebuildPackExpansion(Out.get(), E->getEllipsisLoc(),
-                                              OrigNumExpansions);
+                                              /*NumExpansions=*/std::nullopt);
       if (Out.isInvalid())
         return true;
       ExpandedExprs.push_back(Out.get());
diff --git a/clang/test/SemaCXX/cxx2c-pack-indexing.cpp b/clang/test/SemaCXX/cxx2c-pack-indexing.cpp
index 9ea90a4c3e30fd..028c05168cc332 100644
--- a/clang/test/SemaCXX/cxx2c-pack-indexing.cpp
+++ b/clang/test/SemaCXX/cxx2c-pack-indexing.cpp
@@ -231,3 +231,22 @@ struct type_info {
 namespace GH93650 {
 auto func(auto... inputArgs) { return typeid(inputArgs...[0]); }
 } // namespace GH93650
+
+namespace init_capture_pack {
+
+void init_capture() {
+  auto L = [](auto... x) {
+    return [x...](auto... y) {
+      return [... w = y]() {
+        return w...[3];
+      };
+    };
+  };
+  static_assert(L()(0, 1, 2, 3)() == 3);
+  L()('a', 'b', 'c')();
+  // expected-error at -6 {{invalid index 3 for pack w of size 3}}
+  // expected-note at -8 {{while substituting}}
+  // expected-note at -3 {{requested here}}
+}
+
+}

>From 712d9427e2e1328f48b3b41c97ad5feef831b617 Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Thu, 4 Jul 2024 21:55:40 +0800
Subject: [PATCH 3/3] fix gh97453

---
 clang/lib/Parse/ParseCXXInlineMethods.cpp | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/clang/lib/Parse/ParseCXXInlineMethods.cpp b/clang/lib/Parse/ParseCXXInlineMethods.cpp
index 943ce0fdde3a38..b9000f8f193218 100644
--- a/clang/lib/Parse/ParseCXXInlineMethods.cpp
+++ b/clang/lib/Parse/ParseCXXInlineMethods.cpp
@@ -529,6 +529,9 @@ void Parser::ParseLexedMethodDeclaration(LateParsedMethodDeclaration &LM) {
     ExprResult NoexceptExpr;
     CachedTokens *ExceptionSpecTokens;
 
+    Sema::SynthesizedFunctionScope Scope(Actions,
+                                         cast<FunctionDecl>(LM.Method));
+
     ExceptionSpecificationType EST
       = tryParseExceptionSpecification(/*Delayed=*/false, SpecificationRange,
                                        DynamicExceptions,



More information about the cfe-commits mailing list