Simplify the API and implementation of Stack (#667)

executable_semantics/interpreter/action.cpp

@@ -84,12 +84,13 @@ void Action::Print(llvm::raw_ostream& out) const {
   }
 }
 
-void Action::PrintList(Stack<Action*> ls, llvm::raw_ostream& out) {
-  if (!ls.IsEmpty()) {
-    out << *ls.Pop();
-    if (!ls.IsEmpty()) {
+void Action::PrintList(const Stack<Action*>& ls, llvm::raw_ostream& out) {
+  auto it = ls.begin();
+  while (it != ls.end()) {
+    out << **it;
+    ++it;
+    if (it != ls.end()) {
       out << " :: ";
-      PrintList(ls, out);
     }
   }
 }

executable_semantics/interpreter/action.h

@@ -48,7 +48,7 @@ struct Action {
   static auto MakeStatementAction(const Statement* s) -> Action*;
   static auto MakeValAction(const Value* v) -> Action*;
 
-  static void PrintList(Stack<Action*> ls, llvm::raw_ostream& out);
+  static void PrintList(const Stack<Action*>& ls, llvm::raw_ostream& out);
 
   auto GetLValAction() const -> const LValAction&;
   auto GetExpressionAction() const -> const ExpressionAction&;

executable_semantics/interpreter/frame.h

@@ -44,6 +44,9 @@ struct Frame {
   // the address of the continuation.
   std::optional<Address> continuation;
 
+  Frame(const Frame&) = delete;
+  Frame& operator=(const Frame&) = delete;
+
   Frame(std::string n, Stack<Scope*> s, Stack<Action*> c)
       : name(std::move(std::move(n))), scopes(s), todo(c), continuation() {}
 

executable_semantics/interpreter/interpreter.cpp

@@ -42,12 +42,13 @@ void PrintEnv(Env values, llvm::raw_ostream& out) {
 // State Operations
 //
 
-void PrintStack(Stack<Frame*> ls, llvm::raw_ostream& out) {
-  if (!ls.IsEmpty()) {
-    out << *ls.Pop();
-    if (!ls.IsEmpty()) {
+void PrintStack(const Stack<Frame*>& ls, llvm::raw_ostream& out) {
+  auto it = ls.begin();
+  while (it != ls.end()) {
+    out << **it;
+    ++it;
+    if (it != ls.end()) {
       out << " :: ";
-      PrintStack(ls, out);
     }
   }
 }
@@ -228,8 +229,9 @@ void DeallocateScope(int line_num, Scope* scope) {
 }
 
 void DeallocateLocals(int line_num, Frame* frame) {
-  for (auto scope : frame->scopes) {
-    DeallocateScope(line_num, scope);
+  while (!frame->scopes.IsEmpty()) {
+    DeallocateScope(line_num, frame->scopes.Top());
+    frame->scopes.Pop();
   }
 }
 
@@ -1141,8 +1143,7 @@ auto InterpProgram(std::list<Declaration>* fs) -> int {
     PrintState(llvm::outs());
   }
 
-  while (state->stack.CountExceeds(1) ||
-         state->stack.Top()->todo.CountExceeds(1) ||
+  while (state->stack.Count() > 1 || state->stack.Top()->todo.Count() > 1 ||
          state->stack.Top()->todo.Top()->tag() != ActionKind::ValAction) {
     Step();
     if (tracing_output) {
@@ -1160,8 +1161,7 @@ auto InterpExp(Env values, const Expression* e) -> const Value* {
   auto* frame = new Frame("InterpExp", Stack(scope), todo);
   state->stack = Stack(frame);
 
-  while (state->stack.CountExceeds(1) ||
-         state->stack.Top()->todo.CountExceeds(1) ||
+  while (state->stack.Count() > 1 || state->stack.Top()->todo.Count() > 1 ||
          state->stack.Top()->todo.Top()->tag() != ActionKind::ValAction) {
     Step();
   }

executable_semantics/interpreter/interpreter.h

@@ -28,7 +28,7 @@ struct State {
 extern State* state;
 
 void InitEnv(const Declaration& d, Env* env);
-void PrintStack(Stack<Frame*> ls, llvm::raw_ostream& out);
+void PrintStack(const Stack<Frame*>& ls, llvm::raw_ostream& out);
 void PrintEnv(Env values);
 
 /***** Interpreters *****/

executable_semantics/interpreter/stack.h

@@ -7,75 +7,36 @@
 
 #include <cstddef>
 #include <iterator>
+#include <vector>
 
 #include "common/check.h"
 #include "executable_semantics/interpreter/list_node.h"
 
 namespace Carbon {
 
-// A persistent stack data structure.
-//
-// - Note: this data structure leaks memory.
+// A stack data structure.
 template <class T>
 struct Stack {
-  // A forward iterator over elements of a `Stack`.
-  struct Iterator {
-    using value_type = T;
-    using difference_type = std::ptrdiff_t;
-    using pointer = const T*;
-    using reference = const T&;
-    using iterator_category = std::forward_iterator_tag;
-
-    Iterator(ListNode<T>* x) : p(x) {}
-    Iterator(const Iterator& mit) : p(mit.p) {}
-    Iterator& operator++() {
-      p = p->next;
-      return *this;
-    }
-    Iterator operator++(int) {
-      Iterator tmp(*this);
-      operator++();
-      return tmp;
-    }
-    bool operator==(const Iterator& rhs) const { return p == rhs.p; }
-    bool operator!=(const Iterator& rhs) const { return p != rhs.p; }
-    const T& operator*() { return p->curr; }
-    const T* operator->() { return &p->curr; }
-
-   private:
-    ListNode<T>* p;
-  };
-
-  // The position of the first/`Top()` element, or `end()` if
-  // `this->IsEmpty()`.
-  auto begin() const -> Iterator { return Iterator(head); }
-
-  // The position one past that of the last element.
-  auto end() const -> Iterator { return Iterator(nullptr); }
+  using const_iterator = typename std::vector<T>::const_reverse_iterator;
 
   // Creates an empty instance.
-  Stack() { head = nullptr; }
+  Stack() = default;
 
   // Creates an instance containing just `x`.
-  explicit Stack(T x) : Stack() { Push(x); }
+  // TODO: consider removing this. It's somewhat unconventional, and the
+  // callsite readability is debatable.
+  explicit Stack(T x) : Stack() { Push(std::move(x)); }
 
   // Pushes `x` onto the top of the stack.
-  void Push(T x) { head = new ListNode<T>(x, head); }
-
-  // Returns a copy of `*this`, with `x` pushed onto the top.
-  auto Pushing(T x) const -> Stack {
-    auto r = *this;
-    r.Push(x);
-    return r;
-  }
+  void Push(T x) { elements.push_back(std::move(x)); }
 
   // Removes and returns the top element of the stack.
   //
   // - Requires: !this->IsEmpty()
   auto Pop() -> T {
     CHECK(!IsEmpty()) << "Can't pop from empty stack.";
-    auto r = head->curr;
-    head = head->next;
+    auto r = std::move(elements.back());
+    elements.pop_back();
     return r;
   }
 
@@ -84,19 +45,9 @@ struct Stack {
   // - Requires: n >= 0 && n <= Count()
   void Pop(int n) {
     CHECK(n >= 0) << "Negative pop count disallowed.";
-    while (n--) {
-      CHECK(head != nullptr) << "Can only pop as many elements as stack has.";
-      head = head->next;
-    }
-  }
-
-  // Returns a copy of `*this`, sans the top element.
-  //
-  // - Requires: !this->IsEmpty()
-  auto Popped() const -> Stack {
-    auto r = *this;
-    r.Pop();
-    return r;
+    CHECK(static_cast<size_t>(n) <= elements.size())
+        << "Can only pop as many elements as stack has.";
+    elements.resize(elements.size() - n);
   }
 
   // Returns the top element of the stack.
@@ -104,33 +55,21 @@ struct Stack {
   // - Requires: !this->IsEmpty()
   auto Top() const -> T {
     CHECK(!IsEmpty()) << "Empty stack has no Top().";
-    return head->curr;
+    return elements.back();
   }
 
   // Returns `true` iff `Count() > 0`.
-  auto IsEmpty() const -> bool { return head == nullptr; }
-
-  // Returns `true` iff `Count() > n`.
-  //
-  // - Complexity: O(`n`)
-  auto CountExceeds(int n) const -> bool {
-    if (n < 0)
-      return true;
-
-    for (auto p = head; p != nullptr; p = p->next) {
-      if (n-- == 0)
-        return true;
-    }
-
-    return false;
-  }
+  auto IsEmpty() const -> bool { return elements.empty(); }
 
   // Returns the number of elements in `*this`.
-  auto Count() const -> int { return std::distance(begin(), end()); }
+  auto Count() const -> int { return elements.size(); }
+
+  // Iterates over the Stack from top to bottom.
+  const_iterator begin() const { return elements.crbegin(); }
+  const_iterator end() const { return elements.crend(); }
 
  private:
-  // A linked list of cells containing the elements of self.
-  ListNode<T>* head;
+  std::vector<T> elements;
 };
 
 }  // namespace Carbon