mdebray/tvl-depot
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(tvix/eval/builtins): force acc not list element in foldl'

Browse source 
When investigating discrepancies between foldl' in tvix and C++ Nix,
I discovered that C++ Nix's foldl' doesn't seem to be strict at all.
Since this seemed wrong, I looked into Haskell's foldl' implementation
which doesn't force the list elements (`val` in our code), but the
accumulation value (`res` in our code). You can look at the code here:
https://hackage.haskell.org/package/base-4.17.0.0/docs/src/GHC.List.html#foldl%27

This actually makes a lot of sense: If `res` is not forced after each
application of `op`, we'll end up thunks nested as deeply as the list is
long, potentially taking up a lot of space. This can be limited by
forcing the `res` thunk before applying `op` again (and creating a new
thunk).

I've also PR-ed an equivalent change for C++ Nix at
https://github.com/NixOS/nix/pull/7158. Since this is not merged nor
backported to our Nix 2.3 fork, I've not copied the eval fail test yet,
since it wouldn't when checking our tests against C++ Nix in depot.

Change-Id: I34edf6fc3031fc1485c3e714f2280b4fba8f004b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6947
Autosubmit: sterni <sternenseemann@systemli.org>
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
This commit is contained in:
sterni 2022-10-11 14:34:30 +02:00 committed by clbot
parent b319e00831
commit fcd5e53703
4 changed files with 12 additions and 1 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
tvix/eval/docs/language-issues.md
View file

@ -17,6 +17,7 @@ maybe to get rid of the behavior in all implementations for good. Below is an
* [Behaviour of nested attribute sets depends on definition order][i7111]
* [Partially constructed attribute sets are observable during dynamic attr names construction][i7012]
* [Nix parsers merges multiple attribute set literals for the same key incorrectly depending on definition order](i7115)
* [builtins.foldl' doesn't seem to be strict](p7158)
On the other hand, there is behavior that seems to violate one's expectation
about the language at first, but has good enough reasons from an implementor's
@ -37,3 +38,4 @@ perspective to keep them:
[i7111]: https://github.com/NixOS/nix/issues/7111
[i7012]: https://github.com/NixOS/nix/issues/7012
[i7115]: https://github.com/NixOS/nix/issues/7115
[p7158]: https://github.com/NixOS/nix/pull/7158

2
tvix/eval/src/builtins/mod.rs
View file

@ -245,7 +245,7 @@ fn pure_builtins() -> Vec<Builtin> {
let mut res = args.pop().unwrap();
let op = args.pop().unwrap();
for val in list {
val.force(vm)?;
res.force(vm)?;
res = vm.call_with(&op, [val, res])?;
}

1
tvix/eval/src/tests/tvix_tests/eval-okay-foldlStrict-lazy-elements.exp Normal file
View file

@ -0,0 +1 @@
42

8
tvix/eval/src/tests/tvix_tests/eval-okay-foldlStrict-lazy-elements.nix Normal file
View file

@ -0,0 +1,8 @@
let
lst = builtins.foldl'
(acc: x: acc ++ [ x ])
[ ]
[ 42 (throw "this shouldn't be evaluated") ];
in
builtins.head lst