[AST] Store some data of CXXNewExpr as trailing objects

[riccibruno]

Summary

Store the optional array size expression, optional initialization expression and optional
placement new arguments in a trailing array. Additionally store the range for the parenthesized
type-id in a trailing object if needed since in the vast majority of cases the type is not parenthesized
(not a single new expression in the translation unit of SemaDecl.cpp has a parenthesized type-id).

This saves 2 pointers per CXXNewExpr in all cases, and 2 pointers + 8 bytes per CXXNewExpr
in the common case where the type is not parenthesized.

[rjmccall]

LGTM.

include/clang/AST/ExprCXX.h

1950

If you ever want a somewhat bigger refactoring project that might provide major wins on C++ code, one idea I've had for awhile is to largely eliminate TypeSourceInfo from the AST in favor of storing a TypeLoc in most places. A TSI is basically a TL that stores the location data in a trailing array; it was convenient as a way to retrofit type-location information into the AST without massive changes, and it saves a pointer (at the cost of an extra indirection) when the TSI can be shared between parent nodes. But I think the TSI can rarely be shared between parent nodes (pretty much just template instantiation of a non-dependent type), and the structure forces the location data to be copied when the type changes (e.g. during template instantiation) even if the location data would be the same. It also means we can't use a preallocated zero buffer in getTrivialTypeSourceInfo with a null SourceLocation, and instead we have to allocate and initialize an appropriately-sized zero'ed trailing array.

The win wouldn't be as large as it could be because FunctionProtoTypeLoc includes ParmVarDecls, which get cloned during template instantiation and therefore would always require a new type-location buffer when e.g. instantiating a function temploid. But it might still be quite significant.

[riccibruno]

include/clang/AST/ExprCXX.h

1950

Thanks for the pointer; this type of hint is very useful. I will have to dig into this
a bit before commenting on it, but I will definitely keep this in mind.

If you have some time there are two last patches which deal with
CXXDependentScopeMemberExpr and OverloadExpr, but I don't
want to spam you (with arguably pretty boring patches).

include/clang/AST/ExprCXX.h

public:
   // Iterators
   child_range children() {
     return child_range(child_iterator(), child_iterator());
   }
 };
 
 /// Represents a new-expression for memory allocation and constructor
 /// calls, e.g: "new CXXNewExpr(foo)".
