From e022ff106f41a4cc22b534ea239baf0fd1cb9cf6 Mon Sep 17 00:00:00 2001 From: Geoff Romer Date: Fri, 23 Apr 2021 13:25:51 -0700 Subject: [PATCH] Factor `Heap` class out of `State` (#491) Additional miscellaneous cleanup: - Use the term "deallocate" instead of "kill" in function names, for symmetry with "allocate". - Drop an unnecessary memory allocation in `AllocateValue`. - Implement `PrintHeap` in terms of `PrintAddress`, so that dead values are flagged with `!!`. --- .../interpreter/interpreter.cpp | 149 +++++++++--------- .../interpreter/interpreter.h | 43 +++-- .../interpreter/typecheck.cpp | 24 +-- executable_semantics/interpreter/value.cpp | 20 +-- 4 files changed, 127 insertions(+), 109 deletions(-) diff --git a/executable_semantics/interpreter/interpreter.cpp b/executable_semantics/interpreter/interpreter.cpp index 785ff8d19ddb..87f712eeb7bf 100644 --- a/executable_semantics/interpreter/interpreter.cpp +++ b/executable_semantics/interpreter/interpreter.cpp @@ -31,31 +31,33 @@ void HandleValue(); // Auxiliary Functions // -auto State::AllocateValue(const Value* v) -> Address { +auto Heap::AllocateValue(const Value* v) -> Address { // Putting the following two side effects together in this function // ensures that we don't do anything else in between, which is really bad! // Consider whether to include a copy of the input v in this function // or to leave it up to the caller. - Address a = heap.size(); - heap.push_back(new Value(*v)); - this->alive.push_back(true); + assert(v != nullptr); + Address a = values_.size(); + values_.push_back(v); + alive_.push_back(true); return a; } -auto State::ReadFromMemory(Address a, int line_num) -> const Value* { +auto Heap::Read(Address a, int line_num) -> const Value* { this->CheckAlive(a, line_num); - return heap[a]; + return values_[a]; } -auto State::WriteToMemory(Address a, const Value* v, int line_num) -> void { +auto Heap::Write(Address a, const Value* v, int line_num) -> void { + assert(v != nullptr); this->CheckAlive(a, line_num); - heap[a] = v; + values_[a] = v; } -void State::CheckAlive(Address address, int line_num) { - if (!this->alive[address]) { +void Heap::CheckAlive(Address address, int line_num) { + if (!alive_[address]) { std::cerr << line_num << ": undefined behavior: access to dead value "; - PrintValue(heap[address], std::cerr); + PrintValue(values_[address], std::cerr); std::cerr << std::endl; exit(-1); } @@ -67,16 +69,16 @@ auto CopyVal(const Value* val, int line_num) -> const Value* { auto elts = new std::vector>(); for (auto& i : *val->u.tuple.elts) { const Value* elt = - CopyVal(state->ReadFromMemory(i.second, line_num), line_num); - Address new_address = state->AllocateValue(elt); + CopyVal(state->heap.Read(i.second, line_num), line_num); + Address new_address = state->heap.AllocateValue(elt); elts->push_back(make_pair(i.first, new_address)); } return MakeTupleVal(elts); } case ValKind::AltV: { - const Value* arg = CopyVal( - state->ReadFromMemory(val->u.alt.argument, line_num), line_num); - Address argument_address = state->AllocateValue(arg); + const Value* arg = + CopyVal(state->heap.Read(val->u.alt.argument, line_num), line_num); + Address argument_address = state->heap.AllocateValue(arg); return MakeAltVal(*val->u.alt.alt_name, *val->u.alt.choice_name, argument_address); } @@ -122,18 +124,17 @@ auto CopyVal(const Value* val, int line_num) -> const Value* { } } -// Marks all of the sub-objects of this value as dead. -void KillSubObjects(const Value* val) { +void Heap::DeallocateSubObjects(const Value* val) { switch (val->tag) { case ValKind::AltV: - state->KillObject(val->u.alt.argument); + Deallocate(val->u.alt.argument); break; case ValKind::StructV: - KillSubObjects(val->u.struct_val.inits); + DeallocateSubObjects(val->u.struct_val.inits); break; case ValKind::TupleV: for (auto& elt : *val->u.tuple.elts) { - state->KillObject(elt.second); + Deallocate(elt.second); } break; default: @@ -141,12 +142,13 @@ void KillSubObjects(const Value* val) { } } -void State::KillObject(Address address) { - if (this->alive[address]) { - this->alive[address] = false; - KillSubObjects(heap[address]); +void Heap::Deallocate(Address address) { + if (alive_[address]) { + alive_[address] = false; + DeallocateSubObjects(values_[address]); } else { - std::cerr << "runtime error, killing an already dead value" << std::endl; + std::cerr << "runtime error, deallocating an already dead value" + << std::endl; exit(-1); } } @@ -154,7 +156,7 @@ void State::KillObject(Address address) { void PrintEnv(Env values, std::ostream& out) { for (const auto& [name, address] : values) { out << name << ": "; - state->PrintAddress(address, out); + state->heap.PrintAddress(address, out); out << ", "; } } @@ -180,17 +182,20 @@ void PrintStack(Stack ls, std::ostream& out) { } } -void State::PrintHeap(std::ostream& out) { - for (auto& iter : heap) { - if (iter) { - PrintValue(iter, out); - } else { - out << "_"; - } +void Heap::PrintHeap(std::ostream& out) { + for (Address i = 0; i < values_.size(); ++i) { + PrintAddress(i, out); out << ", "; } } +auto Heap::PrintAddress(Address a, std::ostream& out) -> void { + if (!alive_[a]) { + out << "!!"; + } + PrintValue(values_[a], out); +} + auto CurrentEnv(State* state) -> Env { Frame* frame = state->stack.Top(); return frame->scopes.Top()->values; @@ -201,7 +206,7 @@ void PrintState(std::ostream& out) { out << "stack: "; PrintStack(state->stack, out); out << std::endl << "heap: "; - state->PrintHeap(out); + state->heap.PrintHeap(out); if (!state->stack.IsEmpty() && !state->stack.Top()->scopes.IsEmpty()) { out << std::endl << "values: "; PrintEnv(CurrentEnv(state), out); @@ -300,7 +305,7 @@ auto ChoiceDeclaration::InitGlobals(Env& globals) const -> void { alts->push_back(make_pair(kv.first, t)); } auto ct = MakeChoiceTypeVal(name, alts); - auto a = state->AllocateValue(ct); + auto a = state->heap.AllocateValue(ct); globals.Set(name, a); } @@ -318,7 +323,7 @@ auto StructDeclaration::InitGlobals(Env& globals) const -> void { } } auto st = MakeStructTypeVal(*definition.name, fields, methods); - auto a = state->AllocateValue(st); + auto a = state->heap.AllocateValue(st); globals.Set(*definition.name, a); } @@ -326,7 +331,7 @@ auto FunctionDeclaration::InitGlobals(Env& globals) const -> void { Env values; auto pt = InterpExp(values, definition->param_pattern); auto f = MakeFunVal(definition->name, pt, definition->body); - Address a = state->AllocateValue(f); + Address a = state->heap.AllocateValue(f); globals.Set(definition->name, a); } @@ -334,7 +339,7 @@ auto FunctionDeclaration::InitGlobals(Env& globals) const -> void { // result of evaluating the initializer. auto VariableDeclaration::InitGlobals(Env& globals) const -> void { auto v = InterpExp(globals, initializer); - Address a = state->AllocateValue(v); + Address a = state->heap.AllocateValue(v); globals.Set(name, a); } @@ -373,7 +378,7 @@ void CallFunction(int line_num, std::vector operas, const Value* arg = CopyVal(operas[1], line_num); const Value* av = MakeAltVal(*operas[0]->u.alt_cons.alt_name, *operas[0]->u.alt_cons.choice_name, - state->AllocateValue(arg)); + state->heap.AllocateValue(arg)); Frame* frame = state->stack.Top(); frame->todo.Push(MakeValAct(av)); break; @@ -386,20 +391,20 @@ void CallFunction(int line_num, std::vector operas, } } -void KillScope(int line_num, Scope* scope) { +void DeallocateScope(int line_num, Scope* scope) { for (const auto& l : scope->locals) { std::optional
a = scope->values.Get(l); if (!a) { - std::cerr << "internal error in KillScope" << std::endl; + std::cerr << "internal error in DeallocateScope" << std::endl; exit(-1); } - state->KillObject(*a); + state->heap.Deallocate(*a); } } -void KillLocals(int line_num, Frame* frame) { +void DeallocateLocals(int line_num, Frame* frame) { for (auto scope : frame->scopes) { - KillScope(line_num, scope); + DeallocateScope(line_num, scope); } } @@ -409,7 +414,7 @@ void CreateTuple(Frame* frame, Action* act, const Expression* /*exp*/) { auto elts = new std::vector>(); auto f = act->u.exp->u.tuple.fields->begin(); for (auto i = act->results.begin(); i != act->results.end(); ++i, ++f) { - Address a = state->AllocateValue(*i); // copy? + Address a = state->heap.AllocateValue(*i); // copy? elts->push_back(make_pair(f->first, a)); } const Value* tv = MakeTupleVal(elts); @@ -427,7 +432,7 @@ auto PatternMatch(const Value* p, const Value* v, Env values, -> std::optional { switch (p->tag) { case ValKind::VarPatV: { - Address a = state->AllocateValue(CopyVal(v, line_num)); + Address a = state->heap.AllocateValue(CopyVal(v, line_num)); vars->push_back(*p->u.var_pat.name); values.Set(*p->u.var_pat.name, a); return values; @@ -449,8 +454,8 @@ auto PatternMatch(const Value* p, const Value* v, Env values, exit(-1); } std::optional matches = PatternMatch( - state->ReadFromMemory(elt.second, line_num), - state->ReadFromMemory(*a, line_num), values, vars, line_num); + state->heap.Read(elt.second, line_num), + state->heap.Read(*a, line_num), values, vars, line_num); if (!matches) { return std::nullopt; } @@ -473,8 +478,8 @@ auto PatternMatch(const Value* p, const Value* v, Env values, return std::nullopt; } std::optional matches = - PatternMatch(state->ReadFromMemory(p->u.alt.argument, line_num), - state->ReadFromMemory(v->u.alt.argument, line_num), + PatternMatch(state->heap.Read(p->u.alt.argument, line_num), + state->heap.Read(v->u.alt.argument, line_num), values, vars, line_num); if (!matches) { return std::nullopt; @@ -515,8 +520,8 @@ auto PatternMatch(const Value* p, const Value* v, Env values, void PatternAssignment(const Value* pat, const Value* val, int line_num) { switch (pat->tag) { case ValKind::PtrV: - state->WriteToMemory(ValToPtr(pat, line_num), CopyVal(val, line_num), - line_num); + state->heap.Write(ValToPtr(pat, line_num), CopyVal(val, line_num), + line_num); break; case ValKind::TupleV: { switch (val->tag) { @@ -534,8 +539,8 @@ void PatternAssignment(const Value* pat, const Value* val, int line_num) { std::cerr << std::endl; exit(-1); } - PatternAssignment(state->ReadFromMemory(elt.second, line_num), - state->ReadFromMemory(*a, line_num), line_num); + PatternAssignment(state->heap.Read(elt.second, line_num), + state->heap.Read(*a, line_num), line_num); } break; } @@ -557,9 +562,9 @@ void PatternAssignment(const Value* pat, const Value* val, int line_num) { std::cerr << "internal error in pattern assignment" << std::endl; exit(-1); } - PatternAssignment( - state->ReadFromMemory(pat->u.alt.argument, line_num), - state->ReadFromMemory(val->u.alt.argument, line_num), line_num); + PatternAssignment(state->heap.Read(pat->u.alt.argument, line_num), + state->heap.Read(val->u.alt.argument, line_num), + line_num); break; } default: @@ -699,7 +704,7 @@ void StepExp() { << *(exp->u.variable.name) << "`" << std::endl; exit(-1); } - const Value* pointee = state->ReadFromMemory(*pointer, exp->line_num); + const Value* pointee = state->heap.Read(*pointer, exp->line_num); frame->todo.Pop(1); frame->todo.Push(MakeValAct(pointee)); break; @@ -832,7 +837,7 @@ void StepStmt() { frame->todo.Pop(1); while (!frame->todo.IsEmpty() && !IsWhileAct(frame->todo.Top())) { if (IsBlockAct(frame->todo.Top())) { - KillScope(stmt->line_num, frame->scopes.Top()); + DeallocateScope(stmt->line_num, frame->scopes.Top()); frame->scopes.Pop(1); } frame->todo.Pop(1); @@ -845,7 +850,7 @@ void StepStmt() { frame->todo.Pop(1); while (!frame->todo.IsEmpty() && !IsWhileAct(frame->todo.Top())) { if (IsBlockAct(frame->todo.Top())) { - KillScope(stmt->line_num, frame->scopes.Top()); + DeallocateScope(stmt->line_num, frame->scopes.Top()); frame->scopes.Pop(1); } frame->todo.Pop(1); @@ -863,7 +868,7 @@ void StepStmt() { } } else { Scope* scope = frame->scopes.Top(); - KillScope(stmt->line_num, scope); + DeallocateScope(stmt->line_num, scope); frame->scopes.Pop(1); frame->todo.Pop(1); } @@ -919,7 +924,7 @@ void StepStmt() { todo.Push(MakeStmtAct(stmt->u.continuation.body)); Frame* continuation_frame = new Frame("__continuation", scopes, todo); Address continuation_address = - state->AllocateValue(MakeContinuation({continuation_frame})); + state->heap.AllocateValue(MakeContinuation({continuation_frame})); // Store the continuation's address in the frame. continuation_frame->continuation = continuation_address; // Bind the continuation object to the continuation variable @@ -942,14 +947,14 @@ void StepStmt() { paused.push_back(state->stack.Pop()); } while (!paused.back()->IsContinuation()); // Update the continuation with the paused stack. - state->WriteToMemory(paused.back()->continuation, - MakeContinuation(paused), stmt->line_num); + state->heap.Write(paused.back()->continuation, MakeContinuation(paused), + stmt->line_num); break; } } auto GetMember(Address a, const std::string& f, int line_num) -> Address { - const Value* v = state->ReadFromMemory(a, line_num); + const Value* v = state->heap.Read(a, line_num); switch (v->tag) { case ValKind::StructV: { auto a = FindTupleField(f, v->u.struct_val.inits); @@ -979,7 +984,7 @@ auto GetMember(Address a, const std::string& f, int line_num) -> Address { exit(-1); } auto ac = MakeAltCons(f, *v->u.choice_type.name); - return state->AllocateValue(ac); + return state->heap.AllocateValue(ac); } default: std::cerr << "field access not allowed for value "; @@ -1032,13 +1037,13 @@ void HandleValue() { } switch (act->tag) { case ActionKind::DeleteTmpAction: { - state->KillObject(act->u.delete_tmp); + state->heap.Deallocate(act->u.delete_tmp); frame->todo.Pop(2); frame->todo.Push(val_act); break; } case ActionKind::ExpToLValAction: { - Address a = state->AllocateValue(act->results[0]); + Address a = state->heap.AllocateValue(act->results[0]); auto del = MakeDeleteAct(a); frame->todo.Pop(2); InsertDelete(del, frame->todo); @@ -1145,7 +1150,7 @@ void HandleValue() { exit(-1); } frame->todo.Pop(2); - const Value* element = state->ReadFromMemory(*a, exp->line_num); + const Value* element = state->heap.Read(*a, exp->line_num); frame->todo.Push(MakeValAct(element)); break; } @@ -1164,7 +1169,7 @@ void HandleValue() { // -> { { v_f :: C, E, F} : S, H} auto a = GetMember(ValToPtr(act->results[0], exp->line_num), *exp->u.get_field.field, exp->line_num); - const Value* element = state->ReadFromMemory(a, exp->line_num); + const Value* element = state->heap.Read(a, exp->line_num); frame->todo.Pop(2); frame->todo.Push(MakeValAct(element)); break; @@ -1378,7 +1383,7 @@ void HandleValue() { // { {v :: return [] :: C, E, F} :: {C', E', F'} :: S, H} // -> { {v :: C', E', F'} :: S, H} const Value* ret_val = CopyVal(val_act->u.val, stmt->line_num); - KillLocals(stmt->line_num, frame); + DeallocateLocals(stmt->line_num, frame); state->stack.Pop(1); frame = state->stack.Top(); frame->todo.Push(MakeValAct(ret_val)); diff --git a/executable_semantics/interpreter/interpreter.h b/executable_semantics/interpreter/interpreter.h index f97a5170b7bf..d6608d1d60fd 100644 --- a/executable_semantics/interpreter/interpreter.h +++ b/executable_semantics/interpreter/interpreter.h @@ -59,30 +59,49 @@ struct Frame { continuation(UINT_MAX) {} }; -// TODO(geoffromer): Make this a class, with all members private -struct State { - Stack stack; - std::vector alive; +// A Heap represents the abstract machine's dynamically allocated memory. +class Heap { + public: + // Constructs an empty Heap. + Heap() = default; + + Heap(const Heap&) = delete; + Heap& operator=(const Heap&) = delete; // Returns the value at the given address in the heap after // checking that it is alive. - auto ReadFromMemory(Address a, int line_num) -> const Value*; + auto Read(Address a, int line_num) -> const Value*; + // Writes the given value at the address in the heap after // checking that the address is alive. - auto WriteToMemory(Address a, const Value* v, int line_num) -> void; - // Print the value at the given address to the stream `out`. - auto PrintAddress(Address a, std::ostream& out) -> void; - // Signal an error if the address is no longer alive. - void CheckAlive(Address address, int line_num); + auto Write(Address a, const Value* v, int line_num) -> void; + // Put the given value on the heap and mark it as alive. auto AllocateValue(const Value* v) -> Address; + // Marks the object at this address, and all of its sub-objects, as dead. - auto KillObject(Address address) -> void; + auto Deallocate(Address address) -> void; + + // Print the value at the given address to the stream `out`. + auto PrintAddress(Address a, std::ostream& out) -> void; + // Print all the values on the heap to the stream `out`. auto PrintHeap(std::ostream& out) -> void; private: - std::vector heap; + // Signal an error if the address is no longer alive. + void CheckAlive(Address address, int line_num); + + // Marks all sub-objects of this value as dead. + void DeallocateSubObjects(const Value* val); + + std::vector values_; + std::vector alive_; +}; + +struct State { + Stack stack; + Heap heap; }; extern State* state; diff --git a/executable_semantics/interpreter/typecheck.cpp b/executable_semantics/interpreter/typecheck.cpp index 799de86bd3b0..2cdfceef4a63 100644 --- a/executable_semantics/interpreter/typecheck.cpp +++ b/executable_semantics/interpreter/typecheck.cpp @@ -59,9 +59,9 @@ auto ReifyType(const Value* t, int line_num) -> const Expression* { case ValKind::TupleV: { auto args = new std::vector>(); for (auto& field : *t->u.tuple.elts) { - args->push_back({field.first, ReifyType(state->ReadFromMemory( - field.second, line_num), - line_num)}); + args->push_back( + {field.first, + ReifyType(state->heap.Read(field.second, line_num), line_num)}); } return MakeTuple(0, args); } @@ -161,7 +161,7 @@ auto TypeCheckExp(const Expression* e, TypeEnv types, Env values, std::cerr << std::endl; exit(-1); } - auto field_t = state->ReadFromMemory(*field_address, e->line_num); + auto field_t = state->heap.Read(*field_address, e->line_num); auto new_e = MakeIndex(e->line_num, res.exp, MakeInt(e->line_num, i)); return TCResult(new_e, field_t, res.types); } @@ -200,14 +200,15 @@ auto TypeCheckExp(const Expression* e, TypeEnv types, Env values, << " but got " << arg->first << std::endl; exit(-1); } - arg_expected = state->ReadFromMemory( - (*expected->u.tuple.elts)[i].second, e->line_num); + arg_expected = state->heap.Read((*expected->u.tuple.elts)[i].second, + e->line_num); } auto arg_res = TypeCheckExp(arg->second, new_types, values, arg_expected, context); new_types = arg_res.types; new_args->push_back(std::make_pair(arg->first, arg_res.exp)); - arg_types->push_back({arg->first, state->AllocateValue(arg_res.type)}); + arg_types->push_back( + {arg->first, state->heap.AllocateValue(arg_res.type)}); } auto tuple_e = MakeTuple(e->line_num, new_args); auto tuple_t = MakeTupleVal(arg_types); @@ -245,7 +246,7 @@ auto TypeCheckExp(const Expression* e, TypeEnv types, Env values, auto new_e = MakeGetField(e->line_num, res.exp, *e->u.get_field.field); return TCResult(new_e, - state->ReadFromMemory(field.second, e->line_num), + state->heap.Read(field.second, e->line_num), res.types); } } @@ -715,11 +716,12 @@ auto FunctionDeclaration::TopLevel(TypeCheckContext& tops) const -> void { auto StructDeclaration::TopLevel(TypeCheckContext& tops) const -> void { auto st = TypeOfStructDef(&definition, tops.types, tops.values); - Address a = state->AllocateValue(st); + Address a = state->heap.AllocateValue(st); tops.values.Set(Name(), a); // Is this obsolete? auto field_types = new std::vector>(); for (const auto& [field_name, field_value] : *st->u.struct_type.fields) { - field_types->push_back({field_name, state->AllocateValue(field_value)}); + field_types->push_back( + {field_name, state->heap.AllocateValue(field_value)}); } auto fun_ty = MakeFunTypeVal(MakeTupleVal(field_types), st); tops.types.Set(Name(), fun_ty); @@ -732,7 +734,7 @@ auto ChoiceDeclaration::TopLevel(TypeCheckContext& tops) const -> void { alts->push_back(std::make_pair(a.first, t)); } auto ct = MakeChoiceTypeVal(name, alts); - Address a = state->AllocateValue(ct); + Address a = state->heap.AllocateValue(ct); tops.values.Set(Name(), a); // Is this obsolete? tops.types.Set(Name(), ct); } diff --git a/executable_semantics/interpreter/value.cpp b/executable_semantics/interpreter/value.cpp index 22443d28336e..b571dfa8373e 100644 --- a/executable_semantics/interpreter/value.cpp +++ b/executable_semantics/interpreter/value.cpp @@ -214,13 +214,6 @@ auto MakeChoiceTypeVal(std::string name, return v; } -auto State::PrintAddress(Address a, std::ostream& out) -> void { - if (!this->alive[a]) { - out << "!!"; - } - PrintValue(this->heap[a], out); -} - auto PrintValue(const Value* val, std::ostream& out) -> void { switch (val->tag) { case ValKind::AltConsV: { @@ -235,7 +228,7 @@ auto PrintValue(const Value* val, std::ostream& out) -> void { case ValKind::AltV: { out << "alt " << *val->u.alt.choice_name << "." << *val->u.alt.alt_name << " "; - state->PrintAddress(val->u.alt.argument, out); + state->heap.PrintAddress(val->u.alt.argument, out); break; } case ValKind::StructV: { @@ -254,7 +247,7 @@ auto PrintValue(const Value* val, std::ostream& out) -> void { } out << elt.first << " = "; - state->PrintAddress(elt.second, out); + state->heap.PrintAddress(elt.second, out); out << "@" << elt.second; } out << ")"; @@ -342,9 +335,8 @@ auto TypeEqual(const Value* t1, const Value* t2) -> bool { if ((*t1->u.tuple.elts)[i].first != (*t2->u.tuple.elts)[i].first) { return false; } - if (!TypeEqual( - state->ReadFromMemory((*t1->u.tuple.elts)[i].second, 0), - state->ReadFromMemory((*t2->u.tuple.elts)[i].second, 0))) { + if (!TypeEqual(state->heap.Read((*t1->u.tuple.elts)[i].second, 0), + state->heap.Read((*t2->u.tuple.elts)[i].second, 0))) { return false; } } @@ -378,8 +370,8 @@ static auto FieldsValueEqual(VarAddresses* ts1, VarAddresses* ts2, int line_num) if (iter == ts2->end()) { return false; } - if (!ValueEqual(state->ReadFromMemory(address, line_num), - state->ReadFromMemory(iter->second, line_num), line_num)) { + if (!ValueEqual(state->heap.Read(address, line_num), + state->heap.Read(iter->second, line_num), line_num)) { return false; } }