This came up in the Nix Language channel today and I thought it
warranted a test case.
We did actually implement this correctly.
Change-Id: I4b37c92d06eb6e3a7f59ea3d10af38f2b0a93d53
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7493
Reviewed-by: sterni <sternenseemann@systemli.org>
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
This test case checks two things:
* A sanity check that "pointer equality for functions" means not
just the lambda, but also the upvalues.
* To be pointer-equal, it is not enough for the upvalues to be
normal-form equal (i.e. `nix_eq()`-equal); the upvalues must be
*pointer*-equal. The second part of the test case checks for
this.
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Change-Id: I4e59327a6f199b8212e97197b212e3c3934bb3f0
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7372
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
We have to be careful implementing `builtins.groupBy`, since the
list may contain thunks, and tvix's to_xxx() functions do not work
on thunks.
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Change-Id: I182b6fc2d4296f864ed16744ef70b153e8e6978a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7039
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
The impl Display for NixAttrs needs to wrap double quotes around any
keys which are not valid Nix identifiers. This commit does that,
and adds a test (which fails prior to this commit and passes after
this commit).
Change-Id: Ie31ce91e8637cb27073f23f115db81feefdc6424
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7084
Autosubmit: Adam Joseph <adam@westernsemico.com>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
The current implementation of nix_eq will force one level of thunks
and then switch to the (non-forcing) rust Eq::eq() method. This
gives incorrect results for lists-of-thunks.
This commit changes nix_eq() to be recursive.
A regression test (which fails prior to this commit) is included.
This fix also causes nix_tests/eval-okay-fromjson.nix to pass, so it
is moved out of notyetpassing.
Change-Id: I655fd7a5294208a7b39df8e2c3c12a8b9768292f
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7142
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Lists are compared lexicographically in C++ nix as of [0], and our
updated nix test suites depend on this. This implements comparison of
list values in `Value::nix_cmp` using a very similar algorithm to what
C++ does - similarly to there, this requires passing in the VM so we can
force thunks in the list elements as we go.
[0]: 09471d2680#
Change-Id: I5d8bb07f90647a1fec83f775243e21af856afbb1
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7070
Autosubmit: grfn <grfn@gws.fyi>
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Note that this test (ironically) fails if the observer is used (e.g.
with --trace-runtime), but that's a separate issue.
Change-Id: I952eaeac8b5a7acce9c66cd4744ec570280748e7
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7055
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
This is done via a new `deepForce` function on Value. Since values can
be cyclical (for example, see the test-case), we need to do some extra
work to avoid RefCell borrow errors if we ever hit a graph cycle:
While deep-forcing values, we keep a set of thunks that we have
already seen and avoid doing any work on the same thunk twice. The set
is encapsulated in a separate type to stop potentially invalid
pointers from leaking out.
Finally, since deep_force is conceptually similar to
`VM::force_for_output` (but more suited to usage in eval since it
doesn't clone the values) this removes the latter, replacing it with
the former.
Co-Authored-By: Vincent Ambo <tazjin@tvl.su>
Change-Id: Iefddefcf09fae3b6a4d161a5873febcff54b9157
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7000
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
Reviewed-by: tazjin <tazjin@tvl.su>
When forcing thunks in `force_with_output`, the call stack of the VM
is actually empty (as the calls are synthetic and no longer part of
the evaluation of the top-level expression).
This means that Tvix crashed when constructing error spans for the
`fallible` macro, as the assumption of there being an enclosing span
was violated.
To work around this, we instead pass the span for the whole top-level
expression to force_for_output and set this as the span for the
enclosing error chain. Existing output logic will already avoid
printing the entire expression as an error span.
This fixes b/213.
Change-Id: I93978e0deaf5bcb0f47a6fa95b3f5bebef5bad4c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7052
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
Without this change it was possible to cause situations (see the new
test) in which a `with`-namespace was forced prematurely.
Change-Id: I879ea7763b43edc693feace2c73c890d426fafd3
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7031
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Reviewed-by: Adam Joseph <adam@westernsemico.com>
Now that we're tracking formals on Lambda this ends up being quite easy;
we just pull them off of the Lambda for the argument closure and use
them to construct the result attribute set.
Change-Id: I811cb61ec34c6bef123a4043000b18c0e4ea0125
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7003
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Validate "closed formals" (formal parameters without an ellipsis) via a
new ValidateClosedFormals op, which checks the arguments (in an attr set
at the top of the stack) against the formal parameters on the Lambda in
the current frame, and returns a new UnexpectedArgument error (including
the span of the formals themselves!!) if any arguments aren't allowed
Change-Id: Idcc47a59167a83be1832a6229f137d84e426c56c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7002
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Since we already have infra for forcing arguments to builtins, this ends
up being almost *too* simple - we just return the second argument!
Change-Id: I070d3d0b551c4dcdac095f67b31e22e0de90cbd7
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6999
Reviewed-by: kanepyork <rikingcoding@gmail.com>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Implement an *initial* version of builtins.match, using the rust `regex`
crate for regular expressions. The rust regex crate definitely has
different semantics than nix's regular expressions - but we'd like to
see how far we can get before the incompatibility starts to matter.
This consciously leaves out any sort of memo for compiled regular
expressions (which upstream nix also has) for the sake of expediency -
in the future we should implement that so we don't have to compile the
same regular expression multiple times.
Change-Id: I5b718635831ec83397940e417a9047c4342b6fa1
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6989
Tested-by: BuildkiteCI
Reviewed-by: Adam Joseph <adam@westernsemico.com>
Reviewed-by: tazjin <tazjin@tvl.su>
Using `serde_json` for parsing JSON here, plus an `impl FromJSON for
Value`. The latter is primarily to stay "dependency light" for now -
likely going with an actual serde `Deserialize` impl in the future is
going to be way better as it allows saving significantly on intermediary
allocations.
Change-Id: I152a0448ff7c87cf7ebaac927c38912b99de1c18
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6920
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
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>
This commit causes the test eval-okay-builtins.nix to pass.
It also adds tests/tvix_tests/eval-okay-dirof.nix which has better
coverage than the nix tests for this builtin.
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Change-Id: I71d96b48680696fd6e4fea3a9861742b35cfaa66
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6987
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
This commit implements builtins.baseNameOf and adds a test case
eval-okay-basenameof.nix to the test suite.
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Change-Id: Ib8bbafba2ac9ca0e1d3dc5e844167f94890d9fee
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6997
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
builtins.parseDrvName should not coerce its argument to a string.
This commit fixes that oversight in my previous commit, and adds an
xfail test to cover this condition.
Thanks to @sterni for noticing this.
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Change-Id: I76bc78f1a82e1e08fe5c787c563a221d55de2639
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6991
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
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
Previously, the VM assumed that if an error was returned from `run()`,
the evaluation was "finished" and the state of the VM didn't matter.
This used to be a reasonable assumption, but now that we've got
`tryEval` around we need to actually make sure that we clean up after
ourselves if we're about to return an error. Specifically, if the *last*
instruction in an evaluation frame returns an error, we previously
wouldn't pop that evaluation frame, which could cause all sorts of
bizarre errors based on what happened to be in the stack at the time.
This commit splits out a `run_op` method from `VM::run`, and uses that
to check the evaluation frame return condition even if the op we're
running is about to return an error, and pop the evaluation frame if
we're at the last instruction.
Change-Id: Ib40649d8915ee1571153cb71e3d76492542fc3d7
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6940
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Despite this not being documented, `tryEval` is empirically able to
catch errors caused by a <...> path not resolving (and nixpkgs depends
on this).
Change-Id: Ia3b78a2d9d2d0c603aba829518b351102dc55396
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6926
Reviewed-by: sterni <sternenseemann@systemli.org>
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Since the body of an `if` expr can refer to deferred upvalues, it needs
to be thunked so when we actually compile those deferred upvalues we
have something for the finalize op to point at. Without this all sorts
of weird things can happen due to the finalize op being run in the wrong
lambda context, up to and including a panic.
Change-Id: I040d5e1a7232fd841cfa4953539898fa49cbbb83
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6929
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
With asserts compiled using conditional jumps, this ends up being quite
straightforward - the only real tricky bit is that we have to know
whether an error can or can't be handled.
Change-Id: I75617da73b7a9c5cdd888c0e26ae81d2c5c0d714
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6924
Reviewed-by: sterni <sternenseemann@systemli.org>
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Implement adding paths and strings via OpAdd. Since the nix rules are
quite obscure, I'm electing to test this one with an oracle test to
avoid the danger of getting the actual asserted result wrong.
Change-Id: Icdcca3690ca2e8459e386c1f29cc48eaaa39e9a3
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6914
Autosubmit: grfn <grfn@gws.fyi>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Implement the listToAttrs builtin, which constructs an attribute set
from a list of attribute sets with keys name and value.
This is tested using an adaptation of the nix `eval-ok-listtoattrs.nix`,
with the utilities from `lib.nix` inlined.
Change-Id: Ib5bf743466dda9722c2c1e00797df4b58448cf0f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6894
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
When contrasting the compilation of the desugared version to the
"sugared" version, this was the noticeable difference.
This fixes b/203.
Change-Id: Iae02ffc56e06de1de091b84cdc59d8fe83a17d69
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6898
Reviewed-by: grfn <grfn@gws.fyi>
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
This requires actually passing the source directory into `interpret` in
the eval tests, but otherwise this is fairly straightforward - if we're
trying to import a directory, just push `default.nix` onto it and import
that instead.
Change-Id: I0b7d4234f81977e78d14dfa651bf0cf9721017e5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6893
Autosubmit: grfn <grfn@gws.fyi>
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Thunks correctly force when comparing for equality against other thunks,
but weren't being forced correctly when comparing against non-thunk
values, in either direction.
Change-Id: Ia03702895ec4d70aed3445c1b0a9a7a641d1a300
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6897
Autosubmit: grfn <grfn@gws.fyi>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
