This is more useful than pointing it at the entire assert expression,
as that includes the body as well which is not going to be relevant in
the error.
Pointed out by sterni in cl/6391
Change-Id: I95a5d1edf90df65e7fa53d4d04502afd6e99e89a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6566
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This is no longer required, resolution is now more sane. Pointed out
by sterni in cl/6422.
Change-Id: Icc8983c648f864e66813948df6e2d4bad6a7f312
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6565
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This field no longer needs to be directly accessible by the compiler.
Addresses a sterni lint from cl/6466
Change-Id: I5e6791943d7f0ab3d9b7a30bb1654c4a6a435b1f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6564
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
As suggested by sterni in cl/6453.
Change-Id: I3cf80d97c11fd7d085ab510f6be4b5f937c791ec
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6562
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This very closely follows the way it's done for warnings, but errors
have a lot more information available in some cases which we do not
surface yet.
Note also that due to requiring the `CodeMap`, this is not yet called
from eval.rs as the way that is threaded through needs to be
refactored, so only the method for reporting these errors as strings
is implemented so far.
Next steps for this will be to add a generic diagnostics module that
reduces some of the boilerplate for this between warnings & errors,
and which will also give us a good point in the future to switch to a
fancier diagnostics crate.
Change-Id: If6bb209f8e7a568d866e516a90335b9b2afbf66d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6534
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
This implements an initial fancy display for warnings emitted by the
tvix compiler, using the codemap_diagnostic crate.
Each warning variant has an associated message, and optionally an
associated annotation for the span displayed to the user.
In theory we could get a lot more fancy with the display for specific
variants if needed (e.g. re-parse the AST and actually add multiple
semantic spans based on context), but this is already a good start.
Example:
tvix-repl> let toString = https://tvl.fyi; in let inherit toString; in ({}: 42) rec {}
warning[W004]: declared variable 'toString' shadows a built-in global!
--> [tvix-repl]:1:5
|
1 | let toString = https://tvl.fyi; in let inherit toString; in ({}: 42) rec {}
| ^^^^^^^^ variable declared here
warning[W001]: URL literal syntax is deprecated, use a quoted string instead
--> [tvix-repl]:1:16
|
1 | let toString = https://tvl.fyi; in let inherit toString; in ({}: 42) rec {}
| ^^^^^^^^^^^^^^^
warning[W002]: inherited variable already exists with the same value
--> [tvix-repl]:1:40
|
1 | let toString = https://tvl.fyi; in let inherit toString; in ({}: 42) rec {}
| ^^^^^^^^^^^^^^^^^
warning[W999]: feature not yet implemented in tvix: recursive attribute sets
--> [tvix-repl]:1:70
|
1 | let toString = https://tvl.fyi; in let inherit toString; in ({}: 42) rec {}
| ^^^^^^
warning[W999]: feature not yet implemented in tvix: closed formals
--> [tvix-repl]:1:62
|
1 | let toString = https://tvl.fyi; in let inherit toString; in ({}: 42) rec {}
| ^^
warning[W003]: variable 'toString' is declared, but never used:
--> [tvix-repl]:1:5
|
1 | let toString = https://tvl.fyi; in let inherit toString; in ({}: 42) rec {}
| ^^^^^^^^ variable declared here
=> 42 :: int
These are coloured when output to a terminal.
Change-Id: If315648a07e333895db4ae1d0915ee2013806585
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6532
Autosubmit: tazjin <tazjin@tvl.su>
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
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
