diff --git a/lix/libcmd/repl.cc b/lix/libcmd/repl.cc index 599086118..541811ef1 100644 --- a/lix/libcmd/repl.cc +++ b/lix/libcmd/repl.cc @@ -157,7 +157,7 @@ struct NixRepl void addVarToScope(const Symbol name, Value & v); Expr & parseString(std::string s); void evalString(std::string s, Value & v); - void loadDebugTraceEnv(DebugTrace & dt); + void loadDebugTraceEnv(const DebugTrace & dt); /** * Load the `repl-overlays` and add the resulting AttrSet to the top-level @@ -482,7 +482,7 @@ StorePath NixRepl::getDerivationPath(Value & v) { return *drvPath; } -void NixRepl::loadDebugTraceEnv(DebugTrace & dt) +void NixRepl::loadDebugTraceEnv(const DebugTrace & dt) { initEnv(); @@ -558,16 +558,18 @@ ProcessLineResult NixRepl::processLine(std::string line) } else if (state.debug && state.debug->inDebugger && (command == ":bt" || command == ":backtrace")) { - for (const auto & [idx, i] : enumerate(state.debug->traces)) { + auto traces = state.debug->traces(); + for (const auto & [idx, i] : enumerate(traces)) { std::cout << "\n" << ANSI_BLUE << idx << ANSI_NORMAL << ": "; - showDebugTrace(std::cout, state.positions, i); + showDebugTrace(std::cout, state.positions, *i); } } else if (state.debug && state.debug->inDebugger && (command == ":env")) { - for (const auto & [idx, i] : enumerate(state.debug->traces)) { + auto traces = state.debug->traces(); + for (const auto & [idx, i] : enumerate(traces)) { if (idx == debugTraceIndex) { - printEnvBindings(state, i.expr, i.env); + printEnvBindings(state, i->expr, i->env); break; } } @@ -579,13 +581,14 @@ ProcessLineResult NixRepl::processLine(std::string line) debugTraceIndex = stoi(arg); } catch (...) { } - for (const auto & [idx, i] : enumerate(state.debug->traces)) { + auto traces = state.debug->traces(); + for (const auto & [idx, i] : enumerate(traces)) { if (idx == debugTraceIndex) { std::cout << "\n" << ANSI_BLUE << idx << ANSI_NORMAL << ": "; - showDebugTrace(std::cout, state.positions, i); + showDebugTrace(std::cout, state.positions, *i); std::cout << std::endl; - printEnvBindings(state, i.expr, i.env); - loadDebugTraceEnv(i); + printEnvBindings(state, i->expr, i->env); + loadDebugTraceEnv(*i); break; } } diff --git a/lix/libexpr/eval-error.cc b/lix/libexpr/eval-error.cc index d32c5734d..23b318509 100644 --- a/lix/libexpr/eval-error.cc +++ b/lix/libexpr/eval-error.cc @@ -42,17 +42,14 @@ EvalErrorBuilder & EvalErrorBuilder::withSuggestions(Suggestions & s) template EvalErrorBuilder & EvalErrorBuilder::withFrame(const Env & env, const Expr & expr) { - // NOTE: This is abusing side-effects. - // TODO: check compatibility with nested debugger calls. - // TODO: What side-effects?? if (error.state.debug) { - error.state.debug->traces.push_front(DebugTrace{ + error.frame = error.state.debug->addTrace(DebugTrace{ .pos = error.state.positions[expr.getPos()], .expr = expr, .env = env, .hint = HintFmt("Fake frame for debugging purposes"), .isError = true - }); + }).entry; } return *this; } @@ -77,11 +74,12 @@ EvalErrorBuilder::addTrace(PosIdx pos, std::string_view formatString, const A template void EvalErrorBuilder::debugThrow() { - if (error.state.debug && !error.state.debug->traces.empty()) { - const DebugTrace & last = error.state.debug->traces.front(); - const Env * env = &last.env; - const Expr * expr = &last.expr; - error.state.debug->runDebugRepl(error.state, &error, *env, *expr); + if (error.state.debug) { + if (auto last = error.state.debug->traces().next()) { + const Env * env = &(*last)->env; + const Expr * expr = &(*last)->expr; + error.state.debug->runDebugRepl(error.state, &error, *env, *expr); + } } // `EvalState` is the only class that can construct an `EvalErrorBuilder`, diff --git a/lix/libexpr/eval-error.hh b/lix/libexpr/eval-error.hh index db2643b47..2c18ebc06 100644 --- a/lix/libexpr/eval-error.hh +++ b/lix/libexpr/eval-error.hh @@ -7,6 +7,7 @@ namespace nix { +struct DebugTrace; struct Env; struct Expr; struct Value; @@ -19,6 +20,9 @@ class EvalError : public Error { template friend class EvalErrorBuilder; + + std::shared_ptr frame; + public: EvalState & state; diff --git a/lix/libexpr/eval.cc b/lix/libexpr/eval.cc index 81b907aea..c14375532 100644 --- a/lix/libexpr/eval.cc +++ b/lix/libexpr/eval.cc @@ -646,15 +646,13 @@ void DebugState::runDebugRepl( auto dts = error && expr.getPos() - ? std::make_unique( - evalState, - DebugTrace { - .pos = error->info().pos ? error->info().pos : evalState.positions[expr.getPos()], - .expr = expr, - .env = env, - .hint = error->info().msg, - .isError = true - }) + ? addTrace(DebugTrace { + .pos = error->info().pos ? error->info().pos : evalState.positions[expr.getPos()], + .expr = expr, + .env = env, + .hint = error->info().msg, + .isError = true + }) : nullptr; if (error) @@ -695,31 +693,45 @@ void EvalState::addErrorTrace(Error & e, const PosIdx pos, const Args & ... form e.addTrace(positions[pos], HintFmt(formatArgs...)); } +DebugState::TraceFrame DebugState::addTrace(DebugTrace t) +{ + struct UnlinkDebugTrace + { + DebugState * state; + void operator()(DebugTrace * trace) + { + state->latestTrace = trace->parent; + delete trace; + } + }; + + t.parent = latestTrace.lock(); + std::unique_ptr trace( + new auto(std::move(t)), UnlinkDebugTrace{.state = this} + ); + std::shared_ptr entry(std::move(trace)); + latestTrace = entry; + return TraceFrame{entry}; +} + template -static std::unique_ptr makeDebugTraceStacker( +static DebugState::TraceFrame makeDebugTraceStacker( EvalState & state, Expr & expr, Env & env, std::shared_ptr && pos, const Args & ... formatArgs) { - return std::make_unique(state, - DebugTrace { - .pos = std::move(pos), - .expr = expr, - .env = env, - .hint = HintFmt(formatArgs...), - .isError = false - }); -} - -DebugTraceStacker::DebugTraceStacker(EvalState & evalState, DebugTrace t) - : evalState(evalState) - , trace(std::move(t)) -{ - evalState.debug->traces.push_front(trace); - if (evalState.debug->stop && evalState.debug->repl) - evalState.debug->runDebugRepl(evalState, nullptr, trace.env, trace.expr); + auto trace = state.debug->addTrace(DebugTrace{ + .pos = std::move(pos), + .expr = expr, + .env = env, + .hint = HintFmt(formatArgs...), + .isError = false, + }); + if (state.debug->stop && state.debug->repl) + state.debug->runDebugRepl(state, nullptr, env, expr); + return trace; } inline Value * EvalState::lookupVar(Env * env, const ExprVar & var, bool noEval) diff --git a/lix/libexpr/eval.hh b/lix/libexpr/eval.hh index 50075961d..166bd3749 100644 --- a/lix/libexpr/eval.hh +++ b/lix/libexpr/eval.hh @@ -4,6 +4,7 @@ #include "lix/libexpr/attr-set.hh" #include "lix/libexpr/eval-error.hh" #include "lix/libexpr/gc-alloc.hh" +#include "lix/libutil/generator.hh" #include "lix/libutil/types.hh" #include "lix/libexpr/value.hh" #include "lix/libexpr/nixexpr.hh" @@ -150,14 +151,18 @@ struct DebugTrace { const Env & env; HintFmt hint; bool isError; + std::shared_ptr parent; }; struct DebugState { +private: + std::weak_ptr latestTrace; + +public: std::function repl; bool stop = false; bool inDebugger = false; - std::list traces; std::map> exprEnvs; int trylevel = 0; @@ -178,8 +183,35 @@ struct DebugState } return nullptr; } -}; + class TraceFrame + { + friend struct DebugState; + template + friend class EvalErrorBuilder; + + // holds both the data for this frame *and* a deleter that pulls this frame + // off the trace stack. EvalErrorBuilder uses this for withFrame fake trace + // frames, and to avoid needing to see this class definition in its header. + const std::shared_ptr entry = nullptr; + + explicit TraceFrame(std::shared_ptr entry): entry(std::move(entry)) {} + + public: + TraceFrame(std::nullptr_t) {} + }; + + TraceFrame addTrace(DebugTrace t); + + /// Enumerates the debug frame stack, from the current frame to the root frame. + /// All values are guaranteed to not be null, but must be pointers because C++. + Generator traces() + { + for (auto current = latestTrace.lock(); current; current = current->parent) { + co_yield current.get(); + } + } +}; struct StaticSymbols { @@ -761,16 +793,6 @@ private: friend struct Value; }; -struct DebugTraceStacker { - DebugTraceStacker(EvalState & evalState, DebugTrace t); - ~DebugTraceStacker() - { - evalState.debug->traces.pop_front(); - } - EvalState & evalState; - DebugTrace trace; -}; - /** * @return A string representing the type of the value `v`. * diff --git a/lix/libexpr/primops.cc b/lix/libexpr/primops.cc index c59903469..d722eb59a 100644 --- a/lix/libexpr/primops.cc +++ b/lix/libexpr/primops.cc @@ -566,15 +566,14 @@ static void prim_genericClosure(EvalState & state, const PosIdx pos, Value * * a static void prim_break(EvalState & state, const PosIdx pos, Value * * args, Value & v) { - if (state.debug && !state.debug->traces.empty()) { + if (auto trace = state.debug ? state.debug->traces().next() : std::nullopt) { auto error = EvalError(state, ErrorInfo { .level = lvlInfo, .msg = HintFmt("breakpoint reached"), .pos = state.positions[pos], }); - auto & dt = state.debug->traces.front(); - state.debug->runDebugRepl(state, &error, dt.env, dt.expr); + state.debug->runDebugRepl(state, &error, (*trace)->env, (*trace)->expr); } // Return the value we were passed. @@ -696,9 +695,11 @@ static void prim_trace(EvalState & state, const PosIdx pos, Value * * args, Valu printError("trace: %1%", args[0]->string.s); else printError("trace: %1%", ValuePrinter(state, *args[0])); - if (evalSettings.builtinsTraceDebugger && state.debug && !state.debug->traces.empty()) { - const DebugTrace & last = state.debug->traces.front(); - state.debug->runDebugRepl(state, nullptr, last.env, last.expr); + if (auto last = evalSettings.builtinsTraceDebugger && state.debug + ? state.debug->traces().next() + : std::nullopt) + { + state.debug->runDebugRepl(state, nullptr, (*last)->env, (*last)->expr); } state.forceValue(*args[1], pos); v = *args[1]; diff --git a/tests/functional/repl_characterization/data/debug_frames.test b/tests/functional/repl_characterization/data/debug_frames.test index 25e28ef97..a556b2420 100644 --- a/tests/functional/repl_characterization/data/debug_frames.test +++ b/tests/functional/repl_characterization/data/debug_frames.test @@ -71,3 +71,43 @@ and resume execution the debugger is once again disabled nix-repl> :c error: unknown command ':c' + +leaving the debugger from a toplevel error and entering it again doesn't leave old frames visible + nix-repl> with {}; a + error: undefined variable 'a' + at «string»:1:10: + 1| with {}; a + | ^ + + nix-repl> :s + error: undefined variable 'a' + at «string»:1:10: + 1| with {}; a + | ^ + + nix-repl> with {}; b + error: undefined variable 'b' + at «string»:1:10: + 1| with {}; b + | ^ + + nix-repl> :bt + + 0: error: undefined variable 'b' + «string»:1:10 + + 1| with {}; b + | ^ + + 1: error: Fake frame for debugging purposes + «string»:1:10 + + 1| with {}; b + | ^ + +exiting from here prints the error + nix-repl> :q + error: undefined variable 'b' + at «string»:1:10: + 1| with {}; b + | ^