I assumed that AttrsRep::KV represented attrsets with a single
attribute as a Key-Value pair. That is not the case. Let's warn
other people about this.
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Change-Id: Ie3d2765fcc1ab705c153ab94ffe77bbd6d4ab39e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7093
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Rustc uses wasm32-unknown-unknown, which is rejected by config.sub,
for wasm-in-the-browser environments. Rustc should be using
wasm32-unknown-none, which config.sub accepts. Hopefully the rustc
people will change their triple before stabilising this triple. In
the meantime, we fix it here in order to unbreak tvixbolt.
https://doc.rust-lang.org/beta/nightly-rustc/rustc_target/spec/wasm32_unknown_unknown/index.html
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Change-Id: I941fd8d6f3db4e249901772fd79321ad88cd9cc6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7107
Reviewed-by: tazjin <tazjin@tvl.su>
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This commit contains two search-and-replace renames which are broken
out from I04131501029772f30e28da8281d864427685097f in order to
reduce the noise in that CL:
- `is_thunk -> is_suspended_thunk`, since there are now
OpThunkClosure and OpThunkSuspended
- `compile_lambda_or_thunk` -> `compile_lambda_or_suspension`
Change-Id: I7cc5bbb75ef6605e3428c7be27e812f41a10c127
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7037
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
CL/6867 added support for builtins.import, which required a cyclic
reference import->globals->builtins->import. This was implemented
using a RefCell, which makes it possible to mutate the builtins
during evaluation. The commit message for CL/6867 expressed a
desire to eliminate this possibility:
This opens up a potentially dangerous footgun in which we could
mutate the builtins at runtime leading to different compiler
invocations seeing different builtins, so it'd be nice to have
some kind of "finalised" status for them or some such, but I'm not
sure how to represent that atm.
This CL replaces the RefCell with Rc::new_cyclic(), making the
globals/builtins immutable once again. At VM runtime (once opcodes
start executing) everything is the same as before this CL, except
that the Rc<RefCell<>> introduced by CL/6867 is turned into an
rc::Weak<>.
The function passed to Rc::new_cyclic works very similarly to
overlays in nixpkgs: a function takes its own result as an argument.
However instead of laziness "breaking the cycle", Rust's
Rc::new_cyclic() instead uses an rc::Weak. This is done to prevent
memory leaks rather than divergence.
This CL also resolves the following TODO from CL/6867:
// TODO: encapsulate this import weirdness in builtins
The main disadvantage of this CL is the fact that the VM now must
ensure that it holds a strong reference to the globals while a
program is executing; failure to do so will cause a panic when the
weak reference in the builtins is upgrade()d.
In theory it should be possible to create strong reference cycles
the same way Rc::new_cyclic() creates weak cycles, but these cycles
would cause a permanent memory leak -- without either an rc::Weak or
RefCell there is no way to break the cycle. At some point we will
have to implement some form of cycle collection; whatever library we
choose for that purpose is likely to provide an "immutable strong
reference cycle" primitive similar to Rc::new_cyclic(), and we
should be able to simply drop it in.
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Change-Id: I34bb5821628eb97e426bdb880b02e2097402adb7
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7097
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
This adds a comment noting that StackIdx is an offset relative to
the base of the current CallFrame, whereas UpvalueIdx is an absolute
index into the upvalues array.
It also removes the confusing mention of StackIdx in the descriptive
comment for LocalIdx. They index into totally different structures;
one exists at runtime and the other exists at compile time.
Change-Id: Ib932b1b0679734c15001e8c5c95a08293fa016b4
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7017
Reviewed-by: grfn <grfn@gws.fyi>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
This adds a function NixList::force_elements() which forces each
element of the list shallowly. This behavior is needed for
`builtins.replaceStrings`, and probably a few other builtins as
well.
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Change-Id: I3f0681acbbfe50e781b5f07b6a441647f5e6f8da
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7094
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Unfortunately we have to mangle test case filenames into rust-valid
symbols, since test-generator doesn't use `r#"..."` (deliberately?).
This means that when a test fails, there's nothing on the console
you can copy-and-paste in order to view/edit the code of the failing
test case.
This commit (partially) fixes it by including the unmangled name in
the panic!() string. However failures due to panic!()s inside the
vm (including deliberate panics due to panic!()-debugging) still
won't display an unmangled filename.
Maybe we should reconsider the use of test-generator?
Change-Id: I2208a859ffab1264f17f48fd303ff5e19675967e
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7092
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Rather than implementing all of the interesting semantics of value
comparison with a macro bound to the VM, implement the bulk of the logic
with a method on Value itself that returns an Ordering, and then use the
macro to implement the comparison against that Ordering. This has no
functional change, but paves the way to implementing lexicographic
comparison of list values, which is supported in the latest version of
upstream nix.
Change-Id: I8af1a020b41577021af5939f5edc160c407d4a9e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7069
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
I played around a little bit with doing this in-place, but ended up
going with this perhaps slightly clone-heavy approach for now because
ideally most clones on Value are cheap - but later we should benchmark
alternate approaches that get to reuse allocations better if necessary
or possible.
Change-Id: If998eb2056cedefdf2fb480b0568ac8329ccfc44
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7068
Autosubmit: grfn <grfn@gws.fyi>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
The last bump in langVersion (5->6) in C++ Nix was due to making lists
comparable (commit `09471d2680292af48b2788108de56a8da755d661`), which
we support in Tvix with cl/7070.
Change-Id: Id3beed5150b8fb6e0a46a4d1b7e3942022a65346
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7074
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This commit implements builtins.currentSystem, by capturing the
cargo environment variable `TARGET` and exposing it to rustc as
`TVIX_CURRENT_SYSTEM` so it can be inserted into the source code
using `env!()`.
The resulting value needs to be massaged a bit, since it is an "LLVM
triple". The current code should work for all the platforms for
which cppnix works (thanks qyliss for generating the list!). It
does *not* reject all of the triples that cppnix's configure.ac
rejects -- it is much more forgiving. We can tighten this up in a
future commit.
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Change-Id: I947f504b2af5a7fee8cf0cb301421d2fc9174ce1
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6986
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Since we push arguments onto a stack when calling multi-argument
functions, we actually were ending up calling `call_with` with the
arguments in the *reverse order* - we patched around this by passing the
arguments in the reverse order for `foldl'`, but it makes more sense to
have them just be the order that the function would be called with in
user surface code instead.
Change-Id: Ifddb98f46970ac89872383709c3ce758dc965c65
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7067
Reviewed-by: sterni <sternenseemann@systemli.org>
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
When compiling a lambda, take the name of the outer slot (if
available) and store it as the name on the lambda.
These names are then shown in the observer, and nowhere else (so far).
It is of course common for these things to thread through many
different context levels (e.g. `f = a: b: c: ...`), in this setup only
the outermost closure or thunk gains the name, but it's better than
nothing.
Change-Id: I681ba74e624f2b9e7a147144a27acf364fe6ccc7
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7065
Reviewed-by: grfn <grfn@gws.fyi>
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
There are some rare scope cases with deferred access where this
doesn't behave correctly otherwise.
Change-Id: I6c774f5e62c1cb50b598026c54727017a52cd22d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7064
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
The warning needs to consider whether it is occuring inside of a
thunk, i.e. the dynamic ancestry chain of lambda contexts must be
inspected and not just the current scope.
Change-Id: I5cf5482d67a8bbb9f03b0ecee7a62f58754f8e59
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7063
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: grfn <grfn@gws.fyi>
This check is now actually simply equivalent to checking whether the
target has been initialised or not.
Change-Id: I30660d11073ba313358f3a64234a90ed81abf74c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7062
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
These are builtins which can be basically implemented as the identity
function from the perspective of pure evaluation, and which help us
get closer to evaluating nixpkgs.
For now, builtins added here will be "usable" and just emit a warning
about not being implemented yet.
Change-Id: I0fce94677f01c98c0392aeefb7ab353c7dc7ec82
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7060
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
Checking the computed depth and stack slot against the computed depth
and stack slot is equivalent to just checking the indices into the
locals vector against each other (i.e. "is the slot we're compiling
into the slot we're accessing?")
Change-Id: Ie85a68df073e3b2e3d9aba7fe8634c48eada81fc
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7059
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
When capturing an upvalue, return a detailed TvixBug error that
contains metadata about what exactly was missing.
This particular thing helps with debugging some scope issues we still
seem to have.
Change-Id: I1089a1df4b3bbc63411a4907c3641a5dc3fad984
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7058
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
Using the same method as in Thunk::deep_force, detect cycles when
printing values by maintaining a set of already seen thunks.
With this, display of infinite values matches that of Nix:
> nix-instantiate --eval --strict -E 'let as = { x = 123; y = as; }; in as'
{ x = 123; y = { x = 123; y = <CYCLE>; }; }
> tvix-eval -E 'let as = { x = 123; y = as; }; in as'
=> { x = 123; y = { x = 123; y = <CYCLE>; }; } :: set
Change-Id: I007b918d5131d82c28884e46e46ff365ef691aa8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7056
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
This can be raised in cases where some panics lurk (e.g. indexed
accesses).
Change-Id: I93f4d60568277e9c5635aa52f378645626d68c5e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7057
Reviewed-by: grfn <grfn@gws.fyi>
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>
Failures to resolve a nix search path lookup in angle brackets can be
caught using tryEval (if it reaches the runtime). Resolving relative
paths (either to the current directory or the current user's home) can
never be caught, even if they happen inside a thunk at runtime (which is
currently the case for home-relative paths).
Change-Id: I7f73221df66d82a381dd4063358906257826995a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7025
Autosubmit: sterni <sternenseemann@systemli.org>
Reviewed-by: Adam Joseph <adam@westernsemico.com>
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
cl/7036 moved these paths around, but neglected to update the relative
paths they contain. Without these updated, they will never start
passing!
Change-Id: Ib16468611af59729883e501be8486f43d850fd58
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7046
Autosubmit: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
eval-okay-eq.nix is actually an ancient test case that used the ATerm
syntax for the test result. It was disabled for a while, since the
behavior got reverted for a bit, but works on any reasonably modern
Nix implementation. This change matches my [C++ Nix PR].
[C++ Nix PR]: https://github.com/NixOS/nix/pull/7196
Change-Id: I602fd7c83a0bc104ab502c8b6a74e4591272be1a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7045
Autosubmit: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
The language test suite actually doesn't require flakes and the
new features are mostly sensible (added builtins) as well as some
tests for regressions the C++ implementation experienced.
The path interpolation test is not included in this update because there
is no way to construct an location-independent .exp file for it (the C++
repo also doesn't have one). We may still want to implement that feature
eventually (in case rnix adds support for it).
The C++ Nix revision used is ac0fb38e8a5a25a84fa17704bd31b453211263eb.
Change-Id: I75f1e780ddeeee6f6b1f28cf3c66c288dca2c20c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7043
Autosubmit: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Since cl/7036 we have a mechanism for dealing with the nix_tests we do
not pass yet.
Change-Id: I246c52963ae7f2500253f4035a77d7006dd35307
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7049
Autosubmit: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Reimplement the test discovery of the lang tests script in Nix which
allows for a more flexible skipping logic that can e.g. react to the C++
Nix version used. This allows us to run the test suite against both
C++ Nix 2.3 and the latest C++ Nix version 2.11. The latter is mainly
useful, so we can implement newer Nix features and still verify them
against the C++ implementation.
Change-Id: I30c802844133b86b5e49f5e4f4fefacdb6215e0e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7042
Autosubmit: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
It is helpful to be able to use the test suite as a regression test:
make a change to the compiler/vm, re-run the tests, and if there are
any failures you know it's your fault.
Right now we can't do that, because the expected-to-fail tests are
mixed in with the expected-to-pass tests. So we can't use them as a
regression test.
Change-Id: Ied606882b9835a7effd7e75bfcf3e5f827e0a2c8
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7036
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
This commit deduplicates the Thunk-like functionality from Closure
and unifies it with Thunk.
Specifically, we now have one and only one way of breaking reference
cycles in the Value-graph: Thunk. No other variant contains a
RefCell. This should make it easier to reason about the behavior of
the VM. InnerClosure and UpvaluesCarrier are no longer necessary.
This refactoring allowed an improvement in code generation:
`Rc<RefCell<>>`s are now created only for closures which do not have
self-references or deferred upvalues, instead of for all closures.
OpClosure has been split into two separate opcodes:
- OpClosure creates non-recursive closures with no deferred
upvalues. The VM will not create an `Rc<RefCell<>>` when executing
this instruction.
- OpThunkClosure is used for closures with self-references or
deferred upvalues. The VM will create a Thunk when executing this
opcode, but the Thunk will start out already in the
`ThunkRepr::Evaluated` state, rather than in the
`ThunkRepr::Suspeneded` state.
To avoid confusion, OpThunk has been renamed OpThunkSuspended.
Thanks to @sterni for suggesting that all this could be done without
adding an additional variant to ThunkRepr. This does however mean
that there will be mutating accesses to `ThunkRepr::Evaluated`,
which was not previously the case. The field `is_finalised:bool`
has been added to `Closure` to ensure that these mutating accesses
are performed only on finalised Closures. Both the check and the
field are present only if `#[cfg(debug_assertions)]`.
Change-Id: I04131501029772f30e28da8281d864427685097f
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7019
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
In `a++b`, the previous implementation would move `b` (i.e. memcpy
its elements) twice. Let's do that only once.
We sure do call NixList.clone() a whole lot. At some point in the
future we probably want to do a SmolStr-type split for NixList into
a two-variant enum where one side is an Rc<Vec<Value>> for lists
longer than a certain length.
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Change-Id: I32154d18785a1f663454a8b9d4afd3e78bffdf9c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7040
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
This commit adds a comment for the benefit of non-Lua-users
explaining what an upvalue is. It also adds a comment explaining
what `with_stack` does, including a brief reminder of nix's
slightly-unusual-but-well-justified handling of this construct.
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Change-Id: If6d0e133292451af906e1c728e34010f99be8c7c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7007
Reviewed-by: j4m3s <james.landrein@gmail.com>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
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
In preparation for both implementing the `functionArgs` builtin and
adding support for validating closed formals, record information about
the formal arguments to a function *on the Lambda itself*. This may seem
a little odd for the purposes of just closed formal checking, but is
something we have to have anyway for builtins.functionArgs so I figured
I'd do it this way to kill both birds with one stone.
Change-Id: Ie3770a607bf352a1eb395c79ca29bb25d5978cd8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7001
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
This resolves a TODO.
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Change-Id: If4d2124648ac88094e547e1ad7f1b446feb26182
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7010
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
The previous serialisation format kind of lost the information about
what AST node we're dealing with (e.g. `1234` would serialise to an
AST with a literal `1234`).
That's great for pretty-printing the _code_, but we explicitly want to
serialise how rnix-parser parses something.
To that end, literals are now instead serialised into a structure like
all the other ones (`kind: literal` and appropriate value fields).
Change-Id: I586c95d7db41820b8ec43565ba4016ed3834d1b5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7030
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Reviewed-by: j4m3s <james.landrein@gmail.com>
Reviewed-by: Adam Joseph <adam@westernsemico.com>
Home relative paths depend on the environment to be resolved. We have
elected to do everything that depends on the environment, e.g. resolving
SPATH expressions using NIX_PATH, at runtime, so tvix evaluation would
continue to behave correctly even if we separated the compilation and
execution phases more, e.g. via serializing bytecode. Then the value of
HOME, NIX_PATH etc. could reasonably change in the time until execution,
yielding wrong results if the resolution results were cached in the
bytecode.
We also take the opportunity to fix the broken path concatenation
previously found in the compiler, fixing b/205.
Another thing we could consider is emitting a warning for home relative
path literals, as they are by nature relatively fragile.
One sideeffect of this change is that home path resolution errors
become catchable which is not the case in C++ Nix. This will need to be
fixed up in a subsequent change.
Change-Id: I30bd69b575667c49170a9fdea23a020565d0f9ec
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7024
Autosubmit: sterni <sternenseemann@systemli.org>
Reviewed-by: Adam Joseph <adam@westernsemico.com>
Tested-by: BuildkiteCI
To assert that OpFindFile is only emitted for specially compiled SPATH
expressions, as well as make sure it doesn't accidentally operate on
“ordinary values”, introduce an UnresolvedPath internal value. If
OpFindFile sees a non-UnresolvedPath value, it'll crash.
Note that this change is not done purely for OpFindFile: We may want to
compile SPATH expressions as function calls to __findFile (like C++ Nix
does) in the future, so the UnresolvedPath value would definitely need
to be an ordinary string again then. Rather, this change is done in
preparation for resolving home dir relative paths at runtime (since they
depend on the environment) for which we'll need a similar mechanism to
OpFindFile.
Change-Id: I6acf287f35197cd9e13377079f972b9d36e5b22e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7023
Autosubmit: sterni <sternenseemann@systemli.org>
Reviewed-by: Adam Joseph <adam@westernsemico.com>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
This implements serde::Serialize for the rnix AST through a wrapper
type, and exposes a function for serialising the AST into
a (pretty-printed JSON) string representation.
This can be used to debug issues with the AST, and to display an AST
reprsentation in tools like tvixbolt.
Serialize is implemented manually because we don't own any of the
structs and the way to traverse them is not easily derived
automatically, and this is quite verbose. We might be able to condense
it a little bit, but at the same time it's also fairly straightforward.
Change-Id: I922df43cfc25636f3c8baee7944c75ade516055c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6943
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Reviewed-by: Adam Joseph <adam@westernsemico.com>
Reviewed-by: tazjin <tazjin@tvl.su>
Lambda has a quite large and variable-sized runtime representation,
unlike Rc<Lambda>. It would be easy to accidentally call clone() on
this and create input-dependent performance regressions.
Nothing in the codebase is currently using Lambda.clone(). Let's
remove the derived instance. If it's really needed it is very easy
to add it back in, but whoever does that will have to look at the
struct they're adding Clone to, which will hopefully prompt them to
think about whether or not that's really what they want to do.
Change-Id: I7806a741862ab4402229839ce3f4ea75aafd6d12
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7029
Autosubmit: Adam Joseph <adam@westernsemico.com>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
I believe this variant is left over from a previous implementation.
If not, please let me know.
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Change-Id: I02a3bf2f63794d09e96a5a92a034c0ad3d1ff221
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7027
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
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.toPath. Like OP_ADD, it currently
does not handle string contexts.
This commit allows the
tests::nix_eval_okay_src_tests_nix_tests_eval_okay_pathexists_nix
test to pass.
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Change-Id: Iadd4f7605f8f297adbd0dba187b8481c21370b6e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6996
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Maybe I misunderstood this part of the code, but the use of `unsafe`
appears unnecessary here? In any event it is the one and only
`unsafe` in the codebase.
Hopefully getting to "no `unsafe` anywhere" is worth the extra
never-taken branch caused by unwrap() instead of unwrap_unchecked()?
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Change-Id: I33fbd5aad9d8307ea82c24b6220412783e1973c6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7011
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
The case branch in vm.rs for OpResolveWithOrUpvalue is
unreachable/deadcode.
I believe this opcode is unnecessary, since it should always be
statically detectable (at parse-time) whether a reference is to an
upvalue (i.e. enclosing binding); otherwise, and only then, is
with-resolution applicable.
Perhaps I've misunderstood how with-resolution works. If so, please
explain it to me and -1/-2 this CL.
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Change-Id: I4a90b9eb6cb3396df92a6a943d42ecc301871ba0
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7009
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
This makes it easier to compare currently implemented ones with the full
list.
Change-Id: Ibaffd99d05afa15fc9ab644fd101afa24fc7a1b2
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7008
Tested-by: BuildkiteCI
Autosubmit: j4m3s <james.landrein@gmail.com>
Reviewed-by: tazjin <tazjin@tvl.su>
When I first read the `README.md` I didn't know what this `mg build`
was. Some grepping around turned up `/tools/magrathea`. Apparently
this thing "helps me build planets". Awesome! Let's tell other
people where to find it. It seems that this tool is (currently)
specific to depot, so people who arrive at depot because of tvix
probably won't know where to find `mg`.
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Change-Id: Icb8c51087fd41b696794516abcfee24a4b3b4a14
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6948
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: tazjin <tazjin@tvl.su>
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>
This commit adds support for running the "expected failure" tests in
both the nix and tvix test suites.
I have disabled the eval-fail-blackhole.nix test because it gets
stuck running forever.
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Change-Id: Iba75ce6c8f2becab3c834fcfdd9f4fdc5a4bdb9f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6990
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Reviewed-by: grfn <grfn@gws.fyi>
cppnix has merged #7149, so let's update our copy of their tests:
https://github.com/NixOS/nix/pull/7149
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Change-Id: I0be3bf9da937abd24102e1997daa2087ecfafd98
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6992
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
This path normalisation business causes runtime panics on WebAssembly
because those operations are unsupported.
Maybe this shouldn't be happening in the compiler anyways, not sure,
but for now this commit adds a workaround based on the target to
disable the normalisation if we're compiling for wasm.
Change-Id: I908a84fbdffc3401f8d443e2c73ec673e9f397ff
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7004
Autosubmit: tazjin <tazjin@tvl.su>
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Cppnix immediately absolutizes pathnames at parse time; if you write
`./foo`, it is immediately converted to `$(pwd)/foo` and manipulated
as an absolute path at all times.
To avoid having to introduce filesystem access operations in the
implementation of otherwise-pure builtins, let's guarantee that the
`root_dir` of the VM is always an absolute path.
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Change-Id: I7cbbae2cba4b2716ff3f5ff7c9ce0ad529358c8a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6995
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Right now we're pretending that the Rust library path_clean does the
same thing that cppnix's canonPath() does. This is not true. It's
close enough for the test suite, but may come back to bite us.
Let's create our own canon_path() function and call that in all the
places where we intend to match the behavior of cppnix's
canonPath(). That way when we fix this we can fix it once, in one
place.
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Change-Id: Ia6f9577f62f49ef352ff9cfa5efdf37c32d31b11
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6993
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
On Debian machines (and most FHS distros) /bin is now a symlink to
/usr/bin, which causes this test to fail. Let's use /etc as a
root-relative test case, since it is less likely to be a symlink.
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Change-Id: I1c94de0d2a242394182442fe1c895dc17eb04a4a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6994
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
The nix_tests test suite produces lots of warnings. We can't fix
these, since they are kept in sync with upstream, so there's little
point in cluttering up the console with them every time the tests
are run.
Let's add a clap flag "warnings" and TVIX_WARNINGS environment
variable. The default is "true". The test runner overrides this
default and mutes the warnings.
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Change-Id: I4b065f96fe15838afcca6970491a54e248ae4df7
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6985
Reviewed-by: tazjin <tazjin@tvl.su>
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
This commit passes nix_eval_okay_src_tests_nix_tests_eval_okay_versions_nix.
See also: https://github.com/NixOS/nix/pull/7149
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Change-Id: I24605c2a0cd0da434f37f6c518f20693bfa1b799
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6913
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Since NixString is the Rust type for nix strings, people might
mistake NixPath for the Rust type of nix paths, which it is not.
Let's call it NixSearchPath instead.
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Change-Id: Ib2ea155c4b27cb90d6180a04ea7b951d86607373
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6927
Reviewed-by: kanepyork <rikingcoding@gmail.com>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
I broke the build.
I didn't understand that before hitting "submit" you need to re-test
your changes on latest HEAD (and that CI doesn't do that for you); I
failed to re-test cl/6912 following the merge of cl/6907.
This commit fixes the build by removing the overlapping instances.
Change-Id: I2a720d2c60cc7103b350f78102a8998f93bac828
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6965
Tested-by: BuildkiteCI
Autosubmit: Adam Joseph <adam@westernsemico.com>
Reviewed-by: tazjin <tazjin@tvl.su>
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 *maybe* should do something to check that we don't pass both a file
and an expr, but for now this is useful enough to cut corners (plus
we're probably due for a CLI revamp eventually anyway).
Change-Id: Id44357074150b336b6215ba596cc52d01d037dbd
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6941
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
This is currently implemented with a simple println inline, but in the
future we could hook into this via something pluggable on the VM.
Change-Id: Idd9cc3b34aa13d6ebc64c02aade81ecdf439656a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6938
Autosubmit: grfn <grfn@gws.fyi>
Reviewed-by: tazjin <tazjin@tvl.su>
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>
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>
Instead of just printing the number of errors (useless!) actually emit
separate diagnostics for each nested error.
Change-Id: I97b53c3276c906af5def89077b5b6ba6ec108b37
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6933
Reviewed-by: grfn <grfn@gws.fyi>
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
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
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
Add a simple struct implementing both the string parsing and path
resolution rules of Nix's `NIX_PATH` environment variable, for use in
resolving `<...>`-style paths
Change-Id: Ife75f39aa5c12928278d81fe428fbadc98bac5cc
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6917
Autosubmit: grfn <grfn@gws.fyi>
Reviewed-by: tazjin <tazjin@tvl.su>
Reviewed-by: Adam Joseph <adam@westernsemico.com>
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
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>
This is generally more idiomatic (over just delegating to Debug), and
also allows us to avoid intermediate allocations if we ever end up
using error messages as part of larger strings (because we don't have to
allocate a full String for the return value).
Change-Id: I67e48b44570c72761ed0fcaded9ae4bf3fcbaacf
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6896
Autosubmit: grfn <grfn@gws.fyi>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Similar to what we do for import, push on a `default.nix` to the path
that the top-level is invoked with (if any) if it's a directory.
Change-Id: I281bd44e3c8803b6765c886ae5fd08f549e2e563
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6895
Autosubmit: grfn <grfn@gws.fyi>
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
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
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
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
Adds secondary spans for errors that occur deeply nested within a
thunk.
This is pretty raw right now, there's technically nothing stopping one
of these error chains from being a hundred thunks deep into code,
producing unmanageable error output. We should trim these down
according to some heuristics (e.g. when crossing file boundaries, o r
just - for starters - beginning and end).
Change-Id: Ia73892512737850b6fa3e07cabc37fa9c534c4d5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6872
Reviewed-by: sterni <sternenseemann@systemli.org>
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
This change is quite verbose, so a little bit of explaining:
1. To correctly format parse errors, errors must be able to return
more than one annotated span (the parser returns a list of errors
for each span).
To accomplish this, the structure of how the `Diagnostic` struct
which formats an error is constructed has changed to delegate the
creation of the `SpanLabel` vector to the kind of error.
2. The rnix structures don't have human-readable output formats by
default, so some verbose methods for formatting them in
human-readable ways have been added in the errors module. We might
want to move these out into a submodule.
3. In many cases, the errors returned by rnix are a bit strange - so
while we format them with all information that is easily available
they may look weird or not necessarily help users. Consider this CL
only a first step in the right direction.
Change-Id: Ie7dd74751af9e7ecb35d751f8b087aae5ae6e2e8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6871
Reviewed-by: sterni <sternenseemann@systemli.org>
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
This logic was duplicated between the two rnix types before, but more
crucially - it is also needed for correctly displaying the text ranges
contained in syntax errors.
Change-Id: Ifc6a521de1594d6ced9cba6274f1931df99b6634
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6870
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
This is also useful for error-handling related logic, outside of just
the compiler module.
Change-Id: I5c386e2b4c31cda0a0209b31136ca07f00e39e45
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6869
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
This enables the use of string paths (and, in the future,
derivations), as long as their string values represent an absolute
path.
Change-Id: I4b198efeb70415ed52f58bd1da6fa79a24dad14c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6866
Reviewed-by: sterni <sternenseemann@systemli.org>
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
Adding `import` to builtins causes causes a bootstrap cycle because
the `import` builtin needs to be initialised with the set of globals
before being inserted into the globals, which also must contain
itself.
To break out of the cycle this hack wraps the builtins passed to the
compiler in an `Rc` (probably sensible anyways, as they will end up
getting cloned a bunch), containing a RefCell which gives us mutable
access to the builtins.
This opens up a potentially dangerous footgun in which we could mutate
the builtins at runtime leading to different compiler invocations
seeing different builtins, so it'd be nice to have some kind of
"finalised" status for them or some such, but I'm not sure how to
represent that atm.
Change-Id: I25f8d4d2a7e8472d401c8ba2f4bbf9d86ab2abcb
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6867
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
This adds an initial working version of builtins.import which
encapsulates the entire functionality of `import` within the builtin
itself, without requiring any changes in the compiler or VM.
The key insight that enables this is that we can simply return a Thunk
from `import` that is constructed from the output of running the
compiler and - ta-da! - no other component needs to know about it.
A couple of notes:
* builtins.import needs to capture variables like the SourceCode
structure. This means it can not currently be constructed the same
way as other builtins and has special handling, which leaks out to
`eval.rs`. I have postponed dealing with that until we have this
working a bit more.
* the `globals` are not yet passed through
* the error representation for the new variants is absolutely not done
yet, we probably want to switch to something that supports
cause-chaining now (like miette)
* there is no mechanism for emitting warnings at runtime; we need to
add that
Change-Id: I3117a7ae3ff2432bf44f5ff05ad35f47faca31d5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6857
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: wpcarro <wpcarro@gmail.com>
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
For some upcoming builtins (notably, import) we need to capture
arguments in the builtin's implementation.
To allow this, we can no longer use function pointers for builtins,
but must use a reference-counted closure object instead.
Unfortunately this adds an extra pointer operation to every builtin
call. We should benchmark this later against having a split builtin
representation.
Change-Id: I109d98d0e25998870542f47573eb1ec2e546f2a2
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6856
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
nixpkgs has hardcoded references to Nix versions, we need to provide
it with something that looks like a Nix version while actually being a
Tvix version.
For now, we do this by stealing a trick out of the browser book and
constructing a version that looks like a Nix version to Nix, but like
a Tvix version to people who know what they are looking for.
Nevermind that we don't actually have any kind of versioning for
Tvix (yet?), other than depot revisions.
Change-Id: I7ce8079dd8164a2079891d38e707f09a45f0bbc1
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6858
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This type hides away the lower-level handling of most codemap data
structures, especially to library consumers (see corresponding changes
in tvixbolt).
This will help with implement `import` by giving us central control
over how the codemap works.
Change-Id: Ifcea36776879725871b30c518aeb96ab5fda035a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6855
Tested-by: BuildkiteCI
Reviewed-by: wpcarro <wpcarro@gmail.com>
There's basically nothing that needs *ownership* of an AST
node (which is just a little box full of references to other things
anyways), so we can thread this through as references all the way.
Change-Id: I35a1348a50c0e8e07d51dfc18847829379166fbf
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6853
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
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>
Until we can display a chained representatino of errors in thunks, it
is most useful to forward the error code from the innermost error to
the user.
Change-Id: I8d67254d52313be40387f080e57966c001e0d51c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6854
Reviewed-by: sterni <sternenseemann@systemli.org>
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Returns time since epoch in seconds.
This has a slight behaviour difference from Nix, in that we don't pin
the time between REPL entries (Nix pins it for the program lifetime),
but this is probably inconsequential as long as it is pinned during an
evaluation.
Change-Id: I010c02e93097a209d8ad69e278397c7e30e54c86
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6846
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
Reviewed-by: wpcarro <wpcarro@gmail.com>
Allows impure builtins that have a different shape than a Rust
function pointer; specifically this is required for
builtins.currentTime which does not work in WASM.
Change-Id: I1362d8eeafe770ce4d1c5ebe4d119aeb0abb5c9b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6849
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
Reviewed-by: wpcarro <wpcarro@gmail.com>
Concatenates (but not flattens) a list of lists.
Change-Id: I692e0b3e7b5a5ff93d5768d3a27849b432ec5747
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6843
Autosubmit: tazjin <tazjin@tvl.su>
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This is the same code as before, just moved into a trait impl to gain
access to stuff that needs IntoIterator
Change-Id: Iff9375cd05593dd2681fa85ccc7f4554bf944a02
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6842
Autosubmit: tazjin <tazjin@tvl.su>
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
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
Add a new derivation target to the passthru of tvix.eval that builds the
benchmark binaries, *and* copies them to the outupts of the derivation
via the (somewhat arcane) `copyBinsFilter` jq script arg to naersk. This
is a bit annoying because (as far as I can tell) the derivations
returned by naersk aren't directly overridable, so we have to explicitly
fixpoint the attrs we're passing.
Also, since this is now a separate target to build the benchmarks, we
can remove `--all-targets` from the build of `tvix-eval` itself since
that was only added to build benchmarks in CI, and make
regular (non-benchmark) builds a bit faster.
Change-Id: I136b8526790545e93b1ae666abaefb51cbbee390
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6847
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
The idea is that we can keep track of the more unexpected behavior,
behavior that maybe should not be a thing at all and behavior we are not
sure about yet.
Change-Id: I70933f00af1230a7ab9d30e917b61199fe571caf
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6803
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
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 finishes up the implementation of nested keys after the key
insight that the nesting level does not need to be tracked, and
instead the attribute iterator can simply be retained inside the
structures as is (in an advanced state).
With this implementation, when encountering a nested key, the Tvix
compiler will first analyse whether there is already a matching
binding that can be merged (i.e. a binding that is a literal attribute
set), and perform the merge, or otherwise create a new recursive set
of bindings in which the entry is inserted with the path iterator
advanced beyond the first name component.
With this, all the logic simply applies recursively until there are no
more nested bindings (i.e. until all iterators are "empty").
Note that this has one (potentially insignificant) deviation from Nix
currently: If a non-mergable value is supplied (e.g. `a.b = 1; a =
2;`), Tvix will emit a *runtime* error (whereas it is *parse* time in
Nix) as the branch which could statically analyse this is currently
unreachable. There's a TODO for this, so we can fix it up later.
Change-Id: I53df70e09614ff4281a70b80eac7da3beca12da9
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6806
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This is actually quite useless, as we can just pass
`AstChildren<ast::Attr>` around after partially consuming it.
Change-Id: If0aefa2b53fc801fced1ae0709bff93966bf19f8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6804
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
When encountering a nested binding for the first time, cleanly flip
the representation to `Binding::Set` in `Binding::merge` before
proceeding with the actual merge.
This reduces the number of points where we have to deal with the (soon
to be slightly more complex) construction of the nested binding
representation.
Change-Id: Ifd43aac7b59ebd15a72c3ec512386a5bcf26ec13
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6802
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This adds the scaffolding required for tracking the nesting level (and
appropriately skipping the correct amount of attrpath entries when
inserting nested sets).
In order for all of this to work correctly, we can no longer track
`AttrpathValue` directly in the entries vector as rnix does not allow
us to construct values of that type - so instead we have to track its
inner components.
Change-Id: Icb18e105586bf6c247c2e66c302cde5609ad9789
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6801
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
