[AST] Only store data for the NRVO candidate in ReturnStmt if needed

Only store the NRVO candidate if needed in ReturnStmt.
A good chuck of all of the ReturnStmt have no NRVO candidate
(more than half when parsing all of Boost). For all of them
this saves one pointer. This has no impact on children().

Differential Revision: https://reviews.llvm.org/D53716

Reviewed By: rsmith

llvm-svn: 345605

clang/include/clang/AST/Stmt.h

@@ -256,6 +256,9 @@ protected:
 
     unsigned : NumStmtBits;
 
+    /// True if this ReturnStmt has storage for an NRVO candidate.
+    unsigned HasNRVOCandidate : 1;
+
     /// The location of the "return".
     SourceLocation RetLoc;
   };
@@ -1999,40 +2002,67 @@ public:
 /// return a value, and it allows returning a value in functions declared to
 /// return void.  We explicitly model this in the AST, which means you can't
 /// depend on the return type of the function and the presence of an argument.
-class ReturnStmt : public Stmt {
+class ReturnStmt final
+    : public Stmt,
+      private llvm::TrailingObjects<ReturnStmt, const VarDecl *> {
+  friend TrailingObjects;
+
+  /// The return expression.
   Stmt *RetExpr;
-  const VarDecl *NRVOCandidate;
 
-public:
-  explicit ReturnStmt(SourceLocation RL) : ReturnStmt(RL, nullptr, nullptr) {}
+  // ReturnStmt is followed optionally by a trailing "const VarDecl *"
+  // for the NRVO candidate. Present if and only if hasNRVOCandidate().
 
-  ReturnStmt(SourceLocation RL, Expr *E, const VarDecl *NRVOCandidate)
-      : Stmt(ReturnStmtClass), RetExpr((Stmt *)E),
-        NRVOCandidate(NRVOCandidate) {
-    ReturnStmtBits.RetLoc = RL;
+  /// True if this ReturnStmt has storage for an NRVO candidate.
+  bool hasNRVOCandidate() const { return ReturnStmtBits.HasNRVOCandidate; }
+
+  unsigned numTrailingObjects(OverloadToken<const VarDecl *>) const {
+    return hasNRVOCandidate();
   }
 
-  /// Build an empty return expression.
-  explicit ReturnStmt(EmptyShell Empty) : Stmt(ReturnStmtClass, Empty) {}
+  /// Build a return statement.
+  ReturnStmt(SourceLocation RL, Expr *E, const VarDecl *NRVOCandidate);
 
-  const Expr *getRetValue() const;
-  Expr *getRetValue();
-  void setRetValue(Expr *E) { RetExpr = reinterpret_cast<Stmt*>(E); }
+  /// Build an empty return statement.
+  explicit ReturnStmt(EmptyShell Empty, bool HasNRVOCandidate);
 
-  SourceLocation getReturnLoc() const { return ReturnStmtBits.RetLoc; }
-  void setReturnLoc(SourceLocation L) { ReturnStmtBits.RetLoc = L; }
+public:
+  /// Create a return statement.
+  static ReturnStmt *Create(const ASTContext &Ctx, SourceLocation RL, Expr *E,
+                            const VarDecl *NRVOCandidate);
+
+  /// Create an empty return statement, optionally with
+  /// storage for an NRVO candidate.
+  static ReturnStmt *CreateEmpty(const ASTContext &Ctx, bool HasNRVOCandidate);
+
+  Expr *getRetValue() { return reinterpret_cast<Expr *>(RetExpr); }
+  const Expr *getRetValue() const { return reinterpret_cast<Expr *>(RetExpr); }
+  void setRetValue(Expr *E) { RetExpr = reinterpret_cast<Stmt *>(E); }
 
   /// Retrieve the variable that might be used for the named return
   /// value optimization.
   ///
   /// The optimization itself can only be performed if the variable is
   /// also marked as an NRVO object.
-  const VarDecl *getNRVOCandidate() const { return NRVOCandidate; }
-  void setNRVOCandidate(const VarDecl *Var) { NRVOCandidate = Var; }
+  const VarDecl *getNRVOCandidate() const {
+    return hasNRVOCandidate() ? *getTrailingObjects<const VarDecl *>()
+                              : nullptr;
+  }
+
+  /// Set the variable that might be used for the named return value
+  /// optimization. The return statement must have storage for it,
+  /// which is the case if and only if hasNRVOCandidate() is true.
+  void setNRVOCandidate(const VarDecl *Var) {
+    assert(hasNRVOCandidate() &&
+           "This return statement has no storage for an NRVO candidate!");
+    *getTrailingObjects<const VarDecl *>() = Var;
+  }
+
+  SourceLocation getReturnLoc() const { return ReturnStmtBits.RetLoc; }
+  void setReturnLoc(SourceLocation L) { ReturnStmtBits.RetLoc = L; }
 
   SourceLocation getBeginLoc() const { return getReturnLoc(); }
-
-  SourceLocation getEndLoc() const {
+  SourceLocation getEndLoc() const LLVM_READONLY {
     return RetExpr ? RetExpr->getEndLoc() : getReturnLoc();
   }
 
@@ -2042,7 +2072,8 @@ public:
 
   // Iterators
   child_range children() {
-    if (RetExpr) return child_range(&RetExpr, &RetExpr+1);
+    if (RetExpr)
+      return child_range(&RetExpr, &RetExpr + 1);
     return child_range(child_iterator(), child_iterator());
   }
 };

clang/lib/AST/ASTImporter.cpp

@@ -5957,8 +5957,8 @@ ExpectedStmt ASTNodeImporter::VisitReturnStmt(ReturnStmt *S) {
   const VarDecl *ToNRVOCandidate;
   std::tie(ToReturnLoc, ToRetValue, ToNRVOCandidate) = *Imp;
 
-  return new (Importer.getToContext()) ReturnStmt(
-      ToReturnLoc, ToRetValue, ToNRVOCandidate);
+  return ReturnStmt::Create(Importer.getToContext(), ToReturnLoc, ToRetValue,
+                            ToNRVOCandidate);
 }
 
 ExpectedStmt ASTNodeImporter::VisitCXXCatchStmt(CXXCatchStmt *S) {

clang/lib/AST/Stmt.cpp

@@ -1042,11 +1042,33 @@ LabelDecl *IndirectGotoStmt::getConstantTarget() {
 }
 
 // ReturnStmt
-const Expr* ReturnStmt::getRetValue() const {
-  return cast_or_null<Expr>(RetExpr);
+ReturnStmt::ReturnStmt(SourceLocation RL, Expr *E, const VarDecl *NRVOCandidate)
+    : Stmt(ReturnStmtClass), RetExpr(E) {
+  bool HasNRVOCandidate = NRVOCandidate != nullptr;
+  ReturnStmtBits.HasNRVOCandidate = HasNRVOCandidate;
+  if (HasNRVOCandidate)
+    setNRVOCandidate(NRVOCandidate);
+  setReturnLoc(RL);
 }
-Expr* ReturnStmt::getRetValue() {
-  return cast_or_null<Expr>(RetExpr);
+
+ReturnStmt::ReturnStmt(EmptyShell Empty, bool HasNRVOCandidate)
+    : Stmt(ReturnStmtClass, Empty) {
+  ReturnStmtBits.HasNRVOCandidate = HasNRVOCandidate;
+}
+
+ReturnStmt *ReturnStmt::Create(const ASTContext &Ctx, SourceLocation RL,
+                               Expr *E, const VarDecl *NRVOCandidate) {
+  bool HasNRVOCandidate = NRVOCandidate != nullptr;
+  void *Mem = Ctx.Allocate(totalSizeToAlloc<const VarDecl *>(HasNRVOCandidate),
+                           alignof(ReturnStmt));
+  return new (Mem) ReturnStmt(RL, E, NRVOCandidate);
+}
+
+ReturnStmt *ReturnStmt::CreateEmpty(const ASTContext &Ctx,
+                                    bool HasNRVOCandidate) {
+  void *Mem = Ctx.Allocate(totalSizeToAlloc<const VarDecl *>(HasNRVOCandidate),
+                           alignof(ReturnStmt));
+  return new (Mem) ReturnStmt(EmptyShell(), HasNRVOCandidate);
 }
 
 // CaseStmt

clang/lib/Analysis/BodyFarm.cpp

@@ -201,10 +201,9 @@ ObjCIvarRefExpr *ASTMaker::makeObjCIvarRef(const Expr *Base,
                                  /*arrow=*/true, /*free=*/false);
 }
 
-
 ReturnStmt *ASTMaker::makeReturn(const Expr *RetVal) {
-  return new (C) ReturnStmt(SourceLocation(), const_cast<Expr*>(RetVal),
-                            nullptr);
+  return ReturnStmt::Create(C, SourceLocation(), const_cast<Expr *>(RetVal),
+                            /* NRVOCandidate=*/nullptr);
 }
 
 IntegerLiteral *ASTMaker::makeIntegerLiteral(uint64_t Value, QualType Ty) {

clang/lib/CodeGen/CGObjC.cpp

@@ -883,9 +883,10 @@ CodeGenFunction::generateObjCGetterBody(const ObjCImplementationDecl *classImpl,
   // If there's a non-trivial 'get' expression, we just have to emit that.
   if (!hasTrivialGetExpr(propImpl)) {
     if (!AtomicHelperFn) {
-      ReturnStmt ret(SourceLocation(), propImpl->getGetterCXXConstructor(),
-                     /*nrvo*/ nullptr);
-      EmitReturnStmt(ret);
+      auto *ret = ReturnStmt::Create(getContext(), SourceLocation(),
+                                     propImpl->getGetterCXXConstructor(),
+                                     /* NRVOCandidate=*/nullptr);
+      EmitReturnStmt(*ret);
     }
     else {
       ObjCIvarDecl *ivar = propImpl->getPropertyIvarDecl();

clang/lib/Sema/SemaStmt.cpp

@@ -3226,7 +3226,8 @@ Sema::ActOnCapScopeReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp) {
         return StmtError();
       RetValExp = ER.get();
     }
-    return new (Context) ReturnStmt(ReturnLoc, RetValExp, nullptr);
+    return ReturnStmt::Create(Context, ReturnLoc, RetValExp,
+                              /* NRVOCandidate=*/nullptr);
   }
 
   if (HasDeducedReturnType) {
@@ -3352,8 +3353,8 @@ Sema::ActOnCapScopeReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp) {
       return StmtError();
     RetValExp = ER.get();
   }
-  ReturnStmt *Result = new (Context) ReturnStmt(ReturnLoc, RetValExp,
-                                                NRVOCandidate);
+  auto *Result =
+      ReturnStmt::Create(Context, ReturnLoc, RetValExp, NRVOCandidate);
 
   // If we need to check for the named return value optimization,
   // or if we need to infer the return type,
@@ -3582,7 +3583,8 @@ StmtResult Sema::BuildReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp) {
         return StmtError();
       RetValExp = ER.get();
     }
-    return new (Context) ReturnStmt(ReturnLoc, RetValExp, nullptr);
+    return ReturnStmt::Create(Context, ReturnLoc, RetValExp,
+                              /* NRVOCandidate=*/nullptr);
   }
 
   // FIXME: Add a flag to the ScopeInfo to indicate whether we're performing
@@ -3677,7 +3679,8 @@ StmtResult Sema::BuildReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp) {
       }
     }
 
-    Result = new (Context) ReturnStmt(ReturnLoc, RetValExp, nullptr);
+    Result = ReturnStmt::Create(Context, ReturnLoc, RetValExp,
+                                /* NRVOCandidate=*/nullptr);
   } else if (!RetValExp && !HasDependentReturnType) {
     FunctionDecl *FD = getCurFunctionDecl();
 
@@ -3699,7 +3702,8 @@ StmtResult Sema::BuildReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp) {
     else
       Diag(ReturnLoc, DiagID) << getCurMethodDecl()->getDeclName() << 1/*meth*/;
 
-    Result = new (Context) ReturnStmt(ReturnLoc);
+    Result = ReturnStmt::Create(Context, ReturnLoc, /* RetExpr=*/nullptr,
+                                /* NRVOCandidate=*/nullptr);
   } else {
     assert(RetValExp || HasDependentReturnType);
     const VarDecl *NRVOCandidate = nullptr;
@@ -3752,7 +3756,7 @@ StmtResult Sema::BuildReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp) {
         return StmtError();
       RetValExp = ER.get();
     }
-    Result = new (Context) ReturnStmt(ReturnLoc, RetValExp, NRVOCandidate);
+    Result = ReturnStmt::Create(Context, ReturnLoc, RetValExp, NRVOCandidate);
   }
 
   // If we need to check for the named return value optimization, save the

clang/lib/Serialization/ASTReaderStmt.cpp

@@ -328,9 +328,14 @@ void ASTStmtReader::VisitBreakStmt(BreakStmt *S) {
 
 void ASTStmtReader::VisitReturnStmt(ReturnStmt *S) {
   VisitStmt(S);
+
+  bool HasNRVOCandidate = Record.readInt();
+
   S->setRetValue(Record.readSubExpr());
+  if (HasNRVOCandidate)
+    S->setNRVOCandidate(ReadDeclAs<VarDecl>());
+
   S->setReturnLoc(ReadSourceLocation());
-  S->setNRVOCandidate(ReadDeclAs<VarDecl>());
 }
 
 void ASTStmtReader::VisitDeclStmt(DeclStmt *S) {
@@ -2359,7 +2364,8 @@ Stmt *ASTReader::ReadStmtFromStream(ModuleFile &F) {
       break;
 
     case STMT_RETURN:
-      S = new (Context) ReturnStmt(Empty);
+      S = ReturnStmt::CreateEmpty(
+          Context, /* HasNRVOCandidate=*/Record[ASTStmtReader::NumStmtFields]);
       break;
 
     case STMT_DECL:

clang/lib/Serialization/ASTWriterStmt.cpp

@@ -249,9 +249,15 @@ void ASTStmtWriter::VisitBreakStmt(BreakStmt *S) {
 
 void ASTStmtWriter::VisitReturnStmt(ReturnStmt *S) {
   VisitStmt(S);
+
+  bool HasNRVOCandidate = S->getNRVOCandidate() != nullptr;
+  Record.push_back(HasNRVOCandidate);
+
   Record.AddStmt(S->getRetValue());
+  if (HasNRVOCandidate)
+    Record.AddDeclRef(S->getNRVOCandidate());
+
   Record.AddSourceLocation(S->getReturnLoc());
-  Record.AddDeclRef(S->getNRVOCandidate());
   Code = serialization::STMT_RETURN;
 }