Currently, the span on *all* thunk force errors is the span at which the
thunk is forced, which for recursive thunk forcing ends up just being
the same span over and over again. This changes the span on thunk force
errors to be the span at which point the thunk is *created*, which is a
bit more helpful (though the printing atm is a little... crowded). To
make this work, we have to thread through the span at which a thunk is
created into a field on the thunk itself.
Change-Id: I81474810a763046e2eb3a8f07acf7d8ec708824a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6932
Autosubmit: grfn <grfn@gws.fyi>
Reviewed-by: Adam Joseph <adam@westernsemico.com>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Previously the various call functions either returned `EvalResult<()>`
or `EvalResult<Value>`, which was confusing.
Now only vm::call_with returns a Value directly, and other parts of
the API just leave the stack top in the post-call state.
This makes it easier to reason about what's going on in non-tail-call
cases (which are making a comeback).
Change-Id: I264ffc683a11aca72dd06e2220a5ff6e7c5fc2b0
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6936
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
This commit implements (lazy) resolution of `<...>` paths via either the
NIX_PATH environment variable, or the -I command-line flag - both
handled via EvalOptions. As a result, EvalOptions can no longer derive
Copy, meaning we have to clone it at each line of the repl - this is
probably not a huge deal as repl performance is not exactly an inner
loop and we're not cloning very much.
Internally, this works by creating a thunk which pushes a constant
containing the string inside the brackets to the stack, then a new
opcode to resolve that path via the `NixPath`. To get that opcode to
work, we now have to pass in the NixPath when constructing the VM.
This (intentionally) leaves out proper implementation of path resolution
via `findFile` (cppnix just calls whatever identifier called findFile is
in scope!!!) as that's widely considered a bit of a misfeature, but if
we do decide to implement that down the road it likely wouldn't be more
than a few extra ops within the thunk introduced here.
Change-Id: Ibc979b7e425b65cbe88599940520239a4a10cee2
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6918
Autosubmit: grfn <grfn@gws.fyi>
Reviewed-by: tazjin <tazjin@tvl.su>
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
In order to behave nicely with tryEval, asserts need to leave the
instruction pointer in a reasonable place even if they fail - whereas
with the previous implementation catching a failed assert would still
end up running the op for the *body* of the assert. With this change, we
compile asserts much more like an `if` expression with conditional jumps
rather than having an OpAssert op.
Change-Id: I1b266c3be90185c84000da6b1995ac3e6fd5471b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6925
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
The process of calling a function from a builtin, especially if it's got
more than 1 arrgument, is reasonably involved and easy to get wrong due
to having to interact directly with the stack - instead of having that
done entirely manually in builtins, this wraps it up in a new
`call_with` function which handles pushing arguments onto the stack and
recursively calling the (partially applied) function.
Change-Id: I14700c639a0deca53b9a060f6d70dbc7762e9007
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6910
Autosubmit: grfn <grfn@gws.fyi>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Factor out the construction of Value::Attrs (including the Rc) into a
new `attrs` constructor function, to abstract away the presence of the
Rc itself.
Change-Id: I42fd4c3841e1db368db999ddd651277ff995f025
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6892
Autosubmit: grfn <grfn@gws.fyi>
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
This lets the VM emit warnings when it encounters situations that
should only be warned about at runtime.
For starters, this is used to pass through compilation warnings that
come up when `import` is used.
Change-Id: I0c4bc8c534d699999887c430d93629fadfa662c4
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6868
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
There are actually two different types of observers, the ones that
observe the compiler (and emitted chunks from different kinds of
expressions), and the ones that trace runtime execution.
Use of the NoOpObserver is unchanged, it simply implements both
traits.
Change-Id: I4277b82674c259ec55238a0de3bb1cdf5e21a258
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6852
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
This can actually legitimately be emitted by the compiler currently
when compiling formals with default values. See the scope6 test from
the Nix test suite for an example.
We should restructure this slightly to be able to reintroduce a
runtime error here in case something was compiled incorrectly.
Change-Id: Ib81f0f58ae0e850db9fbc459458b7bd0d3ac6f23
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6841
Autosubmit: tazjin <tazjin@tvl.su>
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This implements __functor calling in situations where `OpTailCall` is
used, but not yet for `OpCall`.
For some reason I have not yet figured out, this same implementation
does not work in call_value, which means that it also doesn't yet work
in builtins that apply functions.
Change-Id: I378f9065ac53d4c05166a7d0151acb1f55c91579
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6826
Autosubmit: tazjin <tazjin@tvl.su>
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This function previously kept a borrow in the form of the
`Thunk::value` result alive while performing arbitrary actions in the
VM, which caused a borrowing error in the test case attached.
The `Ref` value must never be used in cases where control flow is
passed to other parts of the VM.
Change-Id: I41d10aa1882a2166614b670e8ba77aab0e67deca
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6825
Reviewed-by: grfn <grfn@gws.fyi>
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This implementation, which only ever worked for non-recursive
attribute sets, is no longer needed and thus removed here.
We have a new implementation of these nested keys coming up instead.
Change-Id: I0c2875154026a4f5f6e0aa038e465f54444bf721
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6783
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This is a little ugly because the plain Iterator::filter method can
not be used (it does not support fallible primitives), so we need to
resort to an `Iterator::filter_map` and deal with the wrapping in
Options everywhere.
This prevents use of `?` which introduces the need for some matching,
but it's not *too* bad.
Change-Id: Ie2c3c0c9756c4c627176f64fb4e0054e717c26d1
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6765
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
The simplest solution seems to be to pass references to arithmetic_op!()
which avoids the moving annoyance we had to deal with in the
builtins (no more popping!). We then use .force() to force the values
and dereference any Thunks (which arithmetic_op! doesn't do for us).
Change-Id: I0eb8ad60e80a0b3ba9d9f411e973ef8bcf136989
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6724
Tested-by: BuildkiteCI
Reviewed-by: wpcarro <wpcarro@gmail.com>
Reviewed-by: tazjin <tazjin@tvl.su>
This makes it possible to call a callable value (builtin or
closure/lambda) directly, without unwrapping it first. This is needed
for pretty much all higher-order functions to work correctly.
This is mostly equivalent to the previous code in coerce_to_string for
calling `__toString`, except it expects the argument(s) to already be
placed on the stack.
Note that the span for the `NotCallable` error is not currently
guaranteed to make any sense, will experiment with this.
Change-Id: I821224368d438a28900858b343defc1817e46a0a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6717
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Pass in, but ignore, a mutable reference to the VM to the `nix_eq`
functions, in preparation for using that VM to force thunks during
comparison.
Change-Id: I565435d8dfb33768f930fdb5a6b0fb1365d7e161
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6651
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Using rust's PartialEq trait to implement Nix equality semantics is
reasonably fraught with peril, both because the actual laws are
different than what nix expects, and (more importantly) because certain
things actually require extra context to compare for equality (for
example, thunks need to be forced). This converts the manual PartialEq
impl for Value (and all its descendants) to a *derived* PartialEq
impl (which requires a lot of extra PartialEq derives on miscellanious
other types within the codebase), and converts the previous
nix-semantics equality comparison into a new `nix_eq` method. This
returns an EvalResult, even though it can't currently return an error,
to allow it to fail when eg forcing thunks (which it will do soon).
Since the PartialEq impls for Value and NixAttrs are now quite boring,
this converts the generated proptests for those into handwritten ones
that cover `nix_eq` instead
Change-Id: If3da7171f88c22eda5b7a60030d8b00c3b76f672
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6650
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
This matches the name of the AST node from which it was compiled.
Suggested by sterni in cl/6231
Change-Id: Ia51525158d2f47467c01fce2282005b1a8417a47
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6623
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: grfn <grfn@gws.fyi>
With this puzzle piece of string compilation in place, `compile_str`
becomes less redundant, as every part now needs to be compiled the same.
The thunking logic becomes a bit trickier, since we need to thunk even
in the case of `count == 1` if the single part is interpolating.
Splitting the inner (shared) code in a separate function turned out to
be easier for making rustc content.
Change-Id: I6a554ca599926ae5907d7acffce349c9616f568f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6582
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Implement C++ Nix's `EvalState::coerceToString` minus some of the Path
/ store handling. This is currently only used for `toString` which does
all possible coercions, but we've already prepared the weaker coercion
variant which is e.g. used for builtins that expect string arguments.
`EvalState::coerceToPath` is still missing for builtins that need a
path, but it'll be easy to build on top of this.
Change-Id: I78d15576b18921791d04b6b1e964b951fdef22c6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6571
Autosubmit: sterni <sternenseemann@systemli.org>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Avoids accidentally dropping one on the floor if we add more, pointed
out by sterni in cl/6372
Change-Id: Ib7bb0ce9c8331c8337003d20c4d5240dfae1c32a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6570
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
As suggested by sterni in cl/6453.
Change-Id: I3cf80d97c11fd7d085ab510f6be4b5f937c791ec
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6562
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This completely rewrites the handling of "dynamic upvalues" to,
instead of resolving them at thunk/closure instantiation time (which
forces some values too early), capture the entire with stack of parent
contexts if it exists.
There are a couple of things in here that could be written more
efficiently, but I'm first working through this to get to a bug
related to with + recursion and the code complexity of some of the
optimisations is distracting.
Change-Id: Ia538e06c9146e3bf8decb9adf02dd726d2c651cf
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6486
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This struct will be responsible for tracking upvalues (and is a
convenient place to introduce optimisations for reducing value clones)
instead of a plain value vector.
The main motivation for this is that the upvalues will have to capture
the `with`-stack fully and I want to avoid duplicating the logic for
this between the two capturing types.
Change-Id: I6654f8739fc2e04ca046e6667d4a015f51724e99
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6485
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
As pointed out by grfn on cl/6091
Change-Id: I28308577b7cf99dffb4a4fd3cc8783eb9ab4d0d6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6460
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
If the last operation within a chunk is a function call, the call can
be executed in the same call frame without increasing the depth of the
call stack.
To enable this, a new OpTailCall instruction (similar to OpCall) is
introduced, but not yet emitted by the compiler.
Change-Id: I9ffbd7da6d2d6a8ec7a724646435dc6ee89712f2
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6457
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This introduces a macro to do the forcing, but this solution isn't
very nice and also does not work in all cases yet.
Change-Id: Icd18862ec47edb82c0efc3af5835a6cb6126f629
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6456
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
These methods make it possible to trace the runtime execution of the
VM through an observer.
Change-Id: I90e26853ba2fe44748613e7f761ed5c1c5fc9ff7
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6452
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This is a step towards hiding the internal fields of thunk, and making
the interface of the type more predictable.
Part of the preparation for implementing observers.
Change-Id: I1a88a96419c72eb9e2332b56a2dd94afa47e6f88
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6447
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This makes it easier to track exactly which lambda is which when
inspecting e.g. the concrete representation of a thunk.
At runtime all lambdas live in an Rc. To make this print the right
address, the construction of these Rcs had to be moved up right to the
point where the lambda is first emitted (and disassembled).
Change-Id: I6070e6c8ac55f0bd697966c4e7c5565c20d19106
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6435
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
With this change the runtime trace contains much more exact
information about the context of the computation (entering/exiting
calls etc.)
This is in large part due to moving the tracer to be a field on the VM
itself, which enables consistent ordering of traces across the
execution, and tracing an execution with its *input* instead
of *output* stack.
Change-Id: Ibe525e6e7d869756501e52bef1a441619ce7332c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6419
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This makes it much easier to figure out what happened while debugging
this sort of thing.
Change-Id: I2e0e8096709adc647d63c04f213c547c415e5f44
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6418
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Previously, "calling" (setting up the VM run loop for executing a call
frame) and "running" (running this loop to completion) were separate
operations.
This was basically an attempt to avoid nesting `VM::run` invocations.
However, doing things this way introduced some tricky bugs for exiting
out of the call frames of thunks vs. builtins & closures.
For now, we unify the two operations and always return the value to
the caller directly. For now this makes calls a little less effective,
but it gives us a chance to nail down some other strange behaviours
and then re-optimise this afterwards.
To make sure we tackle this again further down I've added it to the
list of known possible optimisations.
Change-Id: I96828ab6a628136e0bac1bf03555faa4e6b74ece
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6415
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
The casting methods of `Value` are pretty verbose, and actually
incorrect before this commit as they did not account for inner thunk
values.
To address this, we first attempt to make them correct by introducing
a standard macro to generate them and traverse the inner thunk(s) if
necessary.
This is likely to be a performance hit as it will now involve more
cloning of values. We can do multiple things to alleviate this, but
should do some measurements first.
Change-Id: If315d6e2afe7b69db727df535bc6cbfb89a691aa
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6412
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This makes it possible for builtins to force values on their own,
without the VM having to apply a strictness mask to the arguments
first.
Change-Id: Ib49a94e56ca2a8d515c39647381ab55a727766e3
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6411
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Previously error spans were optional because the information about
code spans was not available at runtime. Now that this information has
been added, the error type will always carry a span.
This change is very invasive all throughout the codebase. This is due
to the fact that many functions that are called *by* the VM expected
to return `EvalResult`, but this no longer works as the span
information is not available to those functions - only to the VM
itself.
To work around this the majority of these functions have been changed
to return `Result<T, ErrorKind>` instead and an accompanying macro in
the VM constructs the "real" error.
Note that this implementatino currently has a bug where errors
occuring within thunks will yield the location at which the thunk was
forced, not the location at which the error occured within the code.
This will be fixed soon, but the commit is large enough as is.
Change-Id: Ib1ecb81a4d09d464a95ea7ea9e589f3bd08d5202
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6408
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
First pass at supporting `builtins` for tvix. The following tests appear to be
WAI:
```shell
$ cd tvix/eval
$ cargo build
$ cargo test
```
Change-Id: I27cce23d503b17a886d1109e285e8b4be4264977
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6405
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Note that I've allowed `needless_lifetimes` for the attribute set
iterator, as I find the type easier to understand with these
annotations present.
Change-Id: I33abb17837ee4813076cdb9a87f54bac4a37044e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6373
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This essentially makes the VM behave like `nix-instantiate --eval
--strict`, i.e. data structures are traversed strictly and thunks are
forced. Thunks embedded in closures are not forced.
This allows us to re-enable tests that were disabled because they
needed to output nested thunk contents, but is overall a behaviour
that must be configurable later on, as it is not cmopatible with e.g.
an evaluation of nixpkgs.
Change-Id: I5303a5c8e4322feab1384fdb7712fecb950afca5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6372
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
The VM previously took care of repeatedly forcing a thunk until it
reached an evaluated state. This logic is now encapsulated inside of
the `Thunk::force` implementation.
In addition, force no longer returns a reference to the value by
default, leaving it up to callers to decide whether they want to
borrow the value or not (a helper is provided for this).
Change-Id: I2aa7da922058ad1c57fbf8bfc7785aab7971c02b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6365
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This operation forces the evaluation of a thunk.
There is some potential here for making an implementation that avoids
some copies, but the thunk machinery is tricky to get right so the
first priority is to make sure it is correct by keeping the
implementation simple.
Change-Id: Ib381455b02f42ded717faff63f55afed4c8fb7e3
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6352
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Implements an operation very similar to `OpClosure` which populates a
thunk's upvalues and leaves it on the stack.
Change-Id: I753b4dfeeaae6919316c7028ec361aaa13d87646
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6350
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
