Age | Commit message (Collapse) | Author | Files | Lines |
|
This forces users to pass the fully constructed set of globals to the
VM, making it harder to accidentally "lose" the set while weak
references to it still exist.
This doesn't modify any functionality, but is laying the foundation
for simplifying some of the builtins behaviour that has grown more
complex again.
Change-Id: I5120f97861c65dc46d90b8a4e2c92ad32cc53e03
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7877
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
|
|
This makes it possible for users to add additional context to an
error, which will then be rendered as an additional secondary span in
the formatted error output.
We should strive to do this basically anywhere errors are raised that
can occur multiple times, *especially* during type casts. This was
triggered by me debugging a type cast error attached to a fairly
large-ish span (a builtin invocation).
Change-Id: I51be41fabee00cf04de973935daf34fe6424e76f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7849
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
|
|
Instead of having a representation of suspended native thunks that
involves constructing a fake code chunk, make these thunks a
first-class part of the internal thunk representation.
The previous code was not that simple to understand, and actually
contained a critical bug which could lead to Tvix crashes. This
version fixes the particular instance of that bug, but instead
uncovers another (b/238) which can still lead to Tvix crashes.
Fixes: b/237.
Change-Id: I771d03864084d63953bdbb518fec94487481f839
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7750
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
|
|
This is unnecessary, Rc already provides all the boxing we need.
Change-Id: I08cf0939c48da43f04c847526c7e5dae5336d528
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7749
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
Reviewed-by: sterni <sternenseemann@systemli.org>
|
|
Instead of going through Vec/BTreeMap for generating our internal
types, use the proptest strategies from imbl.
The one thing I couldn't figure out in the previous implementation is
where the ranges/sizes of generated collections came from. The
strategies in proptest use different types (Range, with an unknown
default value, and SizeRange with 0..100). I've opted to specify
0..100 directly, but we can probably make it configurable.
Change-Id: I749bc4c703fe424099240cab822b1642e5216361
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7791
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
|
|
External implementors of builtins must be able to force values, which
necessitates publishing a bunch more items from the crate.
Change-Id: I8f6b8ae88156aae417dbe630a698d123d0c1c8d4
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7830
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
|
|
This CL addresses clippy warning len_without_is_empty
which expects `.is_empty()` method to be present when
implementing `.len()` method for an item.
Change-Id: I8878db630b9ef5853649a906b764a33299bb5dc8
Signed-off-by: Aaqa Ishtyaq <aaqaishtyaq@gmail.com>
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7806
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
|
|
Implements `Serialize` for `tvix_eval::Value`. Special care is taken
with serialisation of attribute sets, and forcing of thunks.
The tests should cover both cases well.
Change-Id: I9bb135bacf6f87bc6bd0bd88cef0a42308e6c335
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7803
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Autosubmit: tazjin <tazjin@tvl.su>
|
|
Co-Authored-By: Vincent Ambo <tazjin@tvl.su>
Change-Id: Ib6f7d1f4f4faac36b44f5f75cccc57bf912cf606
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7626
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
|
|
Change-Id: I0d71b82eb7ddc1e457b0996b0668006f55f56751
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7790
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
|
|
With this is_valid_nix_identifier should line up with the upstream lexer
definition:
ID [a-zA-Z\_][a-zA-Z0-9\_\'\-]*
While we're working on this, add a simple test checking the various
formatting rules. Interestingly, it would not be suitable as an identity
test, since you have to write
{ "assert" = null; }
in order to avoid an evaluation error, but C++ Nix is happy to print
this as
{ assert = null; }
– maybe should be considered to be a bug.
Change-Id: I0a4e1ccb5033a80f3767fb8d1c4bba08d303c5d8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7744
Autosubmit: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
|
|
The `im::OrdMap` is already small and cheap to copy while sharing
memory, so this is not required anymore.
Only the `KV` variant may have slightly larger content, but in
practice this doesn't seem to make a difference when comparing the two
variants and this one is less complicated.
Change-Id: I64a563b209a2444125653777551373cb2989ca7d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7677
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
|
|
This uses the `im::OrdMap` for `NixAttrs` to enable sharing of memory
between different iterations of a map.
This slightly speeds up eval, but not significantly. Future work might
include benchmarking whether using a `HashMap` and only ordering in
cases where order is actually required would help.
This switches to a fork of `im` that fixes some bugs with its OrdMap
implementation.
Change-Id: I2f6a5ff471b6d508c1e8a98b13f889f49c0d9537
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7676
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
|
|
The conversion from im::Vector -> Vec is cheaper for NixList
construction (of course), so where possible we should make use of
that.
This updates most builtins dealing with lists to use Vector directly,
and marks the function constructing NixList from Vec as deprecated so
that we get appropriate warnings in places where it's still in use.
These places are currently inside of JSON serialisation logic which is
in flux right now, so lets leave them as-is until it's stabilised.
Change-Id: I037f12a2800f2576db4d9526bd935efd079163f0
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7671
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
|
|
This is a persistent, structurally sharing data structure which is
more efficient in some of our use-cases. I have verified the
efficiency improvement using `hyperfine` repeatedly over expressions
on nixpkgs.
Lists are not the most performance-critical structure in Nix (that
would be attribute sets), but we can already see a small (~5-10%)
improvement.
Note that there are a handful of cases where we still go via `Vec`
that need to be fixed, most notable for `builtins.sort` which can not
currently be implemented directly using `im::Vector` because of a
restrictive type bound.
Change-Id: I237cc50cbd7629a046e5a5e4601fbb40355e551d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7670
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
|
|
It's been a while since the last time, so quite a lot of stuff has
accumulated here.
Change-Id: I0762827c197b30a917ff470fd8ae8f220f6ba247
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7597
Reviewed-by: grfn <grfn@gws.fyi>
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
|
|
Introduces continuation-passing-based trampolining of thunk forcing to
avoid recursing when forcing deeply nested expressions.
This is required for evaluating large expressions.
This change was extracted out of cl/7362.
Co-authored-by: Vincent Ambo <tazjin@tvl.su>
Co-authored-by: Griffin Smith <grfn@gws.fyi>
Change-Id: Ifc1747e712663684b2fff53095de62b8459a47f3
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7551
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
|
|
Change-Id: I287282a195d6f752260242739332b2357791974a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7625
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
|
|
... if they are known. We currently do not propagate names correctly
for curried functions.
Change-Id: I19d57fb30a5c0000ccdf690b91076f6b2191de23
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7596
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
|
|
This value creates a human-readable explanation of a value. This can
be used to implement documentation related functionality.
For some values, the amount of information displayed can be expanded
quite a bit.
Change-Id: Ie8c400feae909e7680af163596f99060262e4241
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7592
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
|
|
This "ties the knot" of importing files into a store when referring
to them through path literals, e.g. inside of strings.
I'm not yet sure if this interface is sufficient for
builtins.path (which we haven't implemented at all yet), but it's
enough to wire up eval & store initially.
In the default implementations nothing interesting happens in this
function at all.
Change-Id: Ie01ff4161617d1e743a68dbd1a5e54c1b40c0990
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7582
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
|
|
This lets users set the `io_handle` field on an `Evaluation`, which is
then propagated to the VM.
Change-Id: I616d7140724fb2b4db47c2ebf95451d5303a487a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7566
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
|
|
This type carries the information required for calculating a
span (i.e. the chunk and offset), instead of the span itself. The span
is then only calculated in cases where it is required (when throwing
errors).
This reduces the eval time for
`builtins.length (builtins.attrNames (import <nixpkgs> {}))` by *one
third*!
The data structure in chunks that carries span information reduces
in-memory size by trading off the speed of retrieving span
information. This is because the span information is only actually
required when throwing errors (or emitting warnings).
However, somewhere along the way we grew a dependency on carrying span
information in thunks (for correctly reporting error chains). Hitting
the code paths for span retrieval was expensive, and carrying the
spans in a different way would still be less cache-efficient. This
change is the best tradeoff I could come up with.
Refs: b/229.
Change-Id: I27d4c4b5c5f9be90ac47f2db61941e123a78a77b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7558
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
|
|
Having thunks which, when forced, execute native Rust code rather
than interpreted opcodes lets us avoid having to bundle
`src/libexpr/primops/derivation.nix` like cppnix does by implementing
it in Rust instead.
Change-Id: If91d77a6736234321eee87ba4b4777eed5a3fe1c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7450
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
|
|
Change-Id: I0fa069fbeff6718a765ece948c2c1bce285496f7
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7449
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
|
|
Change-Id: I595087eff943d38a9fc78a83d37e207bb2ab79bc
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7443
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
|
|
Fixes b/212. Based on feedback in https://cl.tvl.fyi/c/depot/+/7492, all
uses of `NixAttrs::from_map` have been removed. Only `from_iter` and
`from_kv` remain.
Change-Id: I52e25f73018c2aa1843197427516b7a852503e2c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7500
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Autosubmit: IslandUsurper <lyle@menteeth.us>
|
|
Allows for the removal of some BTreeMap usage when constructing NixAttrs
by allowing any iterator over 2-tuples to build a NixAttrs. Some
instances of BTreeMap didn't have anything to do with making NixAttrs,
and some were just the best tool for the job, so they are left using the
old `from_map` interface.
Change-Id: I668ea600b0d93eae700a6b1861ac84502c968d78
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7492
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
|
|
Change-Id: I3a55413e5004777b90c06cd8655f26abb2faf39b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7448
Autosubmit: Adam Joseph <adam@westernsemico.com>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
|
|
Change-Id: I2ab53453ed7370b520bb929ef7285e4f23eec65b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7453
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Autosubmit: Adam Joseph <adam@westernsemico.com>
|
|
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Change-Id: I92acb7e6099a4796d953b2d4d02cca4076ed0fb1
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7426
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
|
|
With this change, the test introduced by cl/7370 passes.
Change-Id: Ie7d2f02a59d61151f14ebd328e6cfa5892cacfb0
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7375
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Autosubmit: Adam Joseph <adam@westernsemico.com>
|
|
This passes all the function/thunk-pointer-equality tests in
cl/7369.
Change-Id: Ib47535ba2fc77a4f1c2cc2fd23d3a879e21d8b4c
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7358
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
|
|
Change-Id: If81ff414dba10a0448b905eec373730a68795376
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7376
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Autosubmit: Adam Joseph <adam@westernsemico.com>
|
|
See cl/7368
Change-Id: I97630994c3d65f4d16414a0da236ce000a5b6d33
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7374
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
|
|
See cl/7372; Nix equality semantics require the ability to track
pointer equality of upvalue-sets.
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Change-Id: I82ba517499cf370189a80355e4e46a5caaab7153
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7373
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
|
|
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Change-Id: Ia373eb30d8516a056f1349f9011dee9816593d6f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7357
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
|
|
Change-Id: I3b1284e28c350bfed84d643ae7f922f3487e1f2a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7355
Autosubmit: Adam Joseph <adam@westernsemico.com>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
|
|
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Change-Id: Ib813d794177c623bf2f12fc2e6a6f304089607d1
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7356
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
|
|
When we start unrecursivifying (sp?) things, Rust's borrow checker
is going to be a headache; its magic only works when you use the CPU
stack as your call stack.
Fixing the borrow checker issues usually involves adding lots of
`clone()`s. Right now `NixList` is the only variant of `Value` that
isn't cheap to clone() -- all the others are either a wrapper around
Rc or else are of bounded size.
Note that this requires dropping the `DerefMut for NixList` instance
and using `Vec<Value>` instead in those situations.
Change-Id: I5a47df66855342aa2064f8f3cb7934ff422d26bd
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7359
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
|
|
When comparing Nix values for equality, an issue can occur where
recursive values contain thunks to themselves which causes borrow
errors when forcing them for comparison later down the line.
To work around this we clone the values for now. There might be some
optimisations possible like checking for thunk equality directly and
short-circuiting on that (we have to check what Nix does).
Change-Id: I7e75c992ea68f100058f52b4b46168da7d671994
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7314
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
|
|
When passing multiple arguments, every intermediate callable needs to
be forced as this is expected by the VM's call_value function.
Also adds a debug assertion for this which makes it easier to spot
exactly what went wrong.
Change-Id: I3aa519cb6cdaab713bd18282bef901c4cd77c535
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7312
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
|
|
Add a new `documentation: Option<&'static str>` field to Builtin, and
populate it in the `#[builtins]` macro with the docstring of the builtin
function, if any.
Change-Id: Ic68fdf9b314d15a780731974234e2ae43f6a44b0
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7205
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
|
|
Refactor the arguments of a Builtin to be a vec of a new BuiltinArgument
struct, which contains the old strictness boolean and also a static
`name` str - this is automatically determined via the ident for the
corresponding function argument in the proc-macro case, and passed in in
the cases where we're still manually calling Builtin::new.
Currently this name is unused, but in the future this can be used as
part of a documentation system for builtins.
Change-Id: Ib9dadb15b69bf8c9ea1983a4f4f197294a2394a6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7204
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
|
|
Change-Id: Idd4ae78ef55891d89b72b5c2f3afc8b697b4b26e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7189
Reviewed-by: grfn <grfn@gws.fyi>
Reviewed-by: Adam Joseph <adam@westernsemico.com>
Tested-by: BuildkiteCI
Autosubmit: tazjin <tazjin@tvl.su>
|
|
It is very confusing that this opcode is called DataLocalIdx, but it
carries a StackIdx rather than a LocalIdx. It seems like this
really ought to be called DataStackIdx, but maybe I've
misunderstood; if so please explain it to me.
Change-Id: I91f6ffa759412beef0b91d3c19ec0d873fe51b99
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7088
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
|
|
The impl Display for NixAttrs needs to wrap double quotes around any
keys which are not valid Nix identifiers. This commit does that,
and adds a test (which fails prior to this commit and passes after
this commit).
Change-Id: Ie31ce91e8637cb27073f23f115db81feefdc6424
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7084
Autosubmit: Adam Joseph <adam@westernsemico.com>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
|
|
It isn't possible to implement PartialEq properly for Value, because
any sensible implementation needs to force() thunks, which cannot be
done without a `&mut VM`.
The existing derive(PartialEq) has false negatives, which caused the
bug which cl/7142 fixed. Fortunately that bug was easy to find, but
a silent false negative deep within the bowels of nixpkgs could be a
real nightmare to hunt down.
Let's just remove the PartialEq impl for Value, and the other
derive(PartialEq)'s that depend on it.
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Change-Id: Iacd3726fefc7fc1edadcd7e9b586e04cf8466775
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7144
Reviewed-by: kanepyork <rikingcoding@gmail.com>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
|
|
The current implementation of nix_eq will force one level of thunks
and then switch to the (non-forcing) rust Eq::eq() method. This
gives incorrect results for lists-of-thunks.
This commit changes nix_eq() to be recursive.
A regression test (which fails prior to this commit) is included.
This fix also causes nix_tests/eval-okay-fromjson.nix to pass, so it
is moved out of notyetpassing.
Change-Id: I655fd7a5294208a7b39df8e2c3c12a8b9768292f
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7142
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
|
|
This is a bit tricky because the comparator can throw errors, so we
need to propagate them out if they exist and try to avoid sorting
forever by returning a reasonable ordering in this case (as
short-circuiting is not available).
Co-Authored-By: Vincent Ambo <tazjin@tvl.su>
Change-Id: Icae1d30f43ec1ae64b2ba51e73ee467605686792
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7072
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
|