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>
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
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>
This is a significant step towards correctly implemented nested
bindings. All attribute sets defined within the same binding scope
will now be merged as in Nix, if they use the same key.
Change-Id: I13e056693d5e73192280043c6dd93b47d1306ed6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6800
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This sets up the required logic for finding and merging attribute sets
into nested bindings if they exist. This is absolutely not complete
yet and can, at this commit, probably cause undefined runtime
behaviour if nested attributes are specified.
The basic idea is that a new helper function on the `TrackedBindings`
struct is called with each encountered attribute and determines
whether the new entry can be merged into an existing attribute or not.
Right now the only effect this has in practice is that a new error
becomes available if somebody attempts to cause a merge into an
inherited key.
Change-Id: Id010df3605055eb1ad7fa65241055889dd21bab0
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6798
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This needs to move here so that we can reuse compile_bindings for the
nested attribute sets we're about to start constructing.
Change-Id: Ie83f52f7e1d128886e96a1da47792211fa826f21
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6796
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This struct will be the key to correctly compiling nested bindings, by
having insertions flow through some logic that will attempt to bind
attribute-set-like things when encountering them.
Change-Id: I8b5b20798de60688f3b6dc4526a460ebb2079f6e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6795
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
As of this commit, all three types of bindings scopes are compiled the
same way (i.e. compilation of non-recursive attribute sets has been
switched over to the new code paths).
This sets us up for doing the final implementation of nested attribute
sets.
HOWEVER, this breaks the existing implementation of nested attributes
in non-recursive attribute sets. That implementation is flawed and
unworkable in practice, so we need to do this dance to be able to
implement it correctly.
Change-Id: Iba2545c0d1d6b51f5e1a31a5d005b8d01da546d3
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6782
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This wires up the new bindings setup logic to be able to thread
through & compile dynamic attributes in recursive attrs.
It seems like we don't actually need to retain the phasing of Nix
exactly, as we can use the phantom mechanism to declare all locals
without making the dynamic ones accessible.
Change-Id: Ic2d43dd8fd97d7ccd56d8c6adf2ff97274cd837a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6781
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Another slice of the salami, but no functionality changes yet (other
than opening a code path that can reach a `todo!()`, but this will be
removed soon).
Change-Id: I56b4ed323f70754ed1ab27964ee3c99cf3bf3292
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6780
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
The previous way of sanitising dynamic keys is going away as we're
slowly introducing the new nested key logic.
While touching this stuff, I've also changed all the related string
types to SmolStr as that is more sensible for identifiers.
Change-Id: If30c74151508719d646d0e68e7d6f62c36f4d23f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6779
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Removes the `compile_inherit_attrs` logic which was only used for
BindingsKind::Attrs (i.e. non-recursive attrs).
This brings us a step closer to fully merging all the binding logic
into one block that can dispatch based on the kind of bindings (and
thus giving us a good point to introduce the final logic for nested
bindings).
Change-Id: If48d7a9497fc084a5cc03a130c2a7da5e2b8ef0c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6776
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This helper is responsible for declaring the bindings in the
compiler's scope tracking structures.
It is almost equivalent to the previous logic, but also accounts for
`BindingsKind::Attrs` - though those code paths do not end up here
yet.
Change-Id: I44f617b99b10f2a7b9675f7b23e2c803a4a93d29
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6775
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This is responsible for actually setting up `TrackedBinding`s on the
stack, i.e. in some sense "actually compiling" values in bindings.
There is no functionality change to before, i.e. this is a salami
slice.
Change-Id: Idb0312038e004470a7d130c020ae0fe87c55c218
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6774
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Splits the large `compile_inherits` function which
previously *compiled* plain inherits and *declared* namespaced
inherits into `compile_plain_inherits` and
`declare_namespaced_inherits`.
This is supposed to make more sense than before, but is still not
consistently used (notably, non-recursive attribute sets still
duplicate most of this logic).
Another salami slice.
Change-Id: Id97fac1cbd5ee97b24d047e7728655e6b7734153
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6773
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
... but do not use it yet.
This refactoring is pretty complicated, so I'm applying salami-slicing
tactics here.
Change-Id: I66e04ee10548f68bf67dc842f3f14cc279426c22
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6772
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
As part of the unification of binding logic between different carriers
of bindings, we need to track which kind of bindings we are dealing
with (attribute set? recursive scope? ...) to correctly emit keys and
declare identifiers in the locals stack.
Right now this changes no functionality as `BindingsKind::Attrs` is
not yet used (only RecAttrs and LetIn, which was previously
represented by the `rec_attrs` boolean).
Change-Id: Id2ac27894079ab584521cb568d75c124f7bf2403
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6771
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This helper will gain the ability to compile both kinds of inherits,
but it is kind of tricky to get right so I am doing it in smaller
steps. Right now there is no change in functionality.
Change-Id: Ie990b88dd90a5e0f9fd79961ee09a6c83f2c872d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6770
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This just describes a binding, and we do need a good name for the kind
of binding*s*, which is going to be introduced soon.
Change-Id: I53900ee52da8a07dae8b918fa6a4cb308e627efb
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6768
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Changes the module structure of the compiler to have a module
dedicated to the logic of setting up bindings. This logic is in the
process of being merged between attribute sets and `let`-expressions,
and the structure of the modules makes more sense when ecapsulating
that specifically.
(Other bits of code related to e.g. attribute sets are pretty
straightforward and can just live in the main compiler module).
Change-Id: I9469b73a7034e5b5f3bb211694d97260c4c9ef54
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6766
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This was added in preparation for some builtins that don't exist yet
and is producing some noise during compilation.
Change-Id: I51fb0d14c3edf0bd6d9a288d50e44dacf35166c6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6769
Reviewed-by: flokli <flokli@flokli.de>
Autosubmit: tazjin <tazjin@tvl.su>
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 previous version had a bug where we assumed that the number of
entries in an attribute set AST node would be equivalent to the number
of entries in the runtime attribute set, but due to inherit nodes
containing a variable number of entries, this did not work out.
Fixes b/199
Change-Id: I6f7f7729f3512b297cf29a2e046302ca28477854
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6749
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Fairly straightforward, only thing of note is that we coerce (weakly) to
string here as well.
Change-Id: I03b427e657e402f1f9eb0f795b689bbf5092aba1
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6745
Tested-by: BuildkiteCI
Reviewed-by: wpcarro <wpcarro@gmail.com>
Reviewed-by: tazjin <tazjin@tvl.su>
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>
Because `inherit` can take string identifiers, we can access
arbitrarily weird identifiers in scopes using it.
Change-Id: Ic868233221befa160538dd2ffaff1cc7f566585a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6748
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This updates rnix-parser to a version where inherits provide an
iterator over `ast::Attr` instead of `ast::Ident`, which mirrors the
behaviour of Nix (inherits can have (statically known) strings as
their identifiers).
This actually required some fairly significant code reshuffling in the
compiler, as there was an implicit assumption in many places that we
would have an `ast::Ident` node available when dealing with variable
access (which is then explicitly only not true in this case).
Change-Id: I12f1e786c0030c85107b1aa409bd49adb5465546
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6747
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
We delegate to Rust std's Ord trait, so we should at least do a sanity
check here.
Change-Id: I9596068f8ab3e51b79602de0bac79af6d558086e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6723
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Sketch out a new set of "impure" builtins, which supplement the existing
set of "pure" builtins but are gated behind a feature flag, which allows
them to be omitted by crates depending on tvix-eval that only want pure
evaluation, such as tvixbolt.
Change-Id: I2736017b5c9b4776bbba8758e108ec84887abd66
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6655
Reviewed-by: wpcarro <wpcarro@gmail.com>
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: tazjin <tazjin@tvl.su>
This was derived from
else if (c1 == "" && n2) return true; // true implies c1 < n2
However, this has no effect since Word always looses out against Number
anyways and the `pre` rules are also unaffected by this change – since
this only affects comparison of an empty Word part with a Number.
Change-Id: Ia04e42ac726352b688c87674b0fdb355f06edbcb
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6722
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
This asserts the not-quite lexicographical property of the comparison.
Change-Id: Iad68081e4b3a7106513f479643de87065dc47739
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6721
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
This is necessary because builtins.compareVersions compares versions in
a subtly not-quite-but-still-lexicographical way: `pre` for example can
have an effect if it is post-fixed: `2.3 < 2.3pre`. This is a violation
of the rule that in a lexicographical ordering, the longer string is
considered greater if they are otherwise equal. builtins.compareVersion
is comparing lexicographically though, if you do the following
transformation beforehand:
2.3 --split--> [ "2" "3" ] --append--> [ "2" "3" "" ]
2.3pre --split--> [ "2" "3" "pre" ] --append--> [ "2" "3" "pre" "" ]
Comparing the transformed version is then done lexicographically:
2.3 < 2.3.0pre since [ "2" "3" "" ] < [ "2" "3" "0" "pre" ]
Here, the `pre` rule never comes into effect because no comparison on it
happens, instead we use the longer string rule of a lexicographical
comparison.
In the C++ codebase, the reason for this behavior is that the
iterator-esque construct they use always yields the empty string before
it exposes it has been fully consumed. This is probably intentional to
support the postfixed `pre` which is, for example, used by NixOS
versions (e.g. unstable post 22.05 is 22.11-pre). We replicate this
behavior using the `Chain` iterator in `VersionPartsIter::new_for_cmp`.
Change-Id: I021c69aa27b0b7deb949dffe50ed18b6de3a7b1f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6720
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
This is based on the [relevant code] in C++ Nix. Our version has more
branches because the C++ one only checks if it is less than or not, so
can save handling a few cases. We on the other hand, can avoid calling
the algorithm twice. It'd be nice to implement proptests for this in the
future and to make sure that this weird little algorithm doesn't violate
the Ord laws.
[relevant code]: cd35bbbeef/src/libstore/names.cc (L81-L94)
Change-Id: I46642e6da5eac7c0883cdce860622cdba04cd12b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6719
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
This was fairly easy, thanks to the work already done by Thomas Frank.
However, the implementation is suboptimal because we parse number parts
only to convert them back to strings afterwards. I've chosen to tackle
this problem in the future, since having an (inefficient) implementation
of splitVersion will be helpful for debugging the slight discrepancies
between C++ Nix and Tvix in the case of compareVersions.
Change-Id: Id6ed8eeb77663ff650c8c53ea952875b1fb7ea84
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6688
Autosubmit: sterni <sternenseemann@systemli.org>
Reviewed-by: grfn <grfn@gws.fyi>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
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>
Generate smaller recursive values for generated Values, and run fewer
cases for the attrs proptests which are particularly egregious.
Change-Id: Ia35c7c120270feaf045be1deb440c87ebb185c27
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6716
Autosubmit: grfn <grfn@gws.fyi>
Reviewed-by: tazjin <tazjin@tvl.su>
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
As we already have a VM passed to the builtins, we can simply execute
the provided closure/lambda in it for each value.
The primary annoyance with this is that we have to clone the upvalues
for each element, but we can try making this cheaper in the
future (it's also a general problem in the VM itself).
Change-Id: I5bcf56d58c509c0eb081e7cf52f6093216451ce4
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6714
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Previously we only matched the outer constructor after forcing which
would mean that we would always return `false` if the inspected value
was a thunk, regardless what value would be present inside.
Change-Id: I361ea6e855e23ef8e5b59098a50b9cd59253803f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6692
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Previously, this would almost always crash because list items are
thunked more often nowadays and selecting from a thunk would fail. Also
we no longer pop from args, accessing it by index should avoid an
unnecessary clone here.
Change-Id: I4410c4c2e28cc255a2c7cf2a5322db3d2c556a0e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6693
Reviewed-by: grfn <grfn@gws.fyi>
Autosubmit: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
And insert the missing newline the C++ Nix test script needs.
Change-Id: I04ddd7268f9caa1414fd23314c281bb7c1e854cf
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6689
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Instead of arity, we pass a array reference to Builtin::new that
describes how many arguments there are and which of them need to be
forced, eliminating the need to force manually.
Note that this change doesn't fix some of the instances where the the
Builtin doesn't consider that the value could be a Thunk.
Change-Id: Iadb58bb79886c30dc6b09dcf0ffad8abf28036a1
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6662
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
TL;DR:
- support `builtins.tail`
- define `ErrorKind::TailEmptyList` and canonical error code
- support basic unit tests
Unsure whether or not the error should be a dedicated `ErrorKind`...
Change-Id: Iae90fda1bb21ce7bdb1aaa2aeb2b8c1e6dcb0f05
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6545
Reviewed-by: wpcarro <wpcarro@gmail.com>
Autosubmit: wpcarro <wpcarro@gmail.com>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Refactor the `force!` macro to a method on `Value` which returns a
smart-pointer-esque type, which simplifies the callsite and eliminates
rightward drift, especially for high-arity builtins.
Change-Id: I97a7837580accfb4bbd03b24f2acdbd38645efa5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6656
Autosubmit: grfn <grfn@gws.fyi>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Previously only the first one was guaranteed to be forced, but we need
to do this for all of them.
Fixes b/190
Change-Id: I76b5667dbfb2f3fde3587e7b91d268cbf32aca00
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6645
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: tazjin <tazjin@tvl.su>
Document the OpAttrs op, paying special attention to the (perhaps
confusing) behavior of taking the number of *pairs*, not the number
of *values*, which will be popped off the stack into the resulting attr
set.
Change-Id: I64df0290308ecae7a5c7e14ead37091d32701507
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6654
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: tazjin <tazjin@tvl.su>
Refactor the environment variable and argument parsing for the tvix repl
to use Clap instead of doing things ad-hoc, and thread through options
obtained from environment variables via explicit arguments rather than
obtaining them from the environment as they're needed. This makes adding
more flags more sustainable, and also makes the binary fully
self-documenting, including supported env vars, via `--help`.
Change-Id: Ib1f6a0cd20056e8c9196760ff755fa5729667760
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6653
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Thunks might be encountered deep in equality comparison (eg nested
inside a list or attr-set), at which point we need to force them in
order to compare them for equality (or else we panic when trying to get
at their value).
Fixes: b/192
Change-Id: I912151085f8298f30d5214c7965251c9266443f2
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6652
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
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 disconnects ownership of the `File` reference in a compiler from
the calling scope, which is required for when we implement `import`.
`import` will need to carry an `Rc<RefCell<CodeMap>>` (or maybe, in
the future, Arc) to give us the ability to add new detected code
files at runtime.
Note that the choice of `Arc` over `Rc` here is not ours - it's the
codemap crate's.
Change-Id: I3aeca4ffc167acbd1701846a332d93550b56ba7d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6630
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
As before, this limits the cases to a relatively small number because
otherwise things get quite large.
Change-Id: I5371dc56418fca52e1dd1d905b20868f647091ba
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6649
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Only running 20 cases for now, since Value can get quite big if you let
it run for a while.
Change-Id: I09ef19da22c789c4869793836c98937c44595340
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6648
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
this was found by proptests!
Change-Id: I16d6a6ece3b20cdddd6f78c94cc87befb1b651e6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6647
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Add an optional config argument to the `<trait>_laws` macros, to allow
configuring the generated tests with a ProptestConfig struct (to limit
the number of cases run)
Change-Id: I2143ddb72c6a870e8be4a9058135b6f9a703039e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6646
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Impl Arbitrary for Value (and NixAttrs and NixList) in the same way we
did for NixString. Value currently only generates non-"internal"
values (no thunks, AttrNotFound, etc.) and can't generate
functions (builtins or closures), because those'd require full
generation of tvix bytecode, which is a bit more work than I'd like to
do now - there's a `todo!` left in the code for a place where we could
allow opting-in to internal values and functions later.
Change-Id: I07a59e2b1d89cfaa912d4ecebd642caf4ddb040a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6627
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
This is pretty boring at the moment, but mostly serves as a foot in the
door in the direction of writing more tests
Change-Id: Id88eb4ec7e53ebb2d5b5c254c8f45ff750238811
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6637
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Invalid integers (eg integers that're too long) end up as error returns
on the `.value()` returned from the literal in the AST - previously we'd
unwrap this error, causing it to panic the compiler, but now we've got a
nice error variant for it (which just unwraps the underlying
std::num::ParseIntError).
Change-Id: I50c3c5ba89407d86659e20d8991b9658415f39a0
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6635
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Add a suite of proptests covering the laws of the handwritten stdlib
trait impls (Eq, Ord, and Hash) for String, generated from a new set of
macros for generating those tests which can be applied to other types.
Change-Id: Ib3276c9e96fca497aece094e5612707d3dc77ccd
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6626
Autosubmit: grfn <grfn@gws.fyi>
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
The From<String> impl for NixString only generates StringRepr::Heap
strings, but we want to make sure we're testing StringRepr::Smol too
Change-Id: I6d04b9cf12ef8462fe2788e0c6414b165f40311d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6629
Autosubmit: grfn <grfn@gws.fyi>
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
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>
... it would be nice if we could thread it through to the `Scope`
stuff (declaring phantoms & locals).
Change-Id: Id3b84e79032b8fbb12138b719e657565355fbc79
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6616
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This trait can be used to convert most structures from rnix-parser
into a codemap::Span. It uses a macro to implement the trait for the
various expression types in the rnix AST, as Rust's silly semantic
versioning restriction stops us from doing a blanket implementation.
This will be used in the next commit to clean up the span handling in
the compiler a bit.
Change-Id: I0a437034e5fa203b5a49c6f25c45932a9f3b2bca
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6615
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
... and emit a warning if anyone decides to use.
Change-Id: Iaa6fe9fa932340e6d0fa9f357155e78823702576
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6611
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Yep.
This is kind of ugly right now. The idea is that the recursive_scope
compilation function is used for recursive sets as well by emitting
the keys. The rest of the logic is pretty much identical.
There is quite a lot of code here that can be cleaned up (duplication
between attrs and let, duplication inside of the recursive scope
compilation function etc.), but I'd like to get it working first and
then make it nice.
Note that nested keys are *not* supported yet.
Change-Id: I45c7cdd5f0e1d35fd94797093904740af3a97134
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6610
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This type is used in the list temporarily populated by the *second*
pass over all identifiers in a recursive scope. This first pass only
serves to make all bindings known to the compiler, without populating
their values yet.
Having a type here is going to be useful once we implement `rec`,
which needs to thread through slightly more information.
Change-Id: Ie33e0f096c5fcb6c864c991255466748b6f0d1eb
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6609
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This needs to be reused between let & `rec` attrs.
Change-Id: I4a3bb90af4be32771b0f9e405c19370e105c0fef
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6608
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This makes the phases of attribute set construction that Nix has very
explicit (inherits, static keys, dynamic keys).
This change focuses on the split between dynamic/static keys by
collecting all dynamic ones while compiling the static ones, and then
phasing them in afterwards. It's possible we also need to do some
additional splitting inside of the inherits.
Change-Id: Icae782e2a5c106e3ce0831dda47ed81c923c0a42
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6530
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This function is necessary for all builtins that expect some form of
path as an argument. It is merely a wrapper around coerce_to_string that
can shortcut if we already have a path. The absolute path check is done
in the same way as in C++ Nix for compatibility, although it should
probably be revised in the long term (think about Windows, for example).
Since coercing to a path is not an operation possible in the language
directly, this function can live in the builtins module as the only
place it is required.
Change-Id: I69ed5455c00d193fea88b8fa83e28907a761cab5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6574
Autosubmit: sterni <sternenseemann@systemli.org>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Define `.len()` method on `NixAttrs` to preallocate the capacity of the result
vector.
Also anchor an errant comment to its context (I think).
Change-Id: I268f15025d453d7b3ae1146558c80e51433dd2a8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6546
Reviewed-by: wpcarro <wpcarro@gmail.com>
Reviewed-by: sterni <sternenseemann@systemli.org>
Autosubmit: wpcarro <wpcarro@gmail.com>
Tested-by: BuildkiteCI
This allows us to get rid of the count local variable which was a bit
confusing. Calling parts.len() multiple times is fine, since the length
doesn't need to be computed.
Change-Id: I4f626729ad1bf23a93cb701385c3f4b50c57456d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6584
Autosubmit: sterni <sternenseemann@systemli.org>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
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
If we have multiple string parts, we need to thunk assembling the
string. If we have a single literal, it is strict (like all literals),
but a single interpolation part may compile to a thunk, depending on how
the expression inside is compiled – we can avoid forcing to early here
compared to the previous behavior.
Note that this CL retains the bug that `"${x}"` is erroneously
translated to `x`, implying e.g. `"${12}" == 12`.
The use of `parts.len()` is unproblematic, since normalized_parts()
builds a `Vec` instead of returning an iterator.
Change-Id: I3aecbfefef65cc627b1b8a65be27cbaeada3582b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6580
Autosubmit: sterni <sternenseemann@systemli.org>
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
OpAttrsIsSet and OpAttrsTrySelect will fail silently if the attribute
set value on the stack is actually a thunk, so we need to make sure to
force at every step of the way.
Emitting the force instructions in the compiler because it is easier to
add, but maybe the VM should do this when handling the relevant opcodes?
Comments welcome.
Change-Id: I65c5ef348d59b2d07c9bb06abb24f9f3e6a0fdb2
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6540
Reviewed-by: grfn <grfn@gws.fyi>
Autosubmit: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
This prevents Nix programs to observe the "internal" type of thunks.
Possibly .type_of() is also an area of the runtime where we should panic
if "internal" would ever be returned.
Change-Id: I9f358044c48ad64896fb6a1b1a42f00a29efac00
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6539
Autosubmit: sterni <sternenseemann@systemli.org>
Reviewed-by: wpcarro <wpcarro@gmail.com>
Reviewed-by: grfn <grfn@gws.fyi>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
The expression inside ${…} may return arbitrary values, including
thunks, so we need to make sure to force them just in case.
Change-Id: Ic11ba00c4c92a10a83becd91233db5f57f6e59c8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6541
Autosubmit: sterni <sternenseemann@systemli.org>
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
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
Pointed out by sterni in cl/6370
Change-Id: I324d8049a2702ced8f30ad43a64d63ae79dd0eab
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6569
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Pointed out by sterni in cl/6389
Change-Id: I648056a760266a8cfd7adcdc478c7ff2132991f7
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6568
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Pointed out by sterni in cl/6395
Change-Id: I2dda2bb11fef702df05fd7a4fd93b9e717a85dad
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6567
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This is more useful than pointing it at the entire assert expression,
as that includes the body as well which is not going to be relevant in
the error.
Pointed out by sterni in cl/6391
Change-Id: I95a5d1edf90df65e7fa53d4d04502afd6e99e89a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6566
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This is no longer required, resolution is now more sane. Pointed out
by sterni in cl/6422.
Change-Id: Icc8983c648f864e66813948df6e2d4bad6a7f312
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6565
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This field no longer needs to be directly accessible by the compiler.
Addresses a sterni lint from cl/6466
Change-Id: I5e6791943d7f0ab3d9b7a30bb1654c4a6a435b1f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6564
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 very closely follows the way it's done for warnings, but errors
have a lot more information available in some cases which we do not
surface yet.
Note also that due to requiring the `CodeMap`, this is not yet called
from eval.rs as the way that is threaded through needs to be
refactored, so only the method for reporting these errors as strings
is implemented so far.
Next steps for this will be to add a generic diagnostics module that
reduces some of the boilerplate for this between warnings & errors,
and which will also give us a good point in the future to switch to a
fancier diagnostics crate.
Change-Id: If6bb209f8e7a568d866e516a90335b9b2afbf66d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6534
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
This implements an initial fancy display for warnings emitted by the
tvix compiler, using the codemap_diagnostic crate.
Each warning variant has an associated message, and optionally an
associated annotation for the span displayed to the user.
In theory we could get a lot more fancy with the display for specific
variants if needed (e.g. re-parse the AST and actually add multiple
semantic spans based on context), but this is already a good start.
Example:
tvix-repl> let toString = https://tvl.fyi; in let inherit toString; in ({}: 42) rec {}
warning[W004]: declared variable 'toString' shadows a built-in global!
--> [tvix-repl]:1:5
|
1 | let toString = https://tvl.fyi; in let inherit toString; in ({}: 42) rec {}
| ^^^^^^^^ variable declared here
warning[W001]: URL literal syntax is deprecated, use a quoted string instead
--> [tvix-repl]:1:16
|
1 | let toString = https://tvl.fyi; in let inherit toString; in ({}: 42) rec {}
| ^^^^^^^^^^^^^^^
warning[W002]: inherited variable already exists with the same value
--> [tvix-repl]:1:40
|
1 | let toString = https://tvl.fyi; in let inherit toString; in ({}: 42) rec {}
| ^^^^^^^^^^^^^^^^^
warning[W999]: feature not yet implemented in tvix: recursive attribute sets
--> [tvix-repl]:1:70
|
1 | let toString = https://tvl.fyi; in let inherit toString; in ({}: 42) rec {}
| ^^^^^^
warning[W999]: feature not yet implemented in tvix: closed formals
--> [tvix-repl]:1:62
|
1 | let toString = https://tvl.fyi; in let inherit toString; in ({}: 42) rec {}
| ^^
warning[W003]: variable 'toString' is declared, but never used:
--> [tvix-repl]:1:5
|
1 | let toString = https://tvl.fyi; in let inherit toString; in ({}: 42) rec {}
| ^^^^^^^^ variable declared here
=> 42 :: int
These are coloured when output to a terminal.
Change-Id: If315648a07e333895db4ae1d0915ee2013806585
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6532
Autosubmit: tazjin <tazjin@tvl.su>
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
It is impossible for tvixbolt to recover from panics, so the user
experience of typing an expression using an unsupported feature was
that it would get sad and stop responding to input.
Instead, raise a normal value-level error of a new variant and
continue where possible.
Change-Id: Ibe016c92cacb87b85095c0f83758eddc6468053e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6528
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This is used to drop an already emitted operation from a chunk again
and clean up its span tracking. This is required in cases where the
compiler has to backtrack.
Change-Id: I8112da9427688bb2dec96a2ddd12390f6e9734c3
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6499
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Refactors the methods used for determining whether an identifier in a
binding (i.e. an `rnix::Attr` node) is a static string, and extracting
it.
Previously all uses of this logic were for `let`-expressions, where
dynamic attributes are always an error. However, we need the same
logic to properly implement the phase separation of attribute set
compilation.
To facilitate this, the actual core logic of these methods now return
`Option`, and are only converted to errors in cases where the errors
are actually required.
Change-Id: Iad7826eff2cb428182521c6f92276310edeae1eb
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6498
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
There's about to be a lot more code for attrsets (hopefully
temporarily as part of an expand&contract cycle), while nested
attribute logic is being refactored in preparation for recursive
attribute sets.
This does not change any functionality.
Change-Id: I667565cd810ca7d9046120d1721c2ceb9095550b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6497
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
We need to make sure that we compile all plain inherits in a let
expression before declaring any other locals. Plain inherits are special
in the sense that they can never be recursive, instead resolving to a
higher scope. Thus we need to compile their value, before declaring
them. If we don't do that, before any other local can be declared,
we cause a situation where the plain inherits' values are placed into
other locals' stack slots.
Note that we can't integrate the plain inherit compilation into the
regular 2-3 phase model where we defer the compilation of the value or
we'd compile `let inherit x; in …` as `let x = x; in …`.
Change-Id: I951d5df3c9661a054e12401546875f4685b5bf08
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6496
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Reviewed-by: sterni <sternenseemann@systemli.org>
This test shows that let bindings' dependencies can form a cyclical
graph, so we need to use thunking to break this cycle.
Change-Id: I2a4de71fd7024f3d3d1166154784139a82f39411
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6495
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Reviewed-by: sterni <sternenseemann@systemli.org>
As the new test case demonstrates, asserts need to be evaluated lazily.
Change-Id: I808046722c5a504e9497855ca5026d255c7a4c34
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6494
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Reviewed-by: sterni <sternenseemann@systemli.org>
The recent change that split declaration of let based locals and the
compilation of their values did not touch locals bound by inherit in
let. These were previously declared and compiled immediately before
starting to work on the other locals introduced in a let.
In the case of plain inherits, this behavior is kept in this change,
because there's nothing wrong with it: The value of a plain inherit will
always resolve to a higher scope, either statically or dynamically.
Since inherit (from) expression might refer to other locals bound in the
same let, we need to handle them in the same three steps as ordinary let
based locals:
1. We need to declare the (uninitialised) locals.
2. We need to compile the expression that obtains their value. For this,
we create a new thunk, since the from expression may very well return
a thunk which we need to force before selecting the value we are
interested in.
3. Thunks need to be finalised.
For 1., we create an extra pass over the inherits that already declares
and initialises plain inherits and notes inherit (from) expressions in
the entries vector after declaring them. 2. only needs a bit of adapting
to create the thunks for selecting if appropriate, the rest of the
existing code can be reused.
Change-Id: Ie4ac1c0f9ffcbf7c07c452036aa8e577443af773
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6490
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: tazjin <tazjin@tvl.su>
With this all other "weird scope" logic starts working for `with` as
well.
Change-Id: I0ea1d8c5fbd9cec5084bd574224f77b71ff2b487
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6487
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
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>
Instead of the reference to the Rc, print the address of the Rc
itself.
Change-Id: I4560598924db7d2864d5c4ae9af847aee2ea7eff
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6471
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Similarly to attribute sets, list elements can be arbitrary
expressions and their (temporary) stack slots during construction must
be accounted for by the compiler.
Change-Id: I3b6f7927860627fd867c64d0cab9104fd636d4f5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6470
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
The temporaries left on the stack as operands to `OpAttrs` must be
accounted for in the locals array in order for operations within them
to receive correct slots.
Some test cases that were previously broken have been added.
Change-Id: Ib52b629bbdf7931f63fd45a45af1073022da923c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6468
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
There are more upcomming uses of declare_phantom where this will come
in handy to avoid some code bloat.
Change-Id: I75cad8caf14511c519ab2f56e87e99bcbf0a082e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6467
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Moves the logic for removing tracked locals from a given scope from
the compiler's locals list, and leaves only the actual
compiler-related stuff (emitting warnings, cleaning up locals at
runtime) in the compiler itself.
Change-Id: I9da6eb54967f0a7775f624d602fe11be4c7ed5c4
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6466
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This will be re-used between the code paths for
recursive/non-recursive sets, and it might even be possible to unify
it with the logic for compiling `let inherit ...`.
Change-Id: I960a061048ac583a6e932e11ff6e642d9fc3093e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6464
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
The comment explains how this works fairly well.
Note that this does not yet have the ability to check "closed
formals", i.e. without an ellipsis Tvix will *NOT* fail if unexpected
attribute set keys are provided.
Change-Id: I0d2b77e893243093d2789baa57f876d35d0a32ff
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6463
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
... even if the code is broken.
Change-Id: I5898bceaebf201b97e8988c94c90e7fafff82529
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6462
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
As pointed out by sterni in cl/6205, this is actually possible in
syntactically valid expressions like
{ ${12 + 13} = 12; }
Change-Id: Id8a1e3aceb551f288f9050c4eea563eb6572f1a7
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6461
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>
When the last instruction in a chunk is OpCall, make it an OpTailCall instead.
Change-Id: I2c80a06ee85e4abf545887b1a79b6d8b5e6123e9
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6458
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
This produces similar output to the previous tracing feature, but can
redirect the output somewhere else.
Change-Id: I9493c260f480904f3932cb74809b622c24d7be96
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6453
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
These are required for tvixbolt to work. This interface is definitely
not stable yet, though.
Change-Id: I4076498e8f42311de74ee4f33c93a3ee0c5f8d3a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6450
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This type implements an observer that is called whenever the compiler
emits a chunk (after the toplevel, thunks, or lambdas) and prints the
output of the disassembler to its internal writer.
This replaces half of the uses of the `disassembler` feature, which
has been removed from the Cargo configuration.
Note that at this commit runtime tracing is not yet implemented as an
observer.
Change-Id: I7894ca1ba445761aba4ad51d98e4a7b6445f1aea
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6449
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This trait will enable library users of tvix-eval to observe internal
happenings of the compilation and runtime processes.
The initial methods of the observer will be called whenever the
compiler emits a chunk.
Change-Id: I668f6c2cfe3d6f4c1a1612c0f293831011768437
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6448
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
With this, most cases of `fix` in attribute sets will work correctly.
A simple test exercising both has been added.
Change-Id: I70fd431177bb6e48ecb33a87518b050c4c3d1c09
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6437
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
At the point where control flow exits Thunk::force (which may be due
to recursing), it is vital that there is no longer a borrow to the
inner thunk representation, otherwise this can cause accidental
infinite recursion (which will be detected, but cause failures on
valid code).
Change-Id: I2846f3142830ae3110a4f5d2299e9d7928634504
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6436
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
There can be different spans on the same line, so the previous
implementation would duplicate line numbers unnecessarily.
Change-Id: I8d8db77177aee0d834a6ec3584641e1bd5f31c3e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6434
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This was fixed by some of the previous commits around scopes. It's
somewhat similar to a few other tests, but I had this one failing
earlier and everything else succeeding, so it is useful to keep it
around for sure.
Change-Id: Ie6cf372b5c805daf992cd87aeb3dfe91542c381c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6431
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Deferred local upvalues can *only* occur at the same depth as the
thing that is closing over them, but there are various situations with
scope nesting where the actual stack indexes of the local and the
closer look like a deferred value is being accessed.
To fix this, simply compare the depth as well.
Change-Id: Ice77424cc87ab0a2c4f01379e68d4399a917b12b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6429
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This is the same as `eval-okay-attrs-simple-inherit`.
Change-Id: I23878accc6cd62c16ec96601239838a385d31306
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6428
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
The condition here was extremely hard to read prior to this change.
As the locals vector is now guaranteed to never be empty (there is
always at least a phantom for the current chunk's root expression),
the logic here can be simplified to just dropping tailing locals
entries while their depth matches that of the scope being closed.
Change-Id: I24973e23bc2ad25e62ece64ab4d8624e6e274c16
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6427
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Similar to setting up a phantom slot when compiling the root value of
a file, closures and thunks need to have a phantom stack slot for the
root of the expression yielded by their thunk to make all accounting
work correctly.
The tricky thing here is that closures & thunks *escape* their inner
lambda context (that's the point!), so the functions emitting them
need to know both the *inner* slot (to resolve everything correctly
while compiling the slot) and the *outer* slot (to correctly emit
instructions for closing over upvalues).
Change-Id: I62ac58e2f639c4b9e09cc702bdbfd2373e985d7f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6426
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Instead of using a sentinel LocalIdx which potentially points to a
value in the locals stack that does not actually exist, set up an
initial uninitialised phantom value representing the result of the
root expression.
Change-Id: I82ea774daab83168020a3850bed57d35ab25c7df
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6424
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
When deciding whether an upvalue needs to have a deferred resolution
step, the *stack* indexes should be compared - not the locals indexes.
The results are almost always the same, but there are tricky
situations where this can cause errors.
It's difficult to reproduce these errors in isolation, as they depend
on other scope behaviour, so this is one in a series of commits to
address the combination of issues which will gain some tests at the
end.
Change-Id: Iaa400b8d9500af58f493ab10e4f95022f3b5dd21
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6423
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Instead of using sentinel values and an additional bool, this tracks
the identifier of a local as an enum that is either a statically known
name, or a phantom.
To make this work correctly some more locals related logic has been
encapsulated in the `scope` module, which is a good thing (that's the
goal).
Phantom values are now not initialised by default, but the only
current call site of phantoms (`with` expression compilation) performs
the initialisation right away.
This commit changes no actual functionality right now, but paves the
way for fixing an issue related to `let` bodies.
Change-Id: I679f93a59a4daeacfe40f4012263cfb7bc05034e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6421
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
The slot is now always known (at the root of the file it is simply
stack slot 0 once the scope drops back down to 0), so it does not need
to be wrapped in an `Option` and accessed in cumbersome ways anymore.
Change-Id: I46bf67a4cf5cb96e4874dffd0e3fb07c551d44f0
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6420
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
This could occur when the disassembler is enabled and tracing the
runtime while a thunk is being evaluated, as it would not be possible
for the *tracer* to borrow the thunk at this exact moment.
However, we know that if the borrowing fails here we are dealing with
a not-fully evaluated thunk (blackhole), which should just print the
internal representation.
Change-Id: I4bdb4f17818d55795368e3d28842048f488f0a91
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6416
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
If the disassembler feature is enabled, make sure that an Rc of the
codemap is available through the chunk.
Change-Id: I700f27ab665a704f73457b19bd2d7efc93828a16
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6414
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
Scope poisoning must be inherited across lambda context boundaries,
e.g. if an outer scope has a poisoned `null`, any lambdas defined on
the same level must reference that poisoned identifier correctly.
Change-Id: I1aac64e1c048a6f3bacadb6d78ed295fa439e8b4
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6410
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
With this, if an error occurs while forcing a thunk (which is very
likely) it is threaded through to the top by wrapping it in the
ErrorKind::ThunkForce variant.
We could use this to generate "stacktrace-like" error output if we
wanted, or simply jump through and discard everything except the
innermost error.
Change-Id: I3c1c8708c2f73ae062815adf490ce935b1979da8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6409
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
Get the length of a list
Change-Id: I41d91e96d833269541a1b3c23b7cc879f96d1e5a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6407
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
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>
Another step towards being able to report accurate errors. The codemap
spans contain strictly more accessible information, as they now retain
information about which input file something came from.
This required some shuffling around in the compiler to thread all the
right information to the right places.
Change-Id: I18ccfb20f07b0c33e1c4f51ca00cd09f7b2d19c6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6404
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
As of this commit, the source spans of all emitted bytecode are fully
tracked.
Change-Id: I4c83deee0fc3f5e6fd6acad5a39047aec693b388
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6403
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
These source spans will always point to the *value* that is being
forced, not the instruction that caused the force to be emitted. This
makes sense so that errors during forcing point at the value and not
the surrounding expression.
Change-Id: I4694414a3281a0de878f71634105b92148ec61f6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6402
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
With this change, the upvalue data instructions used by finalisers for
thunks and closures track the source span of the first identifier that
created the upvalue (if the same value is closed over multiple times
the upvalue will be reused, hence only the first one).
To do this the upvalue struct used by the compiler's scope now carries
an identifier node, which had to be threaded through quite a few
places.
Change-Id: I15a5fcb4c8abbd48544a2325f297a5ad14ec06ae
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6400
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This separation makes it possible to annotate the upvalue itself with
the span that created it, which (due to upvalue reuse) is only the
first one for an instance of the given UpvalueKind.
Change-Id: I9a991da6a3e8d71a92f981314bed900bcf434d44
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6399
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
These are again a bit tricky in terms of emitted errors. The main
error is that the condition is not a boolean, which means that the
jump inspecting the condition must derive from the condition itself to
return an error at the correct position.
For other parts of the expression, it is simply the node itself.
Change-Id: I72411630e5d57dfc199f4c3c48afe443fe966322
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6392
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This one is tricky, specifically the span used for the final jump. I
decided that it makes sense to use the attrpath node, as the final
jump is the one that jumps *over* the default value, so the effect of
this is more closely related to the selector than the default.
It might be more correct to pass through the `or` token itself and
point to this for the jumps, but it depends a bit on what shape of
errors we could end up producing from this.
Change-Id: I29fbc97ba6b9e14e1a0e5f3a7759ddc299dd9c0c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6390
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
These are not actually used yet; this is in preparation for a
multi-commit chain for emitting all the right spans in the right
locations.
Change-Id: Ie99d6add2696c1cc0acb9ab928917a10237159de
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6379
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This instantiates a codemap outside of the compiler and passes a
reference to the file currently under compilation to it. Note that the
"file" might just be a REPL line.
Change-Id: I131ae1ddb6d718e1374750da9ba0b99608c6058d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6378
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This is currently just a wrapper around Chunk::push_op, but will gain
the span resolution logic in a moment.
Change-Id: I862bf9ecff0932f8da6708401ea044b9442c5d5b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6377
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This adds a new vector to the chunk data structure which tracks spans
into a codemap. The compiler will emit this information to the chunk
when adding instructions.
The internal representation of the spans is slightly optimised to
avoid storing duplicate spans, as there are cases where many
instructions might be derived from the same span.
Change-Id: I336f8c912e7eb50ea02ed71e6164f651ca3ca790
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6376
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
This does not require a custom iterator type (for now?)
Change-Id: I5beb194bd8629571bd4040c69c977c27149807fa
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6371
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
If a thunk is already evaluated, there are cases where due to the
memoisation implementation something might observe a value wrapped in
a thunk.
In these cases, the implementation of `Display` and `PartialEq` must
delegate to the underlying value.
Note that there are a handful of other cases like these which we need
to cover.
It is a little tricky to write integration tests for these directly,
especially as some of the open-upvalue optimisations coming down the
pipe will reduce the number of observable thunks.
One test that covers a part of this behaviour is currently
disabled (needs some more machinery), but it's being brought back in
the next commits.
Change-Id: Iaa8cd338c12236af844bbc99d8cec2205f0d0095
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6370
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Implementing iteration over NixAttrs requires a custom iterator type
in order to encapsulate the different representations. The BTreeMap
for example has its own iterator type which needs to be encapsulated.
This is mostly boilerplate code, but for a change some simple unit
tests have been added in.
Change-Id: Ie13b063241d461b810876f95f53878388e918ef2
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6367
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
These static strings show up a bunch when dealing with the internals
of attribute sets, and having them available as static references is
required.
Due to the way const expressions are evaluated, taking a reference to
the existing NixString::NAME / NixString::VALUE items does not work
and the references themselves need to be const-evaluated.
Change-Id: If6e75847af978118a3b266fe6a3242321722434d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6366
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
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 thunks the construction of attribute sets. Because Tvix does not
currently have a "strict output" mode, a test had to be disabled that
now displays a thunk representation.
The test will be re-enabled once that is available.
Change-Id: I360332be64cd5c154f9caea21828f6f1b37a265c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6363
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
As noticed by sterni in cl/6195
Change-Id: Ie9c1e80e2e709284fa8412334af9188d999f64dc
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6361
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This should not have grown a second implementation of the identifier
resolution logic, but it somehow did.
This implementation ended up being incorrect because it did not
account for upvalues inside of thunks.
Change-Id: Ieb1364d8fe43c96aaf4b125fd4b8a522aedff167
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6360
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
All attribute set *key* related operations strictly evaluate all key
fragments, including during construction of an attribute set.
Change-Id: I3519e5e9b0886c2cdc8615ea7dcb5f7be0c59b3f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6358
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
The arguments of all unary/binary operators that are built in to Nix
are forced when encountered. This emits the necessary OpForce operations.
Change-Id: I691fcdbebfe7586cfe217c68d44b10b1192f82d1
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6357
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
With this change any compilation of an expression is aware of its own
stack slot if it is leaving identifiers on the stack.
Change-Id: I0c9f148ae06b078a46b25180c4961686d5f2e166
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6356
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This threads the current local slot through the `compile_attr`
function and all of its callers. At the moment this does not improve
any user-facing behaviour, just internally changes the way in which
some correct expressions would fail to run.
Eventually this slot will need to reach everywhere ...
Change-Id: Iba73123dd1ced421093d8fc18ebeeffc16efacf8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6355
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This is the simplest kind of thunk that can be created (and so far the
only one the compiler knows how to create), in which an identifier
inside a `let` encounters a value that is bound *after* it is
initialised.
Change-Id: I6ea4408a3baef1e7d5137365d70804283f2dbf8e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6354
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>
The capacity (i.e. number of builtins) is known from the lambda, so we
can size it correctly right away.
Change-Id: Iab0b5a3f47d450fa9866c091ebbbed935b934907
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6351
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>
This function is reusable between thunks & closures.
Change-Id: I44d5f9897b087a385c8e75027d2ff39c48a096f0
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6349
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
CallFrame has to work for both thunks & closures (as a thunk is
basically a "weird 0-argument closure").
We opt to store the common, relevant fields directly in the frame to
avoid having to dereference through the nested structures
constantly (which would be especially annoying in the case of thunks).
Change-Id: I47781597b84ec5cd55502dba1713e92cf2592af3
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6348
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This trait abstracts over the commonalities of upvalue handling
between closures and thunks.
It allows the VM to simplify the code used for setting up upvalues,
without duplicating between the two different types.
Note that this does not yet refactor the VM code to optimally make use
of this.
Change-Id: If8de5181f26ae1fa00d554f1ae6ea473ee4b6070
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6347
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
For now, do not distinguish between closing and non-closing thunks, it
will make the initial implementation easier. See Knuth etc.
Change-Id: I0bd51e0f89f2c77e90bac63b507e5027b649e3d8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6346
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
When resolving a value on the same level that is known but not yet
defined, emit a thunk.
Consider for example:
let
# v--- requires a thunk
a = 1 * b;
b = 10;
in a
Change-Id: I922cb50973ebe05e335a7bc7cb851960cf34733b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6345
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
The logic in this method is *very* similar to `compile_lambda`. It is
intended to be called around any expression that should be
thunked (such as function applications, attribute set values, etc.).
Change-Id: Idfbb2daa9f4b735095378fb9c39a2fd07c8cff91
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6344
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Introduces the representation of runtime thunks, that is lazily
evaluated values. Their representation is very similar to closures.
Change-Id: I24d1ab7947c070ae72ca6260a7bbe6198bc8c7c5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6343
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This exact same logic is reused for thunk creation.
Change-Id: I731db9cc659a1f2ca87db55d58d6ff632f417812
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6342
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This test case was previously broken by the bug introduced by
confusing local and stack indexes.
Change-Id: Ibef299dad266c6105deac1da5dde112fe9f640b1
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6341
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Previously the functions in the scope module returned usize values,
which - sometimes from the same function - were either indexes into
the runtime stack *or* indexes into the compiler's local stack.
This is extremely confusing because it requires the caller to be aware
of the difference, and it actually caused subtle bugs.
To avoid this, there is now a new LocalIdx wrapper type which is used
by the scope module to return indexes into the compiler's stack, as
well as helpers for accounting for the differences between these
indexes and the runtime indexes.
Change-Id: I58f0b50ad94b28a304e3372fd9731b6590b3fdb8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6340
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
With this change, the runtime can correctly capture deferred upvalues.
Change-Id: I1e43b7b1ac2553b1812424adfc8bd08ef77bf1ea
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6339
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
When encountering a deferred local upvalue, the compiler will now mark
the corresponding local as needing a finaliser which makes it possible
to emit the OpFinalise instruction for this stack slot a little bit
down the line.
Change-Id: I3962066f10fc6c6e1472722b8bdb415a811e0740
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6338
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This instruction finalises the initialisation of deferred upvalues in
closures (and soon, thunks).
The compiler does not yet emit this instruction, some more accounting
is needed for that.
Change-Id: Ic4181b26e19779e206f51e17388559400da5f93a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6337
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This will leave a sentinel value in the upvalue slot in which the
actual value is to be captured after resolution once a scope is fully
set up.
Change-Id: I12b37b0dc8d32603b03e675c3bd039468e70b354
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6336
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Uses the threaded through slot offset to determine whether
initialisation of a captured local upvalue must be defered to a later
point where all values of a scope are available.
This adds a new data representation to the opcode for this situation,
but the equivalent runtime handling is not yet implemented. This is in
part because there is more compiler machinery needed to find the
resolution point.
Change-Id: Ifd0c393f76abfe6e2d91483faf0f58947ab1dedc
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6329
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
While compiling local bindings, we now know the stack slot of the
value currently being compiled.
This will let us determine whether an upvalue can be captured directly
or whether it needs to wait for a synchronisation point at which the
upvalue can be instantiated.
This machinery lets us avoid unnecessary work at runtime when
instantiating closures that actually do not need complicated recursive
resolution.
This change itself introduces no new functionality, but since the
threading is noisy it is split out as a separate change.
Change-Id: I847c677ee8f6725fda1d2efd689b6a58bdccb779
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6328
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Instead of looking up the local to be initialised by its name again,
we can simply track the index at which it was declared from the point
where the declaration was made.
This reduces some string cloning and removes unnecessary logic. It
also theoretically makes the *current* index available during locals
compilation, which can be used to optimise some recursion cases.
Change-Id: I06f403603d4f86c3d319debfe74b5a52eec00990
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6327
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This actually makes things full-circle, as this tree already had this
implementation once before all the other required components were in
place.
With this commit, the compiler can resolve recursive upvalues within
the same scope (though they will not yet work at runtime).
Change-Id: I6267e477d08f367257c3a6dde054b880d7b47211
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6326
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
In order to resolve recursive references correctly, these two can not
be initialised the same way as a potentially large number of (nested!)
locals can be declared without initialising their depth.
This would lead to issues with detecting things like shadowed
variables, so making both bits explicit is preferable.
Change-Id: I100cdf1724faa4a2b5a0748429841cf8ef206252
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6325
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
The previous closure refactoring introduced a bug in which the same
closure object would get mutated constantly for each instance of a
closure, which is incorrect behaviour.
This commit instead introduces an explicit new Value variant for the
internal "blueprint" that the compiler generates (essentially just the
lambda) and uses this variant to construct the closure at runtime.
If the blueprint ever leaks out to a user somehow that is a critical
bug and tvix-eval will panic.
As a ~treat~ test for this, the fibonacci function is being used as it
is a self-recursive closure (i.e. different instantiations of the same
"blueprint") getting called with different values and it's good to
have it around.
Change-Id: I485de675e9bb0c599ed7d5dc0f001eb34ab4c15f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6323
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This puts together the puzzle pieces for threading dynamic
upvalues (that is, upvalues resolved from the `with`-stack) all the
way through.
Reading the test case enclosed in this commit and walking through it
is recommended to understand what problem is being tackled here.
In short, because the compiler can not statically know *which*
with-scope a dynamic argument is resolved from it needs to lay the
groundwork for resolving from *all* possible scopes.
There are multiple different approaches to doing this. The approach
chosen in this commit is that if a dynamic upvalue is detected, the
compiler will emit instructions to close over this dynamic value
in *all* enclosing lambda contexts.
It uses a new instruction for this that will leave around a sentinel
value in case an identifier could not be resolved, and wire the
location of this found value (or sentinel) up through the upvalues to
the next level of nesting.
In this tradeoff, tvix potentially closes over more upvalues than are
needed (but in practice, how often do people create *really* deep
`with`-stacks? and in *this* kind of code situation? maybe we should
even warn for this!) but avoids keeping the entire attribute sets
themselves around.
Looking at the test case, each surrounding closure will close
over *all* dynamic identifiers that are referenced later on visible to
it, but only the last one for each identifier will actually end up
being used.
This also covers our bases for an additional edge-case this creates,
in which an identifier potentially resolves to a dynamic upvalue *and*
to a dynamic value within the function's own scope (again, would
anyone really do this?) by introducing a resolution instruction for
that particular case.
There is likely some potential for cleaning up this code which is
quite ugly in some parts, but as this implementation is now carefully
calibrated to work I decided it is time to commit it and clean it up
in subsequent commits.
Change-Id: Ib701e3e6da39bd2c95938d1384036ff4f9fb3749
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6322
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
With this change, encountering a dynamic upvalue will thread through
all contexts starting from the lowest context that has a non-empty
`with`-stack.
The additional upvalues are not actually used yet, so the effective
behaviour remains mostly the same. This is done in preparation for an
upcoming change, which will implement proper dynamic resolution for
complex cases of nested dynamic upvalues.
Yes, this whole upvalue + dynamic values thing is a little bit
mind-bending, but we would like to not give up being able to resolve a
large chunk of the scoping behaviour statically.
Change-Id: Ia58cdd47d79212390a6503ef13cef46b6b3e19a2
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6321
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This is a common idiom in both Nix and other languages when a local is
declared without actually being used.
Since Tvix warns for unused locals, having this available is useful
and can be included in the final error message as a suggestion if an
unused variable is intentional.
Change-Id: Ia85f704ba183499a3bae657c58166e2e29f9bde5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6320
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This does not yet correctly resolve them if they are more than one
scope up, however.
Change-Id: I6687073c60aee0282f2b6ffc98b34c1e96a60f20
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6319
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
With this change, it becomes possible for functions to call themselves
as they are being defined in local bindings.
Change-Id: Ib46a39ba17b1452b5673d96fa729d633d237241a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6314
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This is required to efficiently construct the upvalue array at
runtime, as there are situations where during Closure construction
multiple things already have a reference to the closure (e.g. a
self-reference).
Change-Id: I35263b845fdc695dc873de489f5168d39b370f6a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6312
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
The `With` struct no longer contained any internals after the cleanup
logic for the stack had been moved into Compiler::compile_with,
leaving the `Vec<With>` to essentially act as a counter for the number
of things on the with stack.
That's inefficient of course, so with this commit it actually becomes
an integer (with an encapsulated API within scope::Scope).
Change-Id: I67a00987fc8b46b30d369a96d41e83c8af5b1998
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6311
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
The compiler module is getting quite long and this will help keep some
order.
Right now the scope internals are not very well encapsulated; this
paves a way to reducing the API surface of the `scope` type to the
things that are actually used by the compiler instead of giving access
to its internals.
Change-Id: I8c16c26d263f018baa263f395c9cd80715199241
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6310
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This has no effect yet, other than changing the way in which some
upvalue captures break (that are already not working correctly).
However, after this change the compiler correctly detects
self-recursion and can start emitting the instructions to deal with
this at runtime.
Change-Id: Id3b0ac206c0204739597a4325bcc66f9c806c242
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6309
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Calculating the with_idx (i.e. the stack offset of the "phantom"
variable from which a `with` dynamically reads at runtime) needs to
account for unitialised variables the same way as the resolution of
normal locals does.
Change-Id: I9ffe404535bf1c3cb5dfe8d9e005798c857fff94
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6308
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This is required to correctly clean up the `with` values.
At the moment, the attrset from which identifiers are being taken is
always pushed on the stack. This means that it must also be removed
again, otherwise in an expression like
with { a = 15; }; a
The final stack is `[ { a = 15; } 15 ]` *after the last operation*,
which means that the attrset is still on there as garbage.
This has little practical impact right now because it is always
shadowed by the fact that the actual expression value is at the right
location, but becomes relevant when accounting for upvalue captures.
Change-Id: I69e9745bfaa4d6bbcb60ee71f4dc3f8d8695d16a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6303
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This extends the logic of `Scope::resolve_local` to detect cases where
self-recursion is occuring (i.e. an identifier is being accessed in
its own identifier).
These cases are not yet handled specially, and the logic of when
things are marked initialised (which was previously always at the same
spot as their declaration) has not changed, making this commit a
runtime no-op for now.
Change-Id: I3179642a7c55869ad4465fdd2678b0cd51a20f15
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6302
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Nix does not allow things like `let a = 1; a = 2; in a`, but doing it
across depths is allowed.
Change-Id: I6a259f8b01a254b433b58c736e245c9c764641b6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6301
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This does not yet change anything semantically, but will be useful for
resolving simple cases of self-recursion etc.
Change-Id: I139ecb7e4a8a81193774392a96e73e0ea6b9f85d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6300
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
These need to be handled specially by the runtime if the compiler
determines that a given local must be resolved via `with`.
Note that this implementation has a bug: It currently allows `with`
inside of nested lambdas to shadow statically known identifiers. This
will be cleaned up in the next commit.
Change-Id: If196b99cbd1a0f2dbb4a40a0e88cdb09a009c6b9
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6299
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
The previous implementation of OpResolveWith manually controlled the
loop iteration, which skipped over the disassembler's tracing
instruction.
Instead, the resolution of dynamic variables has been delegated to a
new helper function. This has the additional benefit that the loop
labels are no longer required, making things a bit cleaner.
Change-Id: If22b74c3d49c74bf3a1ec4497cb761a9ee6cf2a4
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6298
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Instead of tying the popping of the with stack to scope depth, clean
up the stack immediately after processing a with body.
The previous behaviour was actually incorrect, as it would leave
things on the with-stack longer than they were supposed to be there.
This could lead to false positive resolutions in some situations
involving closures.
Change-Id: I7b0638557503f1f71eb602e3d5ff193cdfcb67cc
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6297
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Implements the final bit of logic remaining for wiring up closures,
which is the runtime construction of closure objects.
When encountering an OpClosure, the VM walks through the bytecode
collecting all the upvalue location operands (see commit introducing
the OpCode::Data* variants for details) and stores the runtime values
in the new closures upvalue vector.
After that, the handling of the closure itself becomes functionally
identical to that of lambdas.
With this initial implementation of closures there are several large
optimisation potentials available, the two most notable ones are:
- Distinguish the runtime representation of lambdas and closures
explicitly.
- Detect and handle multiple-arity functions directly in the compiler.
However, for both of these we should wait until we have appropriate
benchmarking infrastructure in place. This is because our test
implementations have shown that the complexity of either of these
changes is quite significant, and we do not yet know if they really
pay off.
Change-Id: I077e977810fd5cb2b1ecd7f1a119e728025dd786
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6295
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
This resolves an upvalue at runtime by pushing it on the stack from
the closure's upvalue vector.
Change-Id: Ic3e7a7ecd9f7032f679114a1995e5bbf83062fcf
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6294
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
In preparation for implementing calling of closures, store a closure
directly in the VMs call frame.
Change-Id: Iad24cd8c49fee4ebd4d0c84ffaa4c2505ee3dfd6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6293
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
... same as the others
Change-Id: I9c8868388c10b0b6484c5bdd3799d801296c6979
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6292
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
Fully implements the instructions for compiling closure
objects (without runtime handling yet).
Closure (and thunk) objects are created at runtime by capturing all
known upvalues. To represent this, the instructions for creating them
need to have a variable number of arguments. Due to that, this commit
introduces new variants in OpCode that are not actually operations,
but data.
If the VM is implemented correctly, the instruction pointer should
never point at these. Due to this, the VM will panic if it sees a data
operand during an execution run.
Change-Id: Ic56b49b3a42736dc437751e76df0e89c8d0619c6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6291
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
This adds a new upvalue tracking structure in the compiler to resolve
upvalues and track their positions within a function when compiling a
closure.
The compiler will emit runtime upvalue access instructions after this
commit, but the creation of the runtime closure object etc. is not yet
wired up.
Change-Id: Ib0c2c25f686bfd45f797c528753068858e3a770d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6289
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
This adds a transparent wrapper around `usize` used for jump offsets
in the opcodes. This is a step towards getting rid of ambiguous plain
`usize` usage in the opcode.
Change-Id: I21e35e67d94b32d68251908b96c7f62b6f56a8bb
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6282
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
grfn pointed out in cl/6174 that `Result` might cause developers to
believe that this behaves like std::Result, which it does not.
Change-Id: Ia30ab0dcb7e8da7bf842777ee3fe17bcf35cb0c1
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6281
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Refactors the update function to take the attribute sets by value
instead.
To facilitate this, we use an equivalent of the currently unstable
`Rc::clone_or_unwrap` in the VM when encountering attribute sets, so
that in cases where the only references to the attrs being updated are
the ones on the stack those clones are avoided completely.
This does make update() a little bit more tricky internally, as some
optimised branches can directly return the moved value, and others
need to destructure with ownership. For this reason there are now two
different match statements handling the different ownership cases.
Change-Id: Ia77d3ba5c86afb75b9f1f51758bda61729ba5aab
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6279
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This is no longer needed for anything and the extra clone here is not
really more costly than constructing a blackhole value in a different
place.
Change-Id: I5c63085b1b4418b629ea58a42e3bfe9a9b586d76
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6275
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Without this escape, it is possible for Nix to produce escaped
representations which are not literal Nix values again.
This was fixed in upstream Nix in
https://github.com/NixOS/nix/pull/4012 (though only for eval, not in
the REPL) and the updated test is picked from upstream after that commit.
Because we run the C++ Nix tests against our test suite as well, this
also bumps our custom Nix 2.3 to a commit that includes the
cherry-picked fix from the PR above.
Change-Id: I478547ade65f655c606ec46f7143932064192283
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6271
Reviewed-by: grfn <grfn@gws.fyi>
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This is a more sensible place for this function to live and makes
upvalue resolution easier down the line.
Change-Id: I48ee39bdcdb4f96a16a327f7015aff60db5b15fb
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6270
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
This struct will carry the upvalue machinery in addition to the lambda
itself. For now, all lambdas are wrapped in closures (though
technically analysis of the environment can later remove innermost
Closure wrapper, but this optimisation may not be worth it).
Change-Id: If2b68549ec1ea4ab838fdc47a2181c694ac937f2
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6269
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
The blackhole allocation is not going to be cheaper than cloning this.
Change-Id: Id3ad44812decb4392830be06645e67bb0a982b96
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6267
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Compilation of `let`-expressions is going to become a lot more
complicated due to attempts to avoid thunking when encountering
internal references, so this is just being moved out of the way.
Change-Id: Iecfa4b13d14532e21c2540e6561b4235ce29736a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6266
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
This is just for dev comfort, it's not going to be useful for the
final version.
Change-Id: I05fdd590097a61085ed641810655d9ddaf8f3511
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6265
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
In conditions where no dynamic identifiers exist in a scope,
inheriting is usually a no-op - *unless* the identifier is not
statically known and the scope has a non-empty `with`-stack.
Change-Id: Iff4138d9cd4c56e844bc574203708dacc11c3f73
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6264
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This avoids copying around the value more than needed.
Change-Id: I35949d16dad7fb8f76e0f641eaccf48322144777
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6263
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
These do essentially the same, but return different error variants as
upstream Nix considers `throw` to be (sometimes) catchable.
Change-Id: I1a9ea84567d46fb37287dbf3f3f67052f9382cca
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6259
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
The set of things that can leak out of `builtins` into the global
scope is statically known (it is what Nix 2.3 leaks there,
essentially).
This is a mild change over the previous mechanism, where instead at
the point where the `builtins` set is constructed we "lift" the
globals out of there (if they exist).
This way users will still eventually be able to add additional
builtins, HOWEVER they will not be able to leak them into the global
scope.
Note that upstream Nix technically leaks _all_ builtins into the
global scope using the `__*` prefix, but we are trying to avoid this
in Tvix if it is not required in nixpkgs.
Change-Id: Ie9dec2ce33740134f3b2464eba3749f421dd5953
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6258
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Adds a new builtins module in which builtins can be constructed. The
functions in this module should return a correctly structured value to
be passed to the compiler's `globals`.
This is wired up all the way to the compiler with an example
`toString` builtin, available as a global. Note that this does not yet
actually behave like the real toString, which has some differences
from `Display`.
Change-Id: Ibb5f6fbe6207782fdf2434435567fc1bd80039a5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6254
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Previously, the tokens that could poison a scope (`true`, `false`,
`null`) had individual fields in the scope to track whether or not
they were poisoned.
This commit sets up new machinery that instead tracks scope poisoning
dynamically using a HashMap, and which makes it possible to introduce
additional tokens to the top-level ("global") scope that are directly
resolved by the compiler by passing a map of runtime values to be
used.
With this solution, the compiler now contains all machinery required
for wiring up builtins resolution.
The set of builtins to be exposed at runtime must, however, be
constructed *outside* of the compiler and passed in. Everything is
prepared for this, but it is not yet wired up (so the only existing
builtins are the ones we already had before).
Note that this technically opens up an optimisation potential when
compiling selection operations, where the attribute set being selected
from is `builtins`. The compiler could directly resolve the builtins
and place the right values on the stack.
Change-Id: Ia7dad3c2a98703e7ea0c6ace1a722d57cc70a65c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6253
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Builtins are represented as a Rust function pointer that accepts a
vector of arguments, which represents variable arity builtins.
Change-Id: Ibab7e662a646caf1172695d876d2f55e187c03dd
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6251
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
Nix functions always have a single argument and we do not yet make
efforts to optimise this in Tvix for known multi-argument functions
being directly applied.
For this reason, the call instruction is fairly simple and just calls
out to construct a new call frame.
Note that the logic for terminating the run loop has moved to the top
of the dispatch; this is because the loop run needs to be skipped if
the call frame for the current lambda has just been dropped.
Change-Id: I259bc07e19c1e55cd0a65207fa8105b23052b967
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6249
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Compiles lambda definitions of the simple form (i.e. without formals
arguments) and emits them as constants like any other value.
This does not yet implement actually invoking these functions in the VM.
Change-Id: Ie1e0a13220b68c1728be229b875f0992e685c5ef
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6247
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
This structure carries context about the lambda currently being
compiled (which may well be the top-level lambda of an input AST).
Using the indirection helpers in the compiler, things like the scope,
code and constants of the function being compiled are now taken from
the current lambda context instead.
Change-Id: If5f864d826c2e72855cee4b728ea1830e9b5ac06
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6246
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
This wires up most of the machinery for executing different call
frames inside of the VM and stuffs the top-level lambda which the
compiler outputs in there, as well.
Change-Id: Ib6201b3e3be1af96a4d195f6eb147f452860ffc3
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6242
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
This is going to carry the data for a function invocation inside of
the VM.
Change-Id: I86664563a7e35697a64294acd37ffde037fbd32d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6241
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
Changes the internal compiler plumbing to not just return a chunk of
code, but the same chunk wrapped inside of a lambda value.
This is one more step towards compiling runtime lambdas.
Change-Id: If0035f8e65a2970c5ae123fc068a2396e1d8fd72
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6240
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
With these indirections in place it becomes easier to change internals
of the compiler when introducing functions, which need the compiler to
be able to target different code chunks.
Change-Id: I4eb11572a93c140b1d059ba0a5af905756745d65
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6239
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
Instead of exiting the compiler at the first sight of an error,
skip any erroneous nodes and continue compiling, collecting more
errors along the way.
This paves the way for nicer error reporting in which multiple errors
can be reported at once, avoiding situations in which users are
hunting a fault error-by-error and possibly getting distracted by
less useful output.
Change-Id: I80c9a87272e33a31297167ae2eb2706a46adf15a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6236
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
This starts paving the way for nicer, source-code based error
reporting.
Right now the code paths in the VM do not emit annotated errors, as we
do not yet preserve that structure from the compiler. However, error
emitting code paths in the compiler have been amended to include known
nodes.
Change-Id: I1b74410ffd891c40cd913361bd73c4336ec8aa5b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6235
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
In this commit, the string interpolation parsing is identical to
nixpkgs which makes some of the upstream Nix tests for
interpolation-related weirdness pass.
Change-Id: I3a295cfdc404c32228a54846e6efd3c0dcee5842
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6233
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
These were missing an additional level of escaping, silly oversight
caught by an upstream test.
Change-Id: I0312084475e4b88c83945614e9aa5b34c6bc3ec2
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6232
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Since the latest published version of rnix-parser on crates.io, the
crate has undergone major changes which are only available in the git
repository at the moment. This commit updates the compiler to this
newer version of rnix.
Most notably, the entire AST provided by rnix is now wrapped in the
AST type system. As a result of this traversal is much nicer in many
places, especially for things like nested attribute selection.
There are a handful of smaller features missing for full feature
parity with the previous version, especially handling of path
literals, but PRs for these already exist in rnix-parser.
Change-Id: Icde6d393067976549492b7d89c4cc49e5e575fc7
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6231
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This implements `assert`, which evaluates an expression and aborts
evaluation if the value is not `true`.
At this point we should introduce eval-failed-* tests; probably
asserting against some representation of the error enum?
Change-Id: If54c8f616d89b829c1860a4835dde60a2cd70d7a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6230
Reviewed-by: grfn <grfn@gws.fyi>
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This makes for much more readable output especially when long strings
are involved.
Change-Id: I43dd73a0480535d7181a760788c42883a9b083f8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6229
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
`push_local`/`push_phantom` were worse names because they sound like
the value itself is being pushed, where in actuality it is just being
declared to the compiler.
Change-Id: Ibfda5c4c8e47d5d3262bfe005b0f1f84908a117e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6228
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
These tokens are optionally parsed as identifiers by Nix, which means
that within any scopes that resolve them the compiler needs to track
whether they have been overridden to know whether to emit the literal
instructions or resolve a variable.
This is implemented by a new concept of "scope poisoning", where the
compiler's scope structure tracks whether or not any builtin
identifiers have been overridden.
Change-Id: I3ab711146e229f843f6e1f0343385382ee0aecb6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6227
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: grfn <grfn@gws.fyi>
While full recursion through thunking is not available, there are
actually incorrect behaviours introduced by declaring before
binding (example in the newly introduced test).
This commit simplifies the implementation to avoid this issue, and
also because I intend to explore a bit more how far we can get in non
left-to-right bindings *without* introducing thunks immediately.
Change-Id: I21fd3007ac3946570639772d7d624d70bd209958
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6226
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: grfn <grfn@gws.fyi>
If an unknown variable is encountered and the with stack is not empty,
emit instructions for resolving the variable at runtime.
Change-Id: I752f4bd0025335744e4747364abd1bd34130374e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6223
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: grfn <grfn@gws.fyi>
Adds an additional structure to the compiler's scope to track the
runtime "with stack", i.e. the stack of values through which
identifiers should be dynamically resolved within a with-scope.
When encountering a `with` expression, the value from which the
bindings should be resolved is pushed onto the stack and tracked by
the compiler in the "with stack", as well as with a "phantom value"
which indicates that the stack contains an additional slot which is
not available to users via identifiers.
Runtime handling of this is not yet implemented.
Change-Id: I5e96fb55b6378e8e2a59c20c8518caa6df83da1c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6217
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This name is much more sensible actually; its more than just a
collection of locals as it tracks additional scope information in the
case of Nix.
Change-Id: Ia2739bbd39aab222b1c4355e9248828973b0db43
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6216
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Note that at this point recursive bindings do not yet work in either
attrsets or let, so inheriting from the same scope is generally not
possible yet.
Change-Id: I6ca820d04b8ded5c22fb7ea18e2ec203bcaa8e9c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6215
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Straightforward implementation, evaluating the elements of an inherit
and preparing the stack so that `OpAttrs` sees all relevant values
when constructing the attribute set itself.
The emitted instructions for inheriting a lot of values from the same
attribute set are inefficient, but it's too early to say whether this
actually matters.
Change-Id: Icb55a20936d4ef77173f34433811c5fa5d2c9ecc
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6214
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
If the directory in which REPL history is stored does not exist,
gently try to create it, but do not raise an error if it doesn't work.
We may want to warn about it, but in general this sort of
non-essential feature should not cause a hard failure.
Change-Id: If4fe8db0c7893c39627efe72c9cd9ebf7ed63f04
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6213
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Using `OpAttrSelect`, the ? operator will fail when encountering a
nested value that is not an attribute set.
This however breaks valid code, such as:
{ bs = 42; } ? bs.a.b
The fix is simply to use the same operator used in the `or` statement,
which leaves a sentinal on the stack if a field is not found or the
value is not an attribute set.
Change-Id: Ib28fc8a96e6d592b4cdbc3e65ba129ad8faecd66
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6211
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
If a nested attrpath encounters a non-set value, the sentinel value
denoting a lack of next values should be emitted. This mirrors the
behaviour of Nix.
Change-Id: Ia80443d5a11243cc6d98dcab1249a3f5fdf77e27
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6210
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Not sure how exactly this snuck in, but it caused some subtle
breakages in deeply nested attribute sets.
Change-Id: I8049ce912405d3750031f79cc8d86ff1c3c02c2b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6208
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
We're confident that we're handling all branches that can reasonably
occur from valid AST, any other cases should be considered a critical
evaluator bug and panic rather than surfacing something that looks
like user error.
Change-Id: If96966eb32b8ff12fcaeb9ea3b0c8fc51b6abd11
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6205
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
This is actually *tested* behaviour in C++ Nix, so we need to
implement it here, too.
Change-Id: Ic4a4659a2f04cdd928cbe78a85dae90401515371
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6199
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This adds a `disassembler` feature to the crate configuration that
traces the operations executed and the state of the stack at runtime.
This can be enabled by compiling with `--feature disassembler`.
This will also gain a more sensible layout of code slices eventually.
Change-Id: I34c15e1cd346ecc4362b5afba6bf82dd49359d20
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6193
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Using `inherit` in a let-binding can not possibly have an effect, as
the given identifier is already bound exactly the same way in the
current scope.
This introduces a subtle bug that is fixed later on, as there
actually *is* a (single) condition where these inherits are
meaningful.
Change-Id: I8b24f0edcfe80db8153bb7e86cf478d36957d6f8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6192
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
This makes basic `let ... in ...` statements work correctly. It does
not yet account for the call frames pushed into the VM during function
application.
Change-Id: I67155171daf1a43011b96716dd9d1ab04b27db33
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6190
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
Nix does not allow dynamic identifiers in let expressions (only in
attribute sets), but there are several different kinds of things it
considers static identifiers.
The functions introduced here put the path components of a let
expression into normalised (string) form, and surface an error about
dynamic keys if one is encountered.
Change-Id: Ia3ebd95c6f3ed3cd33b94e156930d2e9c39b6cbf
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6189
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
These expressions now leave the binding values on the stack, and clean
up the scope after the body of the expression.
While variable access is not yet implemented (as the identifier node
remains unhandled), this already gives us the correct stack behaviour.
Change-Id: I138c20ace9c64502c94b2c0f99a6077cd912c00d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6188
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
These are going to be used during compilation of `let`-expressions to
determine stack offsets for local variables.
Change-Id: Ibb79f3f1ae86650303f88eacf623ae456458de87
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6187
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Canonicalisation performs much more functionality than what C++ Nix
implements for paths, and causes some undesirable behaviour (e.g.
handling non-existant files becomes difficult, but should be possible
in literals).
Instead, the path_clean crate provides a pure normalisation method.
There is an intention to add this to Rust itself:
https://github.com/rust-lang/rfcs/issues/2208
Change-Id: I775d238136db0a52cf6b12a68985833c8fb32882
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6186
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Resolves relative paths (e.g. `./foo`) either relative to the location
of the Nix file, or relative to the working directory if none is
supplied.
Change-Id: I70ec574657b221b458015117a004b6e4a9c25a30
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6185
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
There are multiple things that can theoretically fail while resolving
a path, as some of it includes I/O. A new error variant has been added
for this and appropriate errors have been introduced.
Change-Id: Ie222245425207dabbf203166eb5ed1eec0114483
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6184
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
These two paths are the easiest to handle, as they are simply built up
from the components supplied in the text node and then normalised.
Note that the normalisation of fs::canonicalize includes symlink
resolution, which Nix does not actually do. We will need to fix that
at some point.
Change-Id: I54158f0684f197dd2a2583f7d0982d54c7619993
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6183
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
grfn pointed out in cl/6082 that this is actually the desugaring of
the write! macro, so it doesn't make sense to write it out.
Change-Id: If7c055b042ad22b034722aec1eaadba92736d684
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6180
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: grfn <grfn@gws.fyi>
grfn pointed out in cl/6069 that naming them like this makes it clear
that things are being added to the end of the state.
Change-Id: I6a23215c4fef713869a3c85b0dde1ebbda7637e9
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6179
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
For representation wrappers that are used to control the visibility of
type internals, this ensures that the wrapper does not increase the
size of the type.
In practice, the optimiser likely does this anyways but it is good to
guarantee it.
Change-Id: Ic6df7d668fe6006dfbd5b6cfcfc2088afa95b810
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6178
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
This makes it possible to quickly detect code errors that might blow
up the size of the OpCode type.
Change-Id: I7662dd0aa30c4762c0f9e4fa346418c9ca8b9994
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6177
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
These can be used predominantly to emit warnings about things that the
compiler can infer, such as deprecated language features.
Change-Id: I3649c625459d7f3f95cdf42d5c651d23d66569ec
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6174
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
The parser creates this node type from literal URL values. Technically
these are deprecated and have been removed from nixpkgs.
Change-Id: I4d05034dd9b4d8348e4ed8a2bbb37c1b6ccef8bc
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6173
Reviewed-by: grfn <grfn@gws.fyi>
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This is a substantially nicer experience, immediately granting us
history, proper exiting and so on.
Change-Id: Iba4cb1713b9ac53d0799722bdbe2cd0e94a2f527
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6171
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
This operator allows for accessing attribute sets (including nested
access) while also providing a default value.
This is one of the more complex operations to compile, as it needs to
keep track of a fairly large number of jumps that all need to be
patched correctly.
To make this easier to understand there's a small diagram included in
the comments.
Change-Id: Ia53bb20d8f779859bfd1692fa3f6d72af74c3a1f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6167
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This sentinel value is going to be used for certain nested accesses
into attribute sets.
There is a new instruction similar to `OpAttrsSelect` which leaves the
sentinel on the stack if a key is not found, instead of raising an
error.
Additionally, a new jump instruction makes its jump operation
conditional on finding such a sentinel value.
Change-Id: I2642f0a0bcc85bbe0ead68ea09a7dd794dbedeac
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6166
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
The only uses of the static variant were for `"name"` and `"value"`,
which are both small enough to fit into a SmolStr. The size of
NixString accomodates `String` anyways, so we may as well inline them.
Additionally smol_str is already in the dependency graph because rnix
uses it, and using it for representations of identifiers is sensible.
Change-Id: I9969312256d1657d69128e54c47dc7294a18ce58
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6165
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: grfn <grfn@gws.fyi>
Having these visible more explicitly is useful while debugging.
Change-Id: I86b497883063d32792b635eb4514b7aeae484af4
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6164
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: grfn <grfn@gws.fyi>
Implements the nested presence check operator for attribuet sets by
traversing the chain of lookups through instructions that push/pop
sequentially deeper attribute sets onto the stack.
Note that this commit introduces a bug in case of nested attributes
not being found, which is fixed in a later commit.
Change-Id: Ic8b4c8648736f6cb048e3aa52592e4d075bf0544
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6163
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: grfn <grfn@gws.fyi>
This makes it possible to check things like `{} ? a` with a single
level of nesting.
Change-Id: I567c36fcfd2f9e2f60071acd3ebfe56dea59b26f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6161
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
Reviewed-by: sterni <sternenseemann@systemli.org>
With this change, attribute set access is working as intended.
Change-Id: Ic5dbbd68aa59156106069289e7375a696909f78b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6159
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: grfn <grfn@gws.fyi>
Fairly straightforward, handling the optimised representations
manually and otherwise delegating to BTreeMap.
Note that parsing of raw identifiers is not yet implemented.
Encountering an identifier node usually means that there is locals
access going on, so we need a special case for compiling a node in
such a way that an identifier's literal value ends up on the stack.
Change-Id: I13fbab7ac657b17ef3f4c5859fe737c321890c8a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6158
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: grfn <grfn@gws.fyi>
This operation is required because both sides of the logical operators
are strictly evaluated by Nix, even if the resulting value is not used
further.
For example, in our implementation of `&&`, if the left-hand side is
`true`, then the result of the expression is simply the right-hand
side value. This value must be asserted to be a boolean for the
semantics of the language to work correctly.
Change-Id: I34f5364f2a444753fa1d8b0a1a2b2d9cdf7c6700
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6157
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: grfn <grfn@gws.fyi>
Add a new `lib.rs` to tvix/eval, which `pub use`s the `interpret`
function, and all types mentioned in its return type, and then uses
*this* instead of direct `mod` statements in the `main.rs` to implement
the entrypoints to the interpreter. This is in preparation for calling
these functions from integrated benchmarking infrastructure using
Criterion, though other things (like integration tests) might want to do
that as well.
Change-Id: I7b585134a96b1c56a2ac64d2036b0e51d321bd27
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6155
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Autosubmit: grfn <grfn@gws.fyi>
Similar to `||`, but inverting the left-hand side.
In other words, `a -> b` is essentially rewritten as `!a || b`.
Change-Id: I8a62da65ff070b389e46048d047a54279060a97b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6152
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
Reviewed-by: sterni <sternenseemann@systemli.org>
This logical operator is implemented in terms of jumping operations
and thus requires slightly different treatment than other binary
operators.
Change-Id: Ib3d768b70dd7e16014c9b47d770aa74eec60ae92
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6150
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
Reviewed-by: sterni <sternenseemann@systemli.org>
These expressions use simple jumps to skip the correct expression
conditionally in the bytecode by advancing the instruction pointer.
Note that these expressions are already covered by a test behind the
`nix_tests` feature flag, but adding more is probably sensible.
Change-Id: Ibe0eba95d216321c883d3b6b5816e2ab6fe7eef1
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6148
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
Reviewed-by: sterni <sternenseemann@systemli.org>
Wraps the string representation in an additional newtype struct with a
private field in order to hide the representation from other modules.
This is done in order to avoid accidental leakage of the internals
outside of value::string.
In fact, this caught a mistake in the compiler module which was
directly constructing an internal variant.
Change-Id: If4b627d3cff7ab9cd50ca1a3ac73245d4dcf7aef
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6147
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Wraps the attrs representation in an additional newtype struct with a
private field in order to hide the representation from other modules.
This is done in order to avoid accidental leakage of the internals
outside of value::attrs.
Change-Id: I68d1d02514aa0443df4c39801001a3f1f6cc5d5c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6146
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Ensuring that the implementation is not leaking out of the module lets
us keep things open for optimisations (e.g. empty list or pairs
through tuples).
Change-Id: I18fd9b7740f28c55736471e16c6b4095a05dd6d0
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6145
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This is accomplished by simply delegating to the Rust implementations
of (Partial)Ord and (Partial)Eq, which are implemented for Value and
underlying wrapper types to behave like they do in Nix.
To ease the implementation overhead, a new comparison operator macro
has been added to the VM module.
Incomparable types will raise a new error variant when a comparison is
attempted, containing both supplied types. This mimics the information
carried in the error thrown by C++ Nix.
Change-Id: Ia19634d69119d40722f3ca672387bc3a80096998
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6143
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This can now fully delegate to PartialEq of Value
Change-Id: Iaa9f4ec9b8830d516d72f83a93ab2df9a6e5697c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6142
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
The underlying implementation does a few tricks based on which pair of
attrset representations is encountered.
Particularly the effect of short-circuiting the empty cases might be
relevant in nixpkgs/NixOS, due to the use of lib.optionalAttrs.
Change-Id: I22b978b1c69af12926489a71087c6a6219c012f3
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6140
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
I assumed that `Ord` is a marker trait like `Eq`, but it actually has
a member. Without this ordering was incoherent.
Change-Id: Id37cbdf333daf748d29b85243046c7e061b1ce29
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6139
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Instead of constructing another runtime value representing the pair on
which to perform arithmetic, implement the same logic in the shape of
a macro.
This is designed to be compatible with operators like `+` that work
both as an arithmetic operator AND as an operator on another pair of
types.
Change-Id: I1c83649ead6117f811f1fb45482d0cadf811125e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6136
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Nix displays a maximum of 5 digits for floating points.
Change-Id: Ifa3c0d96fa0b24e3be8f94dfebc99e602a258355
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6133
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Once we have full coverage they should be enabled by default.
Change-Id: Iace9e1ae9a9f901a0979ad336434004b8028fe8a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6129
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
When printing strings as identifiers (in attribute sets), the string
should only be quoted and escaped if it contains escape characters.
Change-Id: If2bcfa1e93dc8f00be4d7a57ec1d82fc679103c3
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6127
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
Autosubmit: tazjin <tazjin@tvl.su>
This adds scaffolding code for running the Nix language test suite.
The majority of eval-okay-* tests should eventually be runnable as-is
by Tvix, however the eval-fail-* tests might not as we intend to have
more useful error messages than upstream Nix.
Change-Id: I4f3227f0889c55e4274b804a3072850fb78dd1bd
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6126
Tested-by: BuildkiteCI
Autosubmit: tazjin <tazjin@tvl.su>
Reviewed-by: grfn <grfn@gws.fyi>
Nix strings displayed to users must be escaped the same way as they
are in C++ Nix. This adds the scaffolding for escapes, but is most
likely not yet complete.
Change-Id: Icfdcb2ac98d292c567ba894a92b6529a53e0cc17
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6124
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
Instead of comparing the enum variants (which does not yield useful
behaviour), compare &str representations of the string instead.
Change-Id: I5e94b5f6c91b4561e1bc7c36d586f3d23c243764
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6112
Tested-by: BuildkiteCI
Reviewed-by: eta <tvl@eta.st>
For cases where the strings are statically known (such as the
oft-occuring name/value), this can be a useful optimisation.
It's also much more convenient in tests.
Change-Id: Ie462b684805bd4986ea5e85ca4bff663bc2d3c3c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6111
Tested-by: BuildkiteCI
Reviewed-by: eta <tvl@eta.st>
These do not yet test nested attribute sets; we need to add some more
inspection primitives first.
Change-Id: Icfc99bf17c73ebefc0d882a84f0ca73ec688a54d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6110
Reviewed-by: eta <tvl@eta.st>
Tested-by: BuildkiteCI
With this change, nested attribute sets can now be created from
literals.
This required some logic for dealing with cases where at a deeper
nesting point a literal attribute set was constructed from an
optimised representation.
For example, this is valid Nix code:
```nix
{
a = {}; # creates optimised empty representation
a.b = 1; # wants to add a `b = 1` to it
b = { name = "foo"; value = "bar"; }; # creates optimised K/V repr
b.foo = 42; # wants to add an additional `foo = 42`
}
```
In these cases, the attribute set must be coerced to a map
representation first which is achieved by the new internal
NixAttr::map_mut helper.
Change-Id: Ia61d3d9d14c4e0f5e207c00f6a2f4daa3265afb2
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6109
Reviewed-by: eta <tvl@eta.st>
Tested-by: BuildkiteCI
The internal optimisations of the set representation were previously
leaking into the VM, which is highly undesirable.
Keeping it encapsulated allows us to do additional optimisations
within value::attrs without being concerned about its use in the VM.
Change-Id: I7e7020bb0983b9d355d3db747b049b2faa60131f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6108
Reviewed-by: eta <tvl@eta.st>
Tested-by: BuildkiteCI
There are multiple points where an insertion needs to be done into an
attribute set, but copying the key or checking for presence before
insertion should be avoided
As that is a little bit noisy, it's been factored out into a helper
function in this commit.
Change-Id: Ibcb054ebeb25a1236c06c812f47c8e74180d4fc9
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6107
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
This can construct non-overlapping nested attribute sets (i.e. `{ a.b
= 1; b.c = 2; }`, but not `{ a.b = 1; a.c = 2; }`).
In order to do the latter, it's necessary to gain the ability to
manipulate the in-progress attribute set construction. There's
multiple different options for this ...
Change-Id: If1a762a720b175e8eb4216cbf96a7434d22640fb
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6106
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
There are some notions of equality (due to e.g. different backing
variants for types, or Nix particularities) that don't work correctly
when deriving PartialEq.
Change-Id: Ide83ae67d051cc0b3ca89cefb283f17d0207acce
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6105
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
This is required for constructing nested attribute sets at runtime.
There'll be quite a lot of optimisation potential with this solution
eventually, if it should turn out to be a bottleneck.
This introduces a conceptual change, in that the `Value` enum is now
an enum representing "all runtime values" instead of "all Nix language
types". This makes sense in general, as this type will also contain
Chunk representations etc. which are not exposed to users.
Change-Id: Ic5f72b2a0965b146c6a451efad34c6a81ca1aad8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6103
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
For name/value pairs (which occur extremely often in Nix and make up a
significant chunk of the runtime cost of evaluating nixpkgs) we
substitute an optimised representation.
For now this will only be used if the name/value pair keys were
specified as literal identifiers or strings (i.e. if chunks are
encountered as keys they are not forced and a normal attribute set
backed by a map will be constructed).
Change-Id: Ic79746c323e627528bd58b1a6024ee8d0aff7858
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6102
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Users may construct a pair that falls into the name/value optimisation
but where `name` is not actually a string, as from the language
perspective there is nothing special about this attribute set.
We also can not conditionally apply this by forcing the key at this
point, as this would change the language semantics.
Therefore, the name in the optimised representation is also carried as
`Value`.
Change-Id: I5be8a4c98ba19ebdfb7203a929f714a04492512e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6101
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
This adds a new instruction which assembles an interpolated string
from a specified number of fragments, which are already going to be
located on the stack in the right position.
This will raise a type error if any of the fragments do not evaluate
to a string.
Change-Id: I5756248fa3e9fcc3d063c14db40b332f7e20a588
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6098
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
This sets up the scaffolding for compiling interpolation, but those
instructions do not yet exist.
Change-Id: Ife41bbbf432d9661abe566c92437409dd0da44e7
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6097
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
There might be more logic in the future to encapsulate different
backing implementations of lists as well.
Change-Id: Ib7064fab48bf88b0c8913b0ecfa2108177c7c9fd
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6093
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
Autosubmit: tazjin <tazjin@tvl.su>
Implements attribute set literals without nesting. Technically this
already supports dynamic key fragments (evaluating to strings), though
the only way to create these (interpolation) is not yet implemented.
However, creating simple attribute sets like `{ }`, or `{ a = 15; }`
or `{ a = 10 * 2; }` works.
Recursive attribute sets are not yet implemented as we do not have any
kind of scope access yet anyways.
This is implemented using a new instruction that creates an attribute
set with a given number of elements by popping key/value pairs off the
stack.
Change-Id: I0f9aac7a131a112d3f66b131297686b38aaeddf2
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6091
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
Deriving Ord/Eq is required for the ordered BTreeMaps. Once interning
is implemented this will require some extra magic for the sort order,
but that's fine.
Change-Id: I0c654648eb3609a4a01d84868c25f43a4d35bc2e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6089
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
This representation should match what the Nix REPL shows for result
values.
Change-Id: If3143d969fcdc123a6029e2aeb7bbd6ae51aeb71
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6082
Tested-by: BuildkiteCI
Autosubmit: tazjin <tazjin@tvl.su>
Reviewed-by: grfn <grfn@gws.fyi>
This isn't relevant to the value type itself.
Change-Id: I678bc92a8a530b1081ed498bf3ff7925217bcc01
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6081
Tested-by: BuildkiteCI
Autosubmit: tazjin <tazjin@tvl.su>
Reviewed-by: grfn <grfn@gws.fyi>
These are a bit tricky to implement because Nix technically treats
them as identifiers, and only if the identifier is not explicitly
overridden within the scope does it yield the expected literal values.
Note that weirdness even occurs with scopedImport.
Change-Id: Ie55723405ccfcc25da37c5a08fa3332f37cf9ae5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6080
Tested-by: BuildkiteCI
Autosubmit: tazjin <tazjin@tvl.su>
Reviewed-by: grfn <grfn@gws.fyi>
