about summary refs log tree commit diff
path: root/tvix/eval/src/vm.rs
AgeCommit message (Collapse)AuthorFilesLines
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
2022-09-15 r/4857 feat(tvix/eval): implement correct toString behaviorsterni1-3/+3
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
2022-09-13 r/4846 refactor(tvix/eval): cover all Value variants in force_for_outputVincent Ambo1-1/+8
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
2022-09-13 r/4838 refactor(tvix/eval): use CodeIdx wrapper for instruction pointerVincent Ambo1-6/+15
As suggested by sterni in cl/6453. Change-Id: I3cf80d97c11fd7d085ab510f6be4b5f937c791ec Reviewed-on: https://cl.tvl.fyi/c/depot/+/6562 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-11 r/4801 refactor(tvix/eval): capture entire with_stack in upvaluesVincent Ambo1-49/+38
This completely rewrites the handling of "dynamic upvalues" to, instead of resolving them at thunk/closure instantiation time (which forces some values too early), capture the entire with stack of parent contexts if it exists. There are a couple of things in here that could be written more efficiently, but I'm first working through this to get to a bug related to with + recursion and the code complexity of some of the optimisations is distracting. Change-Id: Ia538e06c9146e3bf8decb9adf02dd726d2c651cf Reviewed-on: https://cl.tvl.fyi/c/depot/+/6486 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-11 r/4800 refactor(tvix/eval): introduce Upvalues struct in closures & thunksVincent Ambo1-8/+8
This struct will be responsible for tracking upvalues (and is a convenient place to introduce optimisations for reducing value clones) instead of a plain value vector. The main motivation for this is that the upvalues will have to capture the `with`-stack fully and I want to avoid duplicating the logic for this between the two capturing types. Change-Id: I6654f8739fc2e04ca046e6667d4a015f51724e99 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6485 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-11 r/4793 fix(tvix/eval): avoid forcing with-target until absolutely necessaryVincent Ambo1-4/+11
Change-Id: I00efbbb8b9d3d22f32becf0919c6adf1be8b4b69 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6465 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-10 r/4786 fix(tvix/eval): fix doc comment syntax where applicableVincent Ambo1-11/+11
As pointed out by grfn on cl/6091 Change-Id: I28308577b7cf99dffb4a4fd3cc8783eb9ab4d0d6 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6460 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-10 r/4783 feat(tvix/eval): implement OpTailCallVincent Ambo1-9/+37
If the last operation within a chunk is a function call, the call can be executed in the same call frame without increasing the depth of the call stack. To enable this, a new OpTailCall instruction (similar to OpCall) is introduced, but not yet emitted by the compiler. Change-Id: I9ffbd7da6d2d6a8ec7a724646435dc6ee89712f2 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6457 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-09 r/4782 fix(tvix/eval): force value passed to builtins.toStringVincent Ambo1-1/+1
This introduces a macro to do the forcing, but this solution isn't very nice and also does not work in all cases yet. Change-Id: Icd18862ec47edb82c0efc3af5835a6cb6126f629 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6456 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-09 r/4778 feat(tvix/eval): implement runtime tracing methods for ObserverVincent Ambo1-27/+23
These methods make it possible to trace the runtime execution of the VM through an observer. Change-Id: I90e26853ba2fe44748613e7f761ed5c1c5fc9ff7 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6452 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-09 r/4773 refactor(tvix/eval): index into Chunk with ConstantIdx/CodeIdxVincent Ambo1-4/+4
This is a step towards hiding the internal fields of thunk, and making the interface of the type more predictable. Part of the preparation for implementing observers. Change-Id: I1a88a96419c72eb9e2332b56a2dd94afa47e6f88 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6447 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-09 r/4771 chore(tvix/eval): debug_assert that all jumps are patchedVincent Ambo1-0/+4
Suggestion from sterni in cl/6282 Change-Id: I1adbdda9ff74f55a2f72892ffa524808b305f403 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6445 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-08 r/4768 feat(tvix/eval): print lambda memory adresses in disassemblerVincent Ambo1-3/+4
This makes it easier to track exactly which lambda is which when inspecting e.g. the concrete representation of a thunk. At runtime all lambdas live in an Rc. To make this print the right address, the construction of these Rcs had to be moved up right to the point where the lambda is first emitted (and disassembled). Change-Id: I6070e6c8ac55f0bd697966c4e7c5565c20d19106 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6435 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-08 r/4763 fix(tvix/eval): address current clippy & grfn lintsVincent Ambo1-13/+4
Change-Id: I65c6feb9f817b5b367d37204a1f57acfe4100d97 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6430 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-08 r/4752 refactor(tvix/eval): implement much clearer disassembler outputVincent Ambo1-23/+45
With this change the runtime trace contains much more exact information about the context of the computation (entering/exiting calls etc.) This is in large part due to moving the tracer to be a field on the VM itself, which enables consistent ordering of traces across the execution, and tracing an execution with its *input* instead of *output* stack. Change-Id: Ibe525e6e7d869756501e52bef1a441619ce7332c Reviewed-on: https://cl.tvl.fyi/c/depot/+/6419 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-08 r/4751 chore(tvix/eval): print value when panicking on internalsVincent Ambo1-1/+3
This makes it much easier to figure out what happened while debugging this sort of thing. Change-Id: I2e0e8096709adc647d63c04f213c547c415e5f44 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6418 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-08 r/4748 refactor(tvix/eval): return call frame result from VM::callVincent Ambo1-12/+20
Previously, "calling" (setting up the VM run loop for executing a call frame) and "running" (running this loop to completion) were separate operations. This was basically an attempt to avoid nesting `VM::run` invocations. However, doing things this way introduced some tricky bugs for exiting out of the call frames of thunks vs. builtins & closures. For now, we unify the two operations and always return the value to the caller directly. For now this makes calls a little less effective, but it gives us a chance to nail down some other strange behaviours and then re-optimise this afterwards. To make sure we tackle this again further down I've added it to the list of known possible optimisations. Change-Id: I96828ab6a628136e0bac1bf03555faa4e6b74ece Reviewed-on: https://cl.tvl.fyi/c/depot/+/6415 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-08 r/4745 refactor(tvix/eval): add macros for generating Value castersVincent Ambo1-10/+10
The casting methods of `Value` are pretty verbose, and actually incorrect before this commit as they did not account for inner thunk values. To address this, we first attempt to make them correct by introducing a standard macro to generate them and traverse the inner thunk(s) if necessary. This is likely to be a performance hit as it will now involve more cloning of values. We can do multiple things to alleviate this, but should do some measurements first. Change-Id: If315d6e2afe7b69db727df535bc6cbfb89a691aa Reviewed-on: https://cl.tvl.fyi/c/depot/+/6412 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-08 r/4744 refactor(tvix/eval): pass a VM reference to builtinsVincent Ambo1-1/+1
This makes it possible for builtins to force values on their own, without the VM having to apply a strictness mask to the arguments first. Change-Id: Ib49a94e56ca2a8d515c39647381ab55a727766e3 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6411 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-08 r/4741 feat(tvix/eval): ensure all errors always carry a spanVincent Ambo1-48/+83
Previously error spans were optional because the information about code spans was not available at runtime. Now that this information has been added, the error type will always carry a span. This change is very invasive all throughout the codebase. This is due to the fact that many functions that are called *by* the VM expected to return `EvalResult`, but this no longer works as the span information is not available to those functions - only to the VM itself. To work around this the majority of these functions have been changed to return `Result<T, ErrorKind>` instead and an accompanying macro in the VM constructs the "real" error. Note that this implementatino currently has a bug where errors occuring within thunks will yield the location at which the thunk was forced, not the location at which the error occured within the code. This will be fixed soon, but the commit is large enough as is. Change-Id: Ib1ecb81a4d09d464a95ea7ea9e589f3bd08d5202 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6408 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-07 r/4738 feat(tvix/eval): Support builtins.{add,sub}William Carroll1-0/+1
First pass at supporting `builtins` for tvix. The following tests appear to be WAI: ```shell $ cd tvix/eval $ cargo build $ cargo test ``` Change-Id: I27cce23d503b17a886d1109e285e8b4be4264977 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6405 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-07 r/4706 fix(tvix/eval): address current clippy lintsVincent Ambo1-1/+0
Note that I've allowed `needless_lifetimes` for the attribute set iterator, as I find the type easier to understand with these annotations present. Change-Id: I33abb17837ee4813076cdb9a87f54bac4a37044e Reviewed-on: https://cl.tvl.fyi/c/depot/+/6373 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-07 r/4705 feat(tvix/eval): semi-strictly evaluate output values of the VMVincent Ambo1-1/+35
This essentially makes the VM behave like `nix-instantiate --eval --strict`, i.e. data structures are traversed strictly and thunks are forced. Thunks embedded in closures are not forced. This allows us to re-enable tests that were disabled because they needed to output nested thunk contents, but is overall a behaviour that must be configurable later on, as it is not cmopatible with e.g. an evaluation of nixpkgs. Change-Id: I5303a5c8e4322feab1384fdb7712fecb950afca5 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6372 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-07 r/4700 refactor(tvix/eval): encapsulate all thunk-forcing logic in moduleVincent Ambo1-2/+3
The VM previously took care of repeatedly forcing a thunk until it reached an evaluated state. This logic is now encapsulated inside of the `Thunk::force` implementation. In addition, force no longer returns a reference to the value by default, leaving it up to callers to decide whether they want to borrow the value or not (a helper is provided for this). Change-Id: I2aa7da922058ad1c57fbf8bfc7785aab7971c02b Reviewed-on: https://cl.tvl.fyi/c/depot/+/6365 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-07 r/4688 feat(tvix/eval): implement OpForce in VMVincent Ambo1-1/+11
This operation forces the evaluation of a thunk. There is some potential here for making an implementation that avoids some copies, but the thunk machinery is tricky to get right so the first priority is to make sure it is correct by keeping the implementation simple. Change-Id: Ib381455b02f42ded717faff63f55afed4c8fb7e3 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6352 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>