From c07500e14dce5036ea4b8cf956cf34df4b282873 Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Wed, 1 May 2024 23:02:43 +0200
Subject: [PATCH] refactor: Extract EvalState::{runDebugRepl,canDebug}

---
 src/libexpr/eval-error.cc |  7 +------
 src/libexpr/eval.cc       | 18 ++++++++++++++++++
 src/libexpr/eval.hh       | 12 ++++++++++++
 src/libexpr/primops.cc    | 15 ++++++---------
 4 files changed, 37 insertions(+), 15 deletions(-)

diff --git a/src/libexpr/eval-error.cc b/src/libexpr/eval-error.cc
index 282f5554a..5a0c67514 100644
--- a/src/libexpr/eval-error.cc
+++ b/src/libexpr/eval-error.cc
@@ -73,12 +73,7 @@ EvalErrorBuilder<T>::addTrace(PosIdx pos, std::string_view formatString, const A
 template<class T>
 void EvalErrorBuilder<T>::debugThrow()
 {
-    if (error.state.debugRepl && !error.state.debugTraces.empty()) {
-        const DebugTrace & last = error.state.debugTraces.front();
-        const Env * env = &last.env;
-        const Expr * expr = &last.expr;
-        error.state.runDebugRepl(&error, *env, *expr);
-    }
+    error.state.runDebugRepl(&error);
 
     // `EvalState` is the only class that can construct an `EvalErrorBuilder`,
     // and it does so in dynamic storage. This is the final method called on
diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index c1dadeee0..6a38bbe45 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -785,6 +785,24 @@ public:
     }
 };
 
+bool EvalState::canDebug()
+{
+    return debugRepl && !debugTraces.empty();
+}
+
+void EvalState::runDebugRepl(const Error * error)
+{
+    if (!canDebug())
+        return;
+
+    assert(!debugTraces.empty());
+    const DebugTrace & last = debugTraces.front();
+    const Env & env = last.env;
+    const Expr & expr = last.expr;
+
+    runDebugRepl(error, env, expr);
+}
+
 void EvalState::runDebugRepl(const Error * error, const Env & env, const Expr & expr)
 {
     // Make sure we have a debugger to run and we're not already in a debugger.
diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh
index 7ca2d6227..06a687620 100644
--- a/src/libexpr/eval.hh
+++ b/src/libexpr/eval.hh
@@ -276,6 +276,18 @@ public:
             return std::shared_ptr<const StaticEnv>();;
     }
 
+    /** Whether a debug repl can be started. If `false`, `runDebugRepl(error)` will return without starting a repl. */
+    bool canDebug();
+
+    /** Use front of `debugTraces`; see `runDebugRepl(error,env,expr)` */
+    void runDebugRepl(const Error * error);
+
+    /**
+     * Run a debug repl with the given error, environment and expression.
+     * @param error The error to debug, may be nullptr.
+     * @param env The environment to debug, matching the expression.
+     * @param expr The expression to debug, matching the environment.
+     */
     void runDebugRepl(const Error * error, const Env & env, const Expr & expr);
 
     template<class T, typename... Args>
diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc
index 5ff7d5314..459d19e05 100644
--- a/src/libexpr/primops.cc
+++ b/src/libexpr/primops.cc
@@ -780,15 +780,14 @@ static RegisterPrimOp primop_break({
     )",
     .fun = [](EvalState & state, const PosIdx pos, Value * * args, Value & v)
     {
-        if (state.debugRepl && !state.debugTraces.empty()) {
+        if (state.canDebug()) {
             auto error = Error(ErrorInfo {
                 .level = lvlInfo,
                 .msg = HintFmt("breakpoint reached"),
                 .pos = state.positions[pos],
             });
 
-            auto & dt = state.debugTraces.front();
-            state.runDebugRepl(&error, dt.env, dt.expr);
+            state.runDebugRepl(&error);
         }
 
         // Return the value we were passed.
@@ -1018,9 +1017,8 @@ static void prim_trace(EvalState & state, const PosIdx pos, Value * * args, Valu
         printError("trace: %1%", args[0]->string_view());
     else
         printError("trace: %1%", ValuePrinter(state, *args[0]));
-    if (evalSettings.builtinsTraceDebugger && state.debugRepl && !state.debugTraces.empty()) {
-        const DebugTrace & last = state.debugTraces.front();
-        state.runDebugRepl(nullptr, last.env, last.expr);
+    if (evalSettings.builtinsTraceDebugger) {
+        state.runDebugRepl(nullptr);
     }
     state.forceValue(*args[1], pos);
     v = *args[1];
@@ -1060,9 +1058,8 @@ static void prim_warn(EvalState & state, const PosIdx pos, Value * * args, Value
     if (evalSettings.builtinsAbortOnWarn) {
         state.error<Abort>("aborting to reveal stack trace of warning, as abort-on-warn is set").debugThrow();
     }
-    if ((evalSettings.builtinsTraceDebugger || evalSettings.builtinsDebuggerOnWarn) && state.debugRepl && !state.debugTraces.empty()) {
-        const DebugTrace & last = state.debugTraces.front();
-        state.runDebugRepl(nullptr, last.env, last.expr);
+    if (evalSettings.builtinsTraceDebugger || evalSettings.builtinsDebuggerOnWarn) {
+        state.runDebugRepl(nullptr);
     }
     state.forceValue(*args[1], pos);
     v = *args[1];