[AST] Only store the needed data in IfStmt.

[riccibruno]

Summary

Only store the needed data in IfStmt. This cuts the size of IfStmt
by up to 3 pointers + 1 SourceLocation. The order of the children
is intentionally kept the same even though it would be more
convenient to put the optional trailing objects last.

[riccibruno]

To add on the issue of changing the order of the children. The more I think about
it the less it seems to be a good idea. Every test passes but I strongly suspect
that a lot of code subtly rely on this.

[rjmccall]

Yeah, I agree that changing child order is problematic.

[riccibruno]

Reworked so that the order of the children is kept the same.

[rjmccall]

LGTM.

[riccibruno]

Add a flag to the output of -ast-dump indicating
which sub-statement IfStmt is storing. This removes
the ambiguity in the output of -ast-dump and addresses
rsmith's comment in D53717.

include/clang/AST/Stmt.h

class AttributedStmtBitfields {
     /// Number of attributes.
     unsigned NumAttrs : 32 - NumStmtBits;
 
     /// The location of the attribute.
     SourceLocation AttrLoc;
   };
 
   class IfStmtBitfields {
+    friend class ASTStmtReader;
     friend class IfStmt;
 
     unsigned : NumStmtBits;
 
+    /// True if this if statement is a constexpr if.
     unsigned IsConstexpr : 1;
+
+    /// True if this if statement has storage for an else statement.
+    unsigned HasElse : 1;
+
+    /// True if this if statement has storage for a variable declaration.
+    unsigned HasVar : 1;
+
+    /// True if this if statement has storage for an init statement.
+    unsigned HasInit : 1;
+
+    /// The location of the "if".
+    SourceLocation IfLoc;
   };
 
   class SwitchStmtBitfields {
     friend class SwitchStmt;
 
     unsigned : NumStmtBits;
 
     /// The location of the "switch".
public:
   child_range children() { return child_range(&SubStmt, &SubStmt + 1); }
 
   static bool classof(const Stmt *T) {
     return T->getStmtClass() == AttributedStmtClass;
   }
 };
 
 /// IfStmt - This represents an if/then/else.
