Adds secondary spans for errors that occur deeply nested within a
thunk.
This is pretty raw right now, there's technically nothing stopping one
of these error chains from being a hundred thunks deep into code,
producing unmanageable error output. We should trim these down
according to some heuristics (e.g. when crossing file boundaries, o r
just - for starters - beginning and end).
Change-Id: Ia73892512737850b6fa3e07cabc37fa9c534c4d5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6872
Reviewed-by: sterni <sternenseemann@systemli.org>
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
This change is quite verbose, so a little bit of explaining:
1. To correctly format parse errors, errors must be able to return
more than one annotated span (the parser returns a list of errors
for each span).
To accomplish this, the structure of how the `Diagnostic` struct
which formats an error is constructed has changed to delegate the
creation of the `SpanLabel` vector to the kind of error.
2. The rnix structures don't have human-readable output formats by
default, so some verbose methods for formatting them in
human-readable ways have been added in the errors module. We might
want to move these out into a submodule.
3. In many cases, the errors returned by rnix are a bit strange - so
while we format them with all information that is easily available
they may look weird or not necessarily help users. Consider this CL
only a first step in the right direction.
Change-Id: Ie7dd74751af9e7ecb35d751f8b087aae5ae6e2e8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6871
Reviewed-by: sterni <sternenseemann@systemli.org>
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
This logic was duplicated between the two rnix types before, but more
crucially - it is also needed for correctly displaying the text ranges
contained in syntax errors.
Change-Id: Ifc6a521de1594d6ced9cba6274f1931df99b6634
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6870
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
This is also useful for error-handling related logic, outside of just
the compiler module.
Change-Id: I5c386e2b4c31cda0a0209b31136ca07f00e39e45
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6869
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
This enables the use of string paths (and, in the future,
derivations), as long as their string values represent an absolute
path.
Change-Id: I4b198efeb70415ed52f58bd1da6fa79a24dad14c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6866
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This lets the VM emit warnings when it encounters situations that
should only be warned about at runtime.
For starters, this is used to pass through compilation warnings that
come up when `import` is used.
Change-Id: I0c4bc8c534d699999887c430d93629fadfa662c4
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6868
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Adding `import` to builtins causes causes a bootstrap cycle because
the `import` builtin needs to be initialised with the set of globals
before being inserted into the globals, which also must contain
itself.
To break out of the cycle this hack wraps the builtins passed to the
compiler in an `Rc` (probably sensible anyways, as they will end up
getting cloned a bunch), containing a RefCell which gives us mutable
access to the builtins.
This opens up a potentially dangerous footgun in which we could mutate
the builtins at runtime leading to different compiler invocations
seeing different builtins, so it'd be nice to have some kind of
"finalised" status for them or some such, but I'm not sure how to
represent that atm.
Change-Id: I25f8d4d2a7e8472d401c8ba2f4bbf9d86ab2abcb
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6867
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
This adds an initial working version of builtins.import which
encapsulates the entire functionality of `import` within the builtin
itself, without requiring any changes in the compiler or VM.
The key insight that enables this is that we can simply return a Thunk
from `import` that is constructed from the output of running the
compiler and - ta-da! - no other component needs to know about it.
A couple of notes:
* builtins.import needs to capture variables like the SourceCode
structure. This means it can not currently be constructed the same
way as other builtins and has special handling, which leaks out to
`eval.rs`. I have postponed dealing with that until we have this
working a bit more.
* the `globals` are not yet passed through
* the error representation for the new variants is absolutely not done
yet, we probably want to switch to something that supports
cause-chaining now (like miette)
* there is no mechanism for emitting warnings at runtime; we need to
add that
Change-Id: I3117a7ae3ff2432bf44f5ff05ad35f47faca31d5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6857
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: wpcarro <wpcarro@gmail.com>
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
For some upcoming builtins (notably, import) we need to capture
arguments in the builtin's implementation.
To allow this, we can no longer use function pointers for builtins,
but must use a reference-counted closure object instead.
Unfortunately this adds an extra pointer operation to every builtin
call. We should benchmark this later against having a split builtin
representation.
Change-Id: I109d98d0e25998870542f47573eb1ec2e546f2a2
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6856
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
nixpkgs has hardcoded references to Nix versions, we need to provide
it with something that looks like a Nix version while actually being a
Tvix version.
For now, we do this by stealing a trick out of the browser book and
constructing a version that looks like a Nix version to Nix, but like
a Tvix version to people who know what they are looking for.
Nevermind that we don't actually have any kind of versioning for
Tvix (yet?), other than depot revisions.
Change-Id: I7ce8079dd8164a2079891d38e707f09a45f0bbc1
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6858
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This type hides away the lower-level handling of most codemap data
structures, especially to library consumers (see corresponding changes
in tvixbolt).
This will help with implement `import` by giving us central control
over how the codemap works.
Change-Id: Ifcea36776879725871b30c518aeb96ab5fda035a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6855
Tested-by: BuildkiteCI
Reviewed-by: wpcarro <wpcarro@gmail.com>
There's basically nothing that needs *ownership* of an AST
node (which is just a little box full of references to other things
anyways), so we can thread this through as references all the way.
Change-Id: I35a1348a50c0e8e07d51dfc18847829379166fbf
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6853
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
There are actually two different types of observers, the ones that
observe the compiler (and emitted chunks from different kinds of
expressions), and the ones that trace runtime execution.
Use of the NoOpObserver is unchanged, it simply implements both
traits.
Change-Id: I4277b82674c259ec55238a0de3bb1cdf5e21a258
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6852
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
Until we can display a chained representatino of errors in thunks, it
is most useful to forward the error code from the innermost error to
the user.
Change-Id: I8d67254d52313be40387f080e57966c001e0d51c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6854
Reviewed-by: sterni <sternenseemann@systemli.org>
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Returns time since epoch in seconds.
This has a slight behaviour difference from Nix, in that we don't pin
the time between REPL entries (Nix pins it for the program lifetime),
but this is probably inconsequential as long as it is pinned during an
evaluation.
Change-Id: I010c02e93097a209d8ad69e278397c7e30e54c86
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6846
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
Reviewed-by: wpcarro <wpcarro@gmail.com>
Allows impure builtins that have a different shape than a Rust
function pointer; specifically this is required for
builtins.currentTime which does not work in WASM.
Change-Id: I1362d8eeafe770ce4d1c5ebe4d119aeb0abb5c9b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6849
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
Reviewed-by: wpcarro <wpcarro@gmail.com>
Concatenates (but not flattens) a list of lists.
Change-Id: I692e0b3e7b5a5ff93d5768d3a27849b432ec5747
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6843
Autosubmit: tazjin <tazjin@tvl.su>
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This is the same code as before, just moved into a trait impl to gain
access to stuff that needs IntoIterator
Change-Id: Iff9375cd05593dd2681fa85ccc7f4554bf944a02
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6842
Autosubmit: tazjin <tazjin@tvl.su>
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This can actually legitimately be emitted by the compiler currently
when compiling formals with default values. See the scope6 test from
the Nix test suite for an example.
We should restructure this slightly to be able to reintroduce a
runtime error here in case something was compiled incorrectly.
Change-Id: Ib81f0f58ae0e850db9fbc459458b7bd0d3ac6f23
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6841
Autosubmit: tazjin <tazjin@tvl.su>
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This implements __functor calling in situations where `OpTailCall` is
used, but not yet for `OpCall`.
For some reason I have not yet figured out, this same implementation
does not work in call_value, which means that it also doesn't yet work
in builtins that apply functions.
Change-Id: I378f9065ac53d4c05166a7d0151acb1f55c91579
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6826
Autosubmit: tazjin <tazjin@tvl.su>
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Add a new derivation target to the passthru of tvix.eval that builds the
benchmark binaries, *and* copies them to the outupts of the derivation
via the (somewhat arcane) `copyBinsFilter` jq script arg to naersk. This
is a bit annoying because (as far as I can tell) the derivations
returned by naersk aren't directly overridable, so we have to explicitly
fixpoint the attrs we're passing.
Also, since this is now a separate target to build the benchmarks, we
can remove `--all-targets` from the build of `tvix-eval` itself since
that was only added to build benchmarks in CI, and make
regular (non-benchmark) builds a bit faster.
Change-Id: I136b8526790545e93b1ae666abaefb51cbbee390
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6847
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
The idea is that we can keep track of the more unexpected behavior,
behavior that maybe should not be a thing at all and behavior we are not
sure about yet.
Change-Id: I70933f00af1230a7ab9d30e917b61199fe571caf
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6803
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
This function previously kept a borrow in the form of the
`Thunk::value` result alive while performing arbitrary actions in the
VM, which caused a borrowing error in the test case attached.
The `Ref` value must never be used in cases where control flow is
passed to other parts of the VM.
Change-Id: I41d10aa1882a2166614b670e8ba77aab0e67deca
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6825
Reviewed-by: grfn <grfn@gws.fyi>
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This finishes up the implementation of nested keys after the key
insight that the nesting level does not need to be tracked, and
instead the attribute iterator can simply be retained inside the
structures as is (in an advanced state).
With this implementation, when encountering a nested key, the Tvix
compiler will first analyse whether there is already a matching
binding that can be merged (i.e. a binding that is a literal attribute
set), and perform the merge, or otherwise create a new recursive set
of bindings in which the entry is inserted with the path iterator
advanced beyond the first name component.
With this, all the logic simply applies recursively until there are no
more nested bindings (i.e. until all iterators are "empty").
Note that this has one (potentially insignificant) deviation from Nix
currently: If a non-mergable value is supplied (e.g. `a.b = 1; a =
2;`), Tvix will emit a *runtime* error (whereas it is *parse* time in
Nix) as the branch which could statically analyse this is currently
unreachable. There's a TODO for this, so we can fix it up later.
Change-Id: I53df70e09614ff4281a70b80eac7da3beca12da9
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6806
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This is actually quite useless, as we can just pass
`AstChildren<ast::Attr>` around after partially consuming it.
Change-Id: If0aefa2b53fc801fced1ae0709bff93966bf19f8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6804
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
When encountering a nested binding for the first time, cleanly flip
the representation to `Binding::Set` in `Binding::merge` before
proceeding with the actual merge.
This reduces the number of points where we have to deal with the (soon
to be slightly more complex) construction of the nested binding
representation.
Change-Id: Ifd43aac7b59ebd15a72c3ec512386a5bcf26ec13
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6802
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This adds the scaffolding required for tracking the nesting level (and
appropriately skipping the correct amount of attrpath entries when
inserting nested sets).
In order for all of this to work correctly, we can no longer track
`AttrpathValue` directly in the entries vector as rnix does not allow
us to construct values of that type - so instead we have to track its
inner components.
Change-Id: Icb18e105586bf6c247c2e66c302cde5609ad9789
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6801
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This is a significant step towards correctly implemented nested
bindings. All attribute sets defined within the same binding scope
will now be merged as in Nix, if they use the same key.
Change-Id: I13e056693d5e73192280043c6dd93b47d1306ed6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6800
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This sets up the required logic for finding and merging attribute sets
into nested bindings if they exist. This is absolutely not complete
yet and can, at this commit, probably cause undefined runtime
behaviour if nested attributes are specified.
The basic idea is that a new helper function on the `TrackedBindings`
struct is called with each encountered attribute and determines
whether the new entry can be merged into an existing attribute or not.
Right now the only effect this has in practice is that a new error
becomes available if somebody attempts to cause a merge into an
inherited key.
Change-Id: Id010df3605055eb1ad7fa65241055889dd21bab0
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6798
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This needs to move here so that we can reuse compile_bindings for the
nested attribute sets we're about to start constructing.
Change-Id: Ie83f52f7e1d128886e96a1da47792211fa826f21
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6796
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This struct will be the key to correctly compiling nested bindings, by
having insertions flow through some logic that will attempt to bind
attribute-set-like things when encountering them.
Change-Id: I8b5b20798de60688f3b6dc4526a460ebb2079f6e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6795
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This implementation, which only ever worked for non-recursive
attribute sets, is no longer needed and thus removed here.
We have a new implementation of these nested keys coming up instead.
Change-Id: I0c2875154026a4f5f6e0aa038e465f54444bf721
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6783
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
As of this commit, all three types of bindings scopes are compiled the
same way (i.e. compilation of non-recursive attribute sets has been
switched over to the new code paths).
This sets us up for doing the final implementation of nested attribute
sets.
HOWEVER, this breaks the existing implementation of nested attributes
in non-recursive attribute sets. That implementation is flawed and
unworkable in practice, so we need to do this dance to be able to
implement it correctly.
Change-Id: Iba2545c0d1d6b51f5e1a31a5d005b8d01da546d3
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6782
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This wires up the new bindings setup logic to be able to thread
through & compile dynamic attributes in recursive attrs.
It seems like we don't actually need to retain the phasing of Nix
exactly, as we can use the phantom mechanism to declare all locals
without making the dynamic ones accessible.
Change-Id: Ic2d43dd8fd97d7ccd56d8c6adf2ff97274cd837a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6781
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Another slice of the salami, but no functionality changes yet (other
than opening a code path that can reach a `todo!()`, but this will be
removed soon).
Change-Id: I56b4ed323f70754ed1ab27964ee3c99cf3bf3292
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6780
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
The previous way of sanitising dynamic keys is going away as we're
slowly introducing the new nested key logic.
While touching this stuff, I've also changed all the related string
types to SmolStr as that is more sensible for identifiers.
Change-Id: If30c74151508719d646d0e68e7d6f62c36f4d23f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6779
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Removes the `compile_inherit_attrs` logic which was only used for
BindingsKind::Attrs (i.e. non-recursive attrs).
This brings us a step closer to fully merging all the binding logic
into one block that can dispatch based on the kind of bindings (and
thus giving us a good point to introduce the final logic for nested
bindings).
Change-Id: If48d7a9497fc084a5cc03a130c2a7da5e2b8ef0c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6776
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This helper is responsible for declaring the bindings in the
compiler's scope tracking structures.
It is almost equivalent to the previous logic, but also accounts for
`BindingsKind::Attrs` - though those code paths do not end up here
yet.
Change-Id: I44f617b99b10f2a7b9675f7b23e2c803a4a93d29
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6775
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This is responsible for actually setting up `TrackedBinding`s on the
stack, i.e. in some sense "actually compiling" values in bindings.
There is no functionality change to before, i.e. this is a salami
slice.
Change-Id: Idb0312038e004470a7d130c020ae0fe87c55c218
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6774
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Splits the large `compile_inherits` function which
previously *compiled* plain inherits and *declared* namespaced
inherits into `compile_plain_inherits` and
`declare_namespaced_inherits`.
This is supposed to make more sense than before, but is still not
consistently used (notably, non-recursive attribute sets still
duplicate most of this logic).
Another salami slice.
Change-Id: Id97fac1cbd5ee97b24d047e7728655e6b7734153
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6773
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
... but do not use it yet.
This refactoring is pretty complicated, so I'm applying salami-slicing
tactics here.
Change-Id: I66e04ee10548f68bf67dc842f3f14cc279426c22
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6772
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
As part of the unification of binding logic between different carriers
of bindings, we need to track which kind of bindings we are dealing
with (attribute set? recursive scope? ...) to correctly emit keys and
declare identifiers in the locals stack.
Right now this changes no functionality as `BindingsKind::Attrs` is
not yet used (only RecAttrs and LetIn, which was previously
represented by the `rec_attrs` boolean).
Change-Id: Id2ac27894079ab584521cb568d75c124f7bf2403
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6771
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This helper will gain the ability to compile both kinds of inherits,
but it is kind of tricky to get right so I am doing it in smaller
steps. Right now there is no change in functionality.
Change-Id: Ie990b88dd90a5e0f9fd79961ee09a6c83f2c872d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6770
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This just describes a binding, and we do need a good name for the kind
of binding*s*, which is going to be introduced soon.
Change-Id: I53900ee52da8a07dae8b918fa6a4cb308e627efb
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6768
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Changes the module structure of the compiler to have a module
dedicated to the logic of setting up bindings. This logic is in the
process of being merged between attribute sets and `let`-expressions,
and the structure of the modules makes more sense when ecapsulating
that specifically.
(Other bits of code related to e.g. attribute sets are pretty
straightforward and can just live in the main compiler module).
Change-Id: I9469b73a7034e5b5f3bb211694d97260c4c9ef54
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6766
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This was added in preparation for some builtins that don't exist yet
and is producing some noise during compilation.
Change-Id: I51fb0d14c3edf0bd6d9a288d50e44dacf35166c6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6769
Reviewed-by: flokli <flokli@flokli.de>
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
This is a little ugly because the plain Iterator::filter method can
not be used (it does not support fallible primitives), so we need to
resort to an `Iterator::filter_map` and deal with the wrapping in
Options everywhere.
This prevents use of `?` which introduces the need for some matching,
but it's not *too* bad.
Change-Id: Ie2c3c0c9756c4c627176f64fb4e0054e717c26d1
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6765
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
The previous version had a bug where we assumed that the number of
entries in an attribute set AST node would be equivalent to the number
of entries in the runtime attribute set, but due to inherit nodes
containing a variable number of entries, this did not work out.
Fixes b/199
Change-Id: I6f7f7729f3512b297cf29a2e046302ca28477854
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6749
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Fairly straightforward, only thing of note is that we coerce (weakly) to
string here as well.
Change-Id: I03b427e657e402f1f9eb0f795b689bbf5092aba1
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6745
Tested-by: BuildkiteCI
Reviewed-by: wpcarro <wpcarro@gmail.com>
Reviewed-by: tazjin <tazjin@tvl.su>
The simplest solution seems to be to pass references to arithmetic_op!()
which avoids the moving annoyance we had to deal with in the
builtins (no more popping!). We then use .force() to force the values
and dereference any Thunks (which arithmetic_op! doesn't do for us).
Change-Id: I0eb8ad60e80a0b3ba9d9f411e973ef8bcf136989
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6724
Tested-by: BuildkiteCI
Reviewed-by: wpcarro <wpcarro@gmail.com>
Reviewed-by: tazjin <tazjin@tvl.su>
Because `inherit` can take string identifiers, we can access
arbitrarily weird identifiers in scopes using it.
Change-Id: Ic868233221befa160538dd2ffaff1cc7f566585a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6748
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This updates rnix-parser to a version where inherits provide an
iterator over `ast::Attr` instead of `ast::Ident`, which mirrors the
behaviour of Nix (inherits can have (statically known) strings as
their identifiers).
This actually required some fairly significant code reshuffling in the
compiler, as there was an implicit assumption in many places that we
would have an `ast::Ident` node available when dealing with variable
access (which is then explicitly only not true in this case).
Change-Id: I12f1e786c0030c85107b1aa409bd49adb5465546
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6747
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
We delegate to Rust std's Ord trait, so we should at least do a sanity
check here.
Change-Id: I9596068f8ab3e51b79602de0bac79af6d558086e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6723
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Sketch out a new set of "impure" builtins, which supplement the existing
set of "pure" builtins but are gated behind a feature flag, which allows
them to be omitted by crates depending on tvix-eval that only want pure
evaluation, such as tvixbolt.
Change-Id: I2736017b5c9b4776bbba8758e108ec84887abd66
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6655
Reviewed-by: wpcarro <wpcarro@gmail.com>
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: tazjin <tazjin@tvl.su>
This was derived from
else if (c1 == "" && n2) return true; // true implies c1 < n2
However, this has no effect since Word always looses out against Number
anyways and the `pre` rules are also unaffected by this change – since
this only affects comparison of an empty Word part with a Number.
Change-Id: Ia04e42ac726352b688c87674b0fdb355f06edbcb
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6722
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
This asserts the not-quite lexicographical property of the comparison.
Change-Id: Iad68081e4b3a7106513f479643de87065dc47739
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6721
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
This is necessary because builtins.compareVersions compares versions in
a subtly not-quite-but-still-lexicographical way: `pre` for example can
have an effect if it is post-fixed: `2.3 < 2.3pre`. This is a violation
of the rule that in a lexicographical ordering, the longer string is
considered greater if they are otherwise equal. builtins.compareVersion
is comparing lexicographically though, if you do the following
transformation beforehand:
2.3 --split--> [ "2" "3" ] --append--> [ "2" "3" "" ]
2.3pre --split--> [ "2" "3" "pre" ] --append--> [ "2" "3" "pre" "" ]
Comparing the transformed version is then done lexicographically:
2.3 < 2.3.0pre since [ "2" "3" "" ] < [ "2" "3" "0" "pre" ]
Here, the `pre` rule never comes into effect because no comparison on it
happens, instead we use the longer string rule of a lexicographical
comparison.
In the C++ codebase, the reason for this behavior is that the
iterator-esque construct they use always yields the empty string before
it exposes it has been fully consumed. This is probably intentional to
support the postfixed `pre` which is, for example, used by NixOS
versions (e.g. unstable post 22.05 is 22.11-pre). We replicate this
behavior using the `Chain` iterator in `VersionPartsIter::new_for_cmp`.
Change-Id: I021c69aa27b0b7deb949dffe50ed18b6de3a7b1f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6720
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
This is based on the [relevant code] in C++ Nix. Our version has more
branches because the C++ one only checks if it is less than or not, so
can save handling a few cases. We on the other hand, can avoid calling
the algorithm twice. It'd be nice to implement proptests for this in the
future and to make sure that this weird little algorithm doesn't violate
the Ord laws.
[relevant code]: cd35bbbeef/src/libstore/names.cc (L81-L94)
Change-Id: I46642e6da5eac7c0883cdce860622cdba04cd12b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6719
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
This was fairly easy, thanks to the work already done by Thomas Frank.
However, the implementation is suboptimal because we parse number parts
only to convert them back to strings afterwards. I've chosen to tackle
this problem in the future, since having an (inefficient) implementation
of splitVersion will be helpful for debugging the slight discrepancies
between C++ Nix and Tvix in the case of compareVersions.
Change-Id: Id6ed8eeb77663ff650c8c53ea952875b1fb7ea84
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6688
Autosubmit: sterni <sternenseemann@systemli.org>
Reviewed-by: grfn <grfn@gws.fyi>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
This makes it possible to call a callable value (builtin or
closure/lambda) directly, without unwrapping it first. This is needed
for pretty much all higher-order functions to work correctly.
This is mostly equivalent to the previous code in coerce_to_string for
calling `__toString`, except it expects the argument(s) to already be
placed on the stack.
Note that the span for the `NotCallable` error is not currently
guaranteed to make any sense, will experiment with this.
Change-Id: I821224368d438a28900858b343defc1817e46a0a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6717
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Generate smaller recursive values for generated Values, and run fewer
cases for the attrs proptests which are particularly egregious.
Change-Id: Ia35c7c120270feaf045be1deb440c87ebb185c27
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6716
Autosubmit: grfn <grfn@gws.fyi>
Reviewed-by: tazjin <tazjin@tvl.su>
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
As we already have a VM passed to the builtins, we can simply execute
the provided closure/lambda in it for each value.
The primary annoyance with this is that we have to clone the upvalues
for each element, but we can try making this cheaper in the
future (it's also a general problem in the VM itself).
Change-Id: I5bcf56d58c509c0eb081e7cf52f6093216451ce4
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6714
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Previously we only matched the outer constructor after forcing which
would mean that we would always return `false` if the inspected value
was a thunk, regardless what value would be present inside.
Change-Id: I361ea6e855e23ef8e5b59098a50b9cd59253803f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6692
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Previously, this would almost always crash because list items are
thunked more often nowadays and selecting from a thunk would fail. Also
we no longer pop from args, accessing it by index should avoid an
unnecessary clone here.
Change-Id: I4410c4c2e28cc255a2c7cf2a5322db3d2c556a0e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6693
Reviewed-by: grfn <grfn@gws.fyi>
Autosubmit: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
And insert the missing newline the C++ Nix test script needs.
Change-Id: I04ddd7268f9caa1414fd23314c281bb7c1e854cf
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6689
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Instead of arity, we pass a array reference to Builtin::new that
describes how many arguments there are and which of them need to be
forced, eliminating the need to force manually.
Note that this change doesn't fix some of the instances where the the
Builtin doesn't consider that the value could be a Thunk.
Change-Id: Iadb58bb79886c30dc6b09dcf0ffad8abf28036a1
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6662
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
TL;DR:
- support `builtins.tail`
- define `ErrorKind::TailEmptyList` and canonical error code
- support basic unit tests
Unsure whether or not the error should be a dedicated `ErrorKind`...
Change-Id: Iae90fda1bb21ce7bdb1aaa2aeb2b8c1e6dcb0f05
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6545
Reviewed-by: wpcarro <wpcarro@gmail.com>
Autosubmit: wpcarro <wpcarro@gmail.com>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Refactor the `force!` macro to a method on `Value` which returns a
smart-pointer-esque type, which simplifies the callsite and eliminates
rightward drift, especially for high-arity builtins.
Change-Id: I97a7837580accfb4bbd03b24f2acdbd38645efa5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6656
Autosubmit: grfn <grfn@gws.fyi>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Previously only the first one was guaranteed to be forced, but we need
to do this for all of them.
Fixes b/190
Change-Id: I76b5667dbfb2f3fde3587e7b91d268cbf32aca00
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6645
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: tazjin <tazjin@tvl.su>
Document the OpAttrs op, paying special attention to the (perhaps
confusing) behavior of taking the number of *pairs*, not the number
of *values*, which will be popped off the stack into the resulting attr
set.
Change-Id: I64df0290308ecae7a5c7e14ead37091d32701507
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6654
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: tazjin <tazjin@tvl.su>
Refactor the environment variable and argument parsing for the tvix repl
to use Clap instead of doing things ad-hoc, and thread through options
obtained from environment variables via explicit arguments rather than
obtaining them from the environment as they're needed. This makes adding
more flags more sustainable, and also makes the binary fully
self-documenting, including supported env vars, via `--help`.
Change-Id: Ib1f6a0cd20056e8c9196760ff755fa5729667760
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6653
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Thunks might be encountered deep in equality comparison (eg nested
inside a list or attr-set), at which point we need to force them in
order to compare them for equality (or else we panic when trying to get
at their value).
Fixes: b/192
Change-Id: I912151085f8298f30d5214c7965251c9266443f2
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6652
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Pass in, but ignore, a mutable reference to the VM to the `nix_eq`
functions, in preparation for using that VM to force thunks during
comparison.
Change-Id: I565435d8dfb33768f930fdb5a6b0fb1365d7e161
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6651
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Using rust's PartialEq trait to implement Nix equality semantics is
reasonably fraught with peril, both because the actual laws are
different than what nix expects, and (more importantly) because certain
things actually require extra context to compare for equality (for
example, thunks need to be forced). This converts the manual PartialEq
impl for Value (and all its descendants) to a *derived* PartialEq
impl (which requires a lot of extra PartialEq derives on miscellanious
other types within the codebase), and converts the previous
nix-semantics equality comparison into a new `nix_eq` method. This
returns an EvalResult, even though it can't currently return an error,
to allow it to fail when eg forcing thunks (which it will do soon).
Since the PartialEq impls for Value and NixAttrs are now quite boring,
this converts the generated proptests for those into handwritten ones
that cover `nix_eq` instead
Change-Id: If3da7171f88c22eda5b7a60030d8b00c3b76f672
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6650
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
This disconnects ownership of the `File` reference in a compiler from
the calling scope, which is required for when we implement `import`.
`import` will need to carry an `Rc<RefCell<CodeMap>>` (or maybe, in
the future, Arc) to give us the ability to add new detected code
files at runtime.
Note that the choice of `Arc` over `Rc` here is not ours - it's the
codemap crate's.
Change-Id: I3aeca4ffc167acbd1701846a332d93550b56ba7d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6630
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
As before, this limits the cases to a relatively small number because
otherwise things get quite large.
Change-Id: I5371dc56418fca52e1dd1d905b20868f647091ba
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6649
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Only running 20 cases for now, since Value can get quite big if you let
it run for a while.
Change-Id: I09ef19da22c789c4869793836c98937c44595340
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6648
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
this was found by proptests!
Change-Id: I16d6a6ece3b20cdddd6f78c94cc87befb1b651e6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6647
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Add an optional config argument to the `<trait>_laws` macros, to allow
configuring the generated tests with a ProptestConfig struct (to limit
the number of cases run)
Change-Id: I2143ddb72c6a870e8be4a9058135b6f9a703039e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6646
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Impl Arbitrary for Value (and NixAttrs and NixList) in the same way we
did for NixString. Value currently only generates non-"internal"
values (no thunks, AttrNotFound, etc.) and can't generate
functions (builtins or closures), because those'd require full
generation of tvix bytecode, which is a bit more work than I'd like to
do now - there's a `todo!` left in the code for a place where we could
allow opting-in to internal values and functions later.
Change-Id: I07a59e2b1d89cfaa912d4ecebd642caf4ddb040a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6627
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
This is pretty boring at the moment, but mostly serves as a foot in the
door in the direction of writing more tests
Change-Id: Id88eb4ec7e53ebb2d5b5c254c8f45ff750238811
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6637
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Invalid integers (eg integers that're too long) end up as error returns
on the `.value()` returned from the literal in the AST - previously we'd
unwrap this error, causing it to panic the compiler, but now we've got a
nice error variant for it (which just unwraps the underlying
std::num::ParseIntError).
Change-Id: I50c3c5ba89407d86659e20d8991b9658415f39a0
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6635
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This bumps rnix-parser to a commit that should be unaffected by the
Nix >= 2.4 bug that prevents it from cloning repositories with filters.
Change-Id: Ie01da95245ec6740fa889eb710819e512202f665
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6634
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Just want to note those down somewhere before I forget them again,
we can delete them later if we have it all figured out.
Change-Id: Icafa2d8fc7ca39e38e9637b7eca6f2bbf487c2b8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6632
Autosubmit: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Add a suite of proptests covering the laws of the handwritten stdlib
trait impls (Eq, Ord, and Hash) for String, generated from a new set of
macros for generating those tests which can be applied to other types.
Change-Id: Ib3276c9e96fca497aece094e5612707d3dc77ccd
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6626
Autosubmit: grfn <grfn@gws.fyi>
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
The From<String> impl for NixString only generates StringRepr::Heap
strings, but we want to make sure we're testing StringRepr::Smol too
Change-Id: I6d04b9cf12ef8462fe2788e0c6414b165f40311d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6629
Autosubmit: grfn <grfn@gws.fyi>
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
It's proptest time!
Add deps on the proptest and test_strategy crates, gated behind a
new (default-enabled) "arbitrary" feature flag so that they don't affect
dependencies of things like tvixbolt that depend on tvix.
These are going in dependencies, not dev-dependencies, so that we can
impl Arbitrary for stuff outside of test modules (which will be
important for integration suites which want to run proptests)
Change-Id: I1613bd3ea9a835e22986ad4e59700e8736007963
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6624
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
This matches the name of the AST node from which it was compiled.
Suggested by sterni in cl/6231
Change-Id: Ia51525158d2f47467c01fce2282005b1a8417a47
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6623
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: grfn <grfn@gws.fyi>
Add the start of a test suite that compares tvix eval results against
nix, using the string repr of the value as the comparison. This shells
out to a nix-instantiate binary, which is configurable as an environment
variable, to eval - there's some extra machinery there to setup a new
nix store as a tempdir to allow running this test inside the nix build
for tvix-eval itself.
Currently this has a macro that'll allow writing lots and lots of
hardcoded tests, but going forward I'm also going to be looking into
adding proptest-based generation of expressions to compare.
Change-Id: I9f4895fab1e668ed2b7dfd6f92f8c80de1bbb16b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6307
Autosubmit: grfn <grfn@gws.fyi>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
... it would be nice if we could thread it through to the `Scope`
stuff (declaring phantoms & locals).
Change-Id: Id3b84e79032b8fbb12138b719e657565355fbc79
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6616
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This trait can be used to convert most structures from rnix-parser
into a codemap::Span. It uses a macro to implement the trait for the
various expression types in the rnix AST, as Rust's silly semantic
versioning restriction stops us from doing a blanket implementation.
This will be used in the next commit to clean up the span handling in
the compiler a bit.
Change-Id: I0a437034e5fa203b5a49c6f25c45932a9f3b2bca
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6615
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
... and emit a warning if anyone decides to use.
Change-Id: Iaa6fe9fa932340e6d0fa9f357155e78823702576
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6611
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Yep.
This is kind of ugly right now. The idea is that the recursive_scope
compilation function is used for recursive sets as well by emitting
the keys. The rest of the logic is pretty much identical.
There is quite a lot of code here that can be cleaned up (duplication
between attrs and let, duplication inside of the recursive scope
compilation function etc.), but I'd like to get it working first and
then make it nice.
Note that nested keys are *not* supported yet.
Change-Id: I45c7cdd5f0e1d35fd94797093904740af3a97134
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6610
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This type is used in the list temporarily populated by the *second*
pass over all identifiers in a recursive scope. This first pass only
serves to make all bindings known to the compiler, without populating
their values yet.
Having a type here is going to be useful once we implement `rec`,
which needs to thread through slightly more information.
Change-Id: Ie33e0f096c5fcb6c864c991255466748b6f0d1eb
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6609
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This needs to be reused between let & `rec` attrs.
Change-Id: I4a3bb90af4be32771b0f9e405c19370e105c0fef
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6608
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This makes the phases of attribute set construction that Nix has very
explicit (inherits, static keys, dynamic keys).
This change focuses on the split between dynamic/static keys by
collecting all dynamic ones while compiling the static ones, and then
phasing them in afterwards. It's possible we also need to do some
additional splitting inside of the inherits.
Change-Id: Icae782e2a5c106e3ce0831dda47ed81c923c0a42
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6530
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This function is necessary for all builtins that expect some form of
path as an argument. It is merely a wrapper around coerce_to_string that
can shortcut if we already have a path. The absolute path check is done
in the same way as in C++ Nix for compatibility, although it should
probably be revised in the long term (think about Windows, for example).
Since coercing to a path is not an operation possible in the language
directly, this function can live in the builtins module as the only
place it is required.
Change-Id: I69ed5455c00d193fea88b8fa83e28907a761cab5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6574
Autosubmit: sterni <sternenseemann@systemli.org>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Define `.len()` method on `NixAttrs` to preallocate the capacity of the result
vector.
Also anchor an errant comment to its context (I think).
Change-Id: I268f15025d453d7b3ae1146558c80e51433dd2a8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6546
Reviewed-by: wpcarro <wpcarro@gmail.com>
Reviewed-by: sterni <sternenseemann@systemli.org>
Autosubmit: wpcarro <wpcarro@gmail.com>
Tested-by: BuildkiteCI
This allows us to get rid of the count local variable which was a bit
confusing. Calling parts.len() multiple times is fine, since the length
doesn't need to be computed.
Change-Id: I4f626729ad1bf23a93cb701385c3f4b50c57456d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6584
Autosubmit: sterni <sternenseemann@systemli.org>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
With this puzzle piece of string compilation in place, `compile_str`
becomes less redundant, as every part now needs to be compiled the same.
The thunking logic becomes a bit trickier, since we need to thunk even
in the case of `count == 1` if the single part is interpolating.
Splitting the inner (shared) code in a separate function turned out to
be easier for making rustc content.
Change-Id: I6a554ca599926ae5907d7acffce349c9616f568f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6582
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
If we have multiple string parts, we need to thunk assembling the
string. If we have a single literal, it is strict (like all literals),
but a single interpolation part may compile to a thunk, depending on how
the expression inside is compiled – we can avoid forcing to early here
compared to the previous behavior.
Note that this CL retains the bug that `"${x}"` is erroneously
translated to `x`, implying e.g. `"${12}" == 12`.
The use of `parts.len()` is unproblematic, since normalized_parts()
builds a `Vec` instead of returning an iterator.
Change-Id: I3aecbfefef65cc627b1b8a65be27cbaeada3582b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6580
Autosubmit: sterni <sternenseemann@systemli.org>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Implement C++ Nix's `EvalState::coerceToString` minus some of the Path
/ store handling. This is currently only used for `toString` which does
all possible coercions, but we've already prepared the weaker coercion
variant which is e.g. used for builtins that expect string arguments.
`EvalState::coerceToPath` is still missing for builtins that need a
path, but it'll be easy to build on top of this.
Change-Id: I78d15576b18921791d04b6b1e964b951fdef22c6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6571
Autosubmit: sterni <sternenseemann@systemli.org>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
OpAttrsIsSet and OpAttrsTrySelect will fail silently if the attribute
set value on the stack is actually a thunk, so we need to make sure to
force at every step of the way.
Emitting the force instructions in the compiler because it is easier to
add, but maybe the VM should do this when handling the relevant opcodes?
Comments welcome.
Change-Id: I65c5ef348d59b2d07c9bb06abb24f9f3e6a0fdb2
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6540
Reviewed-by: grfn <grfn@gws.fyi>
Autosubmit: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
This prevents Nix programs to observe the "internal" type of thunks.
Possibly .type_of() is also an area of the runtime where we should panic
if "internal" would ever be returned.
Change-Id: I9f358044c48ad64896fb6a1b1a42f00a29efac00
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6539
Autosubmit: sterni <sternenseemann@systemli.org>
Reviewed-by: wpcarro <wpcarro@gmail.com>
Reviewed-by: grfn <grfn@gws.fyi>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
The expression inside ${…} may return arbitrary values, including
thunks, so we need to make sure to force them just in case.
Change-Id: Ic11ba00c4c92a10a83becd91233db5f57f6e59c8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6541
Autosubmit: sterni <sternenseemann@systemli.org>
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
