Migrate `Tuple` and `FieldInitializer` to value semantics (#605)

* Store tuple elements by value.
* Update FieldInitializer to use IndirectValue.

executable_semantics/ast/expression.cpp

@@ -165,13 +165,13 @@ auto Expression::MakeGetField(int line_num, const Expression* exp,
   return e;
 }
 
-auto Expression::MakeTuple(int line_num, std::vector<FieldInitializer>* args)
+auto Expression::MakeTuple(int line_num, std::vector<FieldInitializer> args)
     -> const Expression* {
   auto* e = new Expression();
   e->line_num = line_num;
   int i = 0;
   bool seen_named_member = false;
-  for (auto& arg : *args) {
+  for (auto& arg : args) {
     if (arg.name == "") {
       if (seen_named_member) {
         std::cerr << line_num
@@ -189,17 +189,6 @@ auto Expression::MakeTuple(int line_num, std::vector<FieldInitializer>* args)
   return e;
 }
 
-// Create an AST node for an empty tuple.
-// TODO(geoffromer): remove this and rewrite its callers to use
-// `MakeTuple(line_num, {})`, once that works.
-auto Expression::MakeUnit(int line_num) -> const Expression* {
-  auto* unit = new Expression();
-  unit->line_num = line_num;
-  auto* args = new std::vector<FieldInitializer>();
-  unit->value = Tuple({.fields = args});
-  return unit;
-}
-
 auto Expression::MakeIndex(int line_num, const Expression* exp,
                            const Expression* i) -> const Expression* {
   auto* e = new Expression();
@@ -237,14 +226,14 @@ static void PrintOp(Operator op) {
   }
 }
 
-static void PrintFields(std::vector<FieldInitializer>* fields) {
+static void PrintFields(const std::vector<FieldInitializer>& fields) {
   int i = 0;
-  for (auto iter = fields->begin(); iter != fields->end(); ++iter, ++i) {
+  for (auto iter = fields.begin(); iter != fields.end(); ++iter, ++i) {
     if (i != 0) {
       std::cout << ", ";
     }
     std::cout << iter->name << " = ";
-    PrintExp(iter->expression);
+    PrintExp(iter->expression.GetPointer());
   }
 }
 

executable_semantics/ast/expression.h

@@ -21,7 +21,7 @@ struct FieldInitializer {
   std::string name;
 
   // The expression that initializes the field.
-  const Expression* expression;
+  IndirectValue<Expression> expression;
 };
 
 enum class ExpressionKind {
@@ -92,7 +92,7 @@ struct BoolLiteral {
 
 struct Tuple {
   static constexpr ExpressionKind Kind = ExpressionKind::Tuple;
-  std::vector<FieldInitializer>* fields;
+  std::vector<FieldInitializer> fields;
 };
 
 struct PrimitiveOperator {
@@ -152,9 +152,8 @@ struct Expression {
                        const Expression* arg) -> const Expression*;
   static auto MakeGetField(int line_num, const Expression* exp,
                            std::string field) -> const Expression*;
-  static auto MakeTuple(int line_num, std::vector<FieldInitializer>* args)
+  static auto MakeTuple(int line_num, std::vector<FieldInitializer> args)
       -> const Expression*;
-  static auto MakeUnit(int line_num) -> const Expression*;
   static auto MakeIndex(int line_num, const Expression* exp,
                         const Expression* i) -> const Expression*;
   static auto MakeTypeType(int line_num) -> const Expression*;

executable_semantics/interpreter/interpreter.cpp

@@ -427,7 +427,7 @@ void CreateTuple(Frame* frame, Action* act, const Expression* /*exp*/) {
   //    { { (v1,...,vn) :: C, E, F} :: S, H}
   // -> { { `(v1,...,vn) :: C, E, F} :: S, H}
   auto elements = new std::vector<TupleElement>();
-  auto f = act->u.exp->GetTuple().fields->begin();
+  auto f = act->u.exp->GetTuple().fields.begin();
   for (auto i = act->results.begin(); i != act->results.end(); ++i, ++f) {
     Address a = state->heap.AllocateValue(*i);  // copy?
     elements->push_back({.name = f->name, .address = a});
@@ -653,7 +653,7 @@ void StepLvalue() {
     case ExpressionKind::Tuple: {
       //    { {(f1=e1,...) :: C, E, F} :: S, H}
       // -> { {e1 :: (f1=[],...) :: C, E, F} :: S, H}
-      const Expression* e1 = (*exp->GetTuple().fields)[0].expression;
+      const Expression* e1 = exp->GetTuple().fields[0].expression.GetPointer();
       frame->todo.Push(MakeLvalAct(e1));
       act->pos++;
       break;
@@ -701,10 +701,11 @@ void StepExp() {
       break;
     }
     case ExpressionKind::Tuple: {
-      if (exp->GetTuple().fields->size() > 0) {
+      if (exp->GetTuple().fields.size() > 0) {
         //    { {(f1=e1,...) :: C, E, F} :: S, H}
         // -> { {e1 :: (f1=[],...) :: C, E, F} :: S, H}
-        const Expression* e1 = (*exp->GetTuple().fields)[0].expression;
+        const Expression* e1 =
+            exp->GetTuple().fields[0].expression.GetPointer();
         frame->todo.Push(MakeExpAct(e1));
         act->pos++;
       } else {
@@ -946,7 +947,7 @@ void StepStmt() {
       scopes.Push(scope);
       Stack<Action*> todo;
       todo.Push(MakeStmtAct(Statement::MakeReturn(
-          stmt->line_num, Expression::MakeUnit(stmt->line_num))));
+          stmt->line_num, Expression::MakeTuple(stmt->line_num, {}))));
       todo.Push(MakeStmtAct(stmt->GetContinuation().body));
       Frame* continuation_frame = new Frame("__continuation", scopes, todo);
       Address continuation_address = state->heap.AllocateValue(
@@ -1111,13 +1112,13 @@ void HandleValue() {
           break;
         }
         case ExpressionKind::Tuple: {
-          if (act->pos != static_cast<int>(exp->GetTuple().fields->size())) {
+          if (act->pos != static_cast<int>(exp->GetTuple().fields.size())) {
             //    { { vk :: (f1=v1,..., fk=[],fk+1=ek+1,...) :: C, E, F} :: S,
             //    H}
             // -> { { ek+1 :: (f1=v1,..., fk=vk, fk+1=[],...) :: C, E, F} :: S,
             // H}
             const Expression* elt =
-                (*exp->GetTuple().fields)[act->pos].expression;
+                exp->GetTuple().fields[act->pos].expression.GetPointer();
             frame->todo.Pop(1);
             frame->todo.Push(MakeLvalAct(elt));
           } else {
@@ -1144,13 +1145,13 @@ void HandleValue() {
           break;
         }
         case ExpressionKind::Tuple: {
-          if (act->pos != static_cast<int>(exp->GetTuple().fields->size())) {
+          if (act->pos != static_cast<int>(exp->GetTuple().fields.size())) {
             //    { { vk :: (f1=v1,..., fk=[],fk+1=ek+1,...) :: C, E, F} :: S,
             //    H}
             // -> { { ek+1 :: (f1=v1,..., fk=vk, fk+1=[],...) :: C, E, F} :: S,
             // H}
             const Expression* elt =
-                (*exp->GetTuple().fields)[act->pos].expression;
+                exp->GetTuple().fields[act->pos].expression.GetPointer();
             frame->todo.Pop(1);
             frame->todo.Push(MakeExpAct(elt));
           } else {
@@ -1424,7 +1425,7 @@ void HandleValue() {
           // Push an expression statement action to ignore the result
           // value from the continuation.
           Action* ignore_result = MakeStmtAct(Statement::MakeExpStmt(
-              stmt->line_num, Expression::MakeUnit(stmt->line_num)));
+              stmt->line_num, Expression::MakeTuple(stmt->line_num, {})));
           ignore_result->pos = 0;
           frame->todo.Push(ignore_result);
           // Push the continuation onto the current stack.
@@ -1497,8 +1498,7 @@ auto InterpProgram(std::list<Declaration>* fs) -> int {
   }
   InitGlobals(fs);
 
-  const Expression* arg =
-      Expression::MakeTuple(0, new std::vector<FieldInitializer>());
+  const Expression* arg = Expression::MakeTuple(0, {});
   const Expression* call_main =
       Expression::MakeCall(0, Expression::MakeVar(0, "main"), arg);
   auto todo = Stack(MakeExpAct(call_main));

executable_semantics/interpreter/typecheck.cpp

@@ -70,12 +70,12 @@ auto ReifyType(const Value* t, int line_num) -> const Expression* {
           0, ReifyType(t->GetFunctionType().param, line_num),
           ReifyType(t->GetFunctionType().ret, line_num));
     case ValKind::TupleV: {
-      auto args = new std::vector<FieldInitializer>();
+      std::vector<FieldInitializer> args;
       for (const TupleElement& field : *t->GetTuple().elements) {
-        args->push_back(
+        args.push_back(
             {.name = field.name,
-             .expression = ReifyType(state->heap.Read(field.address, line_num),
-                                     line_num)});
+             .expression = *ReifyType(state->heap.Read(field.address, line_num),
+                                      line_num)});
       }
       return Expression::MakeTuple(0, args);
     }
@@ -190,7 +190,7 @@ auto TypeCheckExp(const Expression* e, TypeEnv types, Env values,
       }
     }
     case ExpressionKind::Tuple: {
-      auto new_args = new std::vector<FieldInitializer>();
+      std::vector<FieldInitializer> new_args;
       auto arg_types = new std::vector<TupleElement>();
       auto new_types = types;
       if (expected && expected->tag != ValKind::TupleV) {
@@ -198,7 +198,7 @@ auto TypeCheckExp(const Expression* e, TypeEnv types, Env values,
                   << std::endl;
         exit(-1);
       }
-      if (expected && e->GetTuple().fields->size() !=
+      if (expected && e->GetTuple().fields.size() !=
                           expected->GetTuple().elements->size()) {
         std::cerr << e->line_num
                   << ": compilation error, tuples of different length"
@@ -206,8 +206,8 @@ auto TypeCheckExp(const Expression* e, TypeEnv types, Env values,
         exit(-1);
       }
       int i = 0;
-      for (auto arg = e->GetTuple().fields->begin();
-           arg != e->GetTuple().fields->end(); ++arg, ++i) {
+      for (auto arg = e->GetTuple().fields.begin();
+           arg != e->GetTuple().fields.end(); ++arg, ++i) {
         const Value* arg_expected = nullptr;
         if (expected && expected->tag == ValKind::TupleV) {
           if ((*expected->GetTuple().elements)[i].name != arg->name) {
@@ -220,10 +220,10 @@ auto TypeCheckExp(const Expression* e, TypeEnv types, Env values,
           arg_expected = state->heap.Read(
               (*expected->GetTuple().elements)[i].address, e->line_num);
         }
-        auto arg_res = TypeCheckExp(arg->expression, new_types, values,
-                                    arg_expected, context);
+        auto arg_res = TypeCheckExp(arg->expression.GetPointer(), new_types,
+                                    values, arg_expected, context);
         new_types = arg_res.types;
-        new_args->push_back({.name = arg->name, .expression = arg_res.exp});
+        new_args.push_back({.name = arg->name, .expression = *arg_res.exp});
         arg_types->push_back(
             {.name = arg->name,
              .address = state->heap.AllocateValue(arg_res.type)});
@@ -586,7 +586,8 @@ auto CheckOrEnsureReturn(const Statement* stmt, bool void_return, int line_num)
     -> const Statement* {
   if (!stmt) {
     if (void_return) {
-      return Statement::MakeReturn(line_num, Expression::MakeUnit(line_num));
+      return Statement::MakeReturn(line_num,
+                                   Expression::MakeTuple(line_num, {}));
     } else {
       std::cerr
           << "control-flow reaches end of non-void function without a return"
@@ -643,7 +644,7 @@ auto CheckOrEnsureReturn(const Statement* stmt, bool void_return, int line_num)
         return Statement::MakeSeq(
             stmt->line_num, stmt,
             Statement::MakeReturn(stmt->line_num,
-                                  Expression::MakeUnit(stmt->line_num)));
+                                  Expression::MakeTuple(stmt->line_num, {})));
       } else {
         std::cerr
             << stmt->line_num

executable_semantics/syntax/paren_contents.cpp

@@ -9,15 +9,14 @@ namespace Carbon {
 const Expression* ParenContents::AsExpression(int line_number) const {
   if (fields_.size() == 1 && fields_.front().name == "" &&
       has_trailing_comma_ == HasTrailingComma::No) {
-    return fields_.front().expression;
+    return new Expression(*fields_.front().expression);
   } else {
     return AsTuple(line_number);
   }
 }
 
 const Expression* ParenContents::AsTuple(int line_number) const {
-  return Expression::MakeTuple(line_number,
-                               new std::vector<FieldInitializer>(fields_));
+  return Expression::MakeTuple(line_number, fields_);
 }
 
 }  // namespace Carbon

executable_semantics/syntax/paren_contents_test.cpp

@@ -14,7 +14,7 @@ TEST(ParenContentsTest, EmptyAsExpression) {
   const Expression* expression = contents.AsExpression(/*line_num=*/1);
   EXPECT_EQ(expression->line_num, 1);
   ASSERT_EQ(expression->tag(), ExpressionKind::Tuple);
-  EXPECT_EQ(expression->GetTuple().fields->size(), 0);
+  EXPECT_EQ(expression->GetTuple().fields.size(), 0);
 }
 
 TEST(ParenContentsTest, EmptyAsTuple) {
@@ -22,7 +22,7 @@ TEST(ParenContentsTest, EmptyAsTuple) {
   const Expression* tuple = contents.AsTuple(/*line_num=*/1);
   EXPECT_EQ(tuple->line_num, 1);
   ASSERT_EQ(tuple->tag(), ExpressionKind::Tuple);
-  EXPECT_EQ(tuple->GetTuple().fields->size(), 0);
+  EXPECT_EQ(tuple->GetTuple().fields.size(), 0);
 }
 
 TEST(ParenContentsTest, UnaryNoCommaAsExpression) {
@@ -33,7 +33,7 @@ TEST(ParenContentsTest, UnaryNoCommaAsExpression) {
   // )
   // ```
   ParenContents contents(
-      {{.expression = Expression::MakeInt(/*line_num=*/2, 42)}},
+      {{.expression = *Expression::MakeInt(/*line_num=*/2, 42)}},
       ParenContents::HasTrailingComma::No);
 
   const Expression* expression = contents.AsExpression(/*line_num=*/1);
@@ -43,53 +43,53 @@ TEST(ParenContentsTest, UnaryNoCommaAsExpression) {
 
 TEST(ParenContentsTest, UnaryNoCommaAsTuple) {
   ParenContents contents(
-      {{.expression = Expression::MakeInt(/*line_num=*/2, 42)}},
+      {{.expression = *Expression::MakeInt(/*line_num=*/2, 42)}},
       ParenContents::HasTrailingComma::No);
 
   const Expression* tuple = contents.AsTuple(/*line_num=*/1);
   EXPECT_EQ(tuple->line_num, 1);
   ASSERT_EQ(tuple->tag(), ExpressionKind::Tuple);
-  std::vector<FieldInitializer> fields = *tuple->GetTuple().fields;
+  std::vector<FieldInitializer> fields = tuple->GetTuple().fields;
   ASSERT_EQ(fields.size(), 1);
   EXPECT_EQ(fields[0].expression->tag(), ExpressionKind::Integer);
 }
 
 TEST(ParenContentsTest, UnaryWithCommaAsExpression) {
   ParenContents contents(
-      {{.expression = Expression::MakeInt(/*line_num=*/2, 42)}},
+      {{.expression = *Expression::MakeInt(/*line_num=*/2, 42)}},
       ParenContents::HasTrailingComma::Yes);
 
   const Expression* expression = contents.AsExpression(/*line_num=*/1);
   EXPECT_EQ(expression->line_num, 1);
   ASSERT_EQ(expression->tag(), ExpressionKind::Tuple);
-  std::vector<FieldInitializer> fields = *expression->GetTuple().fields;
+  std::vector<FieldInitializer> fields = expression->GetTuple().fields;
   ASSERT_EQ(fields.size(), 1);
   EXPECT_EQ(fields[0].expression->tag(), ExpressionKind::Integer);
 }
 
 TEST(ParenContentsTest, UnaryWithCommaAsTuple) {
   ParenContents contents(
-      {{.expression = Expression::MakeInt(/*line_num=*/2, 42)}},
+      {{.expression = *Expression::MakeInt(/*line_num=*/2, 42)}},
       ParenContents::HasTrailingComma::Yes);
 
   const Expression* tuple = contents.AsTuple(/*line_num=*/1);
   EXPECT_EQ(tuple->line_num, 1);
   ASSERT_EQ(tuple->tag(), ExpressionKind::Tuple);
-  std::vector<FieldInitializer> fields = *tuple->GetTuple().fields;
+  std::vector<FieldInitializer> fields = tuple->GetTuple().fields;
   ASSERT_EQ(fields.size(), 1);
   EXPECT_EQ(fields[0].expression->tag(), ExpressionKind::Integer);
 }
 
 TEST(ParenContentsTest, BinaryAsExpression) {
   ParenContents contents(
-      {{.expression = Expression::MakeInt(/*line_num=*/2, 42)},
-       {.expression = Expression::MakeInt(/*line_num=*/3, 42)}},
+      {{.expression = *Expression::MakeInt(/*line_num=*/2, 42)},
+       {.expression = *Expression::MakeInt(/*line_num=*/3, 42)}},
       ParenContents::HasTrailingComma::Yes);
 
   const Expression* expression = contents.AsExpression(/*line_num=*/1);
   EXPECT_EQ(expression->line_num, 1);
   ASSERT_EQ(expression->tag(), ExpressionKind::Tuple);
-  std::vector<FieldInitializer> fields = *expression->GetTuple().fields;
+  std::vector<FieldInitializer> fields = expression->GetTuple().fields;
   ASSERT_EQ(fields.size(), 2);
   EXPECT_EQ(fields[0].expression->tag(), ExpressionKind::Integer);
   EXPECT_EQ(fields[1].expression->tag(), ExpressionKind::Integer);
@@ -97,14 +97,14 @@ TEST(ParenContentsTest, BinaryAsExpression) {
 
 TEST(ParenContentsTest, BinaryAsTuple) {
   ParenContents contents(
-      {{.expression = Expression::MakeInt(/*line_num=*/2, 42)},
-       {.expression = Expression::MakeInt(/*line_num=*/3, 42)}},
+      {{.expression = *Expression::MakeInt(/*line_num=*/2, 42)},
+       {.expression = *Expression::MakeInt(/*line_num=*/3, 42)}},
       ParenContents::HasTrailingComma::Yes);
 
   const Expression* tuple = contents.AsTuple(/*line_num=*/1);
   EXPECT_EQ(tuple->line_num, 1);
   ASSERT_EQ(tuple->tag(), ExpressionKind::Tuple);
-  std::vector<FieldInitializer> fields = *tuple->GetTuple().fields;
+  std::vector<FieldInitializer> fields = tuple->GetTuple().fields;
   ASSERT_EQ(fields.size(), 2);
   EXPECT_EQ(fields[0].expression->tag(), ExpressionKind::Integer);
   EXPECT_EQ(fields[1].expression->tag(), ExpressionKind::Integer);

executable_semantics/syntax/parser.ypp

@@ -264,9 +264,9 @@ tuple: "(" paren_contents ")"
 ;
 field_initializer:
   pattern
-    { $$ = Carbon::FieldInitializer({"", $1}); }
+    { $$ = Carbon::FieldInitializer({"", *$1}); }
 | designator "=" pattern
-    { $$ = Carbon::FieldInitializer({$1, $3}); }
+    { $$ = Carbon::FieldInitializer({$1, *$3}); }
 ;
 paren_contents:
   // Empty
@@ -350,7 +350,7 @@ statement_list:
 ;
 return_type:
   // Empty
-    { $$ = Carbon::Expression::MakeUnit(yylineno); }
+    { $$ = Carbon::Expression::MakeTuple(yylineno, {}); }
 | ARROW expression %prec FNARROW
     { $$ = $2; }
 ;
@@ -385,7 +385,7 @@ alternative:
 | identifier
     {
       $$ = new std::pair<std::string, const Carbon::Expression*>(
-          $1, Carbon::Expression::MakeUnit(yylineno));
+          $1, Carbon::Expression::MakeTuple(yylineno, {}));
     }
 ;
 alternative_list: