Commit graph

944 commits

Author SHA1 Message Date
Florian Klink
8bd7ced1fb feat(tvix/eval/io): allow &mut self in EvalIO
It's okay if these calls mutate some internal state inside an
implementation.

Change-Id: I12bb11bde0310778c3da1275696bf7de058863a3
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8571
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
2023-05-14 18:59:55 +00:00
Vincent Ambo
969fbcd6d4 fix(tvix/eval): builtins.trace prints to stderr
Change-Id: Icf577396035474d6977e627058aba5805c61985e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8563
Tested-by: BuildkiteCI
Autosubmit: tazjin <tazjin@tvl.su>
Reviewed-by: flokli <flokli@flokli.de>
2023-05-12 12:50:59 +00:00
Florian Klink
6a30eb69e0 chore(tvix/*): bump to smol_str 0.2.0
Change-Id: Ic9ac1b6fecb564eafb41b265bf317cd385fdc170
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8560
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
2023-05-11 18:19:20 +00:00
Vincent Ambo
ce502bdc89 refactor(tvix/eval): improve representation of chunk/span mapping
This switches out the previous compressed representation (count of
instructions per span) with a representation where the chunk's span
list stores the index of the first operation that belongs to a span,
and finds the right span by using a binary search when looking them
up.

This improves the lookup complexity from O(n) to O(log n).

This improvement was suggested and (mostly) implemented by GPT-4. I
only fixed up some names and updated the logic for deleting
spans (which it only did not do because I didn't tell it about that).

The code was verified by producing a complex error before/after the
change and ensuring that all spans in the error match exactly.

Co-Authored-By: GPT-4
Change-Id: Ibfa12cc6973af1c9b0ae55bb464d1975209771f5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8385
Reviewed-by: ezemtsov <eugene.zemtsov@gmail.com>
Tested-by: BuildkiteCI
Autosubmit: tazjin <tazjin@tvl.su>
2023-03-31 16:46:19 +00:00
Vincent Ambo
2d305fd5b3 refactor(tvix/eval): retain call frames when entering calls
This grows the frame stack as the call stack grows, which yields *much*
better user-facing error messages.

I haven't measured the performance impact this has yet, for now I'm
still just trying to add more information to errors and then cut down
again where necessary.

Change-Id: I89f058ef31979edacf4667775d460b60704ce4d7
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8334
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Autosubmit: tazjin <tazjin@tvl.su>
2023-03-27 09:02:43 +00:00
Vincent Ambo
ba138712e4 feat(tvix/eval): add Evaluation::strict to toggle top-level deepseq
This makes it possible for callers to control whether they can receive
partially evaluated values from an evaluation or not.

We're actually flipping the default behaviour to non-strict top-level
evaluation, which means that callers have to set `strict = true` on
the Evaluation to get the previous behaviour.

Change-Id: Ic048e9ba09c88866d4c3177d5fa07db11c4eb20e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8325
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
2023-03-22 13:44:20 +00:00
Vincent Ambo
a5f28eea94 fix(tvix/eval): print unevaluated thunks like Nix does
Change-Id: Ie4c563e933f571f45cb4f4efe650d1b65f119e8d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8324
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Autosubmit: tazjin <tazjin@tvl.su>
2023-03-22 13:44:19 +00:00
Vincent Ambo
80aaadfc19 fix(tvix/eval): use span of set for OpForce in attribute access
Emits the span of the `set` that is being accessed in the `force`
operation of an attribute access.

Looking at traces, it's a lot more useful to get information about
*what* is being forced, as in cases like `foo.bar` it can be
misleading to have an error highlight `bar`, when the error occured
while forcing `foo` to be able to access `bar` in the first place.

Change-Id: Id46ff28f20c67cb4971727ac52cc4811795cea2d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8272
Reviewed-by: flokli <flokli@flokli.de>
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
2023-03-17 19:40:40 +00:00
Vincent Ambo
8c13f18d11 feat(tvix/eval): report all known spans on infinite recursion
This reports the span

1. of the code within a thunk,
2. of the place where the thunk was instantiated,
3. of the place where the thunk was first forced,
4. of the place where the thunk was forced again,

when yielding an infinite recursion error, which hopefully makes it
easier to debug them.

The spans are tracked in the ThunkRepr::Blackhole variant when putting
a thunk under evaluation.

Note that we currently have some loss of span precision in the VM loop
when switching between frame types, so spans 3/4 are currently a bit
wonky. Working on it.

Change-Id: Icbd2a9df903d00e8c2545b3fc46dcd2a9e3e3e55
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8270
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Autosubmit: tazjin <tazjin@tvl.su>
2023-03-17 19:32:38 +00:00
Vincent Ambo
3fa6b13c1e feat(tvix/eval): track span of first force in a thunk blackhole
This is step 1 towards being able to use all 4 spans that we know when
dealing with infinite recursion. It tracks the span at which the
force of a thunk was first requested when constructing a blackhole, so
that we can highlight the spans of the first and second forces.

These are actually the least relevant spans, but the easiest to put in
place, more coming soon.

Change-Id: I4c7e82f6211b98756439d4148a4191457cc46807
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8269
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
2023-03-17 19:31:37 +00:00
Vincent Ambo
5095e4f269 feat(tvix/eval): add generator "name" to NativeError kind
This produces traces in which we can see what kind of native code was
run. Note that these "names" are named after the generator message, so
these aren't *really* intended for end-user consumption, but we can
give them saner names later.

Example:
https://gist.github.com/tazjin/82b24e92ace8e821008954867ee05057

This already makes the traces a little easier to parse.

Change-Id: Idcd601baf84f492211b732ea0f04b377112e10d0
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8268
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Autosubmit: tazjin <tazjin@tvl.su>
2023-03-17 19:31:37 +00:00
Vincent Ambo
ea80e0d3f8 feat(tvix/eval): enrich errors with VM's frame stack information
When emitting an error at runtime, the VM will now use the new
`NativeError` and `BytecodeError` error kinds (which just wrap inner
errors) to create a set of diagnostics to emit.

The primary diagnostic is emitted last, with `error` type (so it will
be coloured red in terminals), the other ones will be emitted with
`note` type, highlighting the causal chain.

Example:
https://gist.github.com/tazjin/25feba7d211702453c9ebd5f8fd378e4

This is currently quite verbose, and we can cut down on this further,
but the purpose of this commit is to surface more information first of
all before worrying about the exact display.

Change-Id: I058104a178c37031c0db6b4b3e4f4170cf76087d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8266
Autosubmit: tazjin <tazjin@tvl.su>
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
2023-03-17 19:31:37 +00:00
Vincent Ambo
b78ae941a4 fix(tvix/eval): use coerce_to_string in builtins.substring
This actually uses coercion under the hood in C++ Nix. See the test
for an example.

Change-Id: Id56b364acf269225b6829d0b600e0222f8b3608d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8322
Reviewed-by: andi <andi@notmuch.email>
Tested-by: BuildkiteCI
2023-03-17 19:17:29 +00:00
Vincent Ambo
b5f5a1595d chore(tvix/eval): remove some dead code
This was commented out and forgotten during the generator refactor, oh
well.

Change-Id: I474b685159a955a846db462da0dd0067af177b04
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8321
Autosubmit: tazjin <tazjin@tvl.su>
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
2023-03-17 11:01:07 +00:00
Adam Joseph
32999cb6f6 docs(tvix/eval): suggested layout adjustment to VM loop diagram
Change-Id: I5467cd66801ad8fe6c4ec0ae337763f1762cea1c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8252
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Autosubmit: Adam Joseph <adam@westernsemico.com>
2023-03-14 09:56:28 +00:00
Vincent Ambo
466e6dc265 docs(tvix/eval): document inner workings of the new VM loop
Change-Id: I8224bf039f739c401900b5a2ddc839810c87cf6e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8226
Tested-by: BuildkiteCI
Reviewed-by: Adam Joseph <adam@westernsemico.com>
2023-03-14 09:22:32 +00:00
Vincent Ambo
d456705352 chore(tvix): Generator{Request|Response} -> VM{Request|Response}
We settled on this being the most reasonable name for this construct.

Change-Id: Ic31c45461a842f22aa05f4446123fe3a61dfdbc0
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8291
Tested-by: BuildkiteCI
Autosubmit: tazjin <tazjin@tvl.su>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2023-03-14 09:22:22 +00:00
Adam Joseph
1e80b9ea8b chore(tvix/eval): mark async functions which are called by the VM
Given Rust's current lack of support for tail calls, we cannot avoid
using `async` for builtins.  This is the only way to avoid
overflowing the cpu stack when we have arbitrarily deep
builtin/interpreted/builtin/interpreted/... "sandwiches"

There are only five `async fn` functions which are not builtins
(some come in multiple "flavors"):

- add_values
- resolve_with
- force, final_deep_force
- nix_eq, nix_cmp_eq
- coerce_to_string

These can be written iteratively rather than recursively (and in
fact nix_eq used to be written that way!).  I volunteer to rewrite
them.  If written iteratively they would no longer need to be
`async`.

There are two motivations for limiting our reliance on `async` to
only the situation (builtins) where we have no other choice:

1. Performance.

   We don't really have any good measurement of the performance hit
   that the Box<dyn Future>s impose on us.  Right now all of our
   large (nixpkgs-eval) tests are swamped by the cost of other
   things (e.g. fork()ing `nix-store`) so we can't really measure
   it.  Builtins tend to be expensive operations anyways
   (regexp-matching, sorting, etc) that are likely to already cost
   more than the `async` overhead.

2. Preserving the ability to switch to `musttail` calls.

   Clang/LLVM recently got `musttail` (mandatory-elimination tail
   calls).  Rust has refused to add this mainly because WASM doesn't
   support, but WASM `tail_call` has been implemented and was
   recently moved to phase 4 (standardization).  It is very likely
   that Rust will get tail calls sometime in the next year; if it
   does, we won't need async anymore.  In the meantime, I'd like to
   avoid adding any further reliance on `async` in places where it
   wouldn't be straightforward to replace it with a tail call.

https://reviews.llvm.org/D99517

https://github.com/WebAssembly/proposals/pull/157

https: //github.com/rust-lang/rfcs/issues/2691#issuecomment-1462152908
Change-Id: Id15945d5a92bf52c16d93456e3437f91d93bdc57
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8290
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Autosubmit: Adam Joseph <adam@westernsemico.com>
2023-03-13 21:33:58 +00:00
Adam Joseph
e7a534e0c6 refactor(tvix/eval): reduce fetch{forced|captured}_with visibility
This commit moves fetch_forced_with and fetch_captured_with into the
scope of their only caller (resolve_with).

Change-Id: I9a8bc27228888729d591e8cb021c431b2b6468f5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8289
Autosubmit: Adam Joseph <adam@westernsemico.com>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
2023-03-13 21:33:58 +00:00
Adam Joseph
47895c4c30 feat(tvix/eval): rewrite nix_cmp_ordering to be nonrecursive
This rewrites nix_cmp_ordering as an iterative loop, which
eliminates the extra pinned-boxing helper function.

Change-Id: I33d0ecc913e02affd8fd4c7bc1c9ecfdf4c7deb9
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8288
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Autosubmit: Adam Joseph <adam@westernsemico.com>
2023-03-13 21:22:12 +00:00
Vincent Ambo
b3f8d66a6a chore(tvix/eval): prune some dependencies & features
* We no longer need backtrace-on-stack-overflow, as we no longer
  overflow the stack with the recent eval refactorings. This was weird
  voodoo anyways, introduced earlier to debug some cases where stack
  overflows occured.

* default features of genawaiter crate are not needed, as we don't use
  their proc macros

Change-Id: I346fc5a18d7f117ee805909a8be8f535b96be76c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8263
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2023-03-13 20:30:59 +00:00
Vincent Ambo
94513525b9 refactor(tvix/eval): reorder bytecode operations match by frequency
This reorders the operations in the VM's main `match` statement while
evaluating bytecode according to the frequency with which these
operations appear in some nixpkgs evaluations.

I used raw data that looks like this:
https://gist.github.com/tazjin/63d0788a78eb8575b04defaad4ef610d

This has a small but noticeable impact on evaluation performance.

No operations have changed in any way, this is purely moving code
around.

Change-Id: Iaa4ef4f0577e98144e8905fec88149c41e8c315c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8262
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
2023-03-13 20:30:59 +00:00
Vincent Ambo
e48a17a6b3 docs(tvix/eval): fix reference to Empty message in a comment
Change-Id: I3dc30cca33fbbd8e8686655635ee471f5937d9f8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8257
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2023-03-13 20:30:59 +00:00
Vincent Ambo
cc59cbf3e2 refactor(tvix/eval): rename VM::tail_call_value -> VM::call_value
The name of this was not accurate anymore after all the recent
shuffling, as noted by amjoseph. Conceptual tail calls here only occur
for Nix bytecode calling Nix bytecode, but things like a builtin call
actually push a new native frame.

Change-Id: I1dea8c9663daf86482b8c7b5a23133254b5ca321
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8256
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2023-03-13 20:30:59 +00:00
Vincent Ambo
5d9bfd7735 fix(tvix/eval): emit warnings from builtins.import again
Wires up generator logic to emit warnings that already have spans
attached again.

Change-Id: I9f878cec3b9d4f6f7819e7c71bab7ae70bd3f08b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8224
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2023-03-13 20:30:59 +00:00
Vincent Ambo
43b0416bd8 fix(tvix/eval): more closely line up path resolution with cppnix
... except now the tests fail, but at least it works

Change-Id: I05e86c173f40533ae65548585c1ddaa200ac5235
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8214
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
2023-03-13 20:30:59 +00:00
Vincent Ambo
c700776733 refactor(tvix/eval): VM struct no longer needs to be public
Change-Id: I93b485ddd280cc15fcbaecf4aed5fcd22e28a8a8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8212
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
2023-03-13 20:30:59 +00:00
Vincent Ambo
939cebd0f1 fix(tvix/eval): implement cppnix JSON-serialisation semantics
This drops the usage of serde::Serialize, as the trait can not be used
to implement the correct semantics (function colouring!).

Instead, a manual JSON serialisation function is written which
correctly handles toString, outPath and other similar weirdnesses.

Unexpectedly, the eval-okay-tojson test from the C++ Nix test suite
now passes, too.

This fixes an issue where serialising data structures containing
derivations to JSON would fail.

Change-Id: I5c39e3d8356ee93a07eda481410f88610f6dd9f8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8209
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
2023-03-13 20:30:59 +00:00
Vincent Ambo
1e37f8b52e feat(tvix/eval): give generators human-readable names
This adds static strings to generator frames that describe the
generator in a human-readable fashion, which are then logged in
observers.

This makes runtime traces very precise, explaining exactly what is
being requested from where.

Change-Id: I695659a6bd0b7b0bdee75bc8049651f62b150e0c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8206
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2023-03-13 20:30:59 +00:00
Vincent Ambo
dfd0066de5 fix(tvix/eval): handle toJSON on attribute sets with outPath
These are serialised as the serialisation of the value of that field.

Change-Id: Ida51708b1f43ce09b0ec835f4e265918aa31dd09
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8205
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
2023-03-13 20:30:59 +00:00
Vincent Ambo
7d339d2762 fix(tvix/eval): handle __toString when JSON-serialising attrsets
These must be serialised to a JSON string of the *result* of coercing
the function application to a string.

Change-Id: Ib7f49ccd950503ddbdbf99643cd59565e26b50da
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8204
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
2023-03-13 20:30:59 +00:00
Vincent Ambo
a9f44721e5 refactor(tvix/eval): move __toString calling to a helper function
It turns out that this is used not just in coerceToString, but also in
toJSON.

Change-Id: I1c324b115a0b8bb6d83446d5bf70453c9b90685e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8203
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2023-03-13 20:30:59 +00:00
Vincent Ambo
54a12577c4 refactor(tvix/eval): print only *types* when observing generators
Do not print the entire value (they're likely to be thunks anyways).
This is useful because there *can* be cases where something like
`nixpkgs` itself is sent through one of these messages, in which case
the observer trying to print it will just blow up.

Change-Id: I1fa37ea071d75efa0eb3428c6e2fe4351c62be6b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8202
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2023-03-13 20:30:59 +00:00
Vincent Ambo
d9371c2f6f fix(tvix/eval): don't print full stack in observer
Print only the top 6 values of the stack, not the entire stack.
There's very few operations that deal with more values anyways, so the
rest are not likely to be useful.

This gets us one step closer to tracing VERY large executions without
blowing up.

Change-Id: I97472321b0321b25d534d9f53b3aadfacc2318fa
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8201
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2023-03-13 20:30:59 +00:00
Vincent Ambo
19106cdaf0 fix(tvix/eval): don't print full values in observer
This can actually blow up when tracing arbitrary execution, as some of
the data structures just get too large to run through a tabwriter.

Change-Id: I6ec4c30ee48655b8a62954ca219107404fb2c256
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8200
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2023-03-13 20:30:59 +00:00
Vincent Ambo
fb4ea1f5a4 refactor(tvix/eval): condense observer's stack writing logic
Change-Id: I1282c3387ac1e0d1528b894814f2a495ca5a6a32
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8199
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2023-03-13 20:30:59 +00:00
Vincent Ambo
eef48b8f1f fix(tvix/eval): correctly thunk deferred formals access
Formals can be initialised with deferred default values (see the test
cases), in which case they need an extra thunk to have something that
can be finalised appropriately when the setup is done.

Fixes: b/255
Change-Id: I380e3770be68eaa83ace96d450c7cead32dacc9f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8196
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2023-03-13 20:30:59 +00:00
Vincent Ambo
43d04d9b98 refactor(tvix/eval): box PathBuf
This shaves another 8 bytes off Value. How did that type get so big?!

Change-Id: I65e9b59a1636bd57e3cc4aec5fea16887070b832
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8153
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
2023-03-13 20:30:59 +00:00
Vincent Ambo
52b7a76268 chore(tvix/eval): remove From<SmolStr> for NixString instance
No longer needed, and in some cases caused some extra work.

Change-Id: I64e8e7292573bdc92a9c7a8e470e33f8c526f311
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8152
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
2023-03-13 20:30:59 +00:00
Vincent Ambo
1941082cbb refactor(tvix/eval): simplify NixString representation(s)
Instead of the two different representations (which we don't really
use much), use a `Box<str>` (which potentially shaves another 8 bytes
off `Value`).

NixString values themselves are immutable anyways (which was a
guarantee we already had with `SmolStr`), so this doesn't change
anything else.

Change-Id: I1d8454c056c21ecb0aebc473cfb3ae06cd70dbb6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8151
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
2023-03-13 20:30:59 +00:00
Vincent Ambo
4bbfeaf1cb refactor(tvix/eval): wrap NixList in Rc
The size of a `Vector<Value>` is 64 *bytes*, which is quite large, and
it bloated the entire Value type to this size.

This change adds an indirection for the inner vector through Rc.
Initially I tried to use a Box, but this breaks pointer equality
guarantees for the Vector when it is small enough to be inlined.

This reduces the size of Value from 64 to 32 bytes.

Change-Id: Ic3211e861b1966c78b2c3d536ba291fea92647fd
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8150
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
2023-03-13 20:30:59 +00:00
Vincent Ambo
83b2290e1b test(tvix/eval): add test for infinite recursion detection
Change-Id: Ief20544a44c3542fe40a5c09f81d0f064a346f44
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8149
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2023-03-13 20:30:59 +00:00
Vincent Ambo
025c67bf4d refactor(tvix/eval): flatten call stack of VM using generators
Warning: This is probably the biggest refactor in tvix-eval history,
so far.

This replaces all instances of trampolines and recursion during
evaluation of the VM loop with generators. A generator is an
asynchronous function that can be suspended to yield a message (in our
case, vm::generators::GeneratorRequest) and receive a
response (vm::generators::GeneratorResponsee).

The `genawaiter` crate provides an interpreter for generators that can
drive their execution and lets us move control flow between the VM and
suspended generators.

To do this, massive changes have occured basically everywhere in the
code. On a high-level:

1. The VM is now organised around a frame stack. A frame is either a
   call frame (execution of Tvix bytecode) or a generator frame (a
   running or suspended generator).

   The VM has an outer loop that pops a frame off the frame stack, and
   then enters an inner loop either driving the execution of the
   bytecode or the execution of a generator.

   Both types of frames have several branches that can result in the
   frame re-enqueuing itself, and enqueuing some other work (in the
   form of a different frame) on top of itself. The VM will eventually
   resume the frame when everything "above" it has been suspended.

   In this way, the VM's new frame stack takes over much of the work
   that was previously achieved by recursion.

2. All methods previously taking a VM have been refactored into async
   functions that instead emit/receive generator messages for
   communication with the VM.

   Notably, this includes *all* builtins.

This has had some other effects:

- Some test have been removed or commented out, either because they
  tested code that was mostly already dead (nix_eq) or because they
  now require generator scaffolding which we do not have in place for
  tests (yet).

- Because generator functions are technically async (though no async
  IO is involved), we lose the ability to use much of the Rust
  standard library e.g. in builtins. This has led to many algorithms
  being unrolled into iterative versions instead of iterator
  combinations, and things like sorting had to be implemented from scratch.

- Many call sites that previously saw a `Result<..., ErrorKind>`
  bubble up now only see the result value, as the error handling is
  encapsulated within the generator loop.

  This reduces number of places inside of builtin implementations
  where error context can be attached to calls that can fail.
  Currently what we gain in this tradeoff is significantly more
  detailed span information (which we still need to bubble up, this
  commit does not change the error display).

  We'll need to do some analysis later of how useful the errors turn
  out to be and potentially introduce some methods for attaching
  context to a generator frame again.

This change is very difficult to do in stages, as it is very much an
"all or nothing" change that affects huge parts of the codebase. I've
tried to isolate changes that can be isolated into the parent CLs of
this one, but this change is still quite difficult to wrap one's mind
and I'm available to discuss it and explain things to any reviewer.

Fixes: b/238, b/237, b/251 and potentially others.
Change-Id: I39244163ff5bbecd169fe7b274df19262b515699
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8104
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Reviewed-by: Adam Joseph <adam@westernsemico.com>
Tested-by: BuildkiteCI
2023-03-13 20:30:59 +00:00
Vincent Ambo
cbb4137dc0 feat(tvix/eval): implement generator-based Nix equality logic
Adds a `Value::neo_nix_eq` method (the `neo_` prefix will be dropped
when we flip over to the generator implementation of the VM) which
implements Nix equality semantics using async, generator-based
comparisons.

Instead of tracking the "kind" of equality that is being compared (see
the pointer-equality doc) through a pair of booleans, I've introduced
an enum that explicitly lists the possible comparisons.

Change-Id: I3354cc1470eeccb3000a5ae24f2418db1a7a2edc
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8241
Tested-by: BuildkiteCI
Reviewed-by: Adam Joseph <adam@westernsemico.com>
2023-03-13 20:30:59 +00:00
Vincent Ambo
cd447e1859 feat(tvix/eval): add generator-related functions to RuntimeObserver
These functions will be used by the changes in the VM to observe the
runtime execution of generator frames, and provide a more linear view
of the execution of the Tvix VM.

Change-Id: I10b1b1933dedc065e7c61d5d6062f0aaeee0097e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8240
Tested-by: BuildkiteCI
Reviewed-by: Adam Joseph <adam@westernsemico.com>
2023-03-13 20:30:59 +00:00
Vincent Ambo
d00229753d feat(tvix/eval): implement asynchronous list sorting algorithm
In order to implement an asynchronous builtins.sort (required for
moving builtins to generators), we need an `async` sorting algorithm
as our comparators involve invoking a Nix function.

This commit implements a fairly simple, optimised bubble sort as the
sorting algorithm used in our `async fn sort_by`.

There don't seem to be any crates providing async versions of things
like this, and they might actually be pretty hard to implement
generically due to some constraints about how `async` works.

Note that this algorithm is less efficient than the hybrid
"timsort/mergesort/insert sort" used in the Rust standard library. I
tried to write a merge sort implementation, but ran into isuses with
the sort becoming unstable because our comparators can not yield
equality. This is the simplest implementation which I know to be
correct.

Note that as of this commit this is *not* covered by the Tvix test
suite, but it will be as soon as the rest of the generator code lands.

Change-Id: Ia9a604f7dd941d6acc9212c902e0e637ed75bebc
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8239
Reviewed-by: Adam Joseph <adam@westernsemico.com>
Tested-by: BuildkiteCI
2023-03-13 20:30:59 +00:00
Florian Klink
c9c8b10370 feat(tvix/eval): don't warn twice about dead code
We currently send two warnings in case of detecting dead code - W008
inside compile_dead_code, and a more detailed warning in all places that
invoke compile_dead_code:

```
warning[W007]: useless operation on boolean: this expression is always false
     --> /nix/store/qz3gjn95gazab4fkb7s8lm6hz17rdzzy-414z9nnj1wy66ymq6vgb693x9xjz6hf2-nixpkgs-src/pkgs/top-level/perl-packages.nix:12079:15
      |
12079 |     doCheck = false && !stdenv.isDarwin;
      |               ^^^^^^^^^^^^^^^^^^^^^^^^^

warning[W008]: this code will never be executed
     --> /nix/store/qz3gjn95gazab4fkb7s8lm6hz17rdzzy-414z9nnj1wy66ymq6vgb693x9xjz6hf2-nixpkgs-src/pkgs/top-level/perl-packages.nix:12079:24
      |
12079 |     doCheck = false && !stdenv.isDarwin;
      |                        ^^^^^^^^^^^^^^^^
```

The place invoking `compile_dead_code` has more context to why the code
is unused, so it's error message is much more useful.

Stop emitting the less informative warning inside compile_dead_code
(W008), and update the comment that we expect the caller to emit a
warning.

I kept W008 itself still around, in case we end up having places this
will get used again.

Change-Id: I2c5d84fc0cb4035872cd4b71cc3e9e34e120eb37
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8024
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2023-03-11 14:26:49 +00:00
Vincent Ambo
2696839770 feat(tvix/eval): introduce generators module
This module contains the request/response types for generators
requesting actions from the VM.

For most of these, an async helper function is added that will be used
inside of generator functions to make use of these requests/responses
instead of constructing them directly.

Change-Id: I1e085f88adaf784a34867957a0e82532d3a83d7c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8148
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2023-03-07 22:04:59 +00:00
Vincent Ambo
9cebae9b56 refactor(tvix/eval): merge OpCall & OpTailCall
As applies are thunked, there was no situation where OpCall could be
emitted. In practice, all calls were already tail calls.

Change-Id: Id0d441dcdd86f804d7cddd0cc14f589bbfc75e5b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8147
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2023-03-07 22:04:59 +00:00
Vincent Ambo
bfb787a6c5 refactor(tvix/eval): remove VM argument from suspended native thunks
Because they do not use it, and it can not be passed with the coming
generator refactoring.

Change-Id: I0d96f2357a7ee79cd8a0f401583d4286230d4a6b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8146
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2023-03-04 15:53:36 +00:00
Vincent Ambo
38fd3cb292 refactor(tvix/eval): insert storeDir "builtin" in eval startup
Instead of using a suspended native thunk, calculate and optionally
insert the storeDir builtin when the VM is constructed.

We already have the IO handle available at this point and can just
check whether a storeDir is present, and insert its absolute value as
a builtin.

Change-Id: If966eee6ff26dc888b6e888e7c46170c0c346b05
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8145
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
2023-03-04 15:53:36 +00:00
Vincent Ambo
e1f082a3ab feat(tvix/eval): add SharedThunkSet
This is a ThunkSet wrapped to be shareable, which will be required
once ThunkSets are embedded in futures.

Change-Id: I5a067b7972ac86e4d354c75ef05c86b2284c1137
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8144
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2023-03-04 15:18:37 +00:00
Vincent Ambo
dccbda5960 fix(tvix/eval): ThunkSet does not need mutable pointers
Change-Id: Iea248870a0ea5d38cb02ff059c968fbd563570b6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8143
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
2023-03-04 15:18:37 +00:00
Vincent Ambo
8c7d5b4f87 chore(tvix/eval): implement From<OrdMap<..>> for NixAttrs
Again simplifying some code down the line, where bits of code that
construct attribute sets already have the final structure available.

Change-Id: I0bb7a1daa63298122b51be73d35d695a4f73f8b0
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8140
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
2023-03-04 15:18:37 +00:00
Vincent Ambo
a11a3e2c59 refactor(tvix/eval): implement From<Span> for LightSpan
This simplifies some code down the line.

Change-Id: I58dd71e796e11479f44516cf24932f8061843d23
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8139
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2023-03-04 15:18:37 +00:00
Vincent Ambo
344c119370 chore(tvix/eval): fix clippy warnings
Change-Id: I4c02f0104c455ac00a3f299c1fbf75cbb08e8972
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8142
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Autosubmit: tazjin <tazjin@tvl.su>
2023-03-03 11:01:52 +00:00
Vincent Ambo
7e9a65dcdb refactor(tvix/eval): remove useless map call
Change-Id: Ifb59ef148ea4fab613f2e4efb133c04baafa3a98
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8141
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
2023-03-03 10:57:50 +00:00
Vincent Ambo
fb4c197b39 refactor(tvix/eval): enhance debug output for bytecode dumps
This adds addresses of thunk and closure chunks to the debug output
displayed when dumping bytecode.

This makes it possible to see in the dump which thunks are referenced
by constants in other thunks.

Change-Id: I2c98de5227e7cb415666cd3134c947a56979dc80
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8137
Autosubmit: tazjin <tazjin@tvl.su>
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
2023-03-03 10:52:48 +00:00
Aaqa Ishtyaq
faffb2a4cb refactor(tvix/eval): remove redundant clone
This CL removes redundant clone from value which is
going to be dropped without further use.

Change-Id: Ibd2a724853c5cfbf8ca40bf0b3adf0fab89b9be5
Signed-off-by: Aaqa Ishtyaq <aaqaishtyaq@gmail.com>
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8125
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
2023-02-16 22:42:14 +00:00
Vincent Ambo
bfe6cace5e docs(tvix/eval): add proposal for VM loop restructuring
Change-Id: Ib991d68724a73886a8343d7f785b5b3aedd637ed
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8103
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
2023-02-16 22:23:51 +00:00
Vincent Ambo
3c311ffab2 fix(tvix/eval): correctly print lambda address in observer
We want the address that the Rc is pointing to, not the address of the
Rc.

Change-Id: I8eba21677f242bbe4166c74d4aa4269c316076e3
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8045
Reviewed-by: flokli <flokli@flokli.de>
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
2023-02-14 08:02:40 +00:00
Aaqa Ishtyaq
fc61dff037 chore(tvix/eval): use writeln for newline string
This CL address clippy warning which expects to
use `writeln` instead of `write` for strings with
new line.

Change-Id: Ia72a07502c60cfd489ecf1e3833b9d42d44a8b17
Signed-off-by: Aaqa Ishtyaq <aaqaishtyaq@gmail.com>
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8030
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
2023-02-13 19:45:58 +00:00
Aaqa Ishtyaq
105069302b chore(tvix/eval): clippy warn is length zero
This CL address clippy warning about finding the zero
length of something using `is_empty()` instead of `len() == 0`.

Change-Id: I2b36c7c7b65b733609fc0dcd33be06f9d772bc9b
Signed-off-by: Aaqa Ishtyaq <aaqaishtyaq@gmail.com>
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8029
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
2023-02-13 19:45:58 +00:00
Vincent Ambo
c98c5399b9 fix(tvix/eval): skip runtime completely on compiler errors
This branch was missing, and an assumption elsewhere just executed the
returned (broken) bytecode.

This fixes b/253.

Change-Id: I015023ba921bc08ea03882167f1f560feca25e50
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8090
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
Autosubmit: tazjin <tazjin@tvl.su>
2023-02-13 16:21:47 +00:00
Vincent Ambo
91a366af46 fix(tvix/eval): make fields of eval's Error type public
These should be inspectable by callers.

Change-Id: Ia9ef871aa63958d06066aaea61b2aecbd217369b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8089
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
2023-02-13 16:21:46 +00:00
Vincent Ambo
720e80c1f9 fix(tvix/eval): fix the default case for path parsing
Plain paths like `foo/bar.nix` are also allowed, so we can not
determine this based on the prefix.

The upstream PR that is referenced in a comment here has a
significantly different interface than we expected, so I'm not
touching that comment yet in this CL before I've had more time to
digest it.

Change-Id: Iea33bbb35de9c00a7d7fedf64d02253c75c1cc9e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8032
Autosubmit: tazjin <tazjin@tvl.su>
Reviewed-by: Alyssa Ross <hi@alyssa.is>
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
2023-02-04 12:44:59 +00:00
Florian Klink
d50044d570 chore(tvix/eval): only use Rc with impure feature
We only use Rc in `impl EvalIO for StdIO`, which is only included when
building with the "impure" feature.

Change-Id: Id29d647c899cbfcdda11abfb9fabd5aa7e24299f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8025
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
2023-02-03 18:58:17 +00:00
Vincent Ambo
f16b0f24e2 refactor(tvix/eval): wrap Builtin type in a Box
This reduces the size of `Builtin` from 88 (!) bytes to 8, and as the
largest variant of `Value`, the size of that type from 96 to 64.

The next largest type is NixList, clocking in at 64 bytes.

This has noticeable performance impact. In an implementation without
disk I/O, evaluating nixpkgs.stdenv looks like this:

Benchmark 1: tvix -E '(import <nixpkgs> {}).stdenv.drvPath'
  Time (mean ± σ):      1.151 s ±  0.003 s    [User: 1.041 s, System: 0.109 s]
  Range (min … max):    1.147 s …  1.155 s    10 runs

After this change, it looks like this:

Benchmark 1: tvix -E '(import <nixpkgs> {}).stdenv.drvPath'
  Time (mean ± σ):      1.046 s ±  0.004 s    [User: 0.954 s, System: 0.092 s]
  Range (min … max):    1.041 s …  1.053 s    10 runs

Change-Id: I5ab7cc02a9a450c0227daf1f1f72966358311ebb
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8027
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
2023-02-03 18:47:33 +00:00
Vincent Ambo
32698766ef refactor(tvix/eval): statically resolve select from constant attrs
When resolving a select expression (`attrs.name` or `attrs.name or
default`), if the set compiles to a constant attribute set (as is most
notably the case with `builtins`) we can backtrack and replace that
attribute set directly with the compiled value.

For something like `builtins.length`, this will directly emit an
`OpConstant` that leaves the `length` builtin on the stack.

Change-Id: I639654e065a06e8cfcbcacb528c6da7ec9e513ee
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7957
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
2023-02-03 18:47:33 +00:00
Vincent Ambo
3419f63575 fix(tvix/eval): ensure all evaluated thunks are correctly memoized
This fixes a very complicated bug (b/246). Evaluation
progresses *much* further after this, leading to several less
complicated bugs likely being uncovered by this

What was the problem?
=====================

Previously, when evaluating a thunk, we had a code path that looked
like this:

    match *thunk {
        ThunkRepr::Evaluated(Value::Thunk(ref inner_thunk)) => {
            let inner_repr = inner_thunk.0.borrow().clone();
            drop(thunk);
            self.0.replace(inner_repr);
        }
        /* ... */
    }

This code path created a copy of the inner `ThunkRepr` of a nested
thunk, and moved that copy into the `ThunkRepr` of the parent.

The effect of this was that the original `ThunkRepr` (unforced!) lived
on in the original thunk, without the memoization of the subsequent
forcing applying to it.

This had the result that Tvix would repeatedly evaluate these thunks
without ever memoizing them, if they occured repeatedly as shared
inner thunks. Most notably, this would *always* occur when
builtins.import was used.

What's the solution?
====================

I have completely rewritten `Thunk::force_trampoline_self` to make all
flows that can occur in it explicit. I have also removed the outer
loop inside of that function, and resorted to more use of trampolining
instead.

The function is now well-commented and it should be possible to read
it from top-to-bottom and get a general sense of what is going on,
though the trampolining itself (which is implemented in the VM) needs
to be at least partially understood for this.

What's the new problem(s)?
==========================

One new (known) problem is that we have to construct `Error` instances
in all error types here, but we do not have spans available in some
thunk-related situations. Due to b/238 we cannot ask the VM for an
arbitrary span from the callsite leading to the force. This means that
there are now code paths where, under certain conditions, causing an
evaluation error during thunk forcing will panic.

To fix this we will need to investigate and fix b/238, and/or add a
span tracking mechanism to thunks themselves.

What other impacts does this have?
==================================

With this commit, eval of nixpkgs mostly succeeds (things like stdenv
evaluate to the same hashes for us and C++ Nix, meaning we now
construct identical derivations without eval breaking).

Due to this we progress much further into nixpkgs, which lets us
uncover more additional bugs. For example, after this commit we can
quickly see that cl/7949 introduces some kind of behavioural issue and
should not be merged as-is (this was not apparent before).

Additionally, tvix-eval is now seemingly very fast. When doing
performance analysis of a nixpkgs eval, we now mostly see the code
path for shelling out to C++ Nix to add things to the store in there.
We still need those code paths, so we can not (yet) do a performance
analysis beyond that.

Change-Id: I738525bad8bc5ede5d8c737f023b14b8f4160612
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8012
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
2023-02-03 10:47:18 +00:00
Vincent Ambo
503e6810e7 fix(tvix/eval): unsafeDiscardStringContext is a no-op
... not just a TODO.

Most use-cases of unsafeDiscardStringContext are for cases where a
string is processed in some ways and no longer contains a "physical"
reference, but still has its context attached in C++ Nix.

We don't need to do this. This does diverge in behaviour in use-cases
related to build scheduling, but that whole behaviour will be
different in Tvix.

Change-Id: I4056d4c09f62d44d6bd52b791db03fe5556672b5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8016
Reviewed-by: flokli <flokli@flokli.de>
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
2023-02-02 14:30:32 +00:00
Vincent Ambo
75d1b72b7c refactor(tvix/eval): import_cache can be a HashMap
... instead of a BTreeMap, as we do not need ordering guarantees here.
HashMaps are noticeably faster here (especially as we've been sorting
essentially random data!).

Change-Id: Ie92d74286df9f763c04c9b226ef1066ee8484c13
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8014
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Autosubmit: tazjin <tazjin@tvl.su>
2023-02-02 14:25:30 +00:00
Vincent Ambo
98ea614c6e chore(tvix/eval): elaborate on internal types in Value::type_of
This aids in debugging quite substantially.

Change-Id: Ic43232aa6165ae1c3db7ac2701938e1dfeeb418c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8013
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
Autosubmit: tazjin <tazjin@tvl.su>
2023-02-02 09:29:43 +00:00
Vincent Ambo
ddf6e6cf47 fix(tvix/eval): allow builtins.toXML to serialise any function
This adds a fake argument name to builtins.toXML which allows toXML to
serialise any value instead of panicking on functions. We do still
have to fix the value itself, eventually, though.

Change-Id: I2e330ecddcd80442b4fac5eced64431ac86123ba
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7962
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
2023-01-31 15:31:30 +00:00
Vincent Ambo
7702941dd9 test(tvix/eval): add tests for internal formals dependencies
Formals can depend on each other when using another formal as a
default value.

This test ensures that the compiler's declaration and initialisation
order of formals is consistent with what actually happens in the VM.

Change-Id: Ibdabe262554e8066d67fac1ebc3b5a48ef626e18
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7948
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
2023-01-31 10:25:02 +00:00
Vincent Ambo
1028aff105 chore(tvix/eval): remove dead comment
Change-Id: Icedb7f272e5067569b8dbf1c2d8b0fdd352b8e12
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7936
Tested-by: BuildkiteCI
Autosubmit: tazjin <tazjin@tvl.su>
Reviewed-by: flokli <flokli@flokli.de>
2023-01-26 23:30:43 +00:00
sterni
8a8325fb9d docs(tvix/eval): builtins.add is not equivalent to +
While it is in the given example, i.e. for integer addition, to claim
that they are equivalent is a bit misleading: builtins.add is less
overloaded than +, i.e. builtins.add "foo" "bar" will fail whereas
"foo" + "bar" performs string concatenation.

