about summary refs log tree commit diff
diff options
context:
space:
mode:
authorsterni <sternenseemann@systemli.org>2022-10-15T13·52+0200
committerclbot <clbot@tvl.fyi>2022-10-15T14·12+0000
commitf3c27f1717f0b92b05340c0bc6ba44007ce9d90b (patch)
tree989566dc26387d1e7849df4de0bdc841cdf963aa
parentcaf9cbf71147054c01c6fdf49773d646cbfb46eb (diff)
fix(tvix/eval): bring foldl' strictness in line with C++ Nix r/5133
Working on https://github.com/NixOS/nix/pull/7158, I discovered that C++
Nix actually is strict in the accumulator, just not in the first value.
This seems due to the fact that in the C++ evaluator, function calls
don't seem to be thunked unconditionally and foldl' just elects not to
wrap it in a thunk (don't quote me on this summary, even though it seems
to line up with the code for primop_foldlStrict and testable behavior).

It doesn't seem worth it to risk breaking the odd Nix expression just to
be strict in one more value per invocation of foldl' (i.e. the initial
accumulator value `nul`), so let's match the existing C++ Nix behavior
here.

Change-Id: If59e62271a90d97cb440f0ca72a58ec7840d1690
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7022
Autosubmit: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
-rw-r--r--tvix/eval/docs/language-issues.md7
-rw-r--r--tvix/eval/src/builtins/mod.rs2
-rw-r--r--tvix/eval/src/tests/tvix_tests/eval-fail-foldlStrict-strict-op-application.nix4
-rw-r--r--tvix/eval/src/tests/tvix_tests/eval-okay-foldlStrict-lazy-initial-accumulator.exp1
-rw-r--r--tvix/eval/src/tests/tvix_tests/eval-okay-foldlStrict-lazy-initial-accumulator.nix4
-rw-r--r--tvix/eval/src/tests/tvix_tests/eval-okay-foldlStrict.exp (renamed from tvix/eval/src/tests/tvix_tests/eval-okay-foldl.exp)0
-rw-r--r--tvix/eval/src/tests/tvix_tests/eval-okay-foldlStrict.nix (renamed from tvix/eval/src/tests/tvix_tests/eval-okay-foldl.nix)0
7 files changed, 16 insertions, 2 deletions
diff --git a/tvix/eval/docs/language-issues.md b/tvix/eval/docs/language-issues.md
index fb74b1d3d324..26401665bbb5 100644
--- a/tvix/eval/docs/language-issues.md
+++ b/tvix/eval/docs/language-issues.md
@@ -17,7 +17,6 @@ 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
@@ -35,6 +34,12 @@ perspective to keep them:
   implementation already merges at parse time, making nested attribute keys
   syntactic sugar effectively.
 
+Other behavior is just odd, surprising or underdocumented:
+
+* `builtins.foldl'` doesn't force the initial accumulator (but all other
+  intermediate accumulator values), differing from e.g. Haskell, see
+  the [relevant PR discussion](p7158).
+
 [i7111]: https://github.com/NixOS/nix/issues/7111
 [i7012]: https://github.com/NixOS/nix/issues/7012
 [i7115]: https://github.com/NixOS/nix/issues/7115
diff --git a/tvix/eval/src/builtins/mod.rs b/tvix/eval/src/builtins/mod.rs
index d767d09b99d5..1ed59ffdc8ed 100644
--- a/tvix/eval/src/builtins/mod.rs
+++ b/tvix/eval/src/builtins/mod.rs
@@ -264,8 +264,8 @@ fn pure_builtins() -> Vec<Builtin> {
                 let mut res = args.pop().unwrap();
                 let op = args.pop().unwrap();
                 for val in list {
-                    res.force(vm)?;
                     res = vm.call_with(&op, [val, res])?;
+                    res.force(vm)?;
                 }
 
                 Ok(res)
diff --git a/tvix/eval/src/tests/tvix_tests/eval-fail-foldlStrict-strict-op-application.nix b/tvix/eval/src/tests/tvix_tests/eval-fail-foldlStrict-strict-op-application.nix
new file mode 100644
index 000000000000..adc029b2f29e
--- /dev/null
+++ b/tvix/eval/src/tests/tvix_tests/eval-fail-foldlStrict-strict-op-application.nix
@@ -0,0 +1,4 @@
+builtins.foldl'
+  (_: f: f null)
+  (throw "This doesn't explode")
+  [ (_: throw "Not the final value, but is still forced!") (_: 23) ]
diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-foldlStrict-lazy-initial-accumulator.exp b/tvix/eval/src/tests/tvix_tests/eval-okay-foldlStrict-lazy-initial-accumulator.exp
new file mode 100644
index 000000000000..d81cc0710eb6
--- /dev/null
+++ b/tvix/eval/src/tests/tvix_tests/eval-okay-foldlStrict-lazy-initial-accumulator.exp
@@ -0,0 +1 @@
+42
diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-foldlStrict-lazy-initial-accumulator.nix b/tvix/eval/src/tests/tvix_tests/eval-okay-foldlStrict-lazy-initial-accumulator.nix
new file mode 100644
index 000000000000..59fd29b55237
--- /dev/null
+++ b/tvix/eval/src/tests/tvix_tests/eval-okay-foldlStrict-lazy-initial-accumulator.nix
@@ -0,0 +1,4 @@
+builtins.foldl'
+  (_: x: x)
+  (throw "This is never forced")
+  [ "but the results of applying op are" 42 ]
diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-foldl.exp b/tvix/eval/src/tests/tvix_tests/eval-okay-foldlStrict.exp
index 8d683a20fab7..8d683a20fab7 100644
--- a/tvix/eval/src/tests/tvix_tests/eval-okay-foldl.exp
+++ b/tvix/eval/src/tests/tvix_tests/eval-okay-foldlStrict.exp
diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-foldl.nix b/tvix/eval/src/tests/tvix_tests/eval-okay-foldlStrict.nix
index 44c0349387ff..44c0349387ff 100644
--- a/tvix/eval/src/tests/tvix_tests/eval-okay-foldl.nix
+++ b/tvix/eval/src/tests/tvix_tests/eval-okay-foldlStrict.nix