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>
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
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>
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
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
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
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
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
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
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
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
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>