Change-Id: Ib52d530d1ab289b367565b286f06a76dd518d4fb
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7929
Autosubmit: sterni <sternenseemann@systemli.org>
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
2023-01-25 14:30:50 +00:00
Florian Klink
669496f0ba test(tvix/eval): add test for total_fmt_float
Change-Id: If6c478ee3d2e4ecf5ef92289614f86535ad05cb7
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7927
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: flokli <flokli@flokli.de>
2023-01-25 11:24:20 +00:00
Vincent Ambo
164005656d refactor(tvix/eval): extract float formatting into a helper
This keeps the actual TotalDisplay implementation readable, as this
float formatting code suddenly made up the majority of its implementation.

Change-Id: I2c0d00e4a691e0b8ffbc72680f680e16feef4bee
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7925
Reviewed-by: flokli <flokli@flokli.de>
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
2023-01-25 11:24:19 +00:00
Florian Klink
e0b05c0fa6 feat(tvix/eval): implement builtins.fromTOML
This allows parsing TOML from Tvix. We can enable the eval-okay-fromTOML
testcase from nix_tests. It uses the `toml` crate, and the serde
integration it brings with it.

Change-Id: Ic6f95aacf2aeb890116629b409752deac49dd655
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7920
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
2023-01-25 07:49:44 +00:00
Florian Klink
1facd889bb feat(tvix/eval): use lexical-core to format float
Apparently our naive implementation of float formatting, which simply
used {:.5}, and trimmed trailing "0" strings not sufficient.

