[AST] Store the expressions in ParenListExpr in a trailing array

[riccibruno]

Summary

Use the newly available space in the bit-fields of Stmt, and store
the expressions in ParenListExpr in a trailing array past ParenListExpr.
This saves 2 pointers per ParenListExpr. Additionally remove the branches
in the loop used to set the bits TypeDependent, ValueDependent,
InstantiationDependent and ContainsUnexpandedParameterPack,
and just access the bit-fields once at the end instead of during each iteration.

include/clang/AST/Expr.h

public:
   child_range children() {
     return child_range(child_iterator(), child_iterator());
   }
   const_child_range children() const {
     return const_child_range(const_child_iterator(), const_child_iterator());
   }
 };
 
-class ParenListExpr : public Expr {
-  Stmt **Exprs;
-  unsigned NumExprs;
+class ParenListExpr final
+    : public Expr,
+      private llvm::TrailingObjects<ParenListExpr, Stmt *> {
+  friend class ASTStmtReader;
+  friend TrailingObjects;
+
+  /// The location of the left and right parentheses.
   SourceLocation LParenLoc, RParenLoc;
 
-public:
-  ParenListExpr(const ASTContext& C, SourceLocation lparenloc,
-                ArrayRef<Expr*> exprs, SourceLocation rparenloc);
+  /// Build a paren list.
+  ParenListExpr(SourceLocation LParenLoc, ArrayRef<Expr *> Exprs,
+                SourceLocation RParenLoc);
 
   /// Build an empty paren list.
-  explicit ParenListExpr(EmptyShell Empty) : Expr(ParenListExprClass, Empty) { }
+  ParenListExpr(EmptyShell Empty, unsigned NumExprs);
 
-  unsigned getNumExprs() const { return NumExprs; }
+public:
+  /// Create a paren list.
+  static ParenListExpr *Create(const ASTContext &Ctx, SourceLocation LParenLoc,
+                               ArrayRef<Expr *> Exprs,
+                               SourceLocation RParenLoc);
 
-  const Expr* getExpr(unsigned Init) const {
-    assert(Init < getNumExprs() && "Initializer access out of range!");
-    return cast_or_null<Expr>(Exprs[Init]);
-  }
+  /// Create an empty paren list.
+  static ParenListExpr *CreateEmpty(const ASTContext &Ctx, unsigned NumExprs);
+
+  /// Return the number of expressions in this paren list.
+  unsigned getNumExprs() const { return ParenListExprBits.NumExprs; }
 
-  Expr* getExpr(unsigned Init) {
+  Expr *getExpr(unsigned Init) {
     assert(Init < getNumExprs() && "Initializer access out of range!");
-    return cast_or_null<Expr>(Exprs[Init]);
+    return getExprs()[Init];
+  }
+
+  const Expr *getExpr(unsigned Init) const {
+    return const_cast<ParenListExpr *>(this)->getExpr(Init);
   }
 
-  Expr **getExprs() { return reinterpret_cast<Expr **>(Exprs); }
+  Expr **getExprs() {
+    return reinterpret_cast<Expr **>(getTrailingObjects<Stmt *>());
+  }
 
   ArrayRef<Expr *> exprs() {
     return llvm::makeArrayRef(getExprs(), getNumExprs());
   }
 
   SourceLocation getLParenLoc() const { return LParenLoc; }
   SourceLocation getRParenLoc() const { return RParenLoc; }
-
-  SourceLocation getBeginLoc() const LLVM_READONLY { return LParenLoc; }
-  SourceLocation getEndLoc() const LLVM_READONLY { return RParenLoc; }
+  SourceLocation getBeginLoc() const { return getLParenLoc(); }
+  SourceLocation getEndLoc() const { return getRParenLoc(); }
 
   static bool classof(const Stmt *T) {
     return T->getStmtClass() == ParenListExprClass;
   }
 
   // Iterators
   child_range children() {
-    return child_range(&Exprs[0], &Exprs[0]+NumExprs);
+    return child_range(getTrailingObjects<Stmt *>(),
+                       getTrailingObjects<Stmt *>() + getNumExprs());
   }
   const_child_range children() const {
-    return const_child_range(&Exprs[0], &Exprs[0] + NumExprs);
+    return const_child_range(getTrailingObjects<Stmt *>(),
+                             getTrailingObjects<Stmt *>() + getNumExprs());
   }
-
-  friend class ASTStmtReader;
-  friend class ASTStmtWriter;
 };
 
 /// Represents a C11 generic selection.
 ///
 /// A generic selection (C11 6.5.1.1) contains an unevaluated controlling
 /// expression, followed by one or more generic associations.  Each generic
 /// association specifies a type name and an expression, or "default" and an
 /// expression (in which case it is known as a default generic association).

include/clang/AST/Stmt.h

class ExprWithCleanupsBitfields {
     unsigned : NumExprBits;
 
     // When false, it must not have side effects.
     unsigned CleanupsHaveSideEffects : 1;
 
     unsigned NumObjects : 32 - 1 - NumExprBits;
   };
 
+  class ParenListExprBitfields {
+    friend class ASTStmtReader;
+    friend class ParenListExpr;
+
+    unsigned : NumExprBits;
+
+    /// The number of expressions in the paren list.
+    unsigned NumExprs;
+  };
+
   class PseudoObjectExprBitfields {
     friend class ASTStmtReader; // deserialization
     friend class PseudoObjectExpr;
 
     unsigned : NumExprBits;
 
     // These don't need to be particularly wide, because they're
     // strictly limited by the forms of expressions we permit.
union {
     ArraySubscriptExprBitfields ArraySubscriptExprBits;
     DeclRefExprBitfields DeclRefExprBits;
     UnaryOperatorBitfields UnaryOperatorBits;
     CallExprBitfields CallExprBits;
     MemberExprBitfields MemberExprBits;
     CastExprBitfields CastExprBits;
     BinaryOperatorBitfields BinaryOperatorBits;
     ExprWithCleanupsBitfields ExprWithCleanupsBits;
+    ParenListExprBitfields ParenListExprBits;
     PseudoObjectExprBitfields PseudoObjectExprBits;
     OpaqueValueExprBitfields OpaqueValueExprBits;
     ObjCIndirectCopyRestoreExprBitfields ObjCIndirectCopyRestoreExprBits;
     InitListExprBitfields InitListExprBits;
     TypeTraitExprBitfields TypeTraitExprBits;
     CoawaitExprBitfields CoawaitBits;
   };
 

lib/AST/ASTImporter.cpp

ExpectedStmt ASTNodeImporter::VisitParenListExpr(ParenListExpr *E) {
   ExpectedSLoc ToLParenLocOrErr = import(E->getLParenLoc());
   if (!ToLParenLocOrErr)
     return ToLParenLocOrErr.takeError();
 
   ExpectedSLoc ToRParenLocOrErr = import(E->getRParenLoc());
   if (!ToRParenLocOrErr)
     return ToRParenLocOrErr.takeError();
 
-  return new (Importer.getToContext()) ParenListExpr(
-      Importer.getToContext(), *ToLParenLocOrErr, ToExprs, *ToRParenLocOrErr);
+  return ParenListExpr::Create(Importer.getToContext(), *ToLParenLocOrErr,
+                               ToExprs, *ToRParenLocOrErr);
 }
 
 ExpectedStmt ASTNodeImporter::VisitStmtExpr(StmtExpr *E) {
   auto Imp = importSeq(
       E->getSubStmt(), E->getType(), E->getLParenLoc(), E->getRParenLoc());
   if (!Imp)
     return Imp.takeError();
 

lib/AST/Expr.cpp

 SourceLocation DesignatedInitUpdateExpr::getBeginLoc() const {
   return getBase()->getBeginLoc();
 }
 
 SourceLocation DesignatedInitUpdateExpr::getEndLoc() const {
   return getBase()->getEndLoc();
 }
 
-ParenListExpr::ParenListExpr(const ASTContext& C, SourceLocation lparenloc,
-                             ArrayRef<Expr*> exprs,
-                             SourceLocation rparenloc)
-  : Expr(ParenListExprClass, QualType(), VK_RValue, OK_Ordinary,
-         false, false, false, false),
-    NumExprs(exprs.size()), LParenLoc(lparenloc), RParenLoc(rparenloc) {
-  Exprs = new (C) Stmt*[exprs.size()];
-  for (unsigned i = 0; i != exprs.size(); ++i) {
-    if (exprs[i]->isTypeDependent())
-      ExprBits.TypeDependent = true;
-    if (exprs[i]->isValueDependent())
-      ExprBits.ValueDependent = true;
-    if (exprs[i]->isInstantiationDependent())
-      ExprBits.InstantiationDependent = true;
-    if (exprs[i]->containsUnexpandedParameterPack())
-      ExprBits.ContainsUnexpandedParameterPack = true;
+ParenListExpr::ParenListExpr(SourceLocation LParenLoc, ArrayRef<Expr *> Exprs,
+                             SourceLocation RParenLoc)
+    : Expr(ParenListExprClass, QualType(), VK_RValue, OK_Ordinary, false, false,
+           false, false),
+      LParenLoc(LParenLoc), RParenLoc(RParenLoc) {
+  ParenListExprBits.NumExprs = Exprs.size();
+
+  bool TypeDependent = false;
+  bool ValueDependent = false;
+  bool InstantiationDependent = false;
+  bool ContainsUnexpandedParameterPack = false;
+
+  for (unsigned I = 0, N = Exprs.size(); I != N; ++I) {
+    TypeDependent |= Exprs[I]->isTypeDependent();
+    ValueDependent |= Exprs[I]->isValueDependent();
+    InstantiationDependent |= Exprs[I]->isInstantiationDependent();
+    ContainsUnexpandedParameterPack |=
+        Exprs[I]->containsUnexpandedParameterPack();
+
+    getTrailingObjects<Stmt *>()[I] = Exprs[I];
+  }
 
-    Exprs[i] = exprs[i];
-  }
+  ExprBits.TypeDependent |= TypeDependent;
+  ExprBits.ValueDependent |= ValueDependent;
+  ExprBits.InstantiationDependent |= InstantiationDependent;
+  ExprBits.ContainsUnexpandedParameterPack |= ContainsUnexpandedParameterPack;
+
+ParenListExpr::ParenListExpr(EmptyShell Empty, unsigned NumExprs)
+    : Expr(ParenListExprClass, Empty) {
+  ParenListExprBits.NumExprs = NumExprs;
+}
+
+ParenListExpr *ParenListExpr::Create(const ASTContext &Ctx,
+                                     SourceLocation LParenLoc,
+                                     ArrayRef<Expr *> Exprs,
+                                     SourceLocation RParenLoc) {
+  void *Mem = Ctx.Allocate(totalSizeToAlloc<Stmt *>(Exprs.size()),
+                           alignof(ParenListExpr));
+  return new (Mem) ParenListExpr(LParenLoc, Exprs, RParenLoc);
+}
+
+ParenListExpr *ParenListExpr::CreateEmpty(const ASTContext &Ctx,
+                                          unsigned NumExprs) {
+  void *Mem =
+      Ctx.Allocate(totalSizeToAlloc<Stmt *>(NumExprs), alignof(ParenListExpr));
+  return new (Mem) ParenListExpr(EmptyShell(), NumExprs);
 }
 
 const OpaqueValueExpr *OpaqueValueExpr::findInCopyConstruct(const Expr *e) {
   if (const ExprWithCleanups *ewc = dyn_cast<ExprWithCleanups>(e))
     e = ewc->getSubExpr();
   if (const MaterializeTemporaryExpr *m = dyn_cast<MaterializeTemporaryExpr>(e))
     e = m->GetTemporaryExpr();
   e = cast<CXXConstructExpr>(e)->getArg(0);

lib/Sema/SemaCoroutine.cpp

RefExpr =
                          ExprValueKind::VK_LValue, FD->getLocation());
     if (RefExpr.isInvalid())
       return nullptr;
     CtorArgExprs.push_back(RefExpr.get());
   }
 
   // Create an initialization sequence for the promise type using the
   // constructor arguments, wrapped in a parenthesized list expression.
-  Expr *PLE = new (Context) ParenListExpr(Context, FD->getLocation(),
-                                          CtorArgExprs, FD->getLocation());
+  Expr *PLE = ParenListExpr::Create(Context, FD->getLocation(),
   InitializedEntity Entity = InitializedEntity::InitializeVariable(VD);
   InitializationKind Kind = InitializationKind::CreateForInit(
       VD->getLocation(), /*DirectInit=*/true, PLE);
   InitializationSequence InitSeq(*this, Entity, Kind, CtorArgExprs,
                                  /*TopLevelOfInitList=*/false,
                                  /*TreatUnavailableAsInvalid=*/false);
 
   // Attempt to initialize the promise type with the arguments.

lib/Sema/SemaDeclCXX.cpp

Sema::ActOnMemInitializer(Decl *ConstructorD,
                           IdentifierInfo *MemberOrBase,
                           ParsedType TemplateTypeTy,
                           const DeclSpec &DS,
                           SourceLocation IdLoc,
                           SourceLocation LParenLoc,
                           ArrayRef<Expr *> Args,
                           SourceLocation RParenLoc,
                           SourceLocation EllipsisLoc) {
-  Expr *List = new (Context) ParenListExpr(Context, LParenLoc,
-                                           Args, RParenLoc);
+  Expr *List = ParenListExpr::Create(Context, LParenLoc, Args, RParenLoc);
   return BuildMemInitializer(ConstructorD, S, SS, MemberOrBase, TemplateTypeTy,
                              DS, IdLoc, List, EllipsisLoc);
 }
 
 namespace {
 
 // Callback to only accept typo corrections that can be a valid C++ member
 // intializer: either a non-static field member or a base class.

lib/Sema/SemaExpr.cpp

Sema::MaybeConvertParenListExprToParenExpr(Scope *S, Expr *OrigExpr) {
   if (Result.isInvalid()) return ExprError();
 
   return ActOnParenExpr(E->getLParenLoc(), E->getRParenLoc(), Result.get());
 }
 
 ExprResult Sema::ActOnParenListExpr(SourceLocation L,
                                     SourceLocation R,
                                     MultiExprArg Val) {
-  Expr *expr = new (Context) ParenListExpr(Context, L, Val, R);
-  return expr;
+  return ParenListExpr::Create(Context, L, Val, R);
 }
 
 /// Emit a specialized diagnostic when one expression is a null pointer
 /// constant and the other is not a pointer.  Returns true if a diagnostic is
 /// emitted.
 bool Sema::DiagnoseConditionalForNull(Expr *LHSExpr, Expr *RHSExpr,
                                       SourceLocation QuestionLoc) {
   Expr *NullExpr = LHSExpr;
ExprResult Sema::ActOnObjCAvailabilityCheckExpr(
   if (getCurFunctionOrMethodDecl())
     getEnclosingFunction()->HasPotentialAvailabilityViolations = true;
   else if (getCurBlock() || getCurLambda())
     getCurFunction()->HasPotentialAvailabilityViolations = true;
 
   return new (Context)
       ObjCAvailabilityCheckExpr(Version, AtLoc, RParen, Context.BoolTy);
 }
 No newline at end of file

lib/Serialization/ASTReaderStmt.cpp

void ASTStmtReader::VisitParenExpr(ParenExpr *E) {
   E->setLParen(ReadSourceLocation());
   E->setRParen(ReadSourceLocation());
   E->setSubExpr(Record.readSubExpr());
 }
 
 void ASTStmtReader::VisitParenListExpr(ParenListExpr *E) {
   VisitExpr(E);
   unsigned NumExprs = Record.readInt();
-  E->Exprs = new (Record.getContext()) Stmt*[NumExprs];
-  for (unsigned i = 0; i != NumExprs; ++i)
-    E->Exprs[i] = Record.readSubStmt();
-  E->NumExprs = NumExprs;
+  assert((NumExprs == E->getNumExprs()) && "Wrong NumExprs!");
+  for (unsigned I = 0; I != NumExprs; ++I)
+    E->getTrailingObjects<Stmt *>()[I] = Record.readSubStmt();
   E->LParenLoc = ReadSourceLocation();
   E->RParenLoc = ReadSourceLocation();
 }
 
 void ASTStmtReader::VisitUnaryOperator(UnaryOperator *E) {
   VisitExpr(E);
   E->setSubExpr(Record.readSubExpr());
   E->setOpcode((UnaryOperator::Opcode)Record.readInt());
case EXPR_CHARACTER_LITERAL:
       S = new (Context) CharacterLiteral(Empty);
       break;
 
     case EXPR_PAREN:
       S = new (Context) ParenExpr(Empty);
       break;
 
     case EXPR_PAREN_LIST:
-      S = new (Context) ParenListExpr(Empty);
+      S = ParenListExpr::CreateEmpty(
+          Context,
+          /* NumExprs=*/Record[ASTStmtReader::NumExprFields + 0]);
       break;
 
     case EXPR_UNARY_OPERATOR:
       S = new (Context) UnaryOperator(Empty);
       break;
 
     case EXPR_OFFSETOF:
       S = OffsetOfExpr::CreateEmpty(Context,

lib/Serialization/ASTWriterStmt.cpp

void ASTStmtWriter::VisitParenExpr(ParenExpr *E) {
   Record.AddSourceLocation(E->getLParen());
   Record.AddSourceLocation(E->getRParen());
   Record.AddStmt(E->getSubExpr());
   Code = serialization::EXPR_PAREN;
 }
 
 void ASTStmtWriter::VisitParenListExpr(ParenListExpr *E) {
   VisitExpr(E);
-  Record.push_back(E->NumExprs);
-  for (unsigned i=0; i != E->NumExprs; ++i)
-    Record.AddStmt(E->Exprs[i]);
-  Record.AddSourceLocation(E->LParenLoc);
-  Record.AddSourceLocation(E->RParenLoc);
+  Record.push_back(E->getNumExprs());
+  for (auto *SubStmt : E->exprs())
+    Record.AddStmt(SubStmt);
+  Record.AddSourceLocation(E->getLParenLoc());
+  Record.AddSourceLocation(E->getRParenLoc());
   Code = serialization::EXPR_PAREN_LIST;
 }
 
 void ASTStmtWriter::VisitUnaryOperator(UnaryOperator *E) {
   VisitExpr(E);
   Record.AddStmt(E->getSubExpr());
   Record.push_back(E->getOpcode()); // FIXME: stable encoding
   Record.AddSourceLocation(E->getOperatorLoc());