-class IfStmt : public Stmt {
-  enum { INIT, VAR, COND, THEN, ELSE, END_EXPR };
-  Stmt* SubExprs[END_EXPR];
+class IfStmt final
+    : public Stmt,
+      private llvm::TrailingObjects<IfStmt, Stmt *, SourceLocation> {
+  friend TrailingObjects;
 
-  SourceLocation IfLoc;
-  SourceLocation ElseLoc;
+  // IfStmt is followed by several trailing objects, some of which optional.
+  // Note that it would be more convenient to put the optional trailing
+  // objects at then end but this would change the order of the children.
+  // The trailing objects are in order:
+  //
+  // * A "Stmt *" for the init statement.
+  //    Present if and only if hasInitStorage().
+  //
+  // * A "Stmt *" for the condition variable.
+  //    Present if and only if hasVarStorage(). This is in fact a "DeclStmt *".
+  //
+  // * A "Stmt *" for the condition.
+  //    Always present. This is in fact a "Expr *".
+  //
+  // * A "Stmt *" for the then statement.
+  //    Always present.
+  //
+  // * A "Stmt *" for the else statement.
+  //    Present if and only if hasElseStorage().
+  //
+  // * A "SourceLocation" for the location of the "else".
+  //    Present if and only if hasElseStorage().
+  enum { InitOffset = 0, ThenOffsetFromCond = 1, ElseOffsetFromCond = 2 };
+  enum { NumMandatoryStmtPtr = 2 };
+
+  unsigned numTrailingObjects(OverloadToken<Stmt *>) const {
+    return NumMandatoryStmtPtr + hasElseStorage() + hasVarStorage() +
+           hasInitStorage();
+  }
+
+  unsigned numTrailingObjects(OverloadToken<SourceLocation>) const {
+    return hasElseStorage();
+  }
+
+  unsigned initOffset() const { return InitOffset; }
+  unsigned varOffset() const { return InitOffset + hasInitStorage(); }
+  unsigned condOffset() const {
+    return InitOffset + hasInitStorage() + hasVarStorage();
+  }
+  unsigned thenOffset() const { return condOffset() + ThenOffsetFromCond; }
+  unsigned elseOffset() const { return condOffset() + ElseOffsetFromCond; }
+
+  /// Build an if/then/else statement.
+  IfStmt(const ASTContext &Ctx, SourceLocation IL, bool IsConstexpr, Stmt *Init,
+         VarDecl *Var, Expr *Cond, Stmt *Then, SourceLocation EL, Stmt *Else);
+
+  /// Build an empty if/then/else statement.
+  explicit IfStmt(EmptyShell Empty, bool HasElse, bool HasVar, bool HasInit);
 
 public:
-  IfStmt(const ASTContext &C, SourceLocation IL,
-         bool IsConstexpr, Stmt *init, VarDecl *var, Expr *cond,
-         Stmt *then, SourceLocation EL = SourceLocation(),
-         Stmt *elsev = nullptr);
+  /// Create an IfStmt.
+  static IfStmt *Create(const ASTContext &Ctx, SourceLocation IL,
+                        bool IsConstexpr, Stmt *Init, VarDecl *Var, Expr *Cond,
+                        Stmt *Then, SourceLocation EL = SourceLocation(),
+                        Stmt *Else = nullptr);
+
+  /// Create an empty IfStmt optionally with storage for an else statement,
+  /// condition variable and init expression.
+  static IfStmt *CreateEmpty(const ASTContext &Ctx, bool HasElse, bool HasVar,
+                             bool HasInit);
+
+  /// True if this IfStmt has the storage for an init statement.
+  bool hasInitStorage() const { return IfStmtBits.HasInit; }
 
-  /// Build an empty if/then/else statement
-  explicit IfStmt(EmptyShell Empty) : Stmt(IfStmtClass, Empty) {}
+  /// True if this IfStmt has storage for a variable declaration.
+  bool hasVarStorage() const { return IfStmtBits.HasVar; }
+
+  /// True if this IfStmt has storage for an else statement.
+  bool hasElseStorage() const { return IfStmtBits.HasElse; }
+
+  Expr *getCond() {
+    return reinterpret_cast<Expr *>(getTrailingObjects<Stmt *>()[condOffset()]);
+  }
+
+  const Expr *getCond() const {
+    return reinterpret_cast<Expr *>(getTrailingObjects<Stmt *>()[condOffset()]);
+  }
+
+  void setCond(Expr *Cond) {
+    getTrailingObjects<Stmt *>()[condOffset()] = reinterpret_cast<Stmt *>(Cond);
+  }
+
+  Stmt *getThen() { return getTrailingObjects<Stmt *>()[thenOffset()]; }
+  const Stmt *getThen() const {
+    return getTrailingObjects<Stmt *>()[thenOffset()];
+  }
+
+  void setThen(Stmt *Then) {
+    getTrailingObjects<Stmt *>()[thenOffset()] = Then;
+  }
+
+  Stmt *getElse() {
+    return hasElseStorage() ? getTrailingObjects<Stmt *>()[elseOffset()]
+                            : nullptr;
+  }
+
+  const Stmt *getElse() const {
+    return hasElseStorage() ? getTrailingObjects<Stmt *>()[elseOffset()]
+                            : nullptr;
+  }
+
+  void setElse(Stmt *Else) {
+    assert(hasElseStorage() &&
+           "This if statement has no storage for an else statement!");
+    getTrailingObjects<Stmt *>()[elseOffset()] = Else;
+  }
 
   /// Retrieve the variable declared in this "if" statement, if any.
   ///
   /// In the following example, "x" is the condition variable.
   /// \code
   /// if (int x = foo()) {
   ///   printf("x is %d", x);
   /// }
   /// \endcode
-  VarDecl *getConditionVariable() const;
-  void setConditionVariable(const ASTContext &C, VarDecl *V);
+  VarDecl *getConditionVariable();
+  const VarDecl *getConditionVariable() const {
+    return const_cast<IfStmt *>(this)->getConditionVariable();
+  }
+
+  /// Set the condition variable for this if statement.
+  /// The if statement must have storage for the condition variable.
+  void setConditionVariable(const ASTContext &Ctx, VarDecl *V);
 
   /// If this IfStmt has a condition variable, return the faux DeclStmt
   /// associated with the creation of that condition variable.
+  DeclStmt *getConditionVariableDeclStmt() {
+    return hasVarStorage() ? static_cast<DeclStmt *>(
+                                 getTrailingObjects<Stmt *>()[varOffset()])
+                           : nullptr;
+  }
+
   const DeclStmt *getConditionVariableDeclStmt() const {
-    return reinterpret_cast<DeclStmt*>(SubExprs[VAR]);
+    return hasVarStorage() ? static_cast<DeclStmt *>(
+                                 getTrailingObjects<Stmt *>()[varOffset()])
+                           : nullptr;
   }
 
-  Stmt *getInit() { return SubExprs[INIT]; }
-  const Stmt *getInit() const { return SubExprs[INIT]; }
-  void setInit(Stmt *S) { SubExprs[INIT] = S; }
-  const Expr *getCond() const { return reinterpret_cast<Expr*>(SubExprs[COND]);}
-  void setCond(Expr *E) { SubExprs[COND] = reinterpret_cast<Stmt *>(E); }
-  const Stmt *getThen() const { return SubExprs[THEN]; }
-  void setThen(Stmt *S) { SubExprs[THEN] = S; }
-  const Stmt *getElse() const { return SubExprs[ELSE]; }
-  void setElse(Stmt *S) { SubExprs[ELSE] = S; }
+  Stmt *getInit() {
+    return hasInitStorage() ? getTrailingObjects<Stmt *>()[initOffset()]
+                            : nullptr;
+  }
 
-  Expr *getCond() { return reinterpret_cast<Expr*>(SubExprs[COND]); }
-  Stmt *getThen() { return SubExprs[THEN]; }
-  Stmt *getElse() { return SubExprs[ELSE]; }
+  const Stmt *getInit() const {
+    return hasInitStorage() ? getTrailingObjects<Stmt *>()[initOffset()]
+                            : nullptr;
+  }
 
-  SourceLocation getIfLoc() const { return IfLoc; }
-  void setIfLoc(SourceLocation L) { IfLoc = L; }
-  SourceLocation getElseLoc() const { return ElseLoc; }
-  void setElseLoc(SourceLocation L) { ElseLoc = L; }
+  void setInit(Stmt *Init) {
+    assert(hasInitStorage() &&
+           "This if statement has no storage for an init statement!");
+    getTrailingObjects<Stmt *>()[initOffset()] = Init;
+  }
+
+  SourceLocation getIfLoc() const { return IfStmtBits.IfLoc; }
+  void setIfLoc(SourceLocation IfLoc) { IfStmtBits.IfLoc = IfLoc; }
+
+  SourceLocation getElseLoc() const {
+    return hasElseStorage() ? *getTrailingObjects<SourceLocation>()
+                            : SourceLocation();
+  }
+
+  void setElseLoc(SourceLocation ElseLoc) {
+    assert(hasElseStorage() &&
+           "This if statement has no storage for an else statement!");
+    *getTrailingObjects<SourceLocation>() = ElseLoc;
+  }
 
   bool isConstexpr() const { return IfStmtBits.IsConstexpr; }
   void setConstexpr(bool C) { IfStmtBits.IsConstexpr = C; }
 
   bool isObjCAvailabilityCheck() const;
 
-  SourceLocation getBeginLoc() const LLVM_READONLY { return IfLoc; }
-
+  SourceLocation getBeginLoc() const { return getIfLoc(); }
   SourceLocation getEndLoc() const LLVM_READONLY {
-    if (SubExprs[ELSE])
-      return SubExprs[ELSE]->getEndLoc();
-    else
-      return SubExprs[THEN]->getEndLoc();
+    if (getElse())
+      return getElse()->getEndLoc();
+    return getThen()->getEndLoc();
   }
 
   // Iterators over subexpressions.  The iterators will include iterating
   // over the initialization expression referenced by the condition variable.
   child_range children() {
-    return child_range(&SubExprs[0], &SubExprs[0]+END_EXPR);
+    return child_range(getTrailingObjects<Stmt *>(),
+                       getTrailingObjects<Stmt *>() +
+                           numTrailingObjects(OverloadToken<Stmt *>()));
   }
 
   static bool classof(const Stmt *T) {
     return T->getStmtClass() == IfStmtClass;
   }
 };
 
 /// SwitchStmt - This represents a 'switch' stmt.

lib/AST/ASTDumper.cpp

public:
     void VisitObjCPropertyDecl(const ObjCPropertyDecl *D);
     void VisitObjCPropertyImplDecl(const ObjCPropertyImplDecl *D);
     void VisitBlockDecl(const BlockDecl *D);
 
     // Stmts.
     void VisitStmt(const Stmt *Node);
     void VisitDeclStmt(const DeclStmt *Node);
     void VisitAttributedStmt(const AttributedStmt *Node);
+    void VisitIfStmt(const IfStmt *Node);
     void VisitLabelStmt(const LabelStmt *Node);
     void VisitGotoStmt(const GotoStmt *Node);
     void VisitCXXCatchStmt(const CXXCatchStmt *Node);
     void VisitCapturedStmt(const CapturedStmt *Node);
 
     // OpenMP
     void VisitOMPExecutableDirective(const OMPExecutableDirective *Node);
 

 void ASTDumper::VisitAttributedStmt(const AttributedStmt *Node) {
   VisitStmt(Node);
   for (ArrayRef<const Attr *>::iterator I = Node->getAttrs().begin(),
                                         E = Node->getAttrs().end();
        I != E; ++I)
     dumpAttr(*I);
 }
 
+void ASTDumper::VisitIfStmt(const IfStmt *Node) {
+  VisitStmt(Node);
+  if (Node->hasInitStorage())
+    OS << " has_init";
+  if (Node->hasVarStorage())
+    OS << " has_var";
+  if (Node->hasElseStorage())
+    OS << " has_else";
+}
+
 void ASTDumper::VisitLabelStmt(const LabelStmt *Node) {
   VisitStmt(Node);
   OS << " '" << Node->getName() << "'";
 }
 
 void ASTDumper::VisitGotoStmt(const GotoStmt *Node) {
   VisitStmt(Node);
   OS << " '" << Node->getLabel()->getName() << "'";

lib/AST/ASTImporter.cpp

ExpectedStmt ASTNodeImporter::VisitIfStmt(IfStmt *S) {
   SourceLocation ToIfLoc, ToElseLoc;
   Stmt *ToInit, *ToThen, *ToElse;
   VarDecl *ToConditionVariable;
   Expr *ToCond;
   std::tie(
       ToIfLoc, ToInit, ToConditionVariable, ToCond, ToThen, ToElseLoc, ToElse) =
           *Imp;
 
-  return new (Importer.getToContext()) IfStmt(
-      Importer.getToContext(),
-      ToIfLoc, S->isConstexpr(), ToInit, ToConditionVariable, ToCond,
-      ToThen, ToElseLoc, ToElse);
+  return IfStmt::Create(Importer.getToContext(), ToIfLoc, S->isConstexpr(),
+                        ToInit, ToConditionVariable, ToCond, ToThen, ToElseLoc,
+                        ToElse);
 }
 
 ExpectedStmt ASTNodeImporter::VisitSwitchStmt(SwitchStmt *S) {
   auto Imp = importSeq(
       S->getInit(), S->getConditionVariable(), S->getCond(),
       S->getBody(), S->getSwitchLoc());
   if (!Imp)
     return Imp.takeError();

lib/AST/Stmt.cpp

void MSAsmStmt::initialize(const ASTContext &C, StringRef asmstr,
   Clobbers = new (C) StringRef[NumClobbers];
   // FIXME: Avoid the allocation/copy if at all possible.
   std::transform(clobbers.begin(), clobbers.end(), Clobbers,
                  [&](StringRef Clobber) {
                    return copyIntoContext(C, Clobber);
                  });
 }
 
-IfStmt::IfStmt(const ASTContext &C, SourceLocation IL, bool IsConstexpr,
-               Stmt *init, VarDecl *var, Expr *cond, Stmt *then,
-               SourceLocation EL, Stmt *elsev)
-    : Stmt(IfStmtClass), IfLoc(IL), ElseLoc(EL) {
+IfStmt::IfStmt(const ASTContext &Ctx, SourceLocation IL, bool IsConstexpr,
+               Stmt *Init, VarDecl *Var, Expr *Cond, Stmt *Then,
+               SourceLocation EL, Stmt *Else)
+    : Stmt(IfStmtClass) {
+  bool HasElse = Else != nullptr;
+  bool HasVar = Var != nullptr;
+  bool HasInit = Init != nullptr;
+  IfStmtBits.HasElse = HasElse;
+  IfStmtBits.HasVar = HasVar;
+  IfStmtBits.HasInit = HasInit;
+
   setConstexpr(IsConstexpr);
-  setConditionVariable(C, var);
-  SubExprs[INIT] = init;
-  SubExprs[COND] = cond;
-  SubExprs[THEN] = then;
-  SubExprs[ELSE] = elsev;
-}
 
-VarDecl *IfStmt::getConditionVariable() const {
-  if (!SubExprs[VAR])
+  setCond(Cond);
+  setThen(Then);
+  if (HasElse)
+    setElse(Else);
+  if (HasVar)
+    setConditionVariable(Ctx, Var);
+  if (HasInit)
+    setInit(Init);
+
+  setIfLoc(IL);
+  if (HasElse)
+    setElseLoc(EL);
+}
+
+IfStmt::IfStmt(EmptyShell Empty, bool HasElse, bool HasVar, bool HasInit)
+    : Stmt(IfStmtClass, Empty) {
+  IfStmtBits.HasElse = HasElse;
+  IfStmtBits.HasVar = HasVar;
+  IfStmtBits.HasInit = HasInit;
+}
+
+IfStmt *IfStmt::Create(const ASTContext &Ctx, SourceLocation IL,
+                       bool IsConstexpr, Stmt *Init, VarDecl *Var, Expr *Cond,
+                       Stmt *Then, SourceLocation EL, Stmt *Else) {
+  bool HasElse = Else != nullptr;
+  bool HasVar = Var != nullptr;
+  bool HasInit = Init != nullptr;
+  void *Mem = Ctx.Allocate(
+      totalSizeToAlloc<Stmt *, SourceLocation>(
+          NumMandatoryStmtPtr + HasElse + HasVar + HasInit, HasElse),
+      alignof(IfStmt));
+  return new (Mem)
+      IfStmt(Ctx, IL, IsConstexpr, Init, Var, Cond, Then, EL, Else);
+}
+
+IfStmt *IfStmt::CreateEmpty(const ASTContext &Ctx, bool HasElse, bool HasVar,
+                            bool HasInit) {
+  void *Mem = Ctx.Allocate(
+      totalSizeToAlloc<Stmt *, SourceLocation>(
+          NumMandatoryStmtPtr + HasElse + HasVar + HasInit, HasElse),
+      alignof(IfStmt));
+  return new (Mem) IfStmt(EmptyShell(), HasElse, HasVar, HasInit);
+}
+
+VarDecl *IfStmt::getConditionVariable() {
+  auto *DS = getConditionVariableDeclStmt();
+  if (!DS)
     return nullptr;
-
-  auto *DS = cast<DeclStmt>(SubExprs[VAR]);
   return cast<VarDecl>(DS->getSingleDecl());
 }
 
-void IfStmt::setConditionVariable(const ASTContext &C, VarDecl *V) {
+void IfStmt::setConditionVariable(const ASTContext &Ctx, VarDecl *V) {
+  assert(hasVarStorage() &&
+         "This if statement has no storage for a condition variable!");
+
   if (!V) {
-    SubExprs[VAR] = nullptr;
+    getTrailingObjects<Stmt *>()[varOffset()] = nullptr;
     return;
   }
 
   SourceRange VarRange = V->getSourceRange();
-  SubExprs[VAR] = new (C) DeclStmt(DeclGroupRef(V), VarRange.getBegin(),
-                                   VarRange.getEnd());
+  getTrailingObjects<Stmt *>()[varOffset()] = new (Ctx)
+      DeclStmt(DeclGroupRef(V), VarRange.getBegin(), VarRange.getEnd());
 }
 
 bool IfStmt::isObjCAvailabilityCheck() const {
-  return isa<ObjCAvailabilityCheckExpr>(SubExprs[COND]);
+  return isa<ObjCAvailabilityCheckExpr>(getCond());
 }
 
 ForStmt::ForStmt(const ASTContext &C, Stmt *Init, Expr *Cond, VarDecl *condVar,
                  Expr *Inc, Stmt *Body, SourceLocation FL, SourceLocation LP,
                  SourceLocation RP)
   : Stmt(ForStmtClass), LParenLoc(LP), RParenLoc(RP)
 {
   SubExprs[INIT] = Init;

lib/Analysis/BodyFarm.cpp

UnaryOperator *FlagCheck = new (C) UnaryOperator(
       /* ExprObjectKind=*/ OK_Ordinary, SourceLocation(),
       /* CanOverflow*/ false);
 
   // Create assignment.
   BinaryOperator *FlagAssignment = M.makeAssignment(
       Deref, M.makeIntegralCast(M.makeIntegerLiteral(1, C.IntTy), DerefType),
       DerefType);
 
-  IfStmt *Out = new (C)
-      IfStmt(C, SourceLocation(),
-             /* IsConstexpr=*/ false,
-             /* init=*/ nullptr,
-             /* var=*/ nullptr,
-             /* cond=*/ FlagCheck,
-             /* then=*/ M.makeCompound({CallbackCall, FlagAssignment}));
+  auto *Out =
+      IfStmt::Create(C, SourceLocation(),
+                     /* IsConstexpr=*/false,
+                     /* init=*/nullptr,
+                     /* var=*/nullptr,
+                     /* cond=*/FlagCheck,
+                     /* then=*/M.makeCompound({CallbackCall, FlagAssignment}));
 
   return Out;
 }
 
 /// Create a fake body for dispatch_once.
 static Stmt *create_dispatch_once(ASTContext &C, const FunctionDecl *D) {
   // Check if we have at least two parameters.
   if (D->param_size() != 2)
M.makeLvalueToRvalue(
         M.makeLvalueToRvalue(
           M.makeDeclRefExpr(Predicate),
           PredicateQPtrTy),
         PredicateTy),
     PredicateTy);
 
   Expr *GuardCondition = M.makeComparison(LValToRval, DoneValue, BO_NE);
   // (5) Create the 'if' statement.
-  IfStmt *If = new (C) IfStmt(C, SourceLocation(),
-                              /* IsConstexpr=*/ false,
-                              /* init=*/ nullptr,
-                              /* var=*/ nullptr,
-                              /* cond=*/ GuardCondition,
-                              /* then=*/ CS);
+  auto *If = IfStmt::Create(C, SourceLocation(),
+                            /* IsConstexpr=*/false,
+                            /* init=*/nullptr,
+                            /* var=*/nullptr,
+                            /* cond=*/GuardCondition,
+                            /* then=*/CS);
   return If;
 }
 
 /// Create a fake body for dispatch_sync.
 static Stmt *create_dispatch_sync(ASTContext &C, const FunctionDecl *D) {
   // Check if we have at least two parameters.
   if (D->param_size() != 2)
     return nullptr;
static Stmt *create_OSAtomicCompareAndSwap(ASTContext &C, const FunctionDecl *D)
 
   // Construct the else clause.
   BoolVal = M.makeObjCBool(false);
   RetVal = isBoolean ? M.makeIntegralCastToBoolean(BoolVal)
                      : M.makeIntegralCast(BoolVal, ResultTy);
   Stmt *Else = M.makeReturn(RetVal);
 
   /// Construct the If.
-  Stmt *If = new (C) IfStmt(C, SourceLocation(), false, nullptr, nullptr,
-                            Comparison, Body, SourceLocation(), Else);
+  auto *If = IfStmt::Create(C, SourceLocation(),
+                            /* IsConstexpr=*/false,
+                            /* init=*/nullptr,
+                            /* var=*/nullptr, Comparison, Body,
+                            SourceLocation(), Else);
 
   return If;
 }
 
 Stmt *BodyFarm::getBody(const FunctionDecl *D) {
   D = D->getCanonicalDecl();
 
   Optional<Stmt *> &Val = Bodies[D];

lib/Sema/SemaStmt.cpp

if (Cond.isInvalid())
     return StmtError();
 
   if (IsConstexpr || isa<ObjCAvailabilityCheckExpr>(Cond.get().second))
     setFunctionHasBranchProtectedScope();
 
   DiagnoseUnusedExprResult(thenStmt);
   DiagnoseUnusedExprResult(elseStmt);
 
-  return new (Context)
-      IfStmt(Context, IfLoc, IsConstexpr, InitStmt, Cond.get().first,
+  return IfStmt::Create(Context, IfLoc, IsConstexpr, InitStmt, Cond.get().first,
                         Cond.get().second, thenStmt, ElseLoc, elseStmt);
 }
 
 namespace {
   struct CaseCompareFunctor {
     bool operator()(const std::pair<llvm::APSInt, CaseStmt*> &LHS,
                     const llvm::APSInt &RHS) {
       return LHS.first < RHS;
     }

lib/Serialization/ASTReaderStmt.cpp

void ASTStmtReader::VisitAttributedStmt(AttributedStmt *S) {
   assert(NumAttrs == Attrs.size());
   std::copy(Attrs.begin(), Attrs.end(), S->getAttrArrayPtr());
   S->SubStmt = Record.readSubStmt();
   S->AttributedStmtBits.AttrLoc = ReadSourceLocation();
 }
 
 void ASTStmtReader::VisitIfStmt(IfStmt *S) {
   VisitStmt(S);
+
   S->setConstexpr(Record.readInt());
-  S->setInit(Record.readSubStmt());
-  S->setConditionVariable(Record.getContext(), ReadDeclAs<VarDecl>());
+  bool HasElse = Record.readInt();
+  bool HasVar = Record.readInt();
+  bool HasInit = Record.readInt();
+
   S->setCond(Record.readSubExpr());
   S->setThen(Record.readSubStmt());
+  if (HasElse)
     S->setElse(Record.readSubStmt());
+  if (HasVar)
+    S->setConditionVariable(Record.getContext(), ReadDeclAs<VarDecl>());
+  if (HasInit)
+    S->setInit(Record.readSubStmt());
+
   S->setIfLoc(ReadSourceLocation());
+  if (HasElse)
     S->setElseLoc(ReadSourceLocation());
 }
 
 void ASTStmtReader::VisitSwitchStmt(SwitchStmt *S) {
   VisitStmt(S);
   S->setInit(Record.readSubStmt());
   S->setConditionVariable(Record.getContext(), ReadDeclAs<VarDecl>());
   S->setCond(Record.readSubExpr());
   S->setBody(Record.readSubStmt());
while (true) {
 
     case STMT_ATTRIBUTED:
       S = AttributedStmt::CreateEmpty(
         Context,
         /*NumAttrs*/Record[ASTStmtReader::NumStmtFields]);
       break;
 
     case STMT_IF:
-      S = new (Context) IfStmt(Empty);
+      S = IfStmt::CreateEmpty(
+          Context,
+          /* HasElse=*/Record[ASTStmtReader::NumStmtFields + 1],
+          /* HasVar=*/Record[ASTStmtReader::NumStmtFields + 2],
+          /* HasInit=*/Record[ASTStmtReader::NumStmtFields + 3]);
       break;
 
     case STMT_SWITCH:
       S = new (Context) SwitchStmt(Empty);
       break;
 
     case STMT_WHILE:
       S = new (Context) WhileStmt(Empty);

lib/Serialization/ASTWriterStmt.cpp

void ASTStmtWriter::VisitAttributedStmt(AttributedStmt *S) {
   Record.AddAttributes(S->getAttrs());
   Record.AddStmt(S->getSubStmt());
   Record.AddSourceLocation(S->getAttrLoc());
   Code = serialization::STMT_ATTRIBUTED;
 }
 
 void ASTStmtWriter::VisitIfStmt(IfStmt *S) {
   VisitStmt(S);
+
+  bool HasElse = S->getElse() != nullptr;
+  bool HasVar = S->getConditionVariableDeclStmt() != nullptr;
+  bool HasInit = S->getInit() != nullptr;
+
   Record.push_back(S->isConstexpr());
-  Record.AddStmt(S->getInit());
-  Record.AddDeclRef(S->getConditionVariable());
+  Record.push_back(HasElse);
+  Record.push_back(HasVar);
+  Record.push_back(HasInit);
+
   Record.AddStmt(S->getCond());
   Record.AddStmt(S->getThen());
+  if (HasElse)
     Record.AddStmt(S->getElse());
+  if (HasVar)
+    Record.AddDeclRef(S->getConditionVariable());
+  if (HasInit)
+    Record.AddStmt(S->getInit());
+
   Record.AddSourceLocation(S->getIfLoc());
+  if (HasElse)
     Record.AddSourceLocation(S->getElseLoc());
+
   Code = serialization::STMT_IF;
 }
 
 void ASTStmtWriter::VisitSwitchStmt(SwitchStmt *S) {
   VisitStmt(S);
   Record.AddStmt(S->getInit());
   Record.AddDeclRef(S->getConditionVariable());
   Record.AddStmt(S->getCond());

test/Import/if-stmt/test.cpp

 // RUN: clang-import-test -dump-ast -import %S/Inputs/F.cpp -expression %s | FileCheck %s
 
 // CHECK: IfStmt
-// CHECK-NEXT: <<NULL>>
-// CHECK-NEXT: <<NULL>>
 // CHECK-NEXT: CXXBoolLiteralExpr
 // CHECK-NEXT: ReturnStmt
-// CHECK-NEXT: <<NULL>>
 
 // CHECK: IfStmt
-// CHECK-NEXT: <<NULL>>
 // CHECK-NEXT: DeclStmt
 // CHECK-NEXT: VarDecl
 // CHECK-NEXT: IntegerLiteral
 // CHECK-NEXT: ImplicitCastExpr
 // CHECK-NEXT: ImplicitCastExpr
 // CHECK-NEXT: DeclRefExpr
 // CHECK-NEXT: ReturnStmt
-// CHECK-NEXT: <<NULL>>
 
 // CHECK: IfStmt
 // CHECK-NEXT: DeclStmt
 // CHECK-NEXT: VarDecl
-// CHECK-NEXT: <<NULL>>
 // CHECK-NEXT: CXXBoolLiteralExpr
 // CHECK-NEXT: ReturnStmt
-// CHECK-NEXT: <<NULL>>
 
 // CHECK: IfStmt
-// CHECK-NEXT: <<NULL>>
-// CHECK-NEXT: <<NULL>>
 // CHECK-NEXT: CXXBoolLiteralExpr
 // CHECK-NEXT: ReturnStmt
 // CHECK-NEXT: ReturnStmt
 
 // CHECK: IfStmt
-// CHECK-NEXT: <<NULL>>
-// CHECK-NEXT: <<NULL>>
 // CHECK-NEXT: CXXBoolLiteralExpr
 // CHECK-NEXT: CompoundStmt
 // CHECK-NEXT: ReturnStmt
 // CHECK-NEXT: CompoundStmt
 // CHECK-NEXT: ReturnStmt
 
 void expr() {
   f();
 }

test/Misc/ast-dump-invalid.cpp

else
     return i;
 }
 }
 // CHECK: NamespaceDecl {{.*}} <{{.*}}> {{.*}} TestInvalidIf
 // CHECK-NEXT: `-FunctionDecl
 // CHECK-NEXT:   |-ParmVarDecl
 // CHECK-NEXT:   `-CompoundStmt
 // CHECK-NEXT:     `-IfStmt {{.*}} <line:25:3, line:28:12>
-// CHECK-NEXT:       |-<<<NULL>>>
-// CHECK-NEXT:       |-<<<NULL>>>
 // CHECK-NEXT:       |-OpaqueValueExpr {{.*}} <<invalid sloc>> 'bool'
 // CHECK-NEXT:       |-ReturnStmt {{.*}} <line:26:5, col:12>
 // CHECK-NEXT:       | `-IntegerLiteral {{.*}} <col:12> 'int' 4
 // CHECK-NEXT:       `-ReturnStmt {{.*}} <line:28:5, col:12>
 // CHECK-NEXT:         `-ImplicitCastExpr {{.*}} <col:12> 'int' <LValueToRValue>
 // CHECK-NEXT:           `-DeclRefExpr {{.*}} <col:12> 'int' lvalue ParmVar {{.*}} 'i' 'int'
 
 namespace TestInvalidFunctionDecl {
 struct Str {
    double foo1(double, invalid_type);
 };
 double Str::foo1(double, invalid_type)
 { return 45; }
 }
 // CHECK: NamespaceDecl {{.*}} <{{.*}}> {{.*}} TestInvalidFunctionDecl
-// CHECK-NEXT: |-CXXRecordDecl {{.*}} <line:46:1, line:48:1> line:46:8 struct Str definition
+// CHECK-NEXT: |-CXXRecordDecl {{.*}} <line:44:1, line:46:1> line:44:8 struct Str definition
 // CHECK:      | |-CXXRecordDecl {{.*}} <col:1, col:8> col:8 implicit struct Str
-// CHECK-NEXT: | `-CXXMethodDecl {{.*}} <line:47:4, col:36> col:11 invalid foo1 'double (double, int)'
+// CHECK-NEXT: | `-CXXMethodDecl {{.*}} <line:45:4, col:36> col:11 invalid foo1 'double (double, int)'
 // CHECK-NEXT: |   |-ParmVarDecl {{.*}} <col:16> col:22 'double'
 // CHECK-NEXT: |   `-ParmVarDecl {{.*}} <col:24, <invalid sloc>> col:36 invalid 'int'
-// CHECK-NEXT: `-CXXMethodDecl {{.*}} parent {{.*}} <line:49:1, line:50:14> line:49:13 invalid foo1 'double (double, int)'
+// CHECK-NEXT: `-CXXMethodDecl {{.*}} parent {{.*}} <line:47:1, line:48:14> line:47:13 invalid foo1 'double (double, int)'
 // CHECK-NEXT:   |-ParmVarDecl {{.*}} <col:18> col:24 'double'
 // CHECK-NEXT:   |-ParmVarDecl {{.*}} <col:26, <invalid sloc>> col:38 invalid 'int'
-// CHECK-NEXT:   `-CompoundStmt {{.*}} <line:50:1, col:14>
+// CHECK-NEXT:   `-CompoundStmt {{.*}} <line:48:1, col:14>
 // CHECK-NEXT:     `-ReturnStmt {{.*}} <col:3, col:10>
 // CHECK-NEXT:       `-ImplicitCastExpr {{.*}} <col:10> 'double' <IntegralToFloating>
 // CHECK-NEXT:         `-IntegerLiteral {{.*}} <col:10> 'int' 45