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>
Updated the rnix hash manually, and ran `cargo update` for the rest.
Change-Id: I457262625d648e25d745efa4d33ae44cb8f21326
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6375
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This will be used to track source spans when emitting bytecode.
The codemap is a data structure which tracks *all* the source files
visited by an evaluation, and makes it possible to represent locations
across all of the files using a simple span (i.e. pair of offsets).
When reporting errors, this even contains enough information to
reconstruct the rnix AST to create fancier reporting in certain cases
if desired.
Change-Id: I4ae98620b9b150fb5a389bd7f1e12670e3192c62
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6374
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>
This lets us create a release build with debug info, for use with e.g.
perf + hotspot
Change-Id: I03897de36c872d318abf1332ca0c1aeabe344ec6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6362
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>
`builtins.getFlake` doesn't interest us, of course, but some others may
be worth (or easy) to implement. They are pretty low priority, though,
since nixpkgs has compatiblity wrappers for the ones it uses.
The new debugging-related builtins (break and traceVerbose) are
interesting to note, but may not make sense to implement at all.
Change-Id: Icae547aa3bd9d6ee6b87897ba8210eb9b9b044c7
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6332
Tested-by: BuildkiteCI
Autosubmit: sterni <sternenseemann@systemli.org>
Reviewed-by: tazjin <tazjin@tvl.su>
This avoids unnecessary rebuilds of //tvix/eval when working on it
locally using impure cargo.
Change-Id: I028033a6345a9655e2877a534448706b8f85a1a1
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6317
Tested-by: BuildkiteCI
Autosubmit: sterni <sternenseemann@systemli.org>
Reviewed-by: tazjin <tazjin@tvl.su>
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>
