Separate alive flag from the Value class (#408)

* separate the alive flag from the Value class

* restored KillValue, added KillAddress

* added comments

* Update executable_semantics/interpreter/interpreter.cpp

Co-authored-by: Dave Abrahams <dabrahams@google.com>

* Update executable_semantics/interpreter/interpreter.cpp

Co-authored-by: Dave Abrahams <dabrahams@google.com>

* Update executable_semantics/interpreter/interpreter.cpp

Co-authored-by: Dave Abrahams <dabrahams@google.com>

* finish edits from Dave

Co-authored-by: Dave Abrahams <dabrahams@google.com>

executable_semantics/interpreter/interpreter.cpp

@@ -49,23 +49,27 @@ auto AllocateValue(Value* v) -> Address {
   // or to leave it up to the caller.
   Address a = state->heap.size();
   state->heap.push_back(v);
+  state->alive.push_back(true);
   return a;
 }
 
 auto CopyVal(Value* val, int line_num) -> Value* {
-  CheckAlive(val, line_num);
   switch (val->tag) {
     case ValKind::TupleV: {
       auto elts = new std::vector<std::pair<std::string, Address>>();
       for (auto& i : *val->u.tuple.elts) {
+        CheckAlive(i.second, line_num);
         Value* elt = CopyVal(state->heap[i.second], line_num);
-        elts->push_back(make_pair(i.first, AllocateValue(elt)));
+        Address new_address = AllocateValue(elt);
+        elts->push_back(make_pair(i.first, new_address));
       }
       return MakeTupleVal(elts);
     }
     case ValKind::AltV: {
-      Value* arg = CopyVal(val->u.alt.arg, line_num);
-      return MakeAltVal(*val->u.alt.alt_name, *val->u.alt.choice_name, arg);
+      Value* arg = CopyVal(state->heap[val->u.alt.argument], line_num);
+      Address argument_address = AllocateValue(arg);
+      return MakeAltVal(*val->u.alt.alt_name, *val->u.alt.choice_name,
+                        argument_address);
     }
     case ValKind::StructV: {
       Value* inits = CopyVal(val->u.struct_val.inits, line_num);
@@ -112,24 +116,20 @@ auto CopyVal(Value* val, int line_num) -> Value* {
   }
 }
 
-void KillValue(Value* val) {
-  val->alive = false;
+void KillObject(Address address);
+
+// Marks all of the sub-objects of this value as dead.
+void KillSubObjects(Value* val) {
   switch (val->tag) {
     case ValKind::AltV:
-      KillValue(val->u.alt.arg);
+      KillObject(val->u.alt.argument);
       break;
     case ValKind::StructV:
-      KillValue(val->u.struct_val.inits);
+      KillSubObjects(val->u.struct_val.inits);
       break;
     case ValKind::TupleV:
       for (auto& elt : *val->u.tuple.elts) {
-        if (state->heap[elt.second]->alive) {
-          KillValue(state->heap[elt.second]);
-        } else {
-          std::cerr << "runtime error, killing an already dead value"
-                    << std::endl;
-          exit(-1);
-        }
+        KillObject(elt.second);
       }
       break;
     default:
@@ -137,6 +137,17 @@ void KillValue(Value* val) {
   }
 }
 
+// Marks the object at this address, and all of its sub-objects, as dead.
+void KillObject(Address address) {
+  if (state->alive[address]) {
+    state->alive[address] = false;
+    KillSubObjects(state->heap[address]);
+  } else {
+    std::cerr << "runtime error, killing an already dead value" << std::endl;
+    exit(-1);
+  }
+}
+
 void PrintEnv(Env env, std::ostream& out) {
   for (const auto& [name, value] : env) {
     out << name << ": ";
@@ -198,7 +209,6 @@ void PrintState(std::ostream& out) {
 //
 
 auto ValToInt(Value* v, int line_num) -> int {
-  CheckAlive(v, line_num);
   switch (v->tag) {
     case ValKind::IntV:
       return v->u.integer;
@@ -210,7 +220,6 @@ auto ValToInt(Value* v, int line_num) -> int {
 }
 
 auto ValToBool(Value* v, int line_num) -> int {
-  CheckAlive(v, line_num);
   switch (v->tag) {
     case ValKind::BoolV:
       return v->u.boolean;
@@ -221,7 +230,7 @@ auto ValToBool(Value* v, int line_num) -> int {
 }
 
 auto ValToPtr(Value* v, int line_num) -> Address {
-  CheckAlive(v, line_num);
+  CheckAlive(v->u.ptr, line_num);
   switch (v->tag) {
     case ValKind::PtrV:
       return v->u.ptr;
@@ -317,7 +326,6 @@ auto VariableDeclaration::InitGlobals(Env& globals) const -> void {
 //       E is the environment (functions + parameters + locals)
 //       F is the function
 void CallFunction(int line_num, std::vector<Value*> operas, State* state) {
-  CheckAlive(operas[0], line_num);
   switch (operas[0]->tag) {
     case ValKind::FunV: {
       // Bind arguments to parameters
@@ -345,8 +353,9 @@ void CallFunction(int line_num, std::vector<Value*> operas, State* state) {
     }
     case ValKind::AltConsV: {
       Value* arg = CopyVal(operas[1], line_num);
-      Value* av = MakeAltVal(*operas[0]->u.alt_cons.alt_name,
-                             *operas[0]->u.alt_cons.choice_name, arg);
+      Value* av =
+          MakeAltVal(*operas[0]->u.alt_cons.alt_name,
+                     *operas[0]->u.alt_cons.choice_name, AllocateValue(arg));
       Frame* frame = state->stack.Top();
       frame->todo.Push(MakeValAct(av));
       break;
@@ -366,7 +375,7 @@ void KillScope(int line_num, Scope* scope) {
       std::cerr << "internal error in KillScope" << std::endl;
       exit(-1);
     }
-    KillValue(state->heap[*a]);
+    KillObject(*a);
   }
 }
 
@@ -444,7 +453,8 @@ auto PatternMatch(Value* p, Value* v, Env env, std::list<std::string>* vars,
             return std::nullopt;
           }
           std::optional<Env> env_with_matches =
-              PatternMatch(p->u.alt.arg, v->u.alt.arg, env, vars, line_num);
+              PatternMatch(state->heap[p->u.alt.argument],
+                           state->heap[v->u.alt.argument], env, vars, line_num);
           if (!env_with_matches) {
             return std::nullopt;
           }
@@ -525,7 +535,8 @@ void PatternAssignment(Value* pat, Value* val, int line_num) {
             std::cerr << "internal error in pattern assignment" << std::endl;
             exit(-1);
           }
-          PatternAssignment(pat->u.alt.arg, val->u.alt.arg, line_num);
+          PatternAssignment(state->heap[pat->u.alt.argument],
+                            state->heap[val->u.alt.argument], line_num);
           break;
         }
         default:
@@ -569,7 +580,7 @@ void StepLvalue() {
         exit(-1);
       }
       Value* v = MakePtrVal(*pointer);
-      CheckAlive(v, exp->line_num);
+      CheckAlive(*pointer, exp->line_num);
       frame->todo.Pop();
       frame->todo.Push(MakeValAct(v));
       break;
@@ -950,7 +961,7 @@ void HandleValue() {
   }
   switch (act->tag) {
     case ActionKind::DeleteTmpAction: {
-      KillValue(state->heap[act->u.delete_tmp]);
+      KillObject(act->u.delete_tmp);
       frame->todo.Pop(2);
       frame->todo.Push(val_act);
       break;

executable_semantics/interpreter/interpreter.h

@@ -41,6 +41,7 @@ struct Frame {
 struct State {
   Stack<Frame*> stack;
   std::vector<Value*> heap;
+  std::vector<bool> alive;
 };
 
 extern State* state;

executable_semantics/interpreter/value.cpp

@@ -38,7 +38,6 @@ auto FieldsEqual(VarValues* ts1, VarValues* ts2) -> bool {
 
 auto MakeIntVal(int i) -> Value* {
   auto* v = new Value();
-  v->alive = true;
   v->tag = ValKind::IntV;
   v->u.integer = i;
   return v;
@@ -46,7 +45,6 @@ auto MakeIntVal(int i) -> Value* {
 
 auto MakeBoolVal(bool b) -> Value* {
   auto* v = new Value();
-  v->alive = true;
   v->tag = ValKind::BoolV;
   v->u.boolean = b;
   return v;
@@ -54,7 +52,6 @@ auto MakeBoolVal(bool b) -> Value* {
 
 auto MakeFunVal(std::string name, Value* param, Statement* body) -> Value* {
   auto* v = new Value();
-  v->alive = true;
   v->tag = ValKind::FunV;
   v->u.fun.name = new std::string(std::move(name));
   v->u.fun.param = param;
@@ -64,7 +61,6 @@ auto MakeFunVal(std::string name, Value* param, Statement* body) -> Value* {
 
 auto MakePtrVal(Address addr) -> Value* {
   auto* v = new Value();
-  v->alive = true;
   v->tag = ValKind::PtrV;
   v->u.ptr = addr;
   return v;
@@ -72,7 +68,6 @@ auto MakePtrVal(Address addr) -> Value* {
 
 auto MakeStructVal(Value* type, Value* inits) -> Value* {
   auto* v = new Value();
-  v->alive = true;
   v->tag = ValKind::StructV;
   v->u.struct_val.type = type;
   v->u.struct_val.inits = inits;
@@ -82,26 +77,23 @@ auto MakeStructVal(Value* type, Value* inits) -> Value* {
 auto MakeTupleVal(std::vector<std::pair<std::string, Address>>* elts)
     -> Value* {
   auto* v = new Value();
-  v->alive = true;
   v->tag = ValKind::TupleV;
   v->u.tuple.elts = elts;
   return v;
 }
 
-auto MakeAltVal(std::string alt_name, std::string choice_name, Value* arg)
+auto MakeAltVal(std::string alt_name, std::string choice_name, Address argument)
     -> Value* {
   auto* v = new Value();
-  v->alive = true;
   v->tag = ValKind::AltV;
   v->u.alt.alt_name = new std::string(std::move(alt_name));
   v->u.alt.choice_name = new std::string(std::move(choice_name));
-  v->u.alt.arg = arg;
+  v->u.alt.argument = argument;
   return v;
 }
 
 auto MakeAltCons(std::string alt_name, std::string choice_name) -> Value* {
   auto* v = new Value();
-  v->alive = true;
   v->tag = ValKind::AltConsV;
   v->u.alt.alt_name = new std::string(std::move(alt_name));
   v->u.alt.choice_name = new std::string(std::move(choice_name));
@@ -110,7 +102,6 @@ auto MakeAltCons(std::string alt_name, std::string choice_name) -> Value* {
 
 auto MakeVarPatVal(std::string name, Value* type) -> Value* {
   auto* v = new Value();
-  v->alive = true;
   v->tag = ValKind::VarPatV;
   v->u.var_pat.name = new std::string(std::move(name));
   v->u.var_pat.type = type;
@@ -119,7 +110,6 @@ auto MakeVarPatVal(std::string name, Value* type) -> Value* {
 
 auto MakeVarTypeVal(std::string name) -> Value* {
   auto* v = new Value();
-  v->alive = true;
   v->tag = ValKind::VarTV;
   v->u.var_type = new std::string(std::move(name));
   return v;
@@ -127,35 +117,30 @@ auto MakeVarTypeVal(std::string name) -> Value* {
 
 auto MakeIntTypeVal() -> Value* {
   auto* v = new Value();
-  v->alive = true;
   v->tag = ValKind::IntTV;
   return v;
 }
 
 auto MakeBoolTypeVal() -> Value* {
   auto* v = new Value();
-  v->alive = true;
   v->tag = ValKind::BoolTV;
   return v;
 }
 
 auto MakeTypeTypeVal() -> Value* {
   auto* v = new Value();
-  v->alive = true;
   v->tag = ValKind::TypeTV;
   return v;
 }
 
 auto MakeAutoTypeVal() -> Value* {
   auto* v = new Value();
-  v->alive = true;
   v->tag = ValKind::AutoTV;
   return v;
 }
 
 auto MakeFunTypeVal(Value* param, Value* ret) -> Value* {
   auto* v = new Value();
-  v->alive = true;
   v->tag = ValKind::FunctionTV;
   v->u.fun_type.param = param;
   v->u.fun_type.ret = ret;
@@ -164,7 +149,6 @@ auto MakeFunTypeVal(Value* param, Value* ret) -> Value* {
 
 auto MakePtrTypeVal(Value* type) -> Value* {
   auto* v = new Value();
-  v->alive = true;
   v->tag = ValKind::PointerTV;
   v->u.ptr_type.type = type;
   return v;
@@ -173,7 +157,6 @@ auto MakePtrTypeVal(Value* type) -> Value* {
 auto MakeStructTypeVal(std::string name, VarValues* fields, VarValues* methods)
     -> Value* {
   auto* v = new Value();
-  v->alive = true;
   v->tag = ValKind::StructTV;
   v->u.struct_type.name = new std::string(std::move(name));
   v->u.struct_type.fields = fields;
@@ -183,7 +166,6 @@ auto MakeStructTypeVal(std::string name, VarValues* fields, VarValues* methods)
 
 auto MakeTupleTypeVal(VarValues* fields) -> Value* {
   auto* v = new Value();
-  v->alive = true;
   v->tag = ValKind::TupleTV;
   v->u.tuple_type.fields = fields;
   return v;
@@ -191,7 +173,6 @@ auto MakeTupleTypeVal(VarValues* fields) -> Value* {
 
 auto MakeVoidTypeVal() -> Value* {
   auto* v = new Value();
-  v->alive = true;
   v->tag = ValKind::TupleTV;
   v->u.tuple_type.fields = new VarValues();
   return v;
@@ -201,7 +182,6 @@ auto MakeChoiceTypeVal(std::string name,
                        std::list<std::pair<std::string, Value*>>* alts)
     -> Value* {
   auto* v = new Value();
-  v->alive = true;
   v->tag = ValKind::ChoiceTV;
   // Transitional leak: when we get rid of all pointers, this will disappear.
   v->u.choice_type.name = new std::string(name);
@@ -210,9 +190,6 @@ auto MakeChoiceTypeVal(std::string name,
 }
 
 void PrintValue(Value* val, std::ostream& out) {
-  if (!val->alive) {
-    out << "!!";
-  }
   switch (val->tag) {
     case ValKind::AltConsV: {
       out << *val->u.alt_cons.choice_name << "." << *val->u.alt_cons.alt_name;
@@ -226,7 +203,7 @@ void PrintValue(Value* val, std::ostream& out) {
     case ValKind::AltV: {
       out << "alt " << *val->u.alt.choice_name << "." << *val->u.alt.alt_name
           << " ";
-      PrintValue(val->u.alt.arg, out);
+      PrintValue(state->heap[val->u.alt.argument], out);
       break;
     }
     case ValKind::StructV: {
@@ -358,8 +335,6 @@ static auto FieldsValueEqual(VarValues* ts1, VarValues* ts2, int line_num)
 }
 
 auto ValueEqual(Value* v1, Value* v2, int line_num) -> bool {
-  CheckAlive(v1, line_num);
-  CheckAlive(v2, line_num);
   if (v1->tag != v2->tag) {
     return false;
   }
@@ -369,6 +344,8 @@ auto ValueEqual(Value* v1, Value* v2, int line_num) -> bool {
     case ValKind::BoolV:
       return v1->u.boolean == v2->u.boolean;
     case ValKind::PtrV:
+      CheckAlive(v1->u.ptr, line_num);
+      CheckAlive(v2->u.ptr, line_num);
       return v1->u.ptr == v2->u.ptr;
     case ValKind::FunV:
       return v1->u.fun.body == v2->u.fun.body;
@@ -391,10 +368,10 @@ auto ToInteger(Value* v) -> int {
   }
 }
 
-void CheckAlive(Value* v, int line_num) {
-  if (!v->alive) {
+void CheckAlive(Address address, int line_num) {
+  if (!state->alive[address]) {
     std::cerr << line_num << ": undefined behavior: access to dead value ";
-    PrintValue(v, std::cerr);
+    PrintValue(state->heap[address], std::cerr);
     std::cerr << std::endl;
     exit(-1);
   }

executable_semantics/interpreter/value.h

@@ -43,7 +43,6 @@ enum class ValKind {
 
 struct Value {
   ValKind tag;
-  bool alive;
   union {
     int integer;
     bool boolean;
@@ -63,7 +62,7 @@ struct Value {
     struct {
       std::string* alt_name;
       std::string* choice_name;
-      Value* arg;
+      Address argument;
     } alt;
     struct {
       std::vector<std::pair<std::string, Address>>* elts;
@@ -107,7 +106,7 @@ auto MakeFunVal(std::string name, Value* param, Statement* body) -> Value*;
 auto MakePtrVal(Address addr) -> Value*;
 auto MakeStructVal(Value* type, Value* inits) -> Value*;
 auto MakeTupleVal(std::vector<std::pair<std::string, Address>>* elts) -> Value*;
-auto MakeAltVal(std::string alt_name, std::string choice_name, Value* arg)
+auto MakeAltVal(std::string alt_name, std::string choice_name, Address argument)
     -> Value*;
 auto MakeAltCons(std::string alt_name, std::string choice_name) -> Value*;
 
@@ -132,7 +131,7 @@ auto TypeEqual(Value* t1, Value* t2) -> bool;
 auto ValueEqual(Value* v1, Value* v2, int line_num) -> bool;
 
 auto ToInteger(Value* v) -> int;
-void CheckAlive(Value* v, int line_num);
+void CheckAlive(Address a, int line_num);
 
 }  // namespace Carbon