about summary refs log tree commit diff
path: root/tvix/eval/src/vm.rs (follow)
AgeCommit message (Collapse)AuthorFilesLines
2022-11-26 r/5339 fix(tvix/eval): OpAdd must weakly stringify if either arg is stringAdam Joseph1-1/+12
Tests included. Change-Id: I7a4905d6103813373e383e2e8629c5fd243d6bca Reviewed-on: https://cl.tvl.fyi/c/depot/+/7377 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: Adam Joseph <adam@westernsemico.com>
2022-11-26 r/5324 feat(tvix/eval): wrap Closure::upvalues in RcAdam Joseph1-1/+2
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
2022-11-23 r/5301 feat(tvix/eval): make NixList::clone() cheapAdam Joseph1-2/+3
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
2022-11-21 r/5297 fix(tvix/eval): ensure callable is forced when using call_withVincent Ambo1-1/+5
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>
2022-11-10 r/5276 feat(tvix/eval): detect division by zerojhahn1-1/+14
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>
2022-11-06 r/5254 refactor(tvix/eval): move `unwrap_or_clone_rc` to lib moduleVincent Ambo1-6/+1
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>
2022-11-05 r/5250 refactor(tvix/eval): rename Opcode::DataLocalIdx to DataStackIdxAdam Joseph1-4/+4
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
2022-11-04 r/5240 fix(tvix_eval): {stack,local}_idx confusionAdam Joseph1-4/+4
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
2022-10-29 r/5221 feat(tvix/eval): Implement comparison for listsGriffin Smith1-1/+1
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
2022-10-24 r/5193 refactor(tvix/eval): Implement value comparison with a methodGriffin Smith1-23/+18
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>
2022-10-23 r/5187 fix(tvix/eval): Use natural arg order for call_withGriffin Smith1-1/+2
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
2022-10-23 r/5179 chore(tvix/eval): return detailed TvixBug if an upvalue is missingVincent Ambo1-1/+17
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>
2022-10-22 r/5175 feat(tvix/eval): Implement builtins.deepSeqGriffin Smith1-56/+9
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>
2022-10-22 r/5174 fix(tvix/eval): use top-level span for `force_with_output`Vincent Ambo1-6/+27
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>
2022-10-21 r/5172 fix(tvix): distinguish search- and relative path resolution errorssterni1-1/+1
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>
2022-10-19 r/5159 feat(tvix/eval): deduplicate overlap between Closure and ThunkAdam Joseph1-27/+32
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>
2022-10-17 r/5154 feat(tvix/eval): Validate closed formalsGriffin Smith1-0/+15
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
2022-10-16 r/5147 fix(tvix/eval): resolve home relative paths at runtimesterni1-0/+20
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
2022-10-16 r/5146 refactor(tvix/eval): make OpFindFile use internal UnresolvedPathsterni1-9/+15
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>
2022-10-16 r/5140 refactor(tvix/eval) remove Value::DynamicUpvalueMissingAdam Joseph1-4/+1
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>
2022-10-14 r/5131 docs(tvix/eval) vm: explain VM::{frames,stack,with_stack}Adam Joseph1-2/+12
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
2022-10-14 r/5129 feat(tvix/eval): eliminate the only `unsafe` in the codebaseAdam Joseph1-4/+2
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>
2022-10-14 r/5128 refactor(tvix/eval): remove OpResolveWithOrUpvalueAdam Joseph1-19/+0
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>
2022-10-13 r/5119 refactor(tvix/eval): factor out all calls to canon_pathAdam Joseph1-3/+1
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>
2022-10-12 r/5113 refactor(tvix/eval) s/NixPath/NixSearchPath/Adam Joseph1-7/+10
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>
2022-10-11 r/5100 fix(tvix/eval): Pop frames even if running op failsGriffin Smith1-268/+283
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>
2022-10-11 r/5099 feat(tvix/eval): observe stack after exiting call frames/builtinsVincent Ambo1-2/+4
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
2022-10-11 r/5098 fix(tvix/eval): implement functor calling for non-tail-callsVincent Ambo1-0/+15
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
2022-10-10 r/5097 fix(tvix/eval): Actually trace spans for thunksGriffin Smith1-2/+2
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
2022-10-10 r/5094 refactor(tvix/eval): after calling, the caller has to popVincent Ambo1-16/+24
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>
2022-10-10 r/5087 feat(tvix/eval): Initial resolution of `<...>` pathsGriffin Smith1-2/+13
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
2022-10-10 r/5085 feat(tvix/eval): Allow adding strings to pathsGriffin Smith1-5/+14
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
2022-10-10 r/5082 refactor(tvix/eval): Compile OpAssert using conditional jumpsGriffin Smith1-4/+2
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
2022-10-10 r/5077 refactor(tvix/eval): Abstract away calling functionsGriffin Smith1-0/+25
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
2022-10-08 r/5067 refactor(tvix/eval): Encapsulate Value::Attrs constructionGriffin Smith1-2/+2
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
2022-10-07 r/5049 feat(tvix/eval): add method for emitting runtime warningsVincent Ambo1-2/+34
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
2022-10-04 r/5033 refactor(tvix/eval): split observer traits in twoVincent Ambo1-4/+4
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>
2022-10-03 r/5020 fix(tvix/eval): do not fail when finalising non-capturing valuesVincent Ambo1-1/+6
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
2022-10-03 r/5019 feat(tvix/eval): implement tail-calling of __functor attributesVincent Ambo1-19/+37
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
2022-09-30 r/5006 fix(tvix/eval): fix thunk borrowing error in force_for_outputVincent Ambo1-1/+2
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
2022-09-29 r/4990 chore(tvix/eval): remove existing nested key implementationVincent Ambo1-21/+1
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
2022-09-23 r/4963 feat(tvix/eval): implement 'builtins.filter'Vincent Ambo1-1/+1
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>
2022-09-22 r/4960 fix(tvix/eval): handle thunks in arithmetic builtinssterni1-4/+4
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>
2022-09-22 r/4955 feat(tvix/eval): Support builtins.lessThanWilliam Carroll1-13/+20
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>
2022-09-20 r/4944 feat(tvix/eval): track other type in NotCallable error kindVincent Ambo1-2/+2
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>
2022-09-20 r/4943 refactor(tvix/eval): add VM::call_value helper methodVincent Ambo1-12/+23
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>
2022-09-18 r/4909 chore(tvix/eval): Pass in VM to nix_eqGriffin Smith1-1/+1
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>
2022-09-18 r/4908 refactor(tvix/eval): Don't (ab)use PartialEq for Nix equalityGriffin Smith1-1/+2
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>
2022-09-17 r/4890 refactor(tvix/eval): rename OpAttrsIsSet -> OpHasAttrVincent Ambo1-1/+1
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>
2022-09-15 r/4860 fix(tvix/eval): coerce string interpolation parts to stringsterni1-1/+11
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