Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
---
synopsis: The `--debugger` will start more reliably in `let` expressions and function calls
prs: 9917
issues: 6649
---

Previously, if you attempted to evaluate this file with the debugger:

```nix
let
a = builtins.trace "before inner break" (
builtins.break "hello"
);
b = builtins.trace "before outer break" (
builtins.break a
);
in
b
```

Nix would correctly enter the debugger at `builtins.break a`, but if you asked
it to `:continue`, it would skip over the `builtins.break "hello"` expression
entirely.

Now, Nix will correctly enter the debugger at both breakpoints.
31 changes: 28 additions & 3 deletions src/libexpr/eval.cc
Original file line number Diff line number Diff line change
Expand Up @@ -846,20 +846,20 @@ void EvalState::addErrorTrace(Error & e, const PosIdx pos, const char * s, const
e.addTrace(positions[pos], hintfmt(s, s2), frame);
}

template<typename... Args>
static std::unique_ptr<DebugTraceStacker> makeDebugTraceStacker(
EvalState & state,
Expr & expr,
Env & env,
std::shared_ptr<Pos> && pos,
const char * s,
const std::string & s2)
const Args & ... formatArgs)
{
return std::make_unique<DebugTraceStacker>(state,
DebugTrace {
.pos = std::move(pos),
.expr = expr,
.env = env,
.hint = hintfmt(s, s2),
.hint = hintfmt(formatArgs...),
.isError = false
});
}
Expand Down Expand Up @@ -1322,6 +1322,19 @@ void ExprLet::eval(EvalState & state, Env & env, Value & v)
for (auto & i : attrs->attrs)
env2.values[displ++] = i.second.e->maybeThunk(state, i.second.inherited ? env : env2);

auto dts = state.debugRepl
? makeDebugTraceStacker(
state,
*this,
env2,
getPos()
? std::make_shared<Pos>(state.positions[getPos()])
: nullptr,
"while evaluating a '%1%' expression",
"let"
Comment on lines +1333 to +1334
Copy link
Member

@thufschmitt thufschmitt Feb 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we just replace that by

Suggested change
"while evaluating a '%1%' expression",
"let"
"while evaluating a 'let' expression"

and not add the formatting logic in makeDebugTraceStacker?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah nvm, I didn't notice that makeDebugTraceStacker was already using hintfmt, just with a hardcoded number of 2 elements 😬 .

It makes sense to make it a bit more generic then

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also colors "let" magenta like the other things in error messages.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, right :) This thing is still too magical for me

)
: nullptr;

body->eval(state, env2, v);
}

Expand Down Expand Up @@ -1718,6 +1731,18 @@ void EvalState::callFunction(Value & fun, size_t nrArgs, Value * * args, Value &

void ExprCall::eval(EvalState & state, Env & env, Value & v)
{
auto dts = state.debugRepl
? makeDebugTraceStacker(
state,
*this,
env,
getPos()
? std::make_shared<Pos>(state.positions[getPos()])
: nullptr,
"while calling a function"
)
: nullptr;

Value vFun;
fun->eval(state, env, vFun);

Expand Down