libcmd/repl: allow :st argument to be relative to current stack index
See [lix-project/lix#1156], but basically currently the `:st <n>` 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
This commit is contained in:
@@ -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`.
|
||||
+88
-24
@@ -1,10 +1,13 @@
|
||||
#include <algorithm>
|
||||
#include <cstdio>
|
||||
#include <editline.h>
|
||||
#include <iostream>
|
||||
#include <cstdlib>
|
||||
#include <cstring>
|
||||
#include <optional>
|
||||
#include <string_view>
|
||||
|
||||
#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, const DebugTrace *> {
|
||||
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<int> maybeIdx = string2Int<int>(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}}}
|
||||
);
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user