This very closely follows the way it's done for warnings, but errors
have a lot more information available in some cases which we do not
surface yet.
Note also that due to requiring the `CodeMap`, this is not yet called
from eval.rs as the way that is threaded through needs to be
refactored, so only the method for reporting these errors as strings
is implemented so far.
Next steps for this will be to add a generic diagnostics module that
reduces some of the boilerplate for this between warnings & errors,
and which will also give us a good point in the future to switch to a
fancier diagnostics crate.
Change-Id: If6bb209f8e7a568d866e516a90335b9b2afbf66d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6534
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
This implements an initial fancy display for warnings emitted by the
tvix compiler, using the codemap_diagnostic crate.
Each warning variant has an associated message, and optionally an
associated annotation for the span displayed to the user.
In theory we could get a lot more fancy with the display for specific
variants if needed (e.g. re-parse the AST and actually add multiple
semantic spans based on context), but this is already a good start.
Example:
tvix-repl> let toString = https://tvl.fyi; in let inherit toString; in ({}: 42) rec {}
warning[W004]: declared variable 'toString' shadows a built-in global!
--> [tvix-repl]:1:5
|
1 | let toString = https://tvl.fyi; in let inherit toString; in ({}: 42) rec {}
| ^^^^^^^^ variable declared here
warning[W001]: URL literal syntax is deprecated, use a quoted string instead
--> [tvix-repl]:1:16
|
1 | let toString = https://tvl.fyi; in let inherit toString; in ({}: 42) rec {}
| ^^^^^^^^^^^^^^^
warning[W002]: inherited variable already exists with the same value
--> [tvix-repl]:1:40
|
1 | let toString = https://tvl.fyi; in let inherit toString; in ({}: 42) rec {}
| ^^^^^^^^^^^^^^^^^
warning[W999]: feature not yet implemented in tvix: recursive attribute sets
--> [tvix-repl]:1:70
|
1 | let toString = https://tvl.fyi; in let inherit toString; in ({}: 42) rec {}
| ^^^^^^
warning[W999]: feature not yet implemented in tvix: closed formals
--> [tvix-repl]:1:62
|
1 | let toString = https://tvl.fyi; in let inherit toString; in ({}: 42) rec {}
| ^^
warning[W003]: variable 'toString' is declared, but never used:
--> [tvix-repl]:1:5
|
1 | let toString = https://tvl.fyi; in let inherit toString; in ({}: 42) rec {}
| ^^^^^^^^ variable declared here
=> 42 :: int
These are coloured when output to a terminal.
Change-Id: If315648a07e333895db4ae1d0915ee2013806585
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6532
Autosubmit: tazjin <tazjin@tvl.su>
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
This is a crate for source-span based error reporting. Since all of
our spans are already codemap spans, it is a good starting point.
We have to figure out quite a bit of logic for neat error printing;
later on if we want fancier presentation we might want to look at one
of the other libraries in this space like miette.
Change-Id: I4e28886af1ed199b7112d9dbf063c9f29b612bf1
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6531
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: grfn <grfn@gws.fyi>
It is impossible for tvixbolt to recover from panics, so the user
experience of typing an expression using an unsupported feature was
that it would get sad and stop responding to input.
Instead, raise a normal value-level error of a new variant and
continue where possible.
Change-Id: Ibe016c92cacb87b85095c0f83758eddc6468053e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6528
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
only run test_nix_store_add() when the feature integration_tests is
enabled.
Change-Id: I600f08ecaefe1ce77651ae07a58d7987107ab969
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6084
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
This is used to drop an already emitted operation from a chunk again
and clean up its span tracking. This is required in cases where the
compiler has to backtrack.
Change-Id: I8112da9427688bb2dec96a2ddd12390f6e9734c3
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6499
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Refactors the methods used for determining whether an identifier in a
binding (i.e. an `rnix::Attr` node) is a static string, and extracting
it.
Previously all uses of this logic were for `let`-expressions, where
dynamic attributes are always an error. However, we need the same
logic to properly implement the phase separation of attribute set
compilation.
To facilitate this, the actual core logic of these methods now return
`Option`, and are only converted to errors in cases where the errors
are actually required.
Change-Id: Iad7826eff2cb428182521c6f92276310edeae1eb
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6498
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
There's about to be a lot more code for attrsets (hopefully
temporarily as part of an expand&contract cycle), while nested
attribute logic is being refactored in preparation for recursive
attribute sets.
This does not change any functionality.
Change-Id: I667565cd810ca7d9046120d1721c2ceb9095550b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6497
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
We need to make sure that we compile all plain inherits in a let
expression before declaring any other locals. Plain inherits are special
in the sense that they can never be recursive, instead resolving to a
higher scope. Thus we need to compile their value, before declaring
them. If we don't do that, before any other local can be declared,
we cause a situation where the plain inherits' values are placed into
other locals' stack slots.
Note that we can't integrate the plain inherit compilation into the
regular 2-3 phase model where we defer the compilation of the value or
we'd compile `let inherit x; in …` as `let x = x; in …`.
Change-Id: I951d5df3c9661a054e12401546875f4685b5bf08
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6496
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Reviewed-by: sterni <sternenseemann@systemli.org>
This test shows that let bindings' dependencies can form a cyclical
graph, so we need to use thunking to break this cycle.
Change-Id: I2a4de71fd7024f3d3d1166154784139a82f39411
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6495
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Reviewed-by: sterni <sternenseemann@systemli.org>
As the new test case demonstrates, asserts need to be evaluated lazily.
Change-Id: I808046722c5a504e9497855ca5026d255c7a4c34
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6494
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Reviewed-by: sterni <sternenseemann@systemli.org>
The recent change that split declaration of let based locals and the
compilation of their values did not touch locals bound by inherit in
let. These were previously declared and compiled immediately before
starting to work on the other locals introduced in a let.
In the case of plain inherits, this behavior is kept in this change,
because there's nothing wrong with it: The value of a plain inherit will
always resolve to a higher scope, either statically or dynamically.
Since inherit (from) expression might refer to other locals bound in the
same let, we need to handle them in the same three steps as ordinary let
based locals:
1. We need to declare the (uninitialised) locals.
2. We need to compile the expression that obtains their value. For this,
we create a new thunk, since the from expression may very well return
a thunk which we need to force before selecting the value we are
interested in.
3. Thunks need to be finalised.
For 1., we create an extra pass over the inherits that already declares
and initialises plain inherits and notes inherit (from) expressions in
the entries vector after declaring them. 2. only needs a bit of adapting
to create the thunks for selecting if appropriate, the rest of the
existing code can be reused.
Change-Id: Ie4ac1c0f9ffcbf7c07c452036aa8e577443af773
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6490
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: tazjin <tazjin@tvl.su>
With this all other "weird scope" logic starts working for `with` as
well.
Change-Id: I0ea1d8c5fbd9cec5084bd574224f77b71ff2b487
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6487
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This completely rewrites the handling of "dynamic upvalues" to,
instead of resolving them at thunk/closure instantiation time (which
forces some values too early), capture the entire with stack of parent
contexts if it exists.
There are a couple of things in here that could be written more
efficiently, but I'm first working through this to get to a bug
related to with + recursion and the code complexity of some of the
optimisations is distracting.
Change-Id: Ia538e06c9146e3bf8decb9adf02dd726d2c651cf
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6486
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This struct will be responsible for tracking upvalues (and is a
convenient place to introduce optimisations for reducing value clones)
instead of a plain value vector.
The main motivation for this is that the upvalues will have to capture
the `with`-stack fully and I want to avoid duplicating the logic for
this between the two capturing types.
Change-Id: I6654f8739fc2e04ca046e6667d4a015f51724e99
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6485
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Instead of the reference to the Rc, print the address of the Rc
itself.
Change-Id: I4560598924db7d2864d5c4ae9af847aee2ea7eff
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6471
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Similarly to attribute sets, list elements can be arbitrary
expressions and their (temporary) stack slots during construction must
be accounted for by the compiler.
Change-Id: I3b6f7927860627fd867c64d0cab9104fd636d4f5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6470
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
The temporaries left on the stack as operands to `OpAttrs` must be
accounted for in the locals array in order for operations within them
to receive correct slots.
Some test cases that were previously broken have been added.
Change-Id: Ib52b629bbdf7931f63fd45a45af1073022da923c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6468
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
There are more upcomming uses of declare_phantom where this will come
in handy to avoid some code bloat.
Change-Id: I75cad8caf14511c519ab2f56e87e99bcbf0a082e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6467
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Moves the logic for removing tracked locals from a given scope from
the compiler's locals list, and leaves only the actual
compiler-related stuff (emitting warnings, cleaning up locals at
runtime) in the compiler itself.
Change-Id: I9da6eb54967f0a7775f624d602fe11be4c7ed5c4
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6466
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This will be re-used between the code paths for
recursive/non-recursive sets, and it might even be possible to unify
it with the logic for compiling `let inherit ...`.
Change-Id: I960a061048ac583a6e932e11ff6e642d9fc3093e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6464
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
The comment explains how this works fairly well.
Note that this does not yet have the ability to check "closed
formals", i.e. without an ellipsis Tvix will *NOT* fail if unexpected
attribute set keys are provided.
Change-Id: I0d2b77e893243093d2789baa57f876d35d0a32ff
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6463
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This doesn't actually do anything other than causing errors for the
current state of this folder.
Change-Id: I0af6410e9eb1700cd0b2b105c8adde077d931a69
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6492
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
... even if the code is broken.
Change-Id: I5898bceaebf201b97e8988c94c90e7fafff82529
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6462
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
As pointed out by sterni in cl/6205, this is actually possible in
syntactically valid expressions like
{ ${12 + 13} = 12; }
Change-Id: Id8a1e3aceb551f288f9050c4eea563eb6572f1a7
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6461
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
As pointed out by grfn on cl/6091
Change-Id: I28308577b7cf99dffb4a4fd3cc8783eb9ab4d0d6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6460
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
When the last instruction in a chunk is OpCall, make it an OpTailCall instead.
Change-Id: I2c80a06ee85e4abf545887b1a79b6d8b5e6123e9
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6458
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
If the last operation within a chunk is a function call, the call can
be executed in the same call frame without increasing the depth of the
call stack.
To enable this, a new OpTailCall instruction (similar to OpCall) is
introduced, but not yet emitted by the compiler.
Change-Id: I9ffbd7da6d2d6a8ec7a724646435dc6ee89712f2
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6457
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This introduces a macro to do the forcing, but this solution isn't
very nice and also does not work in all cases yet.
Change-Id: Icd18862ec47edb82c0efc3af5835a6cb6126f629
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6456
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This produces similar output to the previous tracing feature, but can
redirect the output somewhere else.
Change-Id: I9493c260f480904f3932cb74809b622c24d7be96
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6453
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
These methods make it possible to trace the runtime execution of the
VM through an observer.
Change-Id: I90e26853ba2fe44748613e7f761ed5c1c5fc9ff7
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6452
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
These are required for tvixbolt to work. This interface is definitely
not stable yet, though.
Change-Id: I4076498e8f42311de74ee4f33c93a3ee0c5f8d3a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6450
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This type implements an observer that is called whenever the compiler
emits a chunk (after the toplevel, thunks, or lambdas) and prints the
output of the disassembler to its internal writer.
This replaces half of the uses of the `disassembler` feature, which
has been removed from the Cargo configuration.
Note that at this commit runtime tracing is not yet implemented as an
observer.
Change-Id: I7894ca1ba445761aba4ad51d98e4a7b6445f1aea
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6449
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This trait will enable library users of tvix-eval to observe internal
happenings of the compilation and runtime processes.
The initial methods of the observer will be called whenever the
compiler emits a chunk.
Change-Id: I668f6c2cfe3d6f4c1a1612c0f293831011768437
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6448
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This is a step towards hiding the internal fields of thunk, and making
the interface of the type more predictable.
Part of the preparation for implementing observers.
Change-Id: I1a88a96419c72eb9e2332b56a2dd94afa47e6f88
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6447
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
With this change, it becomes possible to compile tvix-eval to
webassembly if the `repl` feature is disabled.
Change-Id: Icc0a059964cd0bea2054110c682d50fc5c87ec01
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6446
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
With this, most cases of `fix` in attribute sets will work correctly.
A simple test exercising both has been added.
Change-Id: I70fd431177bb6e48ecb33a87518b050c4c3d1c09
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6437
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
At the point where control flow exits Thunk::force (which may be due
to recursing), it is vital that there is no longer a borrow to the
inner thunk representation, otherwise this can cause accidental
infinite recursion (which will be detected, but cause failures on
valid code).
Change-Id: I2846f3142830ae3110a4f5d2299e9d7928634504
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6436
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This makes it easier to track exactly which lambda is which when
inspecting e.g. the concrete representation of a thunk.
At runtime all lambdas live in an Rc. To make this print the right
address, the construction of these Rcs had to be moved up right to the
point where the lambda is first emitted (and disassembled).
Change-Id: I6070e6c8ac55f0bd697966c4e7c5565c20d19106
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6435
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
There can be different spans on the same line, so the previous
implementation would duplicate line numbers unnecessarily.
Change-Id: I8d8db77177aee0d834a6ec3584641e1bd5f31c3e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6434
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This was fixed by some of the previous commits around scopes. It's
somewhat similar to a few other tests, but I had this one failing
earlier and everything else succeeding, so it is useful to keep it
around for sure.
Change-Id: Ie6cf372b5c805daf992cd87aeb3dfe91542c381c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6431
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Deferred local upvalues can *only* occur at the same depth as the
thing that is closing over them, but there are various situations with
scope nesting where the actual stack indexes of the local and the
closer look like a deferred value is being accessed.
To fix this, simply compare the depth as well.
Change-Id: Ice77424cc87ab0a2c4f01379e68d4399a917b12b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6429
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This is the same as `eval-okay-attrs-simple-inherit`.
Change-Id: I23878accc6cd62c16ec96601239838a385d31306
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6428
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
The condition here was extremely hard to read prior to this change.
As the locals vector is now guaranteed to never be empty (there is
always at least a phantom for the current chunk's root expression),
the logic here can be simplified to just dropping tailing locals
entries while their depth matches that of the scope being closed.
Change-Id: I24973e23bc2ad25e62ece64ab4d8624e6e274c16
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6427
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Similar to setting up a phantom slot when compiling the root value of
a file, closures and thunks need to have a phantom stack slot for the
root of the expression yielded by their thunk to make all accounting
work correctly.
The tricky thing here is that closures & thunks *escape* their inner
lambda context (that's the point!), so the functions emitting them
need to know both the *inner* slot (to resolve everything correctly
while compiling the slot) and the *outer* slot (to correctly emit
instructions for closing over upvalues).
Change-Id: I62ac58e2f639c4b9e09cc702bdbfd2373e985d7f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6426
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Instead of using a sentinel LocalIdx which potentially points to a
value in the locals stack that does not actually exist, set up an
initial uninitialised phantom value representing the result of the
root expression.
Change-Id: I82ea774daab83168020a3850bed57d35ab25c7df
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6424
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
When deciding whether an upvalue needs to have a deferred resolution
step, the *stack* indexes should be compared - not the locals indexes.
The results are almost always the same, but there are tricky
situations where this can cause errors.
It's difficult to reproduce these errors in isolation, as they depend
on other scope behaviour, so this is one in a series of commits to
address the combination of issues which will gain some tests at the
end.
Change-Id: Iaa400b8d9500af58f493ab10e4f95022f3b5dd21
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6423
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Instead of using sentinel values and an additional bool, this tracks
the identifier of a local as an enum that is either a statically known
name, or a phantom.
To make this work correctly some more locals related logic has been
encapsulated in the `scope` module, which is a good thing (that's the
goal).
Phantom values are now not initialised by default, but the only
current call site of phantoms (`with` expression compilation) performs
the initialisation right away.
This commit changes no actual functionality right now, but paves the
way for fixing an issue related to `let` bodies.
Change-Id: I679f93a59a4daeacfe40f4012263cfb7bc05034e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6421
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
The slot is now always known (at the root of the file it is simply
stack slot 0 once the scope drops back down to 0), so it does not need
to be wrapped in an `Option` and accessed in cumbersome ways anymore.
Change-Id: I46bf67a4cf5cb96e4874dffd0e3fb07c551d44f0
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6420
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
With this change the runtime trace contains much more exact
information about the context of the computation (entering/exiting
calls etc.)
This is in large part due to moving the tracer to be a field on the VM
itself, which enables consistent ordering of traces across the
execution, and tracing an execution with its *input* instead
of *output* stack.
Change-Id: Ibe525e6e7d869756501e52bef1a441619ce7332c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6419
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This makes it much easier to figure out what happened while debugging
this sort of thing.
Change-Id: I2e0e8096709adc647d63c04f213c547c415e5f44
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6418
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This could occur when the disassembler is enabled and tracing the
runtime while a thunk is being evaluated, as it would not be possible
for the *tracer* to borrow the thunk at this exact moment.
However, we know that if the borrowing fails here we are dealing with
a not-fully evaluated thunk (blackhole), which should just print the
internal representation.
Change-Id: I4bdb4f17818d55795368e3d28842048f488f0a91
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6416
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Previously, "calling" (setting up the VM run loop for executing a call
frame) and "running" (running this loop to completion) were separate
operations.
This was basically an attempt to avoid nesting `VM::run` invocations.
However, doing things this way introduced some tricky bugs for exiting
out of the call frames of thunks vs. builtins & closures.
For now, we unify the two operations and always return the value to
the caller directly. For now this makes calls a little less effective,
but it gives us a chance to nail down some other strange behaviours
and then re-optimise this afterwards.
To make sure we tackle this again further down I've added it to the
list of known possible optimisations.
Change-Id: I96828ab6a628136e0bac1bf03555faa4e6b74ece
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6415
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
If the disassembler feature is enabled, make sure that an Rc of the
codemap is available through the chunk.
Change-Id: I700f27ab665a704f73457b19bd2d7efc93828a16
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6414
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
The casting methods of `Value` are pretty verbose, and actually
incorrect before this commit as they did not account for inner thunk
values.
To address this, we first attempt to make them correct by introducing
a standard macro to generate them and traverse the inner thunk(s) if
necessary.
This is likely to be a performance hit as it will now involve more
cloning of values. We can do multiple things to alleviate this, but
should do some measurements first.
Change-Id: If315d6e2afe7b69db727df535bc6cbfb89a691aa
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6412
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This makes it possible for builtins to force values on their own,
without the VM having to apply a strictness mask to the arguments
first.
Change-Id: Ib49a94e56ca2a8d515c39647381ab55a727766e3
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6411
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Scope poisoning must be inherited across lambda context boundaries,
e.g. if an outer scope has a poisoned `null`, any lambdas defined on
the same level must reference that poisoned identifier correctly.
Change-Id: I1aac64e1c048a6f3bacadb6d78ed295fa439e8b4
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6410
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
With this, if an error occurs while forcing a thunk (which is very
likely) it is threaded through to the top by wrapping it in the
ErrorKind::ThunkForce variant.
We could use this to generate "stacktrace-like" error output if we
wanted, or simply jump through and discard everything except the
innermost error.
Change-Id: I3c1c8708c2f73ae062815adf490ce935b1979da8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6409
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Previously error spans were optional because the information about
code spans was not available at runtime. Now that this information has
been added, the error type will always carry a span.
This change is very invasive all throughout the codebase. This is due
to the fact that many functions that are called *by* the VM expected
to return `EvalResult`, but this no longer works as the span
information is not available to those functions - only to the VM
itself.
To work around this the majority of these functions have been changed
to return `Result<T, ErrorKind>` instead and an accompanying macro in
the VM constructs the "real" error.
Note that this implementatino currently has a bug where errors
occuring within thunks will yield the location at which the thunk was
forced, not the location at which the error occured within the code.
This will be fixed soon, but the commit is large enough as is.
Change-Id: Ib1ecb81a4d09d464a95ea7ea9e589f3bd08d5202
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6408
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Get the length of a list
Change-Id: I41d91e96d833269541a1b3c23b7cc879f96d1e5a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6407
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
First pass at supporting `builtins` for tvix. The following tests appear to be
WAI:
```shell
$ cd tvix/eval
$ cargo build
$ cargo test
```
Change-Id: I27cce23d503b17a886d1109e285e8b4be4264977
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6405
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Another step towards being able to report accurate errors. The codemap
spans contain strictly more accessible information, as they now retain
information about which input file something came from.
This required some shuffling around in the compiler to thread all the
right information to the right places.
Change-Id: I18ccfb20f07b0c33e1c4f51ca00cd09f7b2d19c6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6404
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
As of this commit, the source spans of all emitted bytecode are fully
tracked.
Change-Id: I4c83deee0fc3f5e6fd6acad5a39047aec693b388
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6403
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
These source spans will always point to the *value* that is being
forced, not the instruction that caused the force to be emitted. This
makes sense so that errors during forcing point at the value and not
the surrounding expression.
Change-Id: I4694414a3281a0de878f71634105b92148ec61f6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6402
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
With this change, the upvalue data instructions used by finalisers for
thunks and closures track the source span of the first identifier that
created the upvalue (if the same value is closed over multiple times
the upvalue will be reused, hence only the first one).
To do this the upvalue struct used by the compiler's scope now carries
an identifier node, which had to be threaded through quite a few
places.
Change-Id: I15a5fcb4c8abbd48544a2325f297a5ad14ec06ae
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6400
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This separation makes it possible to annotate the upvalue itself with
the span that created it, which (due to upvalue reuse) is only the
first one for an instance of the given UpvalueKind.
Change-Id: I9a991da6a3e8d71a92f981314bed900bcf434d44
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6399
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
These are again a bit tricky in terms of emitted errors. The main
error is that the condition is not a boolean, which means that the
jump inspecting the condition must derive from the condition itself to
return an error at the correct position.
For other parts of the expression, it is simply the node itself.
Change-Id: I72411630e5d57dfc199f4c3c48afe443fe966322
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6392
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This one is tricky, specifically the span used for the final jump. I
decided that it makes sense to use the attrpath node, as the final
jump is the one that jumps *over* the default value, so the effect of
this is more closely related to the selector than the default.
It might be more correct to pass through the `or` token itself and
point to this for the jumps, but it depends a bit on what shape of
errors we could end up producing from this.
Change-Id: I29fbc97ba6b9e14e1a0e5f3a7759ddc299dd9c0c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6390
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
These are not actually used yet; this is in preparation for a
multi-commit chain for emitting all the right spans in the right
locations.
Change-Id: Ie99d6add2696c1cc0acb9ab928917a10237159de
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6379
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This instantiates a codemap outside of the compiler and passes a
reference to the file currently under compilation to it. Note that the
"file" might just be a REPL line.
Change-Id: I131ae1ddb6d718e1374750da9ba0b99608c6058d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6378
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This is currently just a wrapper around Chunk::push_op, but will gain
the span resolution logic in a moment.
Change-Id: I862bf9ecff0932f8da6708401ea044b9442c5d5b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6377
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This adds a new vector to the chunk data structure which tracks spans
into a codemap. The compiler will emit this information to the chunk
when adding instructions.
The internal representation of the spans is slightly optimised to
avoid storing duplicate spans, as there are cases where many
instructions might be derived from the same span.
Change-Id: I336f8c912e7eb50ea02ed71e6164f651ca3ca790
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6376
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
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>
This has no effect yet, other than changing the way in which some
upvalue captures break (that are already not working correctly).
However, after this change the compiler correctly detects
self-recursion and can start emitting the instructions to deal with
this at runtime.
Change-Id: Id3b0ac206c0204739597a4325bcc66f9c806c242
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6309
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Calculating the with_idx (i.e. the stack offset of the "phantom"
variable from which a `with` dynamically reads at runtime) needs to
account for unitialised variables the same way as the resolution of
normal locals does.
Change-Id: I9ffe404535bf1c3cb5dfe8d9e005798c857fff94
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6308
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This is required to correctly clean up the `with` values.
At the moment, the attrset from which identifiers are being taken is
always pushed on the stack. This means that it must also be removed
again, otherwise in an expression like
with { a = 15; }; a
The final stack is `[ { a = 15; } 15 ]` *after the last operation*,
which means that the attrset is still on there as garbage.
This has little practical impact right now because it is always
shadowed by the fact that the actual expression value is at the right
location, but becomes relevant when accounting for upvalue captures.
Change-Id: I69e9745bfaa4d6bbcb60ee71f4dc3f8d8695d16a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6303
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This extends the logic of `Scope::resolve_local` to detect cases where
self-recursion is occuring (i.e. an identifier is being accessed in
its own identifier).
These cases are not yet handled specially, and the logic of when
things are marked initialised (which was previously always at the same
spot as their declaration) has not changed, making this commit a
runtime no-op for now.
Change-Id: I3179642a7c55869ad4465fdd2678b0cd51a20f15
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6302
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Nix does not allow things like `let a = 1; a = 2; in a`, but doing it
across depths is allowed.
Change-Id: I6a259f8b01a254b433b58c736e245c9c764641b6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6301
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This does not yet change anything semantically, but will be useful for
resolving simple cases of self-recursion etc.
Change-Id: I139ecb7e4a8a81193774392a96e73e0ea6b9f85d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6300
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
These need to be handled specially by the runtime if the compiler
determines that a given local must be resolved via `with`.
Note that this implementation has a bug: It currently allows `with`
inside of nested lambdas to shadow statically known identifiers. This
will be cleaned up in the next commit.
Change-Id: If196b99cbd1a0f2dbb4a40a0e88cdb09a009c6b9
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6299
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
The previous implementation of OpResolveWith manually controlled the
loop iteration, which skipped over the disassembler's tracing
instruction.
Instead, the resolution of dynamic variables has been delegated to a
new helper function. This has the additional benefit that the loop
labels are no longer required, making things a bit cleaner.
Change-Id: If22b74c3d49c74bf3a1ec4497cb761a9ee6cf2a4
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6298
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Instead of tying the popping of the with stack to scope depth, clean
up the stack immediately after processing a with body.
The previous behaviour was actually incorrect, as it would leave
things on the with-stack longer than they were supposed to be there.
This could lead to false positive resolutions in some situations
involving closures.
Change-Id: I7b0638557503f1f71eb602e3d5ff193cdfcb67cc
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6297
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Implements the final bit of logic remaining for wiring up closures,
which is the runtime construction of closure objects.
When encountering an OpClosure, the VM walks through the bytecode
collecting all the upvalue location operands (see commit introducing
the OpCode::Data* variants for details) and stores the runtime values
in the new closures upvalue vector.
After that, the handling of the closure itself becomes functionally
identical to that of lambdas.
With this initial implementation of closures there are several large
optimisation potentials available, the two most notable ones are:
- Distinguish the runtime representation of lambdas and closures
explicitly.
- Detect and handle multiple-arity functions directly in the compiler.
However, for both of these we should wait until we have appropriate
benchmarking infrastructure in place. This is because our test
implementations have shown that the complexity of either of these
changes is quite significant, and we do not yet know if they really
pay off.
Change-Id: I077e977810fd5cb2b1ecd7f1a119e728025dd786
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6295
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
This resolves an upvalue at runtime by pushing it on the stack from
the closure's upvalue vector.
Change-Id: Ic3e7a7ecd9f7032f679114a1995e5bbf83062fcf
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6294
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
In preparation for implementing calling of closures, store a closure
directly in the VMs call frame.
Change-Id: Iad24cd8c49fee4ebd4d0c84ffaa4c2505ee3dfd6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6293
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
... same as the others
Change-Id: I9c8868388c10b0b6484c5bdd3799d801296c6979
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6292
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
Fully implements the instructions for compiling closure
objects (without runtime handling yet).
Closure (and thunk) objects are created at runtime by capturing all
known upvalues. To represent this, the instructions for creating them
need to have a variable number of arguments. Due to that, this commit
introduces new variants in OpCode that are not actually operations,
but data.
If the VM is implemented correctly, the instruction pointer should
never point at these. Due to this, the VM will panic if it sees a data
operand during an execution run.
Change-Id: Ic56b49b3a42736dc437751e76df0e89c8d0619c6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6291
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
This adds a new upvalue tracking structure in the compiler to resolve
upvalues and track their positions within a function when compiling a
closure.
The compiler will emit runtime upvalue access instructions after this
commit, but the creation of the runtime closure object etc. is not yet
wired up.
Change-Id: Ib0c2c25f686bfd45f797c528753068858e3a770d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6289
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
This adds a transparent wrapper around `usize` used for jump offsets
in the opcodes. This is a step towards getting rid of ambiguous plain
`usize` usage in the opcode.
Change-Id: I21e35e67d94b32d68251908b96c7f62b6f56a8bb
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6282
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
grfn pointed out in cl/6174 that `Result` might cause developers to
believe that this behaves like std::Result, which it does not.
Change-Id: Ia30ab0dcb7e8da7bf842777ee3fe17bcf35cb0c1
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6281
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Refactors the update function to take the attribute sets by value
instead.
To facilitate this, we use an equivalent of the currently unstable
`Rc::clone_or_unwrap` in the VM when encountering attribute sets, so
that in cases where the only references to the attrs being updated are
the ones on the stack those clones are avoided completely.
This does make update() a little bit more tricky internally, as some
optimised branches can directly return the moved value, and others
need to destructure with ownership. For this reason there are now two
different match statements handling the different ownership cases.
Change-Id: Ia77d3ba5c86afb75b9f1f51758bda61729ba5aab
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6279
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Add a quick couple of benchmarks for merging attribute sets, large and
small.
Change-Id: I26940a9cf4e0d30e3d9eb07a7b8c366ca4072ca3
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6286
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: grfn <grfn@gws.fyi>
This is no longer needed for anything and the extra clone here is not
really more costly than constructing a blackhole value in a different
place.
Change-Id: I5c63085b1b4418b629ea58a42e3bfe9a9b586d76
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6275
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Without this escape, it is possible for Nix to produce escaped
representations which are not literal Nix values again.
This was fixed in upstream Nix in
https://github.com/NixOS/nix/pull/4012 (though only for eval, not in
the REPL) and the updated test is picked from upstream after that commit.
Because we run the C++ Nix tests against our test suite as well, this
also bumps our custom Nix 2.3 to a commit that includes the
cherry-picked fix from the PR above.
Change-Id: I478547ade65f655c606ec46f7143932064192283
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6271
Reviewed-by: grfn <grfn@gws.fyi>
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This is a more sensible place for this function to live and makes
upvalue resolution easier down the line.
Change-Id: I48ee39bdcdb4f96a16a327f7015aff60db5b15fb
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6270
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
This struct will carry the upvalue machinery in addition to the lambda
itself. For now, all lambdas are wrapped in closures (though
technically analysis of the environment can later remove innermost
Closure wrapper, but this optimisation may not be worth it).
Change-Id: If2b68549ec1ea4ab838fdc47a2181c694ac937f2
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6269
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Primarily to make sure we build benchmark targets, and avoid breaking
them
Change-Id: I0c43f4cf99ddfd38e7545ef2d8276ef6b240a1e8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6285
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Interpret was updated to take an optional path arg in
6fe5e2d75 (feat(tvix/eval): resolve relative path literals, 2022-08-12),
but since benchmarks aren't building in CI the resulting breakage of
benchmarks was missed.
Change-Id: I8a93f1b25ae62e2d032fafc153d91977c6466712
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6284
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
The blackhole allocation is not going to be cheaper than cloning this.
Change-Id: Id3ad44812decb4392830be06645e67bb0a982b96
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6267
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Compilation of `let`-expressions is going to become a lot more
complicated due to attempts to avoid thunking when encountering
internal references, so this is just being moved out of the way.
Change-Id: Iecfa4b13d14532e21c2540e6561b4235ce29736a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6266
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
This is just for dev comfort, it's not going to be useful for the
final version.
Change-Id: I05fdd590097a61085ed641810655d9ddaf8f3511
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6265
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
In conditions where no dynamic identifiers exist in a scope,
inheriting is usually a no-op - *unless* the identifier is not
statically known and the scope has a non-empty `with`-stack.
Change-Id: Iff4138d9cd4c56e844bc574203708dacc11c3f73
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6264
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This avoids copying around the value more than needed.
Change-Id: I35949d16dad7fb8f76e0f641eaccf48322144777
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6263
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
These do essentially the same, but return different error variants as
upstream Nix considers `throw` to be (sometimes) catchable.
Change-Id: I1a9ea84567d46fb37287dbf3f3f67052f9382cca
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6259
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
The set of things that can leak out of `builtins` into the global
scope is statically known (it is what Nix 2.3 leaks there,
essentially).
This is a mild change over the previous mechanism, where instead at
the point where the `builtins` set is constructed we "lift" the
globals out of there (if they exist).
This way users will still eventually be able to add additional
builtins, HOWEVER they will not be able to leak them into the global
scope.
Note that upstream Nix technically leaks _all_ builtins into the
global scope using the `__*` prefix, but we are trying to avoid this
in Tvix if it is not required in nixpkgs.
Change-Id: Ie9dec2ce33740134f3b2464eba3749f421dd5953
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6258
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Adds a new builtins module in which builtins can be constructed. The
functions in this module should return a correctly structured value to
be passed to the compiler's `globals`.
This is wired up all the way to the compiler with an example
`toString` builtin, available as a global. Note that this does not yet
actually behave like the real toString, which has some differences
from `Display`.
Change-Id: Ibb5f6fbe6207782fdf2434435567fc1bd80039a5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6254
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Previously, the tokens that could poison a scope (`true`, `false`,
`null`) had individual fields in the scope to track whether or not
they were poisoned.
This commit sets up new machinery that instead tracks scope poisoning
dynamically using a HashMap, and which makes it possible to introduce
additional tokens to the top-level ("global") scope that are directly
resolved by the compiler by passing a map of runtime values to be
used.
With this solution, the compiler now contains all machinery required
for wiring up builtins resolution.
The set of builtins to be exposed at runtime must, however, be
constructed *outside* of the compiler and passed in. Everything is
prepared for this, but it is not yet wired up (so the only existing
builtins are the ones we already had before).
Note that this technically opens up an optimisation potential when
compiling selection operations, where the attribute set being selected
from is `builtins`. The compiler could directly resolve the builtins
and place the right values on the stack.
Change-Id: Ia7dad3c2a98703e7ea0c6ace1a722d57cc70a65c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6253
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Builtins are represented as a Rust function pointer that accepts a
vector of arguments, which represents variable arity builtins.
Change-Id: Ibab7e662a646caf1172695d876d2f55e187c03dd
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6251
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
Nix functions always have a single argument and we do not yet make
efforts to optimise this in Tvix for known multi-argument functions
being directly applied.
For this reason, the call instruction is fairly simple and just calls
out to construct a new call frame.
Note that the logic for terminating the run loop has moved to the top
of the dispatch; this is because the loop run needs to be skipped if
the call frame for the current lambda has just been dropped.
Change-Id: I259bc07e19c1e55cd0a65207fa8105b23052b967
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6249
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Compiles lambda definitions of the simple form (i.e. without formals
arguments) and emits them as constants like any other value.
This does not yet implement actually invoking these functions in the VM.
Change-Id: Ie1e0a13220b68c1728be229b875f0992e685c5ef
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6247
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
This structure carries context about the lambda currently being
compiled (which may well be the top-level lambda of an input AST).
Using the indirection helpers in the compiler, things like the scope,
code and constants of the function being compiled are now taken from
the current lambda context instead.
Change-Id: If5f864d826c2e72855cee4b728ea1830e9b5ac06
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6246
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
This wires up most of the machinery for executing different call
frames inside of the VM and stuffs the top-level lambda which the
compiler outputs in there, as well.
Change-Id: Ib6201b3e3be1af96a4d195f6eb147f452860ffc3
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6242
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
This is going to carry the data for a function invocation inside of
the VM.
Change-Id: I86664563a7e35697a64294acd37ffde037fbd32d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6241
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
Changes the internal compiler plumbing to not just return a chunk of
code, but the same chunk wrapped inside of a lambda value.
This is one more step towards compiling runtime lambdas.
Change-Id: If0035f8e65a2970c5ae123fc068a2396e1d8fd72
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6240
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
With these indirections in place it becomes easier to change internals
of the compiler when introducing functions, which need the compiler to
be able to target different code chunks.
Change-Id: I4eb11572a93c140b1d059ba0a5af905756745d65
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6239
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
Instead of exiting the compiler at the first sight of an error,
skip any erroneous nodes and continue compiling, collecting more
errors along the way.
This paves the way for nicer error reporting in which multiple errors
can be reported at once, avoiding situations in which users are
hunting a fault error-by-error and possibly getting distracted by
less useful output.
Change-Id: I80c9a87272e33a31297167ae2eb2706a46adf15a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6236
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
This starts paving the way for nicer, source-code based error
reporting.
Right now the code paths in the VM do not emit annotated errors, as we
do not yet preserve that structure from the compiler. However, error
emitting code paths in the compiler have been amended to include known
nodes.
Change-Id: I1b74410ffd891c40cd913361bd73c4336ec8aa5b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6235
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
In this commit, the string interpolation parsing is identical to
nixpkgs which makes some of the upstream Nix tests for
interpolation-related weirdness pass.
Change-Id: I3a295cfdc404c32228a54846e6efd3c0dcee5842
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6233
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
These were missing an additional level of escaping, silly oversight
caught by an upstream test.
Change-Id: I0312084475e4b88c83945614e9aa5b34c6bc3ec2
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6232
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Since the latest published version of rnix-parser on crates.io, the
crate has undergone major changes which are only available in the git
repository at the moment. This commit updates the compiler to this
newer version of rnix.
Most notably, the entire AST provided by rnix is now wrapped in the
AST type system. As a result of this traversal is much nicer in many
places, especially for things like nested attribute selection.
There are a handful of smaller features missing for full feature
parity with the previous version, especially handling of path
literals, but PRs for these already exist in rnix-parser.
Change-Id: Icde6d393067976549492b7d89c4cc49e5e575fc7
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6231
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This implements `assert`, which evaluates an expression and aborts
evaluation if the value is not `true`.
At this point we should introduce eval-failed-* tests; probably
asserting against some representation of the error enum?
Change-Id: If54c8f616d89b829c1860a4835dde60a2cd70d7a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6230
Reviewed-by: grfn <grfn@gws.fyi>
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This makes for much more readable output especially when long strings
are involved.
Change-Id: I43dd73a0480535d7181a760788c42883a9b083f8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6229
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
`push_local`/`push_phantom` were worse names because they sound like
the value itself is being pushed, where in actuality it is just being
declared to the compiler.
Change-Id: Ibfda5c4c8e47d5d3262bfe005b0f1f84908a117e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6228
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
These tokens are optionally parsed as identifiers by Nix, which means
that within any scopes that resolve them the compiler needs to track
whether they have been overridden to know whether to emit the literal
instructions or resolve a variable.
This is implemented by a new concept of "scope poisoning", where the
compiler's scope structure tracks whether or not any builtin
identifiers have been overridden.
Change-Id: I3ab711146e229f843f6e1f0343385382ee0aecb6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6227
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: grfn <grfn@gws.fyi>
While full recursion through thunking is not available, there are
actually incorrect behaviours introduced by declaring before
binding (example in the newly introduced test).
This commit simplifies the implementation to avoid this issue, and
also because I intend to explore a bit more how far we can get in non
left-to-right bindings *without* introducing thunks immediately.
Change-Id: I21fd3007ac3946570639772d7d624d70bd209958
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6226
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: grfn <grfn@gws.fyi>
If an unknown variable is encountered and the with stack is not empty,
emit instructions for resolving the variable at runtime.
Change-Id: I752f4bd0025335744e4747364abd1bd34130374e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6223
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: grfn <grfn@gws.fyi>
Adds an additional structure to the compiler's scope to track the
runtime "with stack", i.e. the stack of values through which
identifiers should be dynamically resolved within a with-scope.
When encountering a `with` expression, the value from which the
bindings should be resolved is pushed onto the stack and tracked by
the compiler in the "with stack", as well as with a "phantom value"
which indicates that the stack contains an additional slot which is
not available to users via identifiers.
Runtime handling of this is not yet implemented.
Change-Id: I5e96fb55b6378e8e2a59c20c8518caa6df83da1c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6217
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This name is much more sensible actually; its more than just a
collection of locals as it tracks additional scope information in the
case of Nix.
Change-Id: Ia2739bbd39aab222b1c4355e9248828973b0db43
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6216
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Note that at this point recursive bindings do not yet work in either
attrsets or let, so inheriting from the same scope is generally not
possible yet.
Change-Id: I6ca820d04b8ded5c22fb7ea18e2ec203bcaa8e9c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6215
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Straightforward implementation, evaluating the elements of an inherit
and preparing the stack so that `OpAttrs` sees all relevant values
when constructing the attribute set itself.
The emitted instructions for inheriting a lot of values from the same
attribute set are inefficient, but it's too early to say whether this
actually matters.
Change-Id: Icb55a20936d4ef77173f34433811c5fa5d2c9ecc
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6214
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
If the directory in which REPL history is stored does not exist,
gently try to create it, but do not raise an error if it doesn't work.
We may want to warn about it, but in general this sort of
non-essential feature should not cause a hard failure.
Change-Id: If4fe8db0c7893c39627efe72c9cd9ebf7ed63f04
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6213
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Using `OpAttrSelect`, the ? operator will fail when encountering a
nested value that is not an attribute set.
This however breaks valid code, such as:
{ bs = 42; } ? bs.a.b
The fix is simply to use the same operator used in the `or` statement,
which leaves a sentinal on the stack if a field is not found or the
value is not an attribute set.
Change-Id: Ib28fc8a96e6d592b4cdbc3e65ba129ad8faecd66
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6211
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
If a nested attrpath encounters a non-set value, the sentinel value
denoting a lack of next values should be emitted. This mirrors the
behaviour of Nix.
Change-Id: Ia80443d5a11243cc6d98dcab1249a3f5fdf77e27
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6210
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Not sure how exactly this snuck in, but it caused some subtle
breakages in deeply nested attribute sets.
Change-Id: I8049ce912405d3750031f79cc8d86ff1c3c02c2b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6208
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
We're confident that we're handling all branches that can reasonably
occur from valid AST, any other cases should be considered a critical
evaluator bug and panic rather than surfacing something that looks
like user error.
Change-Id: If96966eb32b8ff12fcaeb9ea3b0c8fc51b6abd11
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6205
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
This is actually *tested* behaviour in C++ Nix, so we need to
implement it here, too.
Change-Id: Ic4a4659a2f04cdd928cbe78a85dae90401515371
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6199
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This adds a `disassembler` feature to the crate configuration that
traces the operations executed and the state of the stack at runtime.
This can be enabled by compiling with `--feature disassembler`.
This will also gain a more sensible layout of code slices eventually.
Change-Id: I34c15e1cd346ecc4362b5afba6bf82dd49359d20
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6193
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This exposes tvix.nix_cli as a proper attribute to readTree, so it's
actually built by CI.
Change-Id: I3cef085bd872b61c5944270c8926727bf1fa705d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6083
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Using `inherit` in a let-binding can not possibly have an effect, as
the given identifier is already bound exactly the same way in the
current scope.
This introduces a subtle bug that is fixed later on, as there
actually *is* a (single) condition where these inherits are
meaningful.
Change-Id: I8b24f0edcfe80db8153bb7e86cf478d36957d6f8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6192
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
This makes basic `let ... in ...` statements work correctly. It does
not yet account for the call frames pushed into the VM during function
application.
Change-Id: I67155171daf1a43011b96716dd9d1ab04b27db33
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6190
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
Nix does not allow dynamic identifiers in let expressions (only in
attribute sets), but there are several different kinds of things it
considers static identifiers.
The functions introduced here put the path components of a let
expression into normalised (string) form, and surface an error about
dynamic keys if one is encountered.
Change-Id: Ia3ebd95c6f3ed3cd33b94e156930d2e9c39b6cbf
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6189
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
These expressions now leave the binding values on the stack, and clean
up the scope after the body of the expression.
While variable access is not yet implemented (as the identifier node
remains unhandled), this already gives us the correct stack behaviour.
Change-Id: I138c20ace9c64502c94b2c0f99a6077cd912c00d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6188
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
These are going to be used during compilation of `let`-expressions to
determine stack offsets for local variables.
Change-Id: Ibb79f3f1ae86650303f88eacf623ae456458de87
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6187
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Canonicalisation performs much more functionality than what C++ Nix
implements for paths, and causes some undesirable behaviour (e.g.
handling non-existant files becomes difficult, but should be possible
in literals).
Instead, the path_clean crate provides a pure normalisation method.
There is an intention to add this to Rust itself:
https://github.com/rust-lang/rfcs/issues/2208
Change-Id: I775d238136db0a52cf6b12a68985833c8fb32882
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6186
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Resolves relative paths (e.g. `./foo`) either relative to the location
of the Nix file, or relative to the working directory if none is
supplied.
Change-Id: I70ec574657b221b458015117a004b6e4a9c25a30
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6185
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
There are multiple things that can theoretically fail while resolving
a path, as some of it includes I/O. A new error variant has been added
for this and appropriate errors have been introduced.
Change-Id: Ie222245425207dabbf203166eb5ed1eec0114483
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6184
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
These two paths are the easiest to handle, as they are simply built up
from the components supplied in the text node and then normalised.
Note that the normalisation of fs::canonicalize includes symlink
resolution, which Nix does not actually do. We will need to fix that
at some point.
Change-Id: I54158f0684f197dd2a2583f7d0982d54c7619993
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6183
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
grfn pointed out in cl/6082 that this is actually the desugaring of
the write! macro, so it doesn't make sense to write it out.
Change-Id: If7c055b042ad22b034722aec1eaadba92736d684
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6180
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: grfn <grfn@gws.fyi>
grfn pointed out in cl/6069 that naming them like this makes it clear
that things are being added to the end of the state.
Change-Id: I6a23215c4fef713869a3c85b0dde1ebbda7637e9
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6179
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
For representation wrappers that are used to control the visibility of
type internals, this ensures that the wrapper does not increase the
size of the type.
In practice, the optimiser likely does this anyways but it is good to
guarantee it.
Change-Id: Ic6df7d668fe6006dfbd5b6cfcfc2088afa95b810
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6178
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
This makes it possible to quickly detect code errors that might blow
up the size of the OpCode type.
Change-Id: I7662dd0aa30c4762c0f9e4fa346418c9ca8b9994
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6177
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
These can be used predominantly to emit warnings about things that the
compiler can infer, such as deprecated language features.
Change-Id: I3649c625459d7f3f95cdf42d5c651d23d66569ec
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6174
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
The parser creates this node type from literal URL values. Technically
these are deprecated and have been removed from nixpkgs.
Change-Id: I4d05034dd9b4d8348e4ed8a2bbb37c1b6ccef8bc
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6173
Reviewed-by: grfn <grfn@gws.fyi>
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This is a substantially nicer experience, immediately granting us
history, proper exiting and so on.
Change-Id: Iba4cb1713b9ac53d0799722bdbe2cd0e94a2f527
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6171
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
This crate provides sensible default locations for directories on
different platforms.
Change-Id: I0b61cc7f626dc6c8df903ba0f873be24e07d69b5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6170
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
