From bb71669c8342fe4e4313679fa774aca7cc41836e Mon Sep 17 00:00:00 2001
From: Konstantin Nazarov <mail@knazarov.com>
Date: Sun, 15 Sep 2024 03:32:52 +0100
Subject: [PATCH] Implement printing backtraces on VM execution errors

---
 src/common.cpp | 40 ++++++++++++++++++++++++++++++++
 src/common.hpp |  2 ++
 src/valeri.cpp | 63 ++++++++++++++++++++++++++++++++------------------
 src/vm.hpp     |  4 ++++
 4 files changed, 87 insertions(+), 22 deletions(-)

diff --git a/src/common.cpp b/src/common.cpp
index 91ad16e..7aefbf0 100644
--- a/src/common.cpp
+++ b/src/common.cpp
@@ -436,6 +436,46 @@ Result<StackFrame> StackFrame::ret(uint64_t regnum) const {
   return std::move(parent_stack);
 }
 
+Result<String> StackFrame::backtrace(uint64_t indent) const {
+  String res = TRY(String::create(""));
+
+  StackFrame cur = TRY(copy());
+
+  while (true) {
+    Value par = TRY(cur.parent());
+    // If parent frame is empty - we are at the top-level. Top-level is always
+    // <unnamed>, so let's just skip it
+    if (par.is<Nil>()) break;
+
+    auto fun = TRY(cur.fun());
+
+    for (uint64_t i = 0; i < indent; i++) {
+      res = TRY(res.concat(" "));
+    }
+
+    res = TRY(res.concat("Function "));
+    if (fun.is<Function>()) {
+      Function& f = *fun.to<Function>();
+      auto name = TRY(f.name());
+      if (!name.is<Symbol>()) {
+        name = Value(TRY(String::create("<unnamed>")));
+      } else {
+        name = Value(TRY(String::create(*name.to<Symbol>())));
+      }
+      res = TRY(res.concat(*name.to<String>()));
+    } else if (fun.is<StdlibFunction>()) {
+      StdlibFunction& f = *fun.to<StdlibFunction>();
+      auto name = TRY(String::create(TRY(f.name())));
+      res = TRY(res.concat(name));
+    }
+
+    res = TRY(res.concat("\n"));
+
+    cur = TRY(par.to<StackFrame>()->copy());
+  }
+  return res;
+}
+
 Result<Value> Error::copy_value() const {
   return Value(Error(TRY(_value.copy())));
 }
diff --git a/src/common.hpp b/src/common.hpp
index cff02bd..7448353 100644
--- a/src/common.hpp
+++ b/src/common.hpp
@@ -1144,6 +1144,8 @@ class StackFrame : public Object {
   virtual Result<Value> copy_value() const final;
   Result<StackFrame> copy() const;
 
+  Result<String> backtrace(uint64_t indent = 0) const;
+
  private:
   GcRoot<PodStackFrame> _value;
 };
diff --git a/src/valeri.cpp b/src/valeri.cpp
index 463bbed..0dc7d54 100644
--- a/src/valeri.cpp
+++ b/src/valeri.cpp
@@ -13,23 +13,7 @@
 
 StaticArena<64 * 1024 * 1024> arena;
 
-Result<Value> run_string(const String& fname, const String& src) {
-  auto parsed = TRY(read_multiple(src));
-
-  TRY(arena_gc());
-
-  auto compiled = TRY(compile(fname, parsed));
-  Module& mod = *compiled.to<Module>();
-
-  Dict globals = TRY(Dict::create());
-  auto vm = TRY(VM::create(mod, globals));
-
-  auto res = TRY(vm.run());
-
-  return res;
-}
-
-Result<void> print_error(const Result<Value>& res) {
+Result<void> print_error(const Result<Value>& res, const String& backtrace) {
   if (res.has_error()) {
     auto errobj = TRY(geterrobj().copy());
     if (errobj.is<Nil>()) {
@@ -39,11 +23,46 @@ Result<void> print_error(const Result<Value>& res) {
       print_string(*message.to<String>());
       std::cout << "\n";
     }
+
+    if (TRY(backtrace.size()) > 0) {
+      print_string(backtrace);
+    }
   }
 
   return Result<void>();
 }
 
+Result<Value> run_string(const String& fname, const String& src) {
+  auto maybe_parsed = read_multiple(src);
+  auto empty_backtrace = TRY(String::create(""));
+  if (maybe_parsed.has_error()) {
+    print_error(maybe_parsed, empty_backtrace);
+    return maybe_parsed;
+  }
+  auto parsed = maybe_parsed.release_value();
+
+  auto maybe_compiled = compile(fname, parsed);
+  if (maybe_compiled.has_error()) {
+    print_error(maybe_compiled, empty_backtrace);
+    return maybe_compiled;
+  }
+  auto compiled = maybe_compiled.release_value();
+
+  Module& mod = *compiled.to<Module>();
+
+  Dict globals = TRY(Dict::create());
+  auto vm = TRY(VM::create(mod, globals));
+
+  auto maybe_res = vm.run();
+
+  if (maybe_res.has_error()) {
+    auto backtrace = TRY(vm.backtrace(2));
+    print_error(maybe_res, backtrace);
+  }
+
+  return maybe_res.release_value();
+}
+
 Result<void> run_repl() {
   Dict globals = TRY(Dict::create());
 
@@ -51,15 +70,16 @@ Result<void> run_repl() {
   while (true) {
     auto src = TRY(read_line("valeri> "));
     auto maybe_parsed = read_multiple(src);
+    auto empty_backtrace = TRY(String::create(""));
     if (maybe_parsed.has_error()) {
-      print_error(maybe_parsed);
+      print_error(maybe_parsed, empty_backtrace);
       continue;
     }
 
     auto parsed = maybe_parsed.release_value();
     auto maybe_compiled = compile(fname, parsed);
     if (maybe_compiled.has_error()) {
-      print_error(maybe_compiled);
+      print_error(maybe_compiled, empty_backtrace);
       continue;
     }
     auto compiled = maybe_compiled.release_value();
@@ -77,7 +97,8 @@ Result<void> run_repl() {
       }
     } else {
       // Need to print the error
-      print_error(maybe_res);
+      auto backtrace = TRY(vm.backtrace(2));
+      print_error(maybe_res, backtrace);
     }
   }
 
@@ -96,7 +117,6 @@ Result<void> run(int argc, const char* argv[]) {
     auto fname = TRY(String::create("<stdin>"));
     auto res = run_string(fname, src);
     if (res.has_error()) {
-      print_error(res);
       exit(1);
     }
 
@@ -105,7 +125,6 @@ Result<void> run(int argc, const char* argv[]) {
     auto fname = TRY(String::create(argv[1]));
     auto res = run_string(fname, src);
     if (res.has_error()) {
-      print_error(res);
       exit(1);
     }
   }
diff --git a/src/vm.hpp b/src/vm.hpp
index 798e45f..64180fc 100644
--- a/src/vm.hpp
+++ b/src/vm.hpp
@@ -85,6 +85,10 @@ class VM {
     return fun.to<Function>()->closure();
   }
 
+  Result<String> backtrace(uint64_t indent = 0) {
+    return _stack.backtrace(indent);
+  }
+
  private:
   StackFrame _stack;
   Dict _globals;