[Clang][P1061] Consolidate ResolvedUnpexandedPackExpr into FunctionParmPackExpr (llvm#125394)

This merges the functionality of ResolvedUnexpandedPackExpr into
FunctionParmPackExpr. I also added a test to show that
llvm#125103 should be fixed with
this. I put the removal of ResolvedUnexpandedPackExpr in its own commit.
Let me know what you think.

Fixes llvm#125103

Author: ricejasonf

clang/include/clang/AST/DeclCXX.h

@@ -4194,8 +4194,8 @@ class BindingDecl : public ValueDecl {
   /// decomposition declaration, and when the initializer is type-dependent.
   Expr *getBinding() const { return Binding; }
 
-  // Get the array of Exprs when the binding represents a pack.
-  llvm::ArrayRef<Expr *> getBindingPackExprs() const;
+  // Get the array of nested BindingDecls when the binding represents a pack.
+  llvm::ArrayRef<BindingDecl *> getBindingPackDecls() const;
 
   /// Get the decomposition declaration that this binding represents a
   /// decomposition of.
@@ -4246,10 +4246,8 @@ class DecompositionDecl final
     for (auto *B : Bindings) {
       B->setDecomposedDecl(this);
       if (B->isParameterPack() && B->getBinding()) {
-        for (Expr *E : B->getBindingPackExprs()) {
-          auto *DRE = cast<DeclRefExpr>(E);
-          auto *NestedB = cast<BindingDecl>(DRE->getDecl());
-          NestedB->setDecomposedDecl(this);
+        for (BindingDecl *NestedBD : B->getBindingPackDecls()) {
+          NestedBD->setDecomposedDecl(this);
         }
       }
     }
@@ -4278,25 +4276,21 @@ class DecompositionDecl final
   // Provide a flattened range to visit each binding.
   auto flat_bindings() const {
     llvm::ArrayRef<BindingDecl *> Bindings = bindings();
-    llvm::ArrayRef<Expr *> PackExprs;
+    llvm::ArrayRef<BindingDecl *> PackBindings;
 
     // Split the bindings into subranges split by the pack.
-    auto S1 = Bindings.take_until(
+    llvm::ArrayRef<BindingDecl *> BeforePackBindings = Bindings.take_until(
         [](BindingDecl *BD) { return BD->isParameterPack(); });
 
-    Bindings = Bindings.drop_front(S1.size());
+    Bindings = Bindings.drop_front(BeforePackBindings.size());
     if (!Bindings.empty()) {
-      PackExprs = Bindings.front()->getBindingPackExprs();
+      PackBindings = Bindings.front()->getBindingPackDecls();
       Bindings = Bindings.drop_front();
     }
 
-    auto S2 = llvm::map_range(PackExprs, [](Expr *E) {
-      auto *DRE = cast<DeclRefExpr>(E);
-      return cast<BindingDecl>(DRE->getDecl());
-    });
-
-    return llvm::concat<BindingDecl *>(std::move(S1), std::move(S2),
-                                       std::move(Bindings));
+    return llvm::concat<BindingDecl *const>(std::move(BeforePackBindings),
+                                            std::move(PackBindings),
+                                            std::move(Bindings));
   }
 
   void printName(raw_ostream &OS, const PrintingPolicy &Policy) const override;

clang/include/clang/AST/ExprCXX.h

@@ -4633,8 +4633,8 @@ class SubstNonTypeTemplateParmPackExpr : public Expr {
   }
 };
 
-/// Represents a reference to a function parameter pack or init-capture pack
-/// that has been substituted but not yet expanded.
+/// Represents a reference to a function parameter pack, init-capture pack,
+/// or binding pack that has been substituted but not yet expanded.
 ///
 /// When a pack expansion contains multiple parameter packs at different levels,
 /// this node is used to represent a function parameter pack at an outer level
@@ -4649,49 +4649,48 @@ class SubstNonTypeTemplateParmPackExpr : public Expr {
 /// \endcode
 class FunctionParmPackExpr final
     : public Expr,
-      private llvm::TrailingObjects<FunctionParmPackExpr, VarDecl *> {
+      private llvm::TrailingObjects<FunctionParmPackExpr, ValueDecl *> {
   friend class ASTReader;
   friend class ASTStmtReader;
   friend TrailingObjects;
 
   /// The function parameter pack which was referenced.
-  VarDecl *ParamPack;
+  ValueDecl *ParamPack;
 
   /// The location of the function parameter pack reference.
   SourceLocation NameLoc;
 
   /// The number of expansions of this pack.
   unsigned NumParameters;
 
-  FunctionParmPackExpr(QualType T, VarDecl *ParamPack,
-                       SourceLocation NameLoc, unsigned NumParams,
-                       VarDecl *const *Params);
+  FunctionParmPackExpr(QualType T, ValueDecl *ParamPack, SourceLocation NameLoc,
+                       unsigned NumParams, ValueDecl *const *Params);
 
 public:
   static FunctionParmPackExpr *Create(const ASTContext &Context, QualType T,
-                                      VarDecl *ParamPack,
+                                      ValueDecl *ParamPack,
                                       SourceLocation NameLoc,
-                                      ArrayRef<VarDecl *> Params);
+                                      ArrayRef<ValueDecl *> Params);
   static FunctionParmPackExpr *CreateEmpty(const ASTContext &Context,
                                            unsigned NumParams);
 
   /// Get the parameter pack which this expression refers to.
-  VarDecl *getParameterPack() const { return ParamPack; }
+  ValueDecl *getParameterPack() const { return ParamPack; }
 
   /// Get the location of the parameter pack.
   SourceLocation getParameterPackLocation() const { return NameLoc; }
 
   /// Iterators over the parameters which the parameter pack expanded
   /// into.
-  using iterator = VarDecl * const *;
-  iterator begin() const { return getTrailingObjects<VarDecl *>(); }
+  using iterator = ValueDecl *const *;
+  iterator begin() const { return getTrailingObjects<ValueDecl *>(); }
   iterator end() const { return begin() + NumParameters; }
 
   /// Get the number of parameters in this parameter pack.
   unsigned getNumExpansions() const { return NumParameters; }
 
   /// Get an expansion of the parameter pack by index.
-  VarDecl *getExpansion(unsigned I) const { return begin()[I]; }
+  ValueDecl *getExpansion(unsigned I) const { return begin()[I]; }
 
   SourceLocation getBeginLoc() const LLVM_READONLY { return NameLoc; }
   SourceLocation getEndLoc() const LLVM_READONLY { return NameLoc; }
@@ -5319,59 +5318,6 @@ class BuiltinBitCastExpr final
   }
 };
 
-// Represents an unexpanded pack where the list of expressions are
-// known. These are used when structured bindings introduce a pack.
-class ResolvedUnexpandedPackExpr final
-    : public Expr,
-      private llvm::TrailingObjects<ResolvedUnexpandedPackExpr, Expr *> {
-  friend class ASTStmtReader;
-  friend class ASTStmtWriter;
-  friend TrailingObjects;
-
-  SourceLocation BeginLoc;
-  unsigned NumExprs;
-
-  ResolvedUnexpandedPackExpr(SourceLocation BL, QualType QT, unsigned NumExprs);
-
-public:
-  static ResolvedUnexpandedPackExpr *CreateDeserialized(ASTContext &C,
-                                                        unsigned NumExprs);
-  static ResolvedUnexpandedPackExpr *
-  Create(ASTContext &C, SourceLocation BeginLoc, QualType T, unsigned NumExprs);
-  static ResolvedUnexpandedPackExpr *Create(ASTContext &C,
-                                            SourceLocation BeginLoc, QualType T,
-                                            llvm::ArrayRef<Expr *> Exprs);
-
-  unsigned getNumExprs() const { return NumExprs; }
-
-  llvm::MutableArrayRef<Expr *> getExprs() {
-    return {getTrailingObjects<Expr *>(), NumExprs};
-  }
-
-  llvm::ArrayRef<Expr *> getExprs() const {
-    return {getTrailingObjects<Expr *>(), NumExprs};
-  }
-
-  Expr *getExpansion(unsigned Idx) { return getExprs()[Idx]; }
-  Expr *getExpansion(unsigned Idx) const { return getExprs()[Idx]; }
-
-  // Iterators
-  child_range children() {
-    return child_range((Stmt **)getTrailingObjects<Expr *>(),
-                       (Stmt **)getTrailingObjects<Expr *>() + getNumExprs());
-  }
-
-  SourceLocation getBeginLoc() const LLVM_READONLY { return BeginLoc; }
-  SourceLocation getEndLoc() const LLVM_READONLY { return BeginLoc; }
-
-  // Returns the resolved pack of a decl or nullptr
-  static ResolvedUnexpandedPackExpr *getFromDecl(Decl *);
-
-  static bool classof(const Stmt *T) {
-    return T->getStmtClass() == ResolvedUnexpandedPackExprClass;
-  }
-};
-
 } // namespace clang
 
 #endif // LLVM_CLANG_AST_EXPRCXX_H

clang/include/clang/AST/RecursiveASTVisitor.h

@@ -2950,7 +2950,6 @@ DEF_TRAVERSE_STMT(FunctionParmPackExpr, {})
 DEF_TRAVERSE_STMT(CXXFoldExpr, {})
 DEF_TRAVERSE_STMT(AtomicExpr, {})
 DEF_TRAVERSE_STMT(CXXParenListInitExpr, {})
-DEF_TRAVERSE_STMT(ResolvedUnexpandedPackExpr, {})
 
 DEF_TRAVERSE_STMT(MaterializeTemporaryExpr, {
   if (S->getLifetimeExtendedTemporaryDecl()) {

clang/include/clang/Basic/StmtNodes.td

@@ -163,7 +163,6 @@ def MaterializeTemporaryExpr : StmtNode<Expr>;
 def LambdaExpr : StmtNode<Expr>;
 def CXXFoldExpr : StmtNode<Expr>;
 def CXXParenListInitExpr: StmtNode<Expr>;
-def ResolvedUnexpandedPackExpr : StmtNode<Expr>;
 
 // C++ Coroutines expressions
 def CoroutineSuspendExpr : StmtNode<Expr, 1>;

clang/include/clang/Sema/Sema.h

@@ -232,8 +232,7 @@ void threadSafetyCleanup(BeforeSet *Cache);
 
 // FIXME: No way to easily map from TemplateTypeParmTypes to
 // TemplateTypeParmDecls, so we have this horrible PointerUnion.
-typedef std::pair<llvm::PointerUnion<const TemplateTypeParmType *, NamedDecl *,
-                                     ResolvedUnexpandedPackExpr *>,
+typedef std::pair<llvm::PointerUnion<const TemplateTypeParmType *, NamedDecl *>,
                   SourceLocation>
     UnexpandedParameterPack;
 

clang/include/clang/Sema/Template.h

@@ -365,7 +365,7 @@ enum class TemplateSubstitutionKind : char {
   class LocalInstantiationScope {
   public:
     /// A set of declarations.
-    using DeclArgumentPack = SmallVector<VarDecl *, 4>;
+    using DeclArgumentPack = SmallVector<ValueDecl *, 4>;
 
   private:
     /// Reference to the semantic analysis that is performing

clang/include/clang/Serialization/ASTBitCodes.h

@@ -1908,7 +1908,6 @@ enum StmtCode {
   EXPR_PACK_EXPANSION,                    // PackExpansionExpr
   EXPR_PACK_INDEXING,                     // PackIndexingExpr
   EXPR_SIZEOF_PACK,                       // SizeOfPackExpr
-  EXPR_RESOLVED_UNEXPANDED_PACK,          // ResolvedUnexpandedPackExpr
   EXPR_SUBST_NON_TYPE_TEMPLATE_PARM,      // SubstNonTypeTemplateParmExpr
   EXPR_SUBST_NON_TYPE_TEMPLATE_PARM_PACK, // SubstNonTypeTemplateParmPackExpr
   EXPR_FUNCTION_PARM_PACK,                // FunctionParmPackExpr

clang/lib/AST/DeclCXX.cpp

@@ -3504,10 +3504,13 @@ VarDecl *BindingDecl::getHoldingVar() const {
   return VD;
 }
 
-llvm::ArrayRef<Expr *> BindingDecl::getBindingPackExprs() const {
+llvm::ArrayRef<BindingDecl *> BindingDecl::getBindingPackDecls() const {
   assert(Binding && "expecting a pack expr");
-  auto *RP = cast<ResolvedUnexpandedPackExpr>(Binding);
-  return RP->getExprs();
+  auto *FP = cast<FunctionParmPackExpr>(Binding);
+  ValueDecl *const *First = FP->getNumExpansions() > 0 ? FP->begin() : nullptr;
+  assert((!First || isa<BindingDecl>(*First)) && "expecting a BindingDecl");
+  return llvm::ArrayRef<BindingDecl *>(
+      reinterpret_cast<BindingDecl *const *>(First), FP->getNumExpansions());
 }
 
 void DecompositionDecl::anchor() {}

clang/lib/AST/Expr.cpp

@@ -3672,7 +3672,6 @@ bool Expr::HasSideEffects(const ASTContext &Ctx,
   case PackIndexingExprClass:
   case HLSLOutArgExprClass:
   case OpenACCAsteriskSizeExprClass:
-  case ResolvedUnexpandedPackExprClass:
     // These never have a side-effect.
     return false;
 

clang/lib/AST/ExprCXX.cpp

@@ -1779,31 +1779,31 @@ TemplateArgument SubstNonTypeTemplateParmPackExpr::getArgumentPack() const {
   return TemplateArgument(llvm::ArrayRef(Arguments, NumArguments));
 }
 
-FunctionParmPackExpr::FunctionParmPackExpr(QualType T, VarDecl *ParamPack,
+FunctionParmPackExpr::FunctionParmPackExpr(QualType T, ValueDecl *ParamPack,
                                            SourceLocation NameLoc,
                                            unsigned NumParams,
-                                           VarDecl *const *Params)
+                                           ValueDecl *const *Params)
     : Expr(FunctionParmPackExprClass, T, VK_LValue, OK_Ordinary),
       ParamPack(ParamPack), NameLoc(NameLoc), NumParameters(NumParams) {
   if (Params)
     std::uninitialized_copy(Params, Params + NumParams,
-                            getTrailingObjects<VarDecl *>());
+                            getTrailingObjects<ValueDecl *>());
   setDependence(ExprDependence::TypeValueInstantiation |
                 ExprDependence::UnexpandedPack);
 }
 
 FunctionParmPackExpr *
 FunctionParmPackExpr::Create(const ASTContext &Context, QualType T,
-                             VarDecl *ParamPack, SourceLocation NameLoc,
-                             ArrayRef<VarDecl *> Params) {
-  return new (Context.Allocate(totalSizeToAlloc<VarDecl *>(Params.size())))
+                             ValueDecl *ParamPack, SourceLocation NameLoc,
+                             ArrayRef<ValueDecl *> Params) {
+  return new (Context.Allocate(totalSizeToAlloc<ValueDecl *>(Params.size())))
       FunctionParmPackExpr(T, ParamPack, NameLoc, Params.size(), Params.data());
 }
 
 FunctionParmPackExpr *
 FunctionParmPackExpr::CreateEmpty(const ASTContext &Context,
                                   unsigned NumParams) {
-  return new (Context.Allocate(totalSizeToAlloc<VarDecl *>(NumParams)))
+  return new (Context.Allocate(totalSizeToAlloc<ValueDecl *>(NumParams)))
       FunctionParmPackExpr(QualType(), nullptr, SourceLocation(), 0, nullptr);
 }
 
@@ -1965,52 +1965,3 @@ CXXFoldExpr::CXXFoldExpr(QualType T, UnresolvedLookupExpr *Callee,
   SubExprs[SubExpr::RHS] = RHS;
   setDependence(computeDependence(this));
 }
-
-ResolvedUnexpandedPackExpr::ResolvedUnexpandedPackExpr(SourceLocation BL,
-                                                       QualType QT,
-                                                       unsigned NumExprs)
-    : Expr(ResolvedUnexpandedPackExprClass, QT, VK_PRValue, OK_Ordinary),
-      BeginLoc(BL), NumExprs(NumExprs) {
-  // C++ [temp.dep.expr]p3
-  // An id-expression is type-dependent if it is
-  //    - associated by name lookup with a pack
-  setDependence(ExprDependence::TypeValueInstantiation |
-                ExprDependence::UnexpandedPack);
-}
-
-ResolvedUnexpandedPackExpr *
-ResolvedUnexpandedPackExpr::CreateDeserialized(ASTContext &Ctx,
-                                               unsigned NumExprs) {
-  void *Mem = Ctx.Allocate(totalSizeToAlloc<Expr *>(NumExprs),
-                           alignof(ResolvedUnexpandedPackExpr));
-  return new (Mem)
-      ResolvedUnexpandedPackExpr(SourceLocation(), QualType(), NumExprs);
-}
-
-ResolvedUnexpandedPackExpr *
-ResolvedUnexpandedPackExpr::Create(ASTContext &Ctx, SourceLocation BL,
-                                   QualType T, unsigned NumExprs) {
-  void *Mem = Ctx.Allocate(totalSizeToAlloc<Expr *>(NumExprs),
-                           alignof(ResolvedUnexpandedPackExpr));
-  ResolvedUnexpandedPackExpr *New =
-      new (Mem) ResolvedUnexpandedPackExpr(BL, T, NumExprs);
-
-  auto Exprs = New->getExprs();
-  std::uninitialized_fill(Exprs.begin(), Exprs.end(), nullptr);
-
-  return New;
-}
-
-ResolvedUnexpandedPackExpr *
-ResolvedUnexpandedPackExpr::Create(ASTContext &Ctx, SourceLocation BL,
-                                   QualType T, ArrayRef<Expr *> Exprs) {
-  auto *New = Create(Ctx, BL, T, Exprs.size());
-  std::uninitialized_copy(Exprs.begin(), Exprs.end(), New->getExprs().begin());
-  return New;
-}
-
-ResolvedUnexpandedPackExpr *ResolvedUnexpandedPackExpr::getFromDecl(Decl *D) {
-  if (auto *BD = dyn_cast<BindingDecl>(D))
-    return dyn_cast_if_present<ResolvedUnexpandedPackExpr>(BD->getBinding());
-  return nullptr;
-}