Migrate Declaration to variant (#644)

Note this makes some structural choices that I'm not sure how popular they'll be... Most notably, I could remove `DeclarationKind` because I'm using `std::visit` and `Declaration::Visitor` in reality. I could go to `switch(tag())` instead, but I'm wondering if this approach will be well received for the separation of ownership. Alternatively, I could move `Declaration` around so that it actually implements the things like `TopLevel` -- I did feel weird with the old structure of typecheck.cpp implementing members of declaration.h, though.

executable_semantics/ast/declaration.cpp

@@ -8,33 +8,101 @@
 
 namespace Carbon {
 
-void FunctionDeclaration::Print() const { definition.Print(); }
-
-void StructDeclaration::Print() const {
-  std::cout << "struct " << *definition.name << " {" << std::endl;
-  for (auto& member : *definition.members) {
-    member->Print();
-  }
-  std::cout << "}" << std::endl;
+auto Declaration::MakeFunctionDeclaration(FunctionDefinition definition)
+    -> const Declaration {
+  Declaration d;
+  d.value = FunctionDeclaration({.definition = definition});
+  return d;
 }
 
-void ChoiceDeclaration::Print() const {
-  std::cout << "choice " << name << " {" << std::endl;
-  for (const auto& [name, signature] : alternatives) {
-    std::cout << "alt " << name << " ";
-    PrintExp(signature);
-    std::cout << ";" << std::endl;
-  }
-  std::cout << "}" << std::endl;
+auto Declaration::MakeStructDeclaration(int line_num, std::string name,
+                                        std::list<Member*> members)
+    -> const Declaration {
+  Declaration d;
+  d.value = StructDeclaration(
+      {.definition = StructDefinition({.line_num = line_num,
+                                       .name = std::move(name),
+                                       .members = std::move(members)})});
+  return d;
 }
 
-// Print a global variable declaration to standard out.
-void VariableDeclaration::Print() const {
-  std::cout << "var ";
-  PrintExp(type);
-  std::cout << " : " << name << " = ";
-  PrintExp(initializer);
-  std::cout << std::endl;
+auto Declaration::MakeChoiceDeclaration(
+    int line_num, std::string name,
+    std::list<std::pair<std::string, const Expression*>> alternatives)
+    -> const Declaration {
+  Declaration d;
+  d.value = ChoiceDeclaration({.line_num = line_num,
+                               .name = std::move(name),
+                               .alternatives = std::move(alternatives)});
+  return d;
+}
+
+auto Declaration::MakeVariableDeclaration(int source_location, std::string name,
+                                          const Expression* type,
+                                          const Expression* initializer)
+    -> const Declaration {
+  Declaration d;
+  d.value = VariableDeclaration({.source_location = source_location,
+                                 .name = std::move(name),
+                                 .type = type,
+                                 .initializer = initializer});
+  return d;
+}
+
+auto Declaration::GetFunctionDeclaration() const -> const FunctionDeclaration& {
+  return std::get<FunctionDeclaration>(value);
+}
+
+auto Declaration::GetStructDeclaration() const -> const StructDeclaration& {
+  return std::get<StructDeclaration>(value);
+}
+
+auto Declaration::GetChoiceDeclaration() const -> const ChoiceDeclaration& {
+  return std::get<ChoiceDeclaration>(value);
+}
+
+auto Declaration::GetVariableDeclaration() const -> const VariableDeclaration& {
+  return std::get<VariableDeclaration>(value);
+}
+
+void Declaration::Print() const {
+  switch (tag()) {
+    case DeclarationKind::FunctionDeclaration:
+      GetFunctionDeclaration().definition.Print();
+      break;
+
+    case DeclarationKind::StructDeclaration: {
+      const StructDefinition& struct_def = GetStructDeclaration().definition;
+      std::cout << "struct " << struct_def.name << " {" << std::endl;
+      for (Member* m : struct_def.members) {
+        m->Print();
+      }
+      std::cout << "}" << std::endl;
+      break;
+    }
+
+    case DeclarationKind::ChoiceDeclaration: {
+      const auto& choice = GetChoiceDeclaration();
+      std::cout << "choice " << choice.name << " {" << std::endl;
+      for (const auto& [name, signature] : choice.alternatives) {
+        std::cout << "alt " << name << " ";
+        PrintExp(signature);
+        std::cout << ";" << std::endl;
+      }
+      std::cout << "}" << std::endl;
+      break;
+    }
+
+    case DeclarationKind::VariableDeclaration: {
+      const auto& var = GetVariableDeclaration();
+      std::cout << "var ";
+      PrintExp(var.type);
+      std::cout << " : " << var.name << " = ";
+      PrintExp(var.initializer);
+      std::cout << std::endl;
+      break;
+    }
+  }
 }
 
 }  // namespace Carbon

executable_semantics/ast/declaration.h

@@ -14,10 +14,6 @@
 #include "executable_semantics/interpreter/address.h"
 #include "executable_semantics/interpreter/dictionary.h"
 
-namespace yy {
-class parser;
-}
-
 namespace Carbon {
 
 struct Value;
@@ -32,148 +28,72 @@ struct TypeCheckContext {
   Env values;
 };
 
-// An existential AST declaration satisfying the Declaration concept.
-class Declaration {
- public:  // ValueSemantic concept API.
-  Declaration(const Declaration& other) = default;
-  Declaration& operator=(const Declaration& other) = default;
-
-  // Constructs an instance equivalent to `d`, where `Model` satisfies the
-  // Declaration concept.
-  template <class Model>
-  Declaration(Model d) : box(std::make_shared<Boxed<Model>>(d)) {}
-
- public:  // Declaration concept API, in addition to ValueSemantic.
-  void Print() const { box->Print(); }
-  auto Name() const -> std::string { return box->Name(); }
-
-  // Signals a type error if the declaration is not well typed,
-  // otherwise returns this declaration with annotated types.
-  //
-  // - Parameter env: types of run-time names.
-  // - Paraemter ct_env: values of compile-time names.
-  auto TypeChecked(TypeEnv env, Env ct_env) const -> Declaration {
-    return box->TypeChecked(env, ct_env);
-  }
-  // Add an entry in the runtime global symbol table for this declaration.
-  void InitGlobals(Env& globals) const { return box->InitGlobals(globals); }
-  // Add an entry in the compile time global symbol tables for this declaration.
-  auto TopLevel(TypeCheckContext& e) const -> void { return box->TopLevel(e); }
-
- private:
-  // A base class that erases the type of a `Boxed<Content>`, where `Content`
-  // satisfies the Declaration concept.
-  struct Box {
-   protected:
-    Box() {}
-
-   public:
-    Box(const Box& other) = delete;
-    Box& operator=(const Box& other) = delete;
-
-    virtual ~Box() {}
-    virtual auto Print() const -> void = 0;
-    virtual auto Name() const -> std::string = 0;
-    virtual auto TypeChecked(TypeEnv env, Env ct_env) const -> Declaration = 0;
-    virtual auto InitGlobals(Env& globals) const -> void = 0;
-    virtual auto TopLevel(TypeCheckContext&) const -> void = 0;
-  };
-
-  // The derived class that holds an instance of `Content` satisfying the
-  // Declaration concept.
-  template <class Content>
-  struct Boxed final : Box {
-    const Content content;
-    explicit Boxed(Content content) : Box(), content(content) {}
-
-    auto Print() const -> void override { return content.Print(); }
-    auto Name() const -> std::string override { return content.Name(); }
-    auto TypeChecked(TypeEnv env, Env ct_env) const -> Declaration override {
-      return content.TypeChecked(env, ct_env);
-    }
-    auto InitGlobals(Env& globals) const -> void override {
-      content.InitGlobals(globals);
-    }
-    auto TopLevel(TypeCheckContext& e) const -> void override {
-      content.TopLevel(e);
-    }
-  };
-
-  // Constructs an instance in a "partially formed" state, which can only be
-  // assigned to or destroyed.
-  Declaration() = default;
-
-  // Give Bison access to the default constructor.
-  friend class yy::parser;
-
-  // Note: the pointee is const as long as we have no mutating methods. When
-  std::shared_ptr<const Box> box;
+enum class DeclarationKind {
+  FunctionDeclaration,
+  StructDeclaration,
+  ChoiceDeclaration,
+  VariableDeclaration,
 };
 
 struct FunctionDeclaration {
+  static constexpr DeclarationKind Kind = DeclarationKind::FunctionDeclaration;
   FunctionDefinition definition;
-  explicit FunctionDeclaration(FunctionDefinition definition)
-      : definition(std::move(definition)) {}
-
-  auto Print() const -> void;
-  auto Name() const -> std::string;
-  auto TypeChecked(TypeEnv env, Env ct_env) const -> Declaration;
-  auto InitGlobals(Env& globals) const -> void;
-  auto TopLevel(TypeCheckContext&) const -> void;
 };
 
 struct StructDeclaration {
+  static constexpr DeclarationKind Kind = DeclarationKind::StructDeclaration;
   StructDefinition definition;
-  StructDeclaration(int line_num, std::string name, std::list<Member*>* members)
-      : definition{line_num, new std::string(name), members} {}
-
-  void Print() const;
-  auto Name() const -> std::string;
-  auto TypeChecked(TypeEnv env, Env ct_env) const -> Declaration;
-  void InitGlobals(Env& globals) const;
-  auto TopLevel(TypeCheckContext&) const -> void;
 };
 
 struct ChoiceDeclaration {
+  static constexpr DeclarationKind Kind = DeclarationKind::ChoiceDeclaration;
   int line_num;
   std::string name;
   std::list<std::pair<std::string, const Expression*>> alternatives;
-
-  ChoiceDeclaration(
-      int line_num, std::string name,
-      std::list<std::pair<std::string, const Expression*>> alternatives)
-      : line_num(line_num), name(name), alternatives(std::move(alternatives)) {}
-
-  void Print() const;
-  auto Name() const -> std::string;
-  auto TypeChecked(TypeEnv env, Env ct_env) const -> Declaration;
-  void InitGlobals(Env& globals) const;
-  auto TopLevel(TypeCheckContext&) const -> void;
 };
 
 // Global variable definition implements the Declaration concept.
-class VariableDeclaration {
- public:
-  VariableDeclaration(int source_location, std::string name,
-                      const Expression* type, const Expression* initializer)
-      : source_location(source_location),
-        name(name),
-        type(type),
-        initializer(initializer) {}
-
-  void Print() const;
-  auto Name() const -> std::string;
-  auto TypeChecked(TypeEnv env, Env ct_env) const -> Declaration;
-  void InitGlobals(Env& globals) const;
-  auto TopLevel(TypeCheckContext&) const -> void;
-
- private:
+struct VariableDeclaration {
+  static constexpr DeclarationKind Kind = DeclarationKind::VariableDeclaration;
   int source_location;
   std::string name;
   const Expression* type;
   const Expression* initializer;
 };
 
+class Declaration {
+ public:
+  static auto MakeFunctionDeclaration(FunctionDefinition definition)
+      -> const Declaration;
+  static auto MakeStructDeclaration(int line_num, std::string name,
+                                    std::list<Member*> members)
+      -> const Declaration;
+  static auto MakeChoiceDeclaration(
+      int line_num, std::string name,
+      std::list<std::pair<std::string, const Expression*>> alternatives)
+      -> const Declaration;
+  static auto MakeVariableDeclaration(int source_location, std::string name,
+                                      const Expression* type,
+                                      const Expression* initializer)
+      -> const Declaration;
+
+  auto GetFunctionDeclaration() const -> const FunctionDeclaration&;
+  auto GetStructDeclaration() const -> const StructDeclaration&;
+  auto GetChoiceDeclaration() const -> const ChoiceDeclaration&;
+  auto GetVariableDeclaration() const -> const VariableDeclaration&;
+
+  void Print() const;
+
+  inline auto tag() const -> DeclarationKind {
+    return std::visit([](const auto& t) { return t.Kind; }, value);
+  }
+
+ private:
+  std::variant<FunctionDeclaration, StructDeclaration, ChoiceDeclaration,
+               VariableDeclaration>
+      value;
+};
+
 }  // namespace Carbon
 
 #endif  // EXECUTABLE_SEMANTICS_AST_DECLARATION_H_

executable_semantics/ast/struct_definition.h

@@ -14,8 +14,8 @@ namespace Carbon {
 
 struct StructDefinition {
   int line_num;
-  std::string* name;
-  std::list<Member*>* members;
+  std::string name;
+  std::list<Member*> members;
 };
 
 }  // namespace Carbon

executable_semantics/interpreter/interpreter.cpp

@@ -278,57 +278,70 @@ auto EvalPrim(Operator op, const std::vector<const Value*>& args, int line_num)
 }
 
 // Globally-defined entities, such as functions, structs, choices.
-Env globals;
+static Env globals;
 
-void InitGlobals(std::list<Declaration>* fs) {
-  for (auto const& d : *fs) {
-    d.InitGlobals(globals);
-  }
-}
+void InitEnv(const Declaration& d, Env* env) {
+  switch (d.tag()) {
+    case DeclarationKind::FunctionDeclaration: {
+      const FunctionDefinition& func_def =
+          d.GetFunctionDeclaration().definition;
+      auto pt = InterpExp(*env, func_def.param_pattern);
+      auto f = Value::MakeFunctionValue(func_def.name, pt, func_def.body);
+      Address a = state->heap.AllocateValue(f);
+      env->Set(func_def.name, a);
+      break;
+    }
 
-auto ChoiceDeclaration::InitGlobals(Env& globals) const -> void {
-  VarValues alts;
-  for (const auto& [name, signature] : alternatives) {
-    auto t = InterpExp(Env(), signature);
-    alts.push_back(make_pair(name, t));
-  }
-  auto ct = Value::MakeChoiceType(name, std::move(alts));
-  auto a = state->heap.AllocateValue(ct);
-  globals.Set(name, a);
-}
-
-auto StructDeclaration::InitGlobals(Env& globals) const -> void {
-  VarValues fields;
-  VarValues methods;
-  for (Member* m : *definition.members) {
-    switch (m->tag()) {
-      case MemberKind::FieldMember: {
-        const auto& field = m->GetFieldMember();
-        auto t = InterpExp(Env(), field.type);
-        fields.push_back({field.name, t});
-        break;
+    case DeclarationKind::StructDeclaration: {
+      const StructDefinition& struct_def = d.GetStructDeclaration().definition;
+      VarValues fields;
+      VarValues methods;
+      for (const Member* m : struct_def.members) {
+        switch (m->tag()) {
+          case MemberKind::FieldMember: {
+            const auto& field = m->GetFieldMember();
+            auto t = InterpExp(Env(), field.type);
+            fields.push_back(make_pair(field.name, t));
+            break;
+          }
+        }
       }
+      auto st = Value::MakeStructType(struct_def.name, std::move(fields),
+                                      std::move(methods));
+      auto a = state->heap.AllocateValue(st);
+      env->Set(struct_def.name, a);
+      break;
+    }
+
+    case DeclarationKind::ChoiceDeclaration: {
+      const auto& choice = d.GetChoiceDeclaration();
+      VarValues alts;
+      for (const auto& [name, signature] : choice.alternatives) {
+        auto t = InterpExp(Env(), signature);
+        alts.push_back(make_pair(name, t));
+      }
+      auto ct = Value::MakeChoiceType(choice.name, std::move(alts));
+      auto a = state->heap.AllocateValue(ct);
+      env->Set(choice.name, a);
+      break;
+    }
+
+    case DeclarationKind::VariableDeclaration: {
+      const auto& var = d.GetVariableDeclaration();
+      // Adds an entry in `globals` mapping the variable's name to the
+      // result of evaluating the initializer.
+      auto v = InterpExp(*env, var.initializer);
+      Address a = state->heap.AllocateValue(v);
+      env->Set(var.name, a);
+      break;
     }
   }
-  auto st = Value::MakeStructType(*definition.name, std::move(fields),
-                                  std::move(methods));
-  auto a = state->heap.AllocateValue(st);
-  globals.Set(*definition.name, a);
 }
 
-auto FunctionDeclaration::InitGlobals(Env& globals) const -> void {
-  auto pt = InterpExp(globals, definition.param_pattern);
-  auto f = Value::MakeFunctionValue(definition.name, pt, definition.body);
-  Address a = state->heap.AllocateValue(f);
-  globals.Set(definition.name, a);
-}
-
-// Adds an entry in `globals` mapping the variable's name to the
-// result of evaluating the initializer.
-auto VariableDeclaration::InitGlobals(Env& globals) const -> void {
-  auto v = InterpExp(globals, initializer);
-  Address a = state->heap.AllocateValue(v);
-  globals.Set(name, a);
+static void InitGlobals(std::list<Declaration>* fs) {
+  for (auto const& d : *fs) {
+    InitEnv(d, &globals);
+  }
 }
 
 //    { S, H} -> { { C, E, F} :: S, H}

executable_semantics/interpreter/interpreter.h

@@ -97,7 +97,8 @@ struct State {
 
 extern State* state;
 
-auto PrintFrame(Frame* frame, std::ostream& out) -> void;
+void InitEnv(const Declaration& d, Env* env);
+void PrintFrame(Frame* frame, std::ostream& out);
 void PrintStack(Stack<Frame*> ls, std::ostream& out);
 void PrintEnv(Env values);
 auto CopyVal(const Value* val, int line_num) -> const Value*;

executable_semantics/interpreter/typecheck.cpp

@@ -681,65 +681,120 @@ auto TypeOfStructDef(const StructDefinition* sd, TypeEnv /*types*/, Env ct_top)
     -> const Value* {
   VarValues fields;
   VarValues methods;
-  for (Member* m : *sd->members) {
+  for (const Member* m : sd->members) {
     switch (m->tag()) {
       case MemberKind::FieldMember:
-        const auto& field = m->GetFieldMember();
-        auto t = InterpExp(ct_top, field.type);
-        fields.push_back(std::make_pair(field.name, t));
+        auto t = InterpExp(ct_top, m->GetFieldMember().type);
+        fields.push_back(std::make_pair(m->GetFieldMember().name, t));
         break;
     }
   }
-  return Value::MakeStructType(*sd->name, std::move(fields),
-                               std::move(methods));
+  return Value::MakeStructType(sd->name, std::move(fields), std::move(methods));
 }
 
-auto FunctionDeclaration::Name() const -> std::string {
-  return definition.name;
+static auto GetName(const Declaration& d) -> const std::string& {
+  switch (d.tag()) {
+    case DeclarationKind::FunctionDeclaration:
+      return d.GetFunctionDeclaration().definition.name;
+    case DeclarationKind::StructDeclaration:
+      return d.GetStructDeclaration().definition.name;
+    case DeclarationKind::ChoiceDeclaration:
+      return d.GetChoiceDeclaration().name;
+    case DeclarationKind::VariableDeclaration:
+      return d.GetVariableDeclaration().name;
+  }
 }
 
-auto StructDeclaration::Name() const -> std::string { return *definition.name; }
+auto MakeTypeChecked(const Declaration& d, const TypeEnv& types,
+                     const Env& values) -> Declaration {
+  switch (d.tag()) {
+    case DeclarationKind::FunctionDeclaration:
+      return Declaration::MakeFunctionDeclaration(*TypeCheckFunDef(
+          &d.GetFunctionDeclaration().definition, types, values));
 
-auto ChoiceDeclaration::Name() const -> std::string { return name; }
+    case DeclarationKind::StructDeclaration: {
+      const StructDefinition& struct_def = d.GetStructDeclaration().definition;
+      std::list<Member*> fields;
+      for (Member* m : struct_def.members) {
+        switch (m->tag()) {
+          case MemberKind::FieldMember:
+            // TODO: Interpret the type expression and store the result.
+            fields.push_back(m);
+            break;
+        }
+      }
+      return Declaration::MakeStructDeclaration(
+          struct_def.line_num, struct_def.name, std::move(fields));
+    }
 
-// Returns the name of the declared variable.
-auto VariableDeclaration::Name() const -> std::string { return name; }
+    case DeclarationKind::ChoiceDeclaration:
+      // TODO
+      return d;
 
-auto StructDeclaration::TypeChecked(TypeEnv types, Env values) const
-    -> Declaration {
-  auto fields = new std::list<Member*>();
-  for (Member* m : *definition.members) {
-    switch (m->tag()) {
-      case MemberKind::FieldMember:
-        // TODO: Interpret the type expression and store the result.
-        fields->push_back(m);
-        break;
+    case DeclarationKind::VariableDeclaration: {
+      const auto& var = d.GetVariableDeclaration();
+      // Signals a type error if the initializing expression does not have
+      // the declared type of the variable, otherwise returns this
+      // declaration with annotated types.
+      TCResult type_checked_initializer = TypeCheckExp(
+          var.initializer, types, values, nullptr, TCContext::ValueContext);
+      const Value* declared_type = InterpExp(values, var.type);
+      ExpectType(var.source_location, "initializer of variable", declared_type,
+                 type_checked_initializer.type);
+      return d;
     }
   }
-  return StructDeclaration(definition.line_num, *definition.name, fields);
 }
 
-auto FunctionDeclaration::TypeChecked(TypeEnv types, Env values) const
-    -> Declaration {
-  return FunctionDeclaration(*TypeCheckFunDef(&definition, types, values));
-}
+static void TopLevel(const Declaration& d, TypeCheckContext* tops) {
+  switch (d.tag()) {
+    case DeclarationKind::FunctionDeclaration: {
+      const FunctionDefinition& func_def =
+          d.GetFunctionDeclaration().definition;
+      auto t = TypeOfFunDef(tops->types, tops->values, &func_def);
+      tops->types.Set(func_def.name, t);
+      InitEnv(d, &tops->values);
+      break;
+    }
 
-auto ChoiceDeclaration::TypeChecked(TypeEnv types, Env values) const
-    -> Declaration {
-  return *this;  // TODO.
-}
+    case DeclarationKind::StructDeclaration: {
+      const StructDefinition& struct_def = d.GetStructDeclaration().definition;
+      auto st = TypeOfStructDef(&struct_def, tops->types, tops->values);
+      Address a = state->heap.AllocateValue(st);
+      tops->values.Set(struct_def.name, a);  // Is this obsolete?
+      std::vector<TupleElement> field_types;
+      for (const auto& [field_name, field_value] : st->GetStructType().fields) {
+        field_types.push_back({.name = field_name, .value = field_value});
+      }
+      auto fun_ty = Value::MakeFunctionType(
+          Value::MakeTupleValue(std::move(field_types)), st);
+      tops->types.Set(struct_def.name, fun_ty);
+      break;
+    }
 
-// Signals a type error if the initializing expression does not have
-// the declared type of the variable, otherwise returns this
-// declaration with annotated types.
-auto VariableDeclaration::TypeChecked(TypeEnv types, Env values) const
-    -> Declaration {
-  TCResult type_checked_initializer = TypeCheckExp(
-      initializer, types, values, nullptr, TCContext::ValueContext);
-  const Value* declared_type = InterpExp(values, type);
-  ExpectType(source_location, "initializer of variable", declared_type,
-             type_checked_initializer.type);
-  return *this;
+    case DeclarationKind::ChoiceDeclaration: {
+      const auto& choice = d.GetChoiceDeclaration();
+      VarValues alts;
+      for (const auto& [name, signature] : choice.alternatives) {
+        auto t = InterpExp(tops->values, signature);
+        alts.push_back(std::make_pair(name, t));
+      }
+      auto ct = Value::MakeChoiceType(choice.name, std::move(alts));
+      Address a = state->heap.AllocateValue(ct);
+      tops->values.Set(choice.name, a);  // Is this obsolete?
+      tops->types.Set(choice.name, ct);
+      break;
+    }
+
+    case DeclarationKind::VariableDeclaration: {
+      const auto& var = d.GetVariableDeclaration();
+      // Associate the variable name with it's declared type in the
+      // compile-time symbol table.
+      const Value* declared_type = InterpExp(tops->values, var.type);
+      tops->types.Set(var.name, declared_type);
+      break;
+    }
+  }
 }
 
 auto TopLevel(std::list<Declaration>* fs) -> TypeCheckContext {
@@ -747,10 +802,10 @@ auto TopLevel(std::list<Declaration>* fs) -> TypeCheckContext {
   bool found_main = false;
 
   for (auto const& d : *fs) {
-    if (d.Name() == "main") {
+    if (GetName(d) == "main") {
       found_main = true;
     }
-    d.TopLevel(tops);
+    TopLevel(d, &tops);
   }
 
   if (found_main == false) {
@@ -761,42 +816,4 @@ auto TopLevel(std::list<Declaration>* fs) -> TypeCheckContext {
   return tops;
 }
 
-auto FunctionDeclaration::TopLevel(TypeCheckContext& tops) const -> void {
-  auto t = TypeOfFunDef(tops.types, tops.values, &definition);
-  tops.types.Set(Name(), t);
-  InitGlobals(tops.values);
-}
-
-auto StructDeclaration::TopLevel(TypeCheckContext& tops) const -> void {
-  auto st = TypeOfStructDef(&definition, tops.types, tops.values);
-  Address a = state->heap.AllocateValue(st);
-  tops.values.Set(Name(), a);  // Is this obsolete?
-  std::vector<TupleElement> field_types;
-  for (const auto& [field_name, field_value] : st->GetStructType().fields) {
-    field_types.push_back({.name = field_name, .value = field_value});
-  }
-  auto fun_ty = Value::MakeFunctionType(
-      Value::MakeTupleValue(std::move(field_types)), st);
-  tops.types.Set(Name(), fun_ty);
-}
-
-auto ChoiceDeclaration::TopLevel(TypeCheckContext& tops) const -> void {
-  VarValues alts;
-  for (const auto& [name, signature] : alternatives) {
-    auto t = InterpExp(tops.values, signature);
-    alts.push_back(std::make_pair(name, t));
-  }
-  auto ct = Value::MakeChoiceType(name, std::move(alts));
-  Address a = state->heap.AllocateValue(ct);
-  tops.values.Set(Name(), a);  // Is this obsolete?
-  tops.types.Set(Name(), ct);
-}
-
-// Associate the variable name with it's declared type in the
-// compile-time symbol table.
-auto VariableDeclaration::TopLevel(TypeCheckContext& tops) const -> void {
-  const Value* declared_type = InterpExp(tops.values, type);
-  tops.types.Set(Name(), declared_type);
-}
-
 }  // namespace Carbon

executable_semantics/interpreter/typecheck.h

@@ -45,6 +45,8 @@ auto TypeCheckStmt(const Statement*, TypeEnv, Env, Value const*&)
 auto TypeCheckFunDef(struct FunctionDefinition*, TypeEnv)
     -> struct FunctionDefinition*;
 
+auto MakeTypeChecked(const Declaration& decl, const TypeEnv& types,
+                     const Env& values) -> Declaration;
 auto TopLevel(std::list<Declaration>* fs) -> TypeCheckContext;
 
 void PrintErrorString(const std::string& s);

executable_semantics/syntax/parser.ypp

@@ -106,7 +106,7 @@ void yy::parser::error(
 %type <const Carbon::Expression*> tuple
 %type <Carbon::Member*> variable_declaration
 %type <Carbon::Member*> member
-%type <std::list<Carbon::Member*>*> member_list
+%type <std::list<Carbon::Member*>> member_list
 %type <Carbon::FieldInitializer> field_initializer
 %type <Carbon::ParenContents> paren_contents
 %type <std::vector<Carbon::FieldInitializer>> paren_contents_without_trailing_comma
@@ -391,9 +391,9 @@ member: VAR variable_declaration ";"
 ;
 member_list:
   // Empty
-    { $$ = new std::list<Carbon::Member*>(); }
+    { $$ = std::list<Carbon::Member*>(); }
 | member member_list
-    { $$ = $2; $$->push_front($1); }
+    { $$ = $2; $$.push_front($1); }
 ;
 alternative:
   identifier tuple
@@ -417,24 +417,21 @@ alternative_list:
 ;
 declaration:
   function_definition
-    { $$ = Carbon::Declaration(Carbon::FunctionDeclaration{$1}); }
+    { $$ = Carbon::Declaration::MakeFunctionDeclaration(std::move($1)); }
 | function_declaration
-    { $$ = Carbon::Declaration(Carbon::FunctionDeclaration{$1}); }
+    { $$ = Carbon::Declaration::MakeFunctionDeclaration(std::move($1)); }
 | STRUCT identifier "{" member_list "}"
     {
-      $$ = Carbon::Declaration(
-        Carbon::StructDeclaration{yylineno, $2, $4});
+      $$ = Carbon::Declaration::MakeStructDeclaration(yylineno, $2, $4);
     }
 | CHOICE identifier "{" alternative_list "}"
     {
-      $$ = Carbon::Declaration(
-        Carbon::ChoiceDeclaration{yylineno, $2, $4});
+      $$ = Carbon::Declaration::MakeChoiceDeclaration(yylineno, $2, $4);
     }
 | VAR variable_declaration "=" expression ";"
     {
-      $$ = Carbon::Declaration(
-        Carbon::VariableDeclaration(yylineno, $2->GetFieldMember().name,
-                                    $2->GetFieldMember().type, $4));
+      $$ = Carbon::Declaration::MakeVariableDeclaration(
+          yylineno, $2->GetFieldMember().name, $2->GetFieldMember().type, $4);
     }
 ;
 declaration_list:

executable_semantics/syntax/syntax_helpers.cpp

@@ -26,7 +26,7 @@ void ExecProgram(std::list<Declaration>* fs) {
   Env ct_top = p.values;
   std::list<Declaration> new_decls;
   for (const auto& decl : *fs) {
-    new_decls.push_back(decl.TypeChecked(top, ct_top));
+    new_decls.push_back(MakeTypeChecked(decl, top, ct_top));
   }
   if (tracing_output) {
     std::cout << std::endl;