From daadfed9ae84d3126572ba3b2e7602cb21bf18a8 Mon Sep 17 00:00:00 2001 From: blokyk Date: Wed, 18 Mar 2026 16:04:56 +0100 Subject: [PATCH] libcmd/repl: allow :st argument to be relative to current stack index See [lix-project/lix#1156], but basically currently the `:st ` debugger command doesn't allow any negative indices, and putting a plus sign in front of the arg doesn't change anything; thus, we can exploit that "design space" to allow users to move between different stack frames easily, by simply prepending their arg with a +/- sign. The actual behavior is little more nuanced when you account for errors: as suggested by @pennae (thanks! :), when the user inputs an offset that would result in an invalid frame index, the debugger instead clamps it to the closest bound (i.e. 0 for negative offsets, $maxFrame for positive ones) and just prints a warning. Fixes #1156 [lix-project/lix#1156]: https://git.lix.systems/lix-project/lix/issues/1156 Change-Id: I02a0cdb6aaebbdb0515308880a3bf9c0d2fcd25e --- doc/manual/rl-next/repl-relative-st.md | 18 +++ lix/libcmd/repl.cc | 112 ++++++++++++++---- .../data/debug_frames.test | 2 +- .../data/stacktrace_oob.test | 57 +++++++-- .../data/stacktrace_relative.test | 64 ++++++++++ .../repl_characterization.cc | 1 + 6 files changed, 217 insertions(+), 37 deletions(-) create mode 100644 doc/manual/rl-next/repl-relative-st.md create mode 100644 tests/functional/repl_characterization/data/stacktrace_relative.test diff --git a/doc/manual/rl-next/repl-relative-st.md b/doc/manual/rl-next/repl-relative-st.md new file mode 100644 index 000000000..1cbb474ff --- /dev/null +++ b/doc/manual/rl-next/repl-relative-st.md @@ -0,0 +1,18 @@ +--- +synopsis: "Allow moving between stack frames relative to current debugger frame" +issues: [1156] +cls: [5411] +category: "Improvements" +credits: [blokyk] +--- + +Debugging functional programs often involve switching between a bunch of stack +frames to get the full context of what's happening and who's calling who. +Before this change, going up or down the stack in the nix debugger with `:st` +meant remembering the absolute index of each stack frame, instead of their +positions relative to one another; this got tiring *fast*. + +Now, you can prepend `:st`'s argument with a + or - sign to indicate you want to +move relative to the current stack frame. For example, typing `:st +3` when you +were on frame `10` will go frame `13`; vice-versa, typing `:st -4` on frame `6` +will go to frame `2`. diff --git a/lix/libcmd/repl.cc b/lix/libcmd/repl.cc index 0194dc6ca..06a4c2746 100644 --- a/lix/libcmd/repl.cc +++ b/lix/libcmd/repl.cc @@ -1,10 +1,13 @@ +#include #include #include #include #include #include +#include #include +#include "libutil/logging.hh" #include "lix/libexpr/value.hh" #include "lix/libutil/box_ptr.hh" #include "lix/libcmd/repl-interacter.hh" @@ -711,50 +714,111 @@ void NixRepl::initDebugBuiltinCommands() addCommand( "show-trace", + // this command has a bit of nuance to its function and error states. + // it can either: + // 1. be called without any argument + // -> just display the current stack frame (still have to walk up the stack :/) + // 2. be called with an absolute index + // -> try to go to that frame + // -> if it doesn't exist, print an "arg out of range" error + // 3. be called with a relative index + // -> if the final offset is in-bounds, go to that frame + // -> otherwise: clamp the index, i.e. go to 0/$max instead of out-of-bounds + // + // because the collection of frames is lazy and isn't a random-access list, + // we need to iterate the whole stack for most of these if we want to have + // good error messages; this is the biggest reason why this function is so + // long/complex compared to its role + // [](NixRepl & repl, const std::string & arg) { - int requestedTraceIdx = repl.debugTraceIndex; - if (arg.length() != 0) { + auto setTrace = [&](size_t traceIdx, const DebugTrace * trace) { + repl.debugTraceIndex = traceIdx; + std::cout << "\n" << ANSI_BLUE << traceIdx << ANSI_NORMAL << ": "; + showDebugTrace(std::cout, repl.evaluator.positions, *trace); + std::cout << std::endl; + printEnvBindings(repl.state, trace->expr, trace->env); + repl.loadDebugTraceEnv(*trace); + }; + + // tries to find a trace at a given index. + // - if it is found, it returns the requested trace, along with its + // index, which will be *the same* as requested + // - otherwise, it returns the last (=outermost) trace, along with + // its index, which will be *different* than the one requested + auto tryFindTrace = [&](size_t traceIdx) -> std::pair { + size_t lastIndex = 0; + const DebugTrace * lastTrace; + auto traces = repl.evaluator.debug->traces(); + for (const auto & [idx, i] : enumerate(traces)) { + lastTrace = i; + lastIndex = idx; + if (idx == traceIdx) { + return std::pair(idx, i); + } + } + return std::pair(lastIndex, lastTrace); + }; + + bool isRelativeIdx = false; + int requestedTraceIdx; + if (arg.length() == 0) { + // if there's no argument, just re-print the current frame + requestedTraceIdx = repl.debugTraceIndex; + } else { std::optional maybeIdx = string2Int(arg); if (!maybeIdx) { throw Error("argument '%s' is not a valid integer", arg); } - requestedTraceIdx = maybeIdx.value(); + + isRelativeIdx = arg.starts_with('+') || arg.starts_with('-'); + requestedTraceIdx = + isRelativeIdx ? maybeIdx.value() + repl.debugTraceIndex : maybeIdx.value(); } - size_t traceCount = 0; - auto traces = repl.evaluator.debug->traces(); - for (const auto & [idx, i] : enumerate(traces)) { - traceCount++; - if (idx == (size_t) requestedTraceIdx) { - repl.debugTraceIndex = requestedTraceIdx; - std::cout << "\n" << ANSI_BLUE << idx << ANSI_NORMAL << ": "; - showDebugTrace(std::cout, repl.evaluator.positions, *i); - std::cout << std::endl; - printEnvBindings(repl.state, i->expr, i->env); - repl.loadDebugTraceEnv(*i); - break; - } + auto [actualTraceIdx, trace] = tryFindTrace((size_t) requestedTraceIdx); + + // if we *did* find the frame we wanted originally, all is well + // in the world and we can just load it and exit + if (actualTraceIdx == (size_t) requestedTraceIdx) { + setTrace(actualTraceIdx, trace); + return ProcessLineResult::PromptAgain; } - // if we didn't find any trace matching the user's request - if (repl.debugTraceIndex != (size_t) requestedTraceIdx) { - // note: if we get here, then the loop ran fully without matching anything, - // so `traceCount` is the total number of traces + // if we couldn't immediately find the requested trace on the "happy path", then either: + + // a) it was an absolute index but didn't exist + // -> print a specific error showing the exact valid range + if (!isRelativeIdx) { throw Error( "stack index must be between %ld and %ld (inclusive), but was %ld", 0, - // decrease it by one to get the max *index*, since stacks are indexed by 0 - traceCount - 1, + actualTraceIdx, // tryFindTrace sets *idx to the final (max) frame index if it fails requestedTraceIdx ); } - return ProcessLineResult::PromptAgain; + // b) it was a relative index + // -> clamp the index to the bounds and print a warning + if (requestedTraceIdx < 0) { + // just load frame 0 but print a warning about the bounds + std::tie(actualTraceIdx, trace) = tryFindTrace(0); + setTrace(actualTraceIdx, trace); + printTaggedWarning("stopped at stack frame %ld, cannot go any deeper", 0); + return ProcessLineResult::PromptAgain; + } else { + // (if we're here, then requestedTraceIdx > $max, since tryFindTrace failed) + // load the max frame (that `tryFindFrame` kindly already got for us), + // but print a warning that we can't go any further + setTrace(actualTraceIdx, trace); + printTaggedWarning("stopped at stack frame %ld, cannot go any higher", actualTraceIdx); + return ProcessLineResult::PromptAgain; + } }, {.aliases = {"st"}, .debugModeOnly = true, .help = "Show current trace. If an integer is provided, this switches to that stack " - "beforehand.", + "beforehand. If the integer has an explicit + or - sign, it is treated as" + "relative to the current stack index.", .section = "Debug mode", .positionalArgsSpecifiers = {{.placeholderText = "integer index", .optional = true}}} ); diff --git a/tests/functional/repl_characterization/data/debug_frames.test b/tests/functional/repl_characterization/data/debug_frames.test index d0ac05133..ad3198f9a 100644 --- a/tests/functional/repl_characterization/data/debug_frames.test +++ b/tests/functional/repl_characterization/data/debug_frames.test @@ -40,7 +40,7 @@ :bt, :backtrace Show trace stack :c, :continue Go until end of program, exception or builtins.break :s, :step Go one step - :st, :show-trace [integer index] Show current trace. If an integer is provided, this switches to that stack beforehand. + :st, :show-trace [integer index] Show current trace. If an integer is provided, this switches to that stack beforehand. If the integer has an explicit + or - sign, it is treated as relative to the current stack index. Flakes commands diff --git a/tests/functional/repl_characterization/data/stacktrace_oob.test b/tests/functional/repl_characterization/data/stacktrace_oob.test index a282ea8e0..8bafd99fd 100644 --- a/tests/functional/repl_characterization/data/stacktrace_oob.test +++ b/tests/functional/repl_characterization/data/stacktrace_oob.test @@ -40,21 +40,11 @@ frames from 0 up to 4 work fine Env level 2 abort baseNameOf break builtins derivation derivationStrict dirOf false fetchGit fetchMercurial fetchTarball fetchTree fromTOML import isNull map null placeholder removeAttrs scopedImport throw toString true -negative frames print an error - nix-repl> :st -1 - error: stack index must be between 0 and 4, but was -1 - - nix-repl> :st -100 - error: stack index must be between 0 and 4, but was -100 - -positives frames out of bounds also print an error +absolute frames out of bounds print an error nix-repl> :st 5 error: stack index must be between 0 and 4, but was 5 - nix-repl> :st 100 - error: stack index must be between 0 and 4, but was 100 - -argument-less :st is still at the same after oob +argument-less :st is still at the same after absolute oob nix-repl> :st 4: while evaluating a 'let' expression @@ -72,6 +62,49 @@ argument-less :st is still at the same after oob Env level 2 abort baseNameOf break builtins derivation derivationStrict dirOf false fetchGit fetchMercurial fetchTarball fetchTree fromTOML import isNull map null placeholder removeAttrs scopedImport throw toString true +positive relative frames oob clamp to upper bound and print a warning + nix-repl> :st +5 + + 4: while evaluating a 'let' expression + «string»:1:1 + + 1| let f = _: throw "x_x"; x = f 5; in x + | ^ + + Env level 0 + static: f x + + Env level 1 + static: + + Env level 2 + abort baseNameOf break builtins derivation derivationStrict dirOf false fetchGit fetchMercurial fetchTarball fetchTree fromTOML import isNull map null placeholder removeAttrs scopedImport throw toString true + + warning: stopped at stack frame 4, cannot go any higher + +negative relative frames oob clamp to lower bound and print a warning + nix-repl> :st -5 + + 0: error: x_x + «string»:1:12 + + 1| let f = _: throw "x_x"; x = f 5; in x + | ^ + + Env level 0 + static: _ + + Env level 1 + static: f x + + Env level 2 + static: + + Env level 3 + abort baseNameOf break builtins derivation derivationStrict dirOf false fetchGit fetchMercurial fetchTarball fetchTree fromTOML import isNull map null placeholder removeAttrs scopedImport throw toString true + + warning: stopped at stack frame 0, cannot go any deeper + quit nix-repl> :quit error: x_x diff --git a/tests/functional/repl_characterization/data/stacktrace_relative.test b/tests/functional/repl_characterization/data/stacktrace_relative.test new file mode 100644 index 000000000..99539f94d --- /dev/null +++ b/tests/functional/repl_characterization/data/stacktrace_relative.test @@ -0,0 +1,64 @@ +@args --debugger + nix-repl> let f = _: throw "x_x"; x = f 5; in x + error: x_x + +absolute indices still work: + nix-repl> :st 1 + + 1: while calling a function + «string»:1:12 + + 1| let f = _: throw "x_x"; x = f 5; in x + | ^ + + Env level 0 + static: _ + + Env level 1 + static: f x + + Env level 2 + static: + + Env level 3 + abort baseNameOf break builtins derivation derivationStrict dirOf false fetchGit fetchMercurial fetchTarball fetchTree fromTOML import isNull map null placeholder removeAttrs scopedImport throw toString true + +index with + goes up the stack relative to current (1 in this case): + nix-repl> :st +3 + + 4: while evaluating a 'let' expression + «string»:1:1 + + 1| let f = _: throw "x_x"; x = f 5; in x + | ^ + + Env level 0 + static: f x + + Env level 1 + static: + + Env level 2 + abort baseNameOf break builtins derivation derivationStrict dirOf false fetchGit fetchMercurial fetchTarball fetchTree fromTOML import isNull map null placeholder removeAttrs scopedImport throw toString true + +index with - goes down and is also relative to current (4): + nix-repl> :st -1 + + 3: while calling a function + «string»:1:29 + + 1| let f = _: throw "x_x"; x = f 5; in x + | ^ + + Env level 0 + static: f x + + Env level 1 + static: + + Env level 2 + abort baseNameOf break builtins derivation derivationStrict dirOf false fetchGit fetchMercurial fetchTarball fetchTree fromTOML import isNull map null placeholder removeAttrs scopedImport throw toString true + +quit + nix-repl> :quit + error: x_x diff --git a/tests/functional/repl_characterization/repl_characterization.cc b/tests/functional/repl_characterization/repl_characterization.cc index f81367dad..739d24d1b 100644 --- a/tests/functional/repl_characterization/repl_characterization.cc +++ b/tests/functional/repl_characterization/repl_characterization.cc @@ -199,5 +199,6 @@ REPL_TEST(debug_ignore_try); REPL_TEST(debug_ignore_try_defaults); REPL_TEST(stacktrace_invalid_arg); REPL_TEST(stacktrace_oob); +REPL_TEST(stacktrace_relative); }; // namespace nix