| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
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 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>
|
|
This detects if the second argument of a division is a zero (either as integer
or as float). If so, an error message is displayed.
This fixes b/219.
Change-Id: I50203d14a71482bc757832a2c8dee08eb7d35c49
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7258
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
|
|
This is more generally useful than just inside the VM, until it is
stabilised in Rust itself.
Change-Id: Id9aa3d5b533ff38e3d2c6b85ad484394fdd05dcf
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7186
Tested-by: BuildkiteCI
Autosubmit: tazjin <tazjin@tvl.su>
Reviewed-by: grfn <grfn@gws.fyi>
Reviewed-by: Adam Joseph <adam@westernsemico.com>
|
|
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 variable name `local_idx` is used here for a StackIdx, which invites
confusion.
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Change-Id: I2e22db90acdc0d29586ee5b72ea18d42d93badcb
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7086
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
|
|
Lists are compared lexicographically in C++ nix as of [0], and our
updated nix test suites depend on this. This implements comparison of
list values in `Value::nix_cmp` using a very similar algorithm to what
C++ does - similarly to there, this requires passing in the VM so we can
force thunks in the list elements as we go.
[0]: https://github.com/NixOS/nix/commit/09471d2680292af48b2788108de56a8da755d661#
Change-Id: I5d8bb07f90647a1fec83f775243e21af856afbb1
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7070
Autosubmit: grfn <grfn@gws.fyi>
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
|
|
Rather than implementing all of the interesting semantics of value
comparison with a macro bound to the VM, implement the bulk of the logic
with a method on Value itself that returns an Ordering, and then use the
macro to implement the comparison against that Ordering. This has no
functional change, but paves the way to implementing lexicographic
comparison of list values, which is supported in the latest version of
upstream nix.
Change-Id: I8af1a020b41577021af5939f5edc160c407d4a9e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7069
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
|
|
Since we push arguments onto a stack when calling multi-argument
functions, we actually were ending up calling `call_with` with the
arguments in the *reverse order* - we patched around this by passing the
arguments in the reverse order for `foldl'`, but it makes more sense to
have them just be the order that the function would be called with in
user surface code instead.
Change-Id: Ifddb98f46970ac89872383709c3ce758dc965c65
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7067
Reviewed-by: sterni <sternenseemann@systemli.org>
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
|
|
When capturing an upvalue, return a detailed TvixBug error that
contains metadata about what exactly was missing.
This particular thing helps with debugging some scope issues we still
seem to have.
Change-Id: I1089a1df4b3bbc63411a4907c3641a5dc3fad984
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7058
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
|
|
This is done via a new `deepForce` function on Value. Since values can
be cyclical (for example, see the test-case), we need to do some extra
work to avoid RefCell borrow errors if we ever hit a graph cycle:
While deep-forcing values, we keep a set of thunks that we have
already seen and avoid doing any work on the same thunk twice. The set
is encapsulated in a separate type to stop potentially invalid
pointers from leaking out.
Finally, since deep_force is conceptually similar to
`VM::force_for_output` (but more suited to usage in eval since it
doesn't clone the values) this removes the latter, replacing it with
the former.
Co-Authored-By: Vincent Ambo <tazjin@tvl.su>
Change-Id: Iefddefcf09fae3b6a4d161a5873febcff54b9157
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7000
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
Reviewed-by: tazjin <tazjin@tvl.su>
|
|
When forcing thunks in `force_with_output`, the call stack of the VM
is actually empty (as the calls are synthetic and no longer part of
the evaluation of the top-level expression).
This means that Tvix crashed when constructing error spans for the
`fallible` macro, as the assumption of there being an enclosing span
was violated.
To work around this, we instead pass the span for the whole top-level
expression to force_for_output and set this as the span for the
enclosing error chain. Existing output logic will already avoid
printing the entire expression as an error span.
This fixes b/213.
Change-Id: I93978e0deaf5bcb0f47a6fa95b3f5bebef5bad4c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7052
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
|
|
Failures to resolve a nix search path lookup in angle brackets can be
caught using tryEval (if it reaches the runtime). Resolving relative
paths (either to the current directory or the current user's home) can
never be caught, even if they happen inside a thunk at runtime (which is
currently the case for home-relative paths).
Change-Id: I7f73221df66d82a381dd4063358906257826995a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7025
Autosubmit: sterni <sternenseemann@systemli.org>
Reviewed-by: Adam Joseph <adam@westernsemico.com>
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
|
|
This commit deduplicates the Thunk-like functionality from Closure
and unifies it with Thunk.
Specifically, we now have one and only one way of breaking reference
cycles in the Value-graph: Thunk. No other variant contains a
RefCell. This should make it easier to reason about the behavior of
the VM. InnerClosure and UpvaluesCarrier are no longer necessary.
This refactoring allowed an improvement in code generation:
`Rc<RefCell<>>`s are now created only for closures which do not have
self-references or deferred upvalues, instead of for all closures.
OpClosure has been split into two separate opcodes:
- OpClosure creates non-recursive closures with no deferred
upvalues. The VM will not create an `Rc<RefCell<>>` when executing
this instruction.
- OpThunkClosure is used for closures with self-references or
deferred upvalues. The VM will create a Thunk when executing this
opcode, but the Thunk will start out already in the
`ThunkRepr::Evaluated` state, rather than in the
`ThunkRepr::Suspeneded` state.
To avoid confusion, OpThunk has been renamed OpThunkSuspended.
Thanks to @sterni for suggesting that all this could be done without
adding an additional variant to ThunkRepr. This does however mean
that there will be mutating accesses to `ThunkRepr::Evaluated`,
which was not previously the case. The field `is_finalised:bool`
has been added to `Closure` to ensure that these mutating accesses
are performed only on finalised Closures. Both the check and the
field are present only if `#[cfg(debug_assertions)]`.
Change-Id: I04131501029772f30e28da8281d864427685097f
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7019
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
|
|
Validate "closed formals" (formal parameters without an ellipsis) via a
new ValidateClosedFormals op, which checks the arguments (in an attr set
at the top of the stack) against the formal parameters on the Lambda in
the current frame, and returns a new UnexpectedArgument error (including
the span of the formals themselves!!) if any arguments aren't allowed
Change-Id: Idcc47a59167a83be1832a6229f137d84e426c56c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7002
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
|
|
Home relative paths depend on the environment to be resolved. We have
elected to do everything that depends on the environment, e.g. resolving
SPATH expressions using NIX_PATH, at runtime, so tvix evaluation would
continue to behave correctly even if we separated the compilation and
execution phases more, e.g. via serializing bytecode. Then the value of
HOME, NIX_PATH etc. could reasonably change in the time until execution,
yielding wrong results if the resolution results were cached in the
bytecode.
We also take the opportunity to fix the broken path concatenation
previously found in the compiler, fixing b/205.
Another thing we could consider is emitting a warning for home relative
path literals, as they are by nature relatively fragile.
One sideeffect of this change is that home path resolution errors
become catchable which is not the case in C++ Nix. This will need to be
fixed up in a subsequent change.
Change-Id: I30bd69b575667c49170a9fdea23a020565d0f9ec
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7024
Autosubmit: sterni <sternenseemann@systemli.org>
Reviewed-by: Adam Joseph <adam@westernsemico.com>
Tested-by: BuildkiteCI
|
|
To assert that OpFindFile is only emitted for specially compiled SPATH
expressions, as well as make sure it doesn't accidentally operate on
“ordinary values”, introduce an UnresolvedPath internal value. If
OpFindFile sees a non-UnresolvedPath value, it'll crash.
Note that this change is not done purely for OpFindFile: We may want to
compile SPATH expressions as function calls to __findFile (like C++ Nix
does) in the future, so the UnresolvedPath value would definitely need
to be an ordinary string again then. Rather, this change is done in
preparation for resolving home dir relative paths at runtime (since they
depend on the environment) for which we'll need a similar mechanism to
OpFindFile.
Change-Id: I6acf287f35197cd9e13377079f972b9d36e5b22e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7023
Autosubmit: sterni <sternenseemann@systemli.org>
Reviewed-by: Adam Joseph <adam@westernsemico.com>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
|
|
I believe this variant is left over from a previous implementation.
If not, please let me know.
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Change-Id: I02a3bf2f63794d09e96a5a92a034c0ad3d1ff221
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7027
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
|
|
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Change-Id: I94ba31ae25c1ff744f929a722c76a0c33cc361ff
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7016
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
|
|
Maybe I misunderstood this part of the code, but the use of `unsafe`
appears unnecessary here? In any event it is the one and only
`unsafe` in the codebase.
Hopefully getting to "no `unsafe` anywhere" is worth the extra
never-taken branch caused by unwrap() instead of unwrap_unchecked()?
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Change-Id: I33fbd5aad9d8307ea82c24b6220412783e1973c6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7011
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
|
|
The case branch in vm.rs for OpResolveWithOrUpvalue is
unreachable/deadcode.
I believe this opcode is unnecessary, since it should always be
statically detectable (at parse-time) whether a reference is to an
upvalue (i.e. enclosing binding); otherwise, and only then, is
with-resolution applicable.
Perhaps I've misunderstood how with-resolution works. If so, please
explain it to me and -1/-2 this CL.
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Change-Id: I4a90b9eb6cb3396df92a6a943d42ecc301871ba0
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7009
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
|
|
Right now we're pretending that the Rust library path_clean does the
same thing that cppnix's canonPath() does. This is not true. It's
close enough for the test suite, but may come back to bite us.
Let's create our own canon_path() function and call that in all the
places where we intend to match the behavior of cppnix's
canonPath(). That way when we fix this we can fix it once, in one
place.
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Change-Id: Ia6f9577f62f49ef352ff9cfa5efdf37c32d31b11
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6993
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
|
|
Since NixString is the Rust type for nix strings, people might
mistake NixPath for the Rust type of nix paths, which it is not.
Let's call it NixSearchPath instead.
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Change-Id: Ib2ea155c4b27cb90d6180a04ea7b951d86607373
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6927
Reviewed-by: kanepyork <rikingcoding@gmail.com>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
|
|
Previously, the VM assumed that if an error was returned from `run()`,
the evaluation was "finished" and the state of the VM didn't matter.
This used to be a reasonable assumption, but now that we've got
`tryEval` around we need to actually make sure that we clean up after
ourselves if we're about to return an error. Specifically, if the *last*
instruction in an evaluation frame returns an error, we previously
wouldn't pop that evaluation frame, which could cause all sorts of
bizarre errors based on what happened to be in the stack at the time.
This commit splits out a `run_op` method from `VM::run`, and uses that
to check the evaluation frame return condition even if the op we're
running is about to return an error, and pop the evaluation frame if
we're at the last instruction.
Change-Id: Ib40649d8915ee1571153cb71e3d76492542fc3d7
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6940
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
|
|
Change-Id: I1937d37551503a0b6bb0ac899d067302e4791e5f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6939
Reviewed-by: grfn <grfn@gws.fyi>
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
|
|
Change-Id: I1113921c46010021b61c412d74d60193c156e0f1
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6937
Reviewed-by: grfn <grfn@gws.fyi>
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
|
|
Currently, the span on *all* thunk force errors is the span at which the
thunk is forced, which for recursive thunk forcing ends up just being
the same span over and over again. This changes the span on thunk force
errors to be the span at which point the thunk is *created*, which is a
bit more helpful (though the printing atm is a little... crowded). To
make this work, we have to thread through the span at which a thunk is
created into a field on the thunk itself.
Change-Id: I81474810a763046e2eb3a8f07acf7d8ec708824a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6932
Autosubmit: grfn <grfn@gws.fyi>
Reviewed-by: Adam Joseph <adam@westernsemico.com>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
|
|
Previously the various call functions either returned `EvalResult<()>`
or `EvalResult<Value>`, which was confusing.
Now only vm::call_with returns a Value directly, and other parts of
the API just leave the stack top in the post-call state.
This makes it easier to reason about what's going on in non-tail-call
cases (which are making a comeback).
Change-Id: I264ffc683a11aca72dd06e2220a5ff6e7c5fc2b0
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6936
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
|
|
This commit implements (lazy) resolution of `<...>` paths via either the
NIX_PATH environment variable, or the -I command-line flag - both
handled via EvalOptions. As a result, EvalOptions can no longer derive
Copy, meaning we have to clone it at each line of the repl - this is
probably not a huge deal as repl performance is not exactly an inner
loop and we're not cloning very much.
Internally, this works by creating a thunk which pushes a constant
containing the string inside the brackets to the stack, then a new
opcode to resolve that path via the `NixPath`. To get that opcode to
work, we now have to pass in the NixPath when constructing the VM.
This (intentionally) leaves out proper implementation of path resolution
via `findFile` (cppnix just calls whatever identifier called findFile is
in scope!!!) as that's widely considered a bit of a misfeature, but if
we do decide to implement that down the road it likely wouldn't be more
than a few extra ops within the thunk introduced here.
Change-Id: Ibc979b7e425b65cbe88599940520239a4a10cee2
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6918
Autosubmit: grfn <grfn@gws.fyi>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
|
|
Implement adding paths and strings via OpAdd. Since the nix rules are
quite obscure, I'm electing to test this one with an oracle test to
avoid the danger of getting the actual asserted result wrong.
Change-Id: Icdcca3690ca2e8459e386c1f29cc48eaaa39e9a3
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6914
Autosubmit: grfn <grfn@gws.fyi>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
|
|
In order to behave nicely with tryEval, asserts need to leave the
instruction pointer in a reasonable place even if they fail - whereas
with the previous implementation catching a failed assert would still
end up running the op for the *body* of the assert. With this change, we
compile asserts much more like an `if` expression with conditional jumps
rather than having an OpAssert op.
Change-Id: I1b266c3be90185c84000da6b1995ac3e6fd5471b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6925
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
|
|
The process of calling a function from a builtin, especially if it's got
more than 1 arrgument, is reasonably involved and easy to get wrong due
to having to interact directly with the stack - instead of having that
done entirely manually in builtins, this wraps it up in a new
`call_with` function which handles pushing arguments onto the stack and
recursively calling the (partially applied) function.
Change-Id: I14700c639a0deca53b9a060f6d70dbc7762e9007
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6910
Autosubmit: grfn <grfn@gws.fyi>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
|
|
Factor out the construction of Value::Attrs (including the Rc) into a
new `attrs` constructor function, to abstract away the presence of the
Rc itself.
Change-Id: I42fd4c3841e1db368db999ddd651277ff995f025
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6892
Autosubmit: grfn <grfn@gws.fyi>
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: tazjin <tazjin@tvl.su>
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
|
|
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>
|
|
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
|
|
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 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
|
|
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 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>
|
|
Extend and export the `cmp_op`, and this becomes trivial.
Change-Id: I9c93fa4db0f5a1fc8b56928ea144676f79247de1
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6557
Autosubmit: wpcarro <wpcarro@gmail.com>
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
|
|
This makes for slightly nicer error messages if something isn't, well,
callable.
Change-Id: I821c8d7447b93aea9ccaaa52ed329de0cca4b18e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6718
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
|
|
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>
|
|
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 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>
|
|
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
|
|
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
|
|
Avoids accidentally dropping one on the floor if we add more, pointed
out by sterni in cl/6372
Change-Id: Ib7bb0ce9c8331c8337003d20c4d5240dfae1c32a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6570
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
|
