From c4d7adde9a6eff361c97aa9c8f441bfbcd02f3c2 Mon Sep 17 00:00:00 2001
From: Geoff Romer <gromer@google.com>
Date: Mon, 18 Oct 2021 15:58:35 -0700
Subject: [PATCH] Drop `CopyVal` (#895)

Values are immutable, so they should never need to be copied.
---
 .../interpreter/interpreter.cpp               | 15 ++--
 executable_semantics/interpreter/value.cpp    | 84 -------------------
 executable_semantics/interpreter/value.h      |  3 -
 3 files changed, 5 insertions(+), 97 deletions(-)

diff --git a/executable_semantics/interpreter/interpreter.cpp b/executable_semantics/interpreter/interpreter.cpp
index 01b034dbae93..4000d66252bf 100644
--- a/executable_semantics/interpreter/interpreter.cpp
+++ b/executable_semantics/interpreter/interpreter.cpp
@@ -223,7 +223,7 @@ auto Interpreter::PatternMatch(Nonnull<const Value*> p, Nonnull<const Value*> v,
       const auto& placeholder = cast<BindingPlaceholderValue>(*p);
       Env values(arena);
       if (placeholder.name().has_value()) {
-        Address a = heap.AllocateValue(CopyVal(arena, v, source_loc));
+        Address a = heap.AllocateValue(v);
         values.Set(*placeholder.name(), a);
       }
       return values;
@@ -329,8 +329,7 @@ void Interpreter::PatternAssignment(Nonnull<const Value*> pat,
                                     SourceLocation source_loc) {
   switch (pat->kind()) {
     case Value::Kind::PointerValue:
-      heap.Write(cast<PointerValue>(*pat).value(),
-                 CopyVal(arena, val, source_loc), source_loc);
+      heap.Write(cast<PointerValue>(*pat).value(), val, source_loc);
       break;
     case Value::Kind::TupleValue: {
       switch (val->kind()) {
@@ -582,10 +581,8 @@ auto Interpreter::StepExp() -> Transition {
           case Value::Kind::AlternativeConstructorValue: {
             const auto& alt =
                 cast<AlternativeConstructorValue>(*act->results()[0]);
-            Nonnull<const Value*> arg =
-                CopyVal(arena, act->results()[1], exp.source_loc());
-            return Done{arena->New<AlternativeValue>(alt.alt_name(),
-                                                     alt.choice_name(), arg)};
+            return Done{arena->New<AlternativeValue>(
+                alt.alt_name(), alt.choice_name(), act->results()[1])};
           }
           case Value::Kind::FunctionValue:
             return CallFunction{
@@ -937,9 +934,7 @@ auto Interpreter::StepStmt() -> Transition {
       } else {
         //    { {v :: return [] :: C, E, F} :: {C', E', F'} :: S, H}
         // -> { {v :: C', E', F'} :: S, H}
-        Nonnull<const Value*> ret_val =
-            CopyVal(arena, act->results()[0], stmt.source_loc());
-        return UnwindFunctionCall{ret_val};
+        return UnwindFunctionCall{act->results()[0]};
       }
     case Statement::Kind::Sequence: {
       //    { { (s1,s2) :: C, E, F} :: S, H}
diff --git a/executable_semantics/interpreter/value.cpp b/executable_semantics/interpreter/value.cpp
index b5d16c9eebda..f867dfb5d810 100644
--- a/executable_semantics/interpreter/value.cpp
+++ b/executable_semantics/interpreter/value.cpp
@@ -291,90 +291,6 @@ void Value::Print(llvm::raw_ostream& out) const {
   }
 }
 
-auto CopyVal(Nonnull<Arena*> arena, Nonnull<const Value*> val,
-             SourceLocation source_loc) -> Nonnull<const Value*> {
-  switch (val->kind()) {
-    case Value::Kind::TupleValue: {
-      std::vector<Nonnull<const Value*>> elements;
-      for (Nonnull<const Value*> element : cast<TupleValue>(*val).elements()) {
-        elements.push_back(CopyVal(arena, element, source_loc));
-      }
-      return arena->New<TupleValue>(std::move(elements));
-    }
-    case Value::Kind::AlternativeValue: {
-      const auto& alt = cast<AlternativeValue>(*val);
-      Nonnull<const Value*> arg = CopyVal(arena, &alt.argument(), source_loc);
-      return arena->New<AlternativeValue>(alt.alt_name(), alt.choice_name(),
-                                          arg);
-    }
-    case Value::Kind::StructValue: {
-      std::vector<StructElement> elements;
-      for (const StructElement& element : cast<StructValue>(*val).elements()) {
-        elements.push_back(
-            {.name = element.name,
-             .value = CopyVal(arena, element.value, source_loc)});
-      }
-      return arena->New<StructValue>(std::move(elements));
-    }
-    case Value::Kind::NominalClassValue: {
-      const auto& s = cast<NominalClassValue>(*val);
-      Nonnull<const Value*> inits = CopyVal(arena, &s.inits(), source_loc);
-      return arena->New<NominalClassValue>(&s.type(), inits);
-    }
-    case Value::Kind::IntValue:
-      return arena->New<IntValue>(cast<IntValue>(*val).value());
-    case Value::Kind::BoolValue:
-      return arena->New<BoolValue>(cast<BoolValue>(*val).value());
-    case Value::Kind::FunctionValue: {
-      const auto& fn_value = cast<FunctionValue>(*val);
-      return arena->New<FunctionValue>(fn_value.name(), &fn_value.parameters(),
-                                       fn_value.body());
-    }
-    case Value::Kind::PointerValue:
-      return arena->New<PointerValue>(cast<PointerValue>(*val).value());
-    case Value::Kind::ContinuationValue:
-      return arena->New<ContinuationValue>(
-          &cast<ContinuationValue>(*val).stack());
-    case Value::Kind::FunctionType: {
-      const auto& fn_type = cast<FunctionType>(*val);
-      return arena->New<FunctionType>(
-          fn_type.deduced(), CopyVal(arena, &fn_type.parameters(), source_loc),
-          CopyVal(arena, &fn_type.return_type(), source_loc));
-    }
-    case Value::Kind::PointerType:
-      return arena->New<PointerType>(
-          CopyVal(arena, &cast<PointerType>(*val).type(), source_loc));
-    case Value::Kind::IntType:
-      return arena->New<IntType>();
-    case Value::Kind::BoolType:
-      return arena->New<BoolType>();
-    case Value::Kind::TypeType:
-      return arena->New<TypeType>();
-    case Value::Kind::AutoType:
-      return arena->New<AutoType>();
-    case Value::Kind::ContinuationType:
-      return arena->New<ContinuationType>();
-    case Value::Kind::StringType:
-      return arena->New<StringType>();
-    case Value::Kind::StringValue:
-      return arena->New<StringValue>(cast<StringValue>(*val).value());
-    case Value::Kind::StructType: {
-      VarValues fields;
-      for (const auto& [name, type] : cast<StructType>(*val).fields()) {
-        fields.push_back({name, CopyVal(arena, type, source_loc)});
-      }
-      return arena->New<StructType>(fields);
-    }
-    case Value::Kind::VariableType:
-    case Value::Kind::NominalClassType:
-    case Value::Kind::ChoiceType:
-    case Value::Kind::BindingPlaceholderValue:
-    case Value::Kind::AlternativeConstructorValue:
-      // TODO: These should be copied so that they don't get destructed.
-      return val;
-  }
-}
-
 auto TypeEqual(Nonnull<const Value*> t1, Nonnull<const Value*> t2) -> bool {
   if (t1->kind() != t2->kind()) {
     return false;
diff --git a/executable_semantics/interpreter/value.h b/executable_semantics/interpreter/value.h
index a29664043bea..46aea8235d07 100644
--- a/executable_semantics/interpreter/value.h
+++ b/executable_semantics/interpreter/value.h
@@ -540,9 +540,6 @@ class StringValue : public Value {
   std::string value_;
 };
 
-auto CopyVal(Nonnull<Arena*> arena, Nonnull<const Value*> val,
-             SourceLocation source_loc) -> Nonnull<const Value*>;
-
 auto TypeEqual(Nonnull<const Value*> t1, Nonnull<const Value*> t2) -> bool;
 auto ValueEqual(Nonnull<const Value*> v1, Nonnull<const Value*> v2,
                 SourceLocation source_loc) -> bool;