diff --git a/doc/manual/rl-next/libexpr-fix-break.md b/doc/manual/rl-next/libexpr-fix-break.md new file mode 100644 index 000000000..28a835098 --- /dev/null +++ b/doc/manual/rl-next/libexpr-fix-break.md @@ -0,0 +1,14 @@ +--- +synopsis: "builtins.break doesn't break expression anymore" +issues: [1165] +cls: [5422] +category: "Fixes" +credits: [blokyk] +--- + +Wrapping an expression in `builtins.break` used to break some builtins like +`map` and the `is*` functions, which could modify the execution path of code +inadvertently, made debugging nix harder than it already is, and in some cases +even crashed the interpreter. Now, using `break` should be completely +transparent to whatever function receives it as an input, preventing the +above-mentioned issues. diff --git a/lix/libexpr/primops.cc b/lix/libexpr/primops.cc index f09decb15..0c1c52055 100644 --- a/lix/libexpr/primops.cc +++ b/lix/libexpr/primops.cc @@ -656,6 +656,7 @@ static void prim_break(EvalState & state, Value * * args, Value & v) } // Return the value we were passed. + state.forceValue(*args[0], noPos); v = *args[0]; } diff --git a/tests/functional/repl_characterization/data/regression_9917.test b/tests/functional/repl_characterization/data/regression_9917.test index 8a2bb3678..451c4efec 100644 --- a/tests/functional/repl_characterization/data/regression_9917.test +++ b/tests/functional/repl_characterization/data/regression_9917.test @@ -14,26 +14,56 @@ This test ensures that continues don't skip opportunities to enter the debugger. 0: error: breakpoint reached $TEST_DATA/regression_9917.nix:3:5 - 2| a = builtins.trace "before inner break" ( - 3| builtins.break { msg = "hello"; } - | ^ - 4| ); + 2| a = builtins.trace "before inner break" ( + 3| builtins.break { msg = "hello"; } + | ^ + 4| ); 1: while calling a function $TEST_DATA/regression_9917.nix:3:5 - 2| a = builtins.trace "before inner break" ( - 3| builtins.break { msg = "hello"; } - | ^ - 4| ); + 2| a = builtins.trace "before inner break" ( + 3| builtins.break { msg = "hello"; } + | ^ + 4| ); 2: while calling a function $TEST_DATA/regression_9917.nix:2:7 - 1| let - 2| a = builtins.trace "before inner break" ( - | ^ - 3| builtins.break { msg = "hello"; } + 1| let + 2| a = builtins.trace "before inner break" ( + | ^ + 3| builtins.break { msg = "hello"; } + + 3: while calling a function + $TEST_DATA/regression_9917.nix:6:5 + + 5| b = builtins.trace "before outer break" ( + 6| builtins.break a + | ^ + 7| ); + + 4: while calling a function + $TEST_DATA/regression_9917.nix:5:7 + + 4| ); + 5| b = builtins.trace "before outer break" ( + | ^ + 6| builtins.break a + + 5: while evaluating a 'let' expression + $TEST_DATA/regression_9917.nix:1:1 + + 1| let + | ^ + 2| a = builtins.trace "before inner break" ( + + 6: while evaluating the file '$TEST_DATA/regression_9917.nix': + $TEST_DATA/regression_9917.nix:1:1 + + 1| let + | ^ + 2| a = builtins.trace "before inner break" ( nix-repl> :c diff --git a/tests/functional/repl_characterization/data/stack_vars.test b/tests/functional/repl_characterization/data/stack_vars.test index 354969075..bd845087c 100644 --- a/tests/functional/repl_characterization/data/stack_vars.test +++ b/tests/functional/repl_characterization/data/stack_vars.test @@ -87,18 +87,22 @@ If we :st past the frame in the backtrace with the meow in it, the meow should n nix-repl> :quit error: - … while calling the 'trace' builtin - at $TEST_DATA/stack_vars.nix:2:7: - 1| let - 2| a = builtins.trace "before inner break" ( - | ^ - 3| let meow' = 3; in builtins.break { msg = "hello"; } + … while evaluating the file '$TEST_DATA/stack_vars.nix': - … while calling the 'break' builtin - at $TEST_DATA/stack_vars.nix:3:23: - 2| a = builtins.trace "before inner break" ( - 3| let meow' = 3; in builtins.break { msg = "hello"; } - | ^ + … while evaluating b + at $TEST_DATA/stack_vars.nix:9:3: + 8| in + 9| b + | ^ + 10| + + … while calling the 'trace' builtin + at $TEST_DATA/stack_vars.nix:5:7: 4| ); + 5| b = builtins.trace "before outer break" ( + | ^ + 6| let meow = 2; in builtins.break a + + (stack trace truncated; use '--show-trace' to show the full trace) error: breakpoint reached diff --git a/tests/functional2/eval/test_debugger.py b/tests/functional2/eval/test_debugger.py index 60af030ef..d2d427dd6 100644 --- a/tests/functional2/eval/test_debugger.py +++ b/tests/functional2/eval/test_debugger.py @@ -27,3 +27,59 @@ def test_debugger_output(nix: Nix): assert "error: oh snap" in res.stderr_plain assert re.findall(r"with: .*a", res.stdout_plain) assert re.findall(r"static: .*x", res.stdout_plain) + + +def test_transparent_break(nix: Nix): + """ + Make sure that adding a call to builtins.break doesn't + change the result of an expression + """ + + expr = dedent(""" + let + inherit (builtins) + attrNames + break + elem + functionArgs + head + isAttrs + isPath + isFunction + map + mapAttrs + removeAttrs + toJSON + typeOf; + in + builtins.all (b: b) [ + ((attrNames { a = 5; }) == (attrNames (break { a = 5; }))) + ((elem 5 [1 5]) == (elem 5 (break [1 5]))) + ((elem (2+3) [1 (2+3)]) == (elem (2+3) (break [1 (2+3)]))) + ((functionArgs ({ a }: 5)) == (functionArgs (break ({ a }: 5)))) + ((head [1 2]) == (head (break [1 2]))) + ((isAttrs { a = 5; }) == (isAttrs (break { a = 5; }))) + ((isPath ./.) == (isPath (break ./.))) + ((isPath ./${".meow"}) == (isPath (break ./${".meow"}))) + ((isFunction (x: x)) == (isFunction (break (x: x)))) + ((map (x: x) [1 5]) == (map (x: x) (break [1 5]))) + ((mapAttrs (n: v: v) { a = 5; }) == (mapAttrs (n: v: v) (break { a = 5; }))) + ((removeAttrs { a = 5; b = 6; } ["a"]) == (removeAttrs (break { a = 5; b = 6; }) ["a"])) + ((removeAttrs { ab = 5; } [("a"+"b")]) == (removeAttrs { ab = 5; } [(break ("a"+"b"))])) + ((toJSON { a = 5; }) == (toJSON (break { a = 5; }))) + ((toJSON { a = [(1+2)]; }) == (toJSON { a = break [(1+2)]; })) + ((typeOf { a = 5; }) == (typeOf (break { a = 5; }))) + ((typeOf (1+2)) == (typeOf (break (1+2)))) + ] + """) + + res_no_dbg = nix.nix(["eval", "--expr", expr], flake=True).run().expect(0) + assert "true" in res_no_dbg.stdout_plain + + res_with_dbg = ( + nix.nix(["eval", "--debugger", "--expr", expr], flake=True) + .with_stdin(b":c\n" * 50) + .run() + .expect(0) + ) + assert "true" in res_with_dbg.stdout_plain