-class CXXNewExpr : public Expr {
+class CXXNewExpr final
+    : public Expr,
+      private llvm::TrailingObjects<CXXNewExpr, Stmt *, SourceRange> {
   friend class ASTStmtReader;
   friend class ASTStmtWriter;
-
-  /// Contains an optional array size expression, an optional initialization
-  /// expression, and any number of optional placement arguments, in that order.
-  Stmt **SubExprs = nullptr;
+  friend TrailingObjects;
 
   /// Points to the allocation function used.
   FunctionDecl *OperatorNew;
 
-  /// Points to the deallocation function used in case of error. May be
-  /// null.
+  /// Points to the deallocation function used in case of error. May be null.
   FunctionDecl *OperatorDelete;
 
   /// The allocated type-source information, as written in the source.
   TypeSourceInfo *AllocatedTypeInfo;
 
-  /// If the allocated type was expressed as a parenthesized type-id,
-  /// the source range covering the parenthesized type-id.
-  SourceRange TypeIdParens;
-
   /// Range of the entire new expression.
   SourceRange Range;
 
   /// Source-range of a paren-delimited initializer.
   SourceRange DirectInitRange;
 
-  /// Was the usage ::new, i.e. is the global new to be used?
-  unsigned GlobalNew : 1;
-
-  /// Do we allocate an array? If so, the first SubExpr is the size expression.
-  unsigned Array : 1;
+  // CXXNewExpr is followed by several optional trailing objects.
+  // They are in order:
+  //
+  // * An optional "Stmt *" for the array size expression.
+  //    Present if and ony if isArray().
+  //
+  // * An optional "Stmt *" for the init expression.
+  //    Present if and only if hasInitializer().
+  //
+  // * An array of getNumPlacementArgs() "Stmt *" for the placement new
+  //   arguments, if any.
+  //
+  // * An optional SourceRange for the range covering the parenthesized type-id
+  //    if the allocated type was expressed as a parenthesized type-id.
+  //    Present if and only if isParenTypeId().
+  unsigned arraySizeOffset() const { return 0; }
+  unsigned initExprOffset() const { return arraySizeOffset() + isArray(); }
+  unsigned placementNewArgsOffset() const {
+    return initExprOffset() + hasInitializer();
+  }
 
-  /// Should the alignment be passed to the allocation function?
-  unsigned PassAlignment : 1;
+  unsigned numTrailingObjects(OverloadToken<Stmt *>) const {
+    return isArray() + hasInitializer() + getNumPlacementArgs();
+  }
 
-  /// If this is an array allocation, does the usual deallocation
-  /// function for the allocated type want to know the allocated size?
-  unsigned UsualArrayDeleteWantsSize : 1;
-
-  /// The number of placement new arguments.
-  unsigned NumPlacementArgs : 26;
-
-  /// What kind of initializer do we have? Could be none, parens, or braces.
-  /// In storage, we distinguish between "none, and no initializer expr", and
-  /// "none, but an implicit initializer expr".
-  unsigned StoredInitializationStyle : 2;
+  unsigned numTrailingObjects(OverloadToken<SourceRange>) const {
+    return isParenTypeId();
+  }
 
 public:
   enum InitializationStyle {
     /// New-expression has no initializer as written.
     NoInit,
 
     /// New-expression has a C++98 paren-delimited initializer.
     CallInit,
 
     /// New-expression has a C++11 list-initializer.
     ListInit
   };
 
-  CXXNewExpr(const ASTContext &C, bool globalNew, FunctionDecl *operatorNew,
-             FunctionDecl *operatorDelete, bool PassAlignment,
-             bool usualArrayDeleteWantsSize, ArrayRef<Expr*> placementArgs,
-             SourceRange typeIdParens, Expr *arraySize,
-             InitializationStyle initializationStyle, Expr *initializer,
-             QualType ty, TypeSourceInfo *AllocatedTypeInfo,
-             SourceRange Range, SourceRange directInitRange);
-  explicit CXXNewExpr(EmptyShell Shell)
-      : Expr(CXXNewExprClass, Shell) {}
-
-  void AllocateArgsArray(const ASTContext &C, bool isArray,
-                         unsigned numPlaceArgs, bool hasInitializer);
+private:
+  /// Build a c++ new expression.
+  CXXNewExpr(bool IsGlobalNew, FunctionDecl *OperatorNew,
+             FunctionDecl *OperatorDelete, bool ShouldPassAlignment,
+             bool UsualArrayDeleteWantsSize, ArrayRef<Expr *> PlacementArgs,
+             SourceRange TypeIdParens, Expr *ArraySize,
+             InitializationStyle InitializationStyle, Expr *Initializer,
+             QualType Ty, TypeSourceInfo *AllocatedTypeInfo, SourceRange Range,
+             SourceRange DirectInitRange);
+
+  /// Build an empty c++ new expression.
+  CXXNewExpr(EmptyShell Empty, bool IsArray, unsigned NumPlacementArgs,
+             bool IsParenTypeId);
+
+public:
+  /// Create a c++ new expression.
+  static CXXNewExpr *
+  Create(const ASTContext &Ctx, bool IsGlobalNew, FunctionDecl *OperatorNew,
+         FunctionDecl *OperatorDelete, bool ShouldPassAlignment,
+         bool UsualArrayDeleteWantsSize, ArrayRef<Expr *> PlacementArgs,
+         SourceRange TypeIdParens, Expr *ArraySize,
+         InitializationStyle InitializationStyle, Expr *Initializer,
+         QualType Ty, TypeSourceInfo *AllocatedTypeInfo, SourceRange Range,
+         SourceRange DirectInitRange);
+
+  /// Create an empty c++ new expression.
+  static CXXNewExpr *CreateEmpty(const ASTContext &Ctx, bool IsArray,
+                                 bool HasInit, unsigned NumPlacementArgs,
+                                 bool IsParenTypeId);
 
   QualType getAllocatedType() const {
     assert(getType()->isPointerType());
     return getType()->getAs<PointerType>()->getPointeeType();
   }
 
   TypeSourceInfo *getAllocatedTypeSourceInfo() const {
     return AllocatedTypeInfo;
public:
   /// C++ DR1748:
   ///   If the allocation function is a reserved placement allocation
   ///   function that returns null, the behavior is undefined.
   ///
   /// An allocation function is not allowed to return null unless it
   /// has a non-throwing exception-specification.  The '03 rule is
   /// identical except that the definition of a non-throwing
   /// exception specification is just "is it throw()?".
-  bool shouldNullCheckAllocation(const ASTContext &Ctx) const;
+  bool shouldNullCheckAllocation() const;
 
   FunctionDecl *getOperatorNew() const { return OperatorNew; }
   void setOperatorNew(FunctionDecl *D) { OperatorNew = D; }
   FunctionDecl *getOperatorDelete() const { return OperatorDelete; }
   void setOperatorDelete(FunctionDecl *D) { OperatorDelete = D; }
 
-  bool isArray() const { return Array; }
+  bool isArray() const { return CXXNewExprBits.IsArray; }
 
   Expr *getArraySize() {
-    return Array ? cast<Expr>(SubExprs[0]) : nullptr;
+    return isArray()
+               ? cast<Expr>(getTrailingObjects<Stmt *>()[arraySizeOffset()])
+               : nullptr;
   }
   const Expr *getArraySize() const {
-    return Array ? cast<Expr>(SubExprs[0]) : nullptr;
+    return isArray()
+               ? cast<Expr>(getTrailingObjects<Stmt *>()[arraySizeOffset()])
+               : nullptr;
   }
 
-  unsigned getNumPlacementArgs() const { return NumPlacementArgs; }
+  unsigned getNumPlacementArgs() const {
+    return CXXNewExprBits.NumPlacementArgs;
+  }
 
   Expr **getPlacementArgs() {
-    return reinterpret_cast<Expr **>(SubExprs + Array + hasInitializer());
+    return reinterpret_cast<Expr **>(getTrailingObjects<Stmt *>() +
+                                     placementNewArgsOffset());
   }
 
-  Expr *getPlacementArg(unsigned i) {
-    assert(i < NumPlacementArgs && "Index out of range");
-    return getPlacementArgs()[i];
+  Expr *getPlacementArg(unsigned I) {
+    assert((I < getNumPlacementArgs()) && "Index out of range!");
+    return getPlacementArgs()[I];
   }
-  const Expr *getPlacementArg(unsigned i) const {
-    assert(i < NumPlacementArgs && "Index out of range");
-    return const_cast<CXXNewExpr*>(this)->getPlacementArg(i);
+  const Expr *getPlacementArg(unsigned I) const {
+    return const_cast<CXXNewExpr *>(this)->getPlacementArg(I);
   }
 
-  bool isParenTypeId() const { return TypeIdParens.isValid(); }
-  SourceRange getTypeIdParens() const { return TypeIdParens; }
+  bool isParenTypeId() const { return CXXNewExprBits.IsParenTypeId; }
+  SourceRange getTypeIdParens() const {
+    return isParenTypeId() ? getTrailingObjects<SourceRange>()[0]
+                           : SourceRange();
+  }
 
-  bool isGlobalNew() const { return GlobalNew; }
+  bool isGlobalNew() const { return CXXNewExprBits.IsGlobalNew; }
 
   /// Whether this new-expression has any initializer at all.
-  bool hasInitializer() const { return StoredInitializationStyle > 0; }
+  bool hasInitializer() const {
+    return CXXNewExprBits.StoredInitializationStyle > 0;
+  }
 
   /// The kind of initializer this new-expression has.
   InitializationStyle getInitializationStyle() const {
-    if (StoredInitializationStyle == 0)
+    if (CXXNewExprBits.StoredInitializationStyle == 0)
       return NoInit;
-    return static_cast<InitializationStyle>(StoredInitializationStyle-1);
+    return static_cast<InitializationStyle>(
+        CXXNewExprBits.StoredInitializationStyle - 1);
   }
 
   /// The initializer of this new-expression.
   Expr *getInitializer() {
-    return hasInitializer() ? cast<Expr>(SubExprs[Array]) : nullptr;
+    return hasInitializer()
+               ? cast<Expr>(getTrailingObjects<Stmt *>()[initExprOffset()])
+               : nullptr;
   }
   const Expr *getInitializer() const {
-    return hasInitializer() ? cast<Expr>(SubExprs[Array]) : nullptr;
+    return hasInitializer()
+               ? cast<Expr>(getTrailingObjects<Stmt *>()[initExprOffset()])
+               : nullptr;
   }
 
   /// Returns the CXXConstructExpr from this new-expression, or null.
   const CXXConstructExpr *getConstructExpr() const {
     return dyn_cast_or_null<CXXConstructExpr>(getInitializer());
   }
 
   /// Indicates whether the required alignment should be implicitly passed to
   /// the allocation function.
-  bool passAlignment() const {
-    return PassAlignment;
-  }
+  bool passAlignment() const { return CXXNewExprBits.ShouldPassAlignment; }
 
   /// Answers whether the usual array deallocation function for the
   /// allocated type expects the size of the allocation as a
   /// parameter.
   bool doesUsualArrayDeleteWantSize() const {
-    return UsualArrayDeleteWantsSize;
+    return CXXNewExprBits.UsualArrayDeleteWantsSize;
   }
 
   using arg_iterator = ExprIterator;
   using const_arg_iterator = ConstExprIterator;
 
   llvm::iterator_range<arg_iterator> placement_arguments() {
     return llvm::make_range(placement_arg_begin(), placement_arg_end());
   }
 
   llvm::iterator_range<const_arg_iterator> placement_arguments() const {
     return llvm::make_range(placement_arg_begin(), placement_arg_end());
   }
 
   arg_iterator placement_arg_begin() {
-    return SubExprs + Array + hasInitializer();
+    return getTrailingObjects<Stmt *>() + placementNewArgsOffset();
   }
   arg_iterator placement_arg_end() {
-    return SubExprs + Array + hasInitializer() + getNumPlacementArgs();
+    return placement_arg_begin() + getNumPlacementArgs();
   }
   const_arg_iterator placement_arg_begin() const {
-    return SubExprs + Array + hasInitializer();
+    return getTrailingObjects<Stmt *>() + placementNewArgsOffset();
   }
   const_arg_iterator placement_arg_end() const {
-    return SubExprs + Array + hasInitializer() + getNumPlacementArgs();
+    return placement_arg_begin() + getNumPlacementArgs();
   }
 
   using raw_arg_iterator = Stmt **;
 
-  raw_arg_iterator raw_arg_begin() { return SubExprs; }
+  raw_arg_iterator raw_arg_begin() { return getTrailingObjects<Stmt *>(); }
   raw_arg_iterator raw_arg_end() {
-    return SubExprs + Array + hasInitializer() + getNumPlacementArgs();
+    return raw_arg_begin() + numTrailingObjects(OverloadToken<Stmt *>());
+  }
+  const_arg_iterator raw_arg_begin() const {
+    return getTrailingObjects<Stmt *>();
   }
-  const_arg_iterator raw_arg_begin() const { return SubExprs; }
   const_arg_iterator raw_arg_end() const {
-    return SubExprs + Array + hasInitializer() + getNumPlacementArgs();
+    return raw_arg_begin() + numTrailingObjects(OverloadToken<Stmt *>());
   }
 
   SourceLocation getBeginLoc() const { return Range.getBegin(); }
   SourceLocation getEndLoc() const { return Range.getEnd(); }
 
   SourceRange getDirectInitRange() const { return DirectInitRange; }
-
-  SourceRange getSourceRange() const LLVM_READONLY {
-    return Range;
-  }
-
+  SourceRange getSourceRange() const { return Range; }
 
   static bool classof(const Stmt *T) {
     return T->getStmtClass() == CXXNewExprClass;
   }
 
   // Iterators
-  child_range children() {
-    return child_range(raw_arg_begin(), raw_arg_end());
-  }
+  child_range children() { return child_range(raw_arg_begin(), raw_arg_end()); }
 };
 
 /// Represents a \c delete expression for memory deallocation and
 /// destructor calls, e.g. "delete[] pArray".
 class CXXDeleteExpr : public Expr {
   friend class ASTStmtReader;
 
   /// Points to the operator delete overload that is used. Could be a member.

include/clang/AST/Stmt.h

class CXXDefaultInitExprBitfields {
     friend class CXXDefaultInitExpr;
 
     unsigned : NumExprBits;
 
     /// The location where the default initializer expression was used.
     SourceLocation Loc;
   };
 
+  class CXXNewExprBitfields {
+    friend class ASTStmtReader;
+    friend class ASTStmtWriter;
+    friend class CXXNewExpr;
+
+    unsigned : NumExprBits;
+
+    /// Was the usage ::new, i.e. is the global new to be used?
+    unsigned IsGlobalNew : 1;
+
+    /// Do we allocate an array? If so, the first trailing "Stmt *" is the
+    /// size expression.
+    unsigned IsArray : 1;
+
+    /// Should the alignment be passed to the allocation function?
+    unsigned ShouldPassAlignment : 1;
+
+    /// If this is an array allocation, does the usual deallocation
+    /// function for the allocated type want to know the allocated size?
+    unsigned UsualArrayDeleteWantsSize : 1;
+
+    /// What kind of initializer do we have? Could be none, parens, or braces.
+    /// In storage, we distinguish between "none, and no initializer expr", and
+    /// "none, but an implicit initializer expr".
+    unsigned StoredInitializationStyle : 2;
+
+    /// True if the allocated type was expressed as a parenthesized type-id.
+    unsigned IsParenTypeId : 1;
+
+    /// The number of placement new arguments.
+    unsigned NumPlacementArgs;
+  };
+
   class CXXDeleteExprBitfields {
     friend class ASTStmtReader;
     friend class CXXDeleteExpr;
 
     unsigned : NumExprBits;
 
     /// Is this a forced global delete, i.e. "::delete"?
     unsigned GlobalDelete : 1;
union {
     // C++ Expressions
     CXXOperatorCallExprBitfields CXXOperatorCallExprBits;
     CXXBoolLiteralExprBitfields CXXBoolLiteralExprBits;
     CXXNullPtrLiteralExprBitfields CXXNullPtrLiteralExprBits;
     CXXThisExprBitfields CXXThisExprBits;
     CXXThrowExprBitfields CXXThrowExprBits;
     CXXDefaultArgExprBitfields CXXDefaultArgExprBits;
     CXXDefaultInitExprBitfields CXXDefaultInitExprBits;
+    CXXNewExprBitfields CXXNewExprBits;
     CXXDeleteExprBitfields CXXDeleteExprBits;
     TypeTraitExprBitfields TypeTraitExprBits;
     DependentScopeDeclRefExprBitfields DependentScopeDeclRefExprBits;
     CXXConstructExprBitfields CXXConstructExprBits;
     ExprWithCleanupsBitfields ExprWithCleanupsBits;
     CXXUnresolvedConstructExprBitfields CXXUnresolvedConstructExprBits;
 
     // C++ Coroutines TS expressions

lib/AST/ASTImporter.cpp

std::tie(
     ToOperatorNew, ToOperatorDelete, ToTypeIdParens, ToArraySize, ToInitializer,
     ToType, ToAllocatedTypeSourceInfo, ToSourceRange, ToDirectInitRange) = *Imp;
 
   SmallVector<Expr *, 4> ToPlacementArgs(E->getNumPlacementArgs());
   if (Error Err =
       ImportContainerChecked(E->placement_arguments(), ToPlacementArgs))
     return std::move(Err);
 
-  return new (Importer.getToContext()) CXXNewExpr(
+  return CXXNewExpr::Create(
       Importer.getToContext(), E->isGlobalNew(), ToOperatorNew,
       ToOperatorDelete, E->passAlignment(), E->doesUsualArrayDeleteWantSize(),
       ToPlacementArgs, ToTypeIdParens, ToArraySize, E->getInitializationStyle(),
       ToInitializer, ToType, ToAllocatedTypeSourceInfo, ToSourceRange,
       ToDirectInitRange);
 }
 
 ExpectedStmt ASTNodeImporter::VisitCXXDeleteExpr(CXXDeleteExpr *E) {

lib/AST/ExprCXX.cpp

 }
 
 // CXXScalarValueInitExpr
 SourceLocation CXXScalarValueInitExpr::getBeginLoc() const {
   return TypeInfo ? TypeInfo->getTypeLoc().getBeginLoc() : RParenLoc;
 }
 
 // CXXNewExpr
-CXXNewExpr::CXXNewExpr(const ASTContext &C, bool globalNew,
-                       FunctionDecl *operatorNew, FunctionDecl *operatorDelete,
-                       bool PassAlignment, bool usualArrayDeleteWantsSize,
-                       ArrayRef<Expr*> placementArgs,
-                       SourceRange typeIdParens, Expr *arraySize,
-                       InitializationStyle initializationStyle,
-                       Expr *initializer, QualType ty,
-                       TypeSourceInfo *allocatedTypeInfo,
-                       SourceRange Range, SourceRange directInitRange)
-    : Expr(CXXNewExprClass, ty, VK_RValue, OK_Ordinary, ty->isDependentType(),
-           ty->isDependentType(), ty->isInstantiationDependentType(),
-           ty->containsUnexpandedParameterPack()),
-      OperatorNew(operatorNew), OperatorDelete(operatorDelete),
-      AllocatedTypeInfo(allocatedTypeInfo), TypeIdParens(typeIdParens),
-      Range(Range), DirectInitRange(directInitRange), GlobalNew(globalNew),
-      PassAlignment(PassAlignment),
-      UsualArrayDeleteWantsSize(usualArrayDeleteWantsSize) {
-  assert((initializer != nullptr || initializationStyle == NoInit) &&
-         "Only NoInit can have no initializer.");
-  StoredInitializationStyle = initializer ? initializationStyle + 1 : 0;
-  AllocateArgsArray(C, arraySize != nullptr, placementArgs.size(),
-                    initializer != nullptr);
-  unsigned i = 0;
-  if (Array) {
-    if (arraySize->isInstantiationDependent())
-      ExprBits.InstantiationDependent = true;
+CXXNewExpr::CXXNewExpr(bool IsGlobalNew, FunctionDecl *OperatorNew,
+                       FunctionDecl *OperatorDelete, bool ShouldPassAlignment,
+                       bool UsualArrayDeleteWantsSize,
+                       ArrayRef<Expr *> PlacementArgs, SourceRange TypeIdParens,
+                       Expr *ArraySize, InitializationStyle InitializationStyle,
+                       Expr *Initializer, QualType Ty,
+                       TypeSourceInfo *AllocatedTypeInfo, SourceRange Range,
+                       SourceRange DirectInitRange)
+    : Expr(CXXNewExprClass, Ty, VK_RValue, OK_Ordinary, Ty->isDependentType(),
+           Ty->isDependentType(), Ty->isInstantiationDependentType(),
+           Ty->containsUnexpandedParameterPack()),
+      OperatorNew(OperatorNew), OperatorDelete(OperatorDelete),
+      AllocatedTypeInfo(AllocatedTypeInfo), Range(Range),
+      DirectInitRange(DirectInitRange) {
+
+  assert((Initializer != nullptr || InitializationStyle == NoInit) &&
+         "Only NoInit can have no initializer!");
+
+  CXXNewExprBits.IsGlobalNew = IsGlobalNew;
+  CXXNewExprBits.IsArray = ArraySize != nullptr;
+  CXXNewExprBits.ShouldPassAlignment = ShouldPassAlignment;
+  CXXNewExprBits.UsualArrayDeleteWantsSize = UsualArrayDeleteWantsSize;
+  CXXNewExprBits.StoredInitializationStyle =
+      Initializer ? InitializationStyle + 1 : 0;
+  bool IsParenTypeId = TypeIdParens.isValid();
+  CXXNewExprBits.IsParenTypeId = IsParenTypeId;
+  CXXNewExprBits.NumPlacementArgs = PlacementArgs.size();
 
-    if (arraySize->containsUnexpandedParameterPack())
+  if (ArraySize) {
+    if (ArraySize->isInstantiationDependent())
+      ExprBits.InstantiationDependent = true;
+    if (ArraySize->containsUnexpandedParameterPack())
       ExprBits.ContainsUnexpandedParameterPack = true;
 
-    SubExprs[i++] = arraySize;
+    getTrailingObjects<Stmt *>()[arraySizeOffset()] = ArraySize;
   }
 
-  if (initializer) {
-    if (initializer->isInstantiationDependent())
+  if (Initializer) {
+    if (Initializer->isInstantiationDependent())
       ExprBits.InstantiationDependent = true;
-
-    if (initializer->containsUnexpandedParameterPack())
+    if (Initializer->containsUnexpandedParameterPack())
       ExprBits.ContainsUnexpandedParameterPack = true;
 
-    SubExprs[i++] = initializer;
+    getTrailingObjects<Stmt *>()[initExprOffset()] = Initializer;
   }
 
-  for (unsigned j = 0; j != placementArgs.size(); ++j) {
-    if (placementArgs[j]->isInstantiationDependent())
+  for (unsigned I = 0; I != PlacementArgs.size(); ++I) {
+    if (PlacementArgs[I]->isInstantiationDependent())
       ExprBits.InstantiationDependent = true;
-    if (placementArgs[j]->containsUnexpandedParameterPack())
+    if (PlacementArgs[I]->containsUnexpandedParameterPack())
       ExprBits.ContainsUnexpandedParameterPack = true;
 
-    SubExprs[i++] = placementArgs[j];
+    getTrailingObjects<Stmt *>()[placementNewArgsOffset() + I] =
+        PlacementArgs[I];
   }
 
+  if (IsParenTypeId)
+    getTrailingObjects<SourceRange>()[0] = TypeIdParens;
+
   switch (getInitializationStyle()) {
   case CallInit:
-    this->Range.setEnd(DirectInitRange.getEnd()); break;
+    this->Range.setEnd(DirectInitRange.getEnd());
+    break;
   case ListInit:
-    this->Range.setEnd(getInitializer()->getSourceRange().getEnd()); break;
+    this->Range.setEnd(getInitializer()->getSourceRange().getEnd());
+    break;
   default:
-    if (TypeIdParens.isValid())
+    if (IsParenTypeId)
       this->Range.setEnd(TypeIdParens.getEnd());
     break;
   }
 }
 
-void CXXNewExpr::AllocateArgsArray(const ASTContext &C, bool isArray,
-                                   unsigned numPlaceArgs, bool hasInitializer){
-  assert(SubExprs == nullptr && "SubExprs already allocated");
-  Array = isArray;
-  NumPlacementArgs = numPlaceArgs;
+CXXNewExpr::CXXNewExpr(EmptyShell Empty, bool IsArray,
+                       unsigned NumPlacementArgs, bool IsParenTypeId)
+    : Expr(CXXNewExprClass, Empty) {
+  CXXNewExprBits.IsArray = IsArray;
+  CXXNewExprBits.NumPlacementArgs = NumPlacementArgs;
+  CXXNewExprBits.IsParenTypeId = IsParenTypeId;
+}
+
+CXXNewExpr *
+CXXNewExpr::Create(const ASTContext &Ctx, bool IsGlobalNew,
+                   FunctionDecl *OperatorNew, FunctionDecl *OperatorDelete,
+                   bool ShouldPassAlignment, bool UsualArrayDeleteWantsSize,
+                   ArrayRef<Expr *> PlacementArgs, SourceRange TypeIdParens,
+                   Expr *ArraySize, InitializationStyle InitializationStyle,
+                   Expr *Initializer, QualType Ty,
+                   TypeSourceInfo *AllocatedTypeInfo, SourceRange Range,
+                   SourceRange DirectInitRange) {
+  bool IsArray = ArraySize != nullptr;
+  bool HasInit = Initializer != nullptr;
+  unsigned NumPlacementArgs = PlacementArgs.size();
+  bool IsParenTypeId = TypeIdParens.isValid();
+  void *Mem =
+      Ctx.Allocate(totalSizeToAlloc<Stmt *, SourceRange>(
+                       IsArray + HasInit + NumPlacementArgs, IsParenTypeId),
+                   alignof(CXXNewExpr));
+  return new (Mem)
+      CXXNewExpr(IsGlobalNew, OperatorNew, OperatorDelete, ShouldPassAlignment,
+                 UsualArrayDeleteWantsSize, PlacementArgs, TypeIdParens,
+                 ArraySize, InitializationStyle, Initializer, Ty,
+                 AllocatedTypeInfo, Range, DirectInitRange);
+}
 
-  unsigned TotalSize = Array + hasInitializer + NumPlacementArgs;
-  SubExprs = new (C) Stmt*[TotalSize];
+CXXNewExpr *CXXNewExpr::CreateEmpty(const ASTContext &Ctx, bool IsArray,
+                                    bool HasInit, unsigned NumPlacementArgs,
+                                    bool IsParenTypeId) {
+  void *Mem =
+      Ctx.Allocate(totalSizeToAlloc<Stmt *, SourceRange>(
+                       IsArray + HasInit + NumPlacementArgs, IsParenTypeId),
+                   alignof(CXXNewExpr));
+  return new (Mem)
+      CXXNewExpr(EmptyShell(), IsArray, NumPlacementArgs, IsParenTypeId);
 }
 
-bool CXXNewExpr::shouldNullCheckAllocation(const ASTContext &Ctx) const {
-  return getOperatorNew()->getType()->castAs<FunctionProtoType>()
-                                          ->isNothrow() &&
+bool CXXNewExpr::shouldNullCheckAllocation() const {
+  return getOperatorNew()
+             ->getType()
+             ->castAs<FunctionProtoType>()
          !getOperatorNew()->isReservedGlobalPlacementOperator();
 }
 
 // CXXDeleteExpr
 QualType CXXDeleteExpr::getDestroyedType() const {
   const Expr *Arg = getArgument();
 
   // For a destroying operator delete, we may have implicitly converted the

lib/CodeGen/CGExprCXX.cpp

if (allocator->isReservedGlobalPlacementOperator()) {
 
     allocation = Address(RV.getScalarVal(), allocationAlign);
   }
 
   // Emit a null check on the allocation result if the allocation
   // function is allowed to return null (because it has a non-throwing
   // exception spec or is the reserved placement new) and we have an
   // interesting initializer.
-  bool nullCheck = E->shouldNullCheckAllocation(getContext()) &&
+  bool nullCheck = E->shouldNullCheckAllocation() &&
                    (!allocType.isPODType(getContext()) || E->hasInitializer());
 
   llvm::BasicBlock *nullCheckBB = nullptr;
   llvm::BasicBlock *contBB = nullptr;
 
   // The null-check means that the initializer is conditionally
   // evaluated.
   ConditionalEvaluation conditional(*this);
 

lib/Sema/SemaExprCXX.cpp

if (const RecordType *BaseRecordType = BaseAllocType->getAs<RecordType>()) {
                               PDiag(diag::err_access_dtor)
                                 << BaseAllocType);
         if (DiagnoseUseOfDecl(dtor, StartLoc))
           return ExprError();
       }
     }
   }
 
-  return new (Context)
-      CXXNewExpr(Context, UseGlobal, OperatorNew, OperatorDelete, PassAlignment,
-                 UsualArrayDeleteWantsSize, PlacementArgs, TypeIdParens,
-                 ArraySize, initStyle, Initializer, ResultType, AllocTypeInfo,
-                 Range, DirectInitRange);
+  return CXXNewExpr::Create(Context, UseGlobal, OperatorNew, OperatorDelete,
+                            PassAlignment, UsualArrayDeleteWantsSize,
+                            PlacementArgs, TypeIdParens, ArraySize, initStyle,
+                            Initializer, ResultType, AllocTypeInfo, Range,
+                            DirectInitRange);
 }
 
 /// Checks that a type is suitable as the allocated type
 /// in a new-expression.
 bool Sema::CheckAllocatedType(QualType AllocType, SourceLocation Loc,
                               SourceRange R) {
   // C++ 5.3.4p1: "[The] type shall be a complete object type, but not an
   //   abstract class type or array thereof.

lib/Serialization/ASTReaderStmt.cpp

 void ASTStmtReader::VisitCXXScalarValueInitExpr(CXXScalarValueInitExpr *E) {
   VisitExpr(E);
   E->TypeInfo = GetTypeSourceInfo();
   E->RParenLoc = ReadSourceLocation();
 }
 
 void ASTStmtReader::VisitCXXNewExpr(CXXNewExpr *E) {
   VisitExpr(E);
-  E->GlobalNew = Record.readInt();
-  bool isArray = Record.readInt();
-  E->PassAlignment = Record.readInt();
-  E->UsualArrayDeleteWantsSize = Record.readInt();
+
+  bool IsArray = Record.readInt();
+  bool HasInit = Record.readInt();
   unsigned NumPlacementArgs = Record.readInt();
-  E->StoredInitializationStyle = Record.readInt();
+  bool IsParenTypeId = Record.readInt();
+
+  E->CXXNewExprBits.IsGlobalNew = Record.readInt();
+  E->CXXNewExprBits.ShouldPassAlignment = Record.readInt();
+  E->CXXNewExprBits.UsualArrayDeleteWantsSize = Record.readInt();
+  E->CXXNewExprBits.StoredInitializationStyle = Record.readInt();
+
+  assert((IsArray == E->isArray()) && "Wrong IsArray!");
+  assert((HasInit == E->hasInitializer()) && "Wrong HasInit!");
+  assert((NumPlacementArgs == E->getNumPlacementArgs()) &&
+         "Wrong NumPlacementArgs!");
+  assert((IsParenTypeId == E->isParenTypeId()) && "Wrong IsParenTypeId!");
+  (void)IsArray;
+  (void)HasInit;
+  (void)NumPlacementArgs;
+
   E->setOperatorNew(ReadDeclAs<FunctionDecl>());
   E->setOperatorDelete(ReadDeclAs<FunctionDecl>());
   E->AllocatedTypeInfo = GetTypeSourceInfo();
-  E->TypeIdParens = ReadSourceRange();
+  if (IsParenTypeId)
+    E->getTrailingObjects<SourceRange>()[0] = ReadSourceRange();
   E->Range = ReadSourceRange();
   E->DirectInitRange = ReadSourceRange();
 
-  E->AllocateArgsArray(Record.getContext(), isArray, NumPlacementArgs,
-                       E->StoredInitializationStyle != 0);
-
   // Install all the subexpressions.
-  for (CXXNewExpr::raw_arg_iterator I = E->raw_arg_begin(),e = E->raw_arg_end();
-       I != e; ++I)
+  for (CXXNewExpr::raw_arg_iterator I = E->raw_arg_begin(), N = E->raw_arg_end();
+       I != N; ++I)
     *I = Record.readSubStmt();
 }
 
 void ASTStmtReader::VisitCXXDeleteExpr(CXXDeleteExpr *E) {
   VisitExpr(E);
   E->CXXDeleteExprBits.GlobalDelete = Record.readInt();
   E->CXXDeleteExprBits.ArrayForm = Record.readInt();
   E->CXXDeleteExprBits.ArrayFormAsWritten = Record.readInt();
case EXPR_CXX_BIND_TEMPORARY:
       S = new (Context) CXXBindTemporaryExpr(Empty);
       break;
 
     case EXPR_CXX_SCALAR_VALUE_INIT:
       S = new (Context) CXXScalarValueInitExpr(Empty);
       break;
 
     case EXPR_CXX_NEW:
-      S = new (Context) CXXNewExpr(Empty);
+      S = CXXNewExpr::CreateEmpty(
+          Context,
+          /*IsArray=*/Record[ASTStmtReader::NumExprFields + 0],
+          /*HasInit=*/Record[ASTStmtReader::NumExprFields + 1],
+          /*NumPlacementArgs=*/Record[ASTStmtReader::NumExprFields + 2],
+          /*IsParenTypeId=*/Record[ASTStmtReader::NumExprFields + 3]);
       break;
 
     case EXPR_CXX_DELETE:
       S = new (Context) CXXDeleteExpr(Empty);
       break;
 
     case EXPR_CXX_PSEUDO_DESTRUCTOR:
       S = new (Context) CXXPseudoDestructorExpr(Empty);

lib/Serialization/ASTWriterStmt.cpp

void ASTStmtWriter::VisitCXXScalarValueInitExpr(CXXScalarValueInitExpr *E) {
   VisitExpr(E);
   Record.AddTypeSourceInfo(E->getTypeSourceInfo());
   Record.AddSourceLocation(E->getRParenLoc());
   Code = serialization::EXPR_CXX_SCALAR_VALUE_INIT;
 }
 
 void ASTStmtWriter::VisitCXXNewExpr(CXXNewExpr *E) {
   VisitExpr(E);
-  Record.push_back(E->isGlobalNew());
+
   Record.push_back(E->isArray());
+  Record.push_back(E->hasInitializer());
+  Record.push_back(E->getNumPlacementArgs());
+  Record.push_back(E->isParenTypeId());
+
+  Record.push_back(E->isGlobalNew());
   Record.push_back(E->passAlignment());
   Record.push_back(E->doesUsualArrayDeleteWantSize());
-  Record.push_back(E->getNumPlacementArgs());
-  Record.push_back(E->StoredInitializationStyle);
+  Record.push_back(E->CXXNewExprBits.StoredInitializationStyle);
+
   Record.AddDeclRef(E->getOperatorNew());
   Record.AddDeclRef(E->getOperatorDelete());
   Record.AddTypeSourceInfo(E->getAllocatedTypeSourceInfo());
+  if (E->isParenTypeId())
     Record.AddSourceRange(E->getTypeIdParens());
   Record.AddSourceRange(E->getSourceRange());
   Record.AddSourceRange(E->getDirectInitRange());
-  for (CXXNewExpr::arg_iterator I = E->raw_arg_begin(), e = E->raw_arg_end();
-       I != e; ++I)
+
+  for (CXXNewExpr::arg_iterator I = E->raw_arg_begin(), N = E->raw_arg_end();
+       I != N; ++I)
     Record.AddStmt(*I);
 
   Code = serialization::EXPR_CXX_NEW;
 }
 
 void ASTStmtWriter::VisitCXXDeleteExpr(CXXDeleteExpr *E) {
   VisitExpr(E);
   Record.push_back(E->isGlobalDelete());