From 7222f349d6126717e680331265dec2bb7c35a44b Mon Sep 17 00:00:00 2001
From: Adrien Leravat <Pixep@users.noreply.github.com>
Date: Tue, 24 Jan 2023 13:24:05 -0700
Subject: [PATCH] Explorer: move deallocation to `StepCleanUp` (#2547)

Move deallocation logic to `StepCleanUp` to have all necessary cleanup actions in the same place. Currently this means `DestroyAction` and `heap_.Deallocate`.
---
 explorer/interpreter/action.cpp      |  7 -------
 explorer/interpreter/action.h        |  3 ---
 explorer/interpreter/interpreter.cpp | 23 ++++++++++++++---------
 3 files changed, 14 insertions(+), 19 deletions(-)

diff --git a/explorer/interpreter/action.cpp b/explorer/interpreter/action.cpp
index bda16c35dad1..d946a7be9cd1 100644
--- a/explorer/interpreter/action.cpp
+++ b/explorer/interpreter/action.cpp
@@ -35,13 +35,6 @@ auto RuntimeScope::operator=(RuntimeScope&& rhs) noexcept -> RuntimeScope& {
   return *this;
 }
 
-RuntimeScope::~RuntimeScope() {
-  for (auto allocation : allocations_) {
-    // TODO: move this into StepCleanUp
-    heap_->Deallocate(allocation);
-  }
-}
-
 void RuntimeScope::Print(llvm::raw_ostream& out) const {
   out << "{";
   llvm::ListSeparator sep;
diff --git a/explorer/interpreter/action.h b/explorer/interpreter/action.h
index c5ebd13f8927..21b10b70d957 100644
--- a/explorer/interpreter/action.h
+++ b/explorer/interpreter/action.h
@@ -42,9 +42,6 @@ class RuntimeScope {
   RuntimeScope(RuntimeScope&&) noexcept;
   auto operator=(RuntimeScope&&) noexcept -> RuntimeScope&;
 
-  // Deallocates any allocations in this scope from `heap`.
-  ~RuntimeScope();
-
   void Print(llvm::raw_ostream& out) const;
   LLVM_DUMP_METHOD void Dump() const { Print(llvm::errs()); }
 
diff --git a/explorer/interpreter/interpreter.cpp b/explorer/interpreter/interpreter.cpp
index d57c78d1efc5..6f640d8ac3d3 100644
--- a/explorer/interpreter/interpreter.cpp
+++ b/explorer/interpreter/interpreter.cpp
@@ -2214,16 +2214,21 @@ auto Interpreter::StepDestroy() -> ErrorOr<Success> {
 auto Interpreter::StepCleanUp() -> ErrorOr<Success> {
   const Action& act = todo_.CurrentAction();
   const auto& cleanup = cast<CleanUpAction>(act);
-  if (act.pos() < cleanup.allocations_count()) {
-    auto allocation =
-        act.scope()->allocations()[cleanup.allocations_count() - act.pos() - 1];
-    const auto* lvalue = arena_->New<LValue>(Address(allocation));
-    SourceLocation source_loc("destructor", 1);
-    auto value = heap_.Read(lvalue->address(), source_loc);
-    // Step over uninitialized values
-    if (value.ok()) {
-      return todo_.Spawn(std::make_unique<DestroyAction>(lvalue, *value));
+  if (act.pos() < cleanup.allocations_count() * 2) {
+    const size_t alloc_index = cleanup.allocations_count() - act.pos() / 2 - 1;
+    auto allocation = act.scope()->allocations()[alloc_index];
+    if (act.pos() % 2 == 0) {
+      auto* lvalue = arena_->New<LValue>(Address(allocation));
+      auto value =
+          heap_.Read(lvalue->address(), SourceLocation("destructor", 1));
+      // Step over uninitialized values.
+      if (value.ok()) {
+        return todo_.Spawn(std::make_unique<DestroyAction>(lvalue, *value));
+      } else {
+        return todo_.RunAgain();
+      }
     } else {
+      heap_.Deallocate(allocation);
       return todo_.RunAgain();
     }
   }