It wrongly trimmed numbers with zeroes but no decimal point, like
`10000` got trimmed to `1`.

Nix uses `std::to_string` on the double, which according to
https://en.cppreference.com/w/cpp/string/basic_string/to_string
is equivalent to `std::sprintf(buf, "%f", value)`.

https://en.cppreference.com/w/cpp/io/c/fprintf mentions this is treated
like this:

> Precision specifies the exact number of digits to appear after
> the decimal point character. The default precision is 6. In the
> alternative implementation decimal point character is written even if
> no digits follow it. For infinity and not-a-number conversion style
> see notes.

This doesn't seem to be the case though, and Nix uses scientific
notation in some cases.

There's a whole bunch of strategies to determine which is a more compact
notation, and which notation should be used for a given number.
https://github.com/rust-lang/rust/issues/24556 provides some pointers
into various rabbit holes for those interested.

This gist seems to be that currently a different formatting is not
exposed in rust directly, at least not for public consumption.

There is the
[lexical-core](https://github.com/Alexhuszagh/rust-lexical) crate
though, which provides a way to format floats with various strategies
and formats.

Change our implementation of `TotalDisplay` for the `Value::Float` case
to use that. We still need to do some post-processing, because Nix
always adds the sign in scientific notation (and there's no way to
configure lexical-core to do that), and lexical-core in some cases keeps
the trailing zeros.

Even with all that in place, there as a difference in `eval-okay-
fromjson.nix` (from tvix-tests), which I couldn't get to work. I updated
the fixture to a less problematic number.

With this, the testsuite passes again, and does for the upcoming CL
introducing builtins.fromTOML, and enabling the nix testsuite bits for
it, too.

Change-Id: Ie6fba5619e1d9fd7ce669a51594658b029057acc
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7922
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
2023-01-25 07:49:44 +00:00
Vincent Ambo
9ecf668452 chore(tvix/eval): delete "useless parenthesis" warning/optimisation
Two main reasons:

1. Traversing the structure to do this optimisation is
   actually *slower* than not optimising it.

2. There are literally hundreds of thousands of incidences of this in
   nixpkgs, and with some of the weird code there some of
   these (functionally) useless parens are actually required for
   readability reasons.

Change-Id: I1044b1c5f9fe20df4b6085851fc3b191277c65dc
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7917
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
2023-01-23 17:59:06 +00:00
Vincent Ambo
d2f1a290a4 fix(tvix/eval): force functors before applying them
call_value in the VM expects the callable to be forced when calling
it, which was not the case for functors.

Change-Id: Id55a2fe32a9573be42aef8669e268df519a989cd
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7909
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
2023-01-23 17:27:53 +00:00
Vincent Ambo
5719763fd3 feat(tvix/eval): support builtins implemented in Nix itself
This makes it possible to inject builtins into the builtin set that
are written in Nix code, and which at runtime are represented by a
thunk that will compile them the first time they are used.

Change-Id: Ia632367328f66fb2f26cb64ae464f8f3dc9c6d30
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7891
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
2023-01-22 20:48:53 +00:00
Vincent Ambo
8513a58b37 docs(tvix/eval): update some outdated comments
These don't apply anymore since the "antidote-CL".

Change-Id: I40ee73ef43d44bbfc650a8fe6c2b33263dd06959
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7890
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
2023-01-22 19:39:00 +00:00
Florian Klink
758b94f47c docs(tvix): move most of //tvix/eval README up to //tvix
While moving the CLI out of the evaluator, we forgot to update the
README in //tvix/eval. Move this up to //tvix, so people know where
to start.

Keep the instructions on how to build only `//tvix/eval` in `//tvix/
eval/README.md`.

Change-Id: Ie2755e8b5a0056225dbf3a0ee040f70f7f6a1f27
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7887
Reviewed-by: sterni <sternenseemann@systemli.org>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
2023-01-21 15:08:38 +00:00
Adam Joseph
22b9e6ff09 refactor(tvix/eval): administer antidote for poison
The codebase contains a lot of complexity and odd roundabout
handling for shadowing globals.  I'm pretty sure none of this is
necessary, and all of it disappears if you simply make the globals
part of the ordinary identifier resolution chain, with their own
scope up above the root scope.  Then the ordinary shadowing routines
do the right thing, and no special cases or new terminology are
required.

This commit does that.

Note by tazjin: This commit was originally abandoned when Adam decided
not to take away reviewer bandwidth for this at the time (eval was
still in a much earlier stage). As we've recently done some
significant refactoring of globals initialisation this came up again,
and it seems we can easily cover the use-cases of the poison tracking
in other ways now, so I've rebased, updated and resurrected the CL.

Co-Authored-By: Vincent Ambo <tazjin@tvl.su>
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Change-Id: Ib3309a47a7b31fa5bf10466bade0d876b76ae462
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7089
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
2023-01-21 10:19:15 +00:00
Vincent Ambo
7442558b33 refactor(tvix/eval): keep globals alive through VM struct
This forces users to pass the fully constructed set of globals to the
VM, making it harder to accidentally "lose" the set while weak
references to it still exist.

This doesn't modify any functionality, but is laying the foundation
for simplifying some of the builtins behaviour that has grown more
complex again.

Change-Id: I5120f97861c65dc46d90b8a4e2c92ad32cc53e03
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7877
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
2023-01-20 22:48:13 +00:00
Vincent Ambo
6d03e31060 docs(tvix/eval): add doc comments to VM fields
Change-Id: Ia4857c217de15aec8b61e1abd39e22c50e2d816a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7876
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Autosubmit: tazjin <tazjin@tvl.su>
2023-01-20 22:48:13 +00:00
Vincent Ambo
7d0456fa0e feat(tvix/eval): let builtin macro capture external state
This adds a feature to the `#[builtins]` macro which lets users
specify an additional state type to (optionally) thread through to
builtins when constructing them.

This makes it possible for builtins-macro users to pass external state
handles (specifically, in our case, known path tracking) into a set of
builtins.

Change-Id: I3ade20d333fc3ba90a80822cdfa5f87a9cfada75
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7840
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
2023-01-20 15:39:51 +00:00
Vincent Ambo
f12f938166 refactor(tvix/eval): directly return builtin tuples from macro
All invocations of the builtin macro had to previously filter through
the `builtin_tuple` function, but it's more sensible to directly
return these from the macro.

Change-Id: I45600ba84d56c9528d3e92570461c319eea595ce
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7825
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
2023-01-20 15:39:51 +00:00
Vincent Ambo
972c867b36 feat(tvix/eval): add error contexts to annotate error kinds
This makes it possible for users to add additional context to an
error, which will then be rendered as an additional secondary span in
the formatted error output.

We should strive to do this basically anywhere errors are raised that
can occur multiple times, *especially* during type casts. This was
triggered by me debugging a type cast error attached to a fairly
large-ish span (a builtin invocation).

Change-Id: I51be41fabee00cf04de973935daf34fe6424e76f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7849
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
2023-01-20 15:39:51 +00:00
Vincent Ambo
bf6f6a0b3f refactor(tvix/eval): non-hacky suspended native thunks
Instead of having a representation of suspended native thunks that
involves constructing a fake code chunk, make these thunks a
first-class part of the internal thunk representation.

The previous code was not that simple to understand, and actually
contained a critical bug which could lead to Tvix crashes. This
version fixes the particular instance of that bug, but instead
uncovers another (b/238) which can still lead to Tvix crashes.

Fixes: b/237.
Change-Id: I771d03864084d63953bdbb518fec94487481f839
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7750
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
2023-01-17 10:38:06 +00:00
Vincent Ambo
559a09c5e6 refactor(tvix/eval): remove Box in new_suspended_native
This is unnecessary, Rc already provides all the boxing we need.

Change-Id: I08cf0939c48da43f04c847526c7e5dae5336d528
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7749
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
Reviewed-by: sterni <sternenseemann@systemli.org>
2023-01-17 10:38:06 +00:00
Vincent Ambo
b9646ab40c feat(tvix/eval): add builtins to builtins
This is a somewhat terrifying hack that enables us to support
`builtins.builtins`, by running a "fake compilation" inside of a
suspended native thunk that can resolve the weak pointer to the
globals.

With this implementation, the thunk at `builtins.builtins` actually
resolves to the "real" `builtins` (verified with a new test).

This is kind of ugly, and it's something users shouldn't use, but
bubbling a warning out of this is difficult at the moment due to a
little bit of trickery with how the spans in suspended native thunks
work (they don't) (see b/237, b/238)

Change-Id: I67d0e93246dd5b279c960aeda00402031aa12af3
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7748
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
2023-01-17 10:38:06 +00:00
Vincent Ambo
940251b87f refactor(tvix/value): use proptest strategies from imbl crate
Instead of going through Vec/BTreeMap for generating our internal
types, use the proptest strategies from imbl.

The one thing I couldn't figure out in the previous implementation is
where the ranges/sizes of generated collections came from. The
strategies in proptest use different types (Range, with an unknown
default value, and SizeRange with 0..100). I've opted to specify
0..100 directly, but we can probably make it configurable.

Change-Id: I749bc4c703fe424099240cab822b1642e5216361
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7791
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
2023-01-17 10:20:38 +00:00
Vincent Ambo
67addf3b3d feat(tvix/eval): add error variant for threading through errors
This variant is required for external builtins (which in our case
includes `derivation`) to thread through reasonable error messages.

This has some potential for improvement, but it's an improvement over
the status quo of panicking in the external builtins when no
appropriate error is available.

Change-Id: I7e4bdb0a156c7717092dde30aa4785192182dc66
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7841
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
2023-01-16 13:43:14 +00:00
Vincent Ambo
d365b09226 feat(tvix/eval): implement builtins.toXML
Change-Id: I009efc53a8e98f0650ae660c4decd8216e8a06e7
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7835
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
2023-01-16 13:43:14 +00:00
Vincent Ambo
1786b4c835 chore(tvix/eval): add other required items to public API
External implementors of builtins must be able to force values, which
necessitates publishing a bunch more items from the crate.

Change-Id: I8f6b8ae88156aae417dbe630a698d123d0c1c8d4
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7830
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
2023-01-16 13:43:14 +00:00
Aaqa Ishtyaq
158f4d1d69 fix(tvix/eval): len_without_is_empty clippy warn
This CL addresses clippy warning len_without_is_empty
which expects `.is_empty()` method to be present when
implementing `.len()` method for an item.

Change-Id: I8878db630b9ef5853649a906b764a33299bb5dc8
Signed-off-by: Aaqa Ishtyaq <aaqaishtyaq@gmail.com>
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7806
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
2023-01-12 18:06:49 +00:00