about summary refs log tree commit diff
path: root/tvix/eval/src/value
AgeCommit message (Collapse)AuthorFilesLines
2022-10-13 r/5119 refactor(tvix/eval): factor out all calls to canon_pathAdam Joseph2-0/+16
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/5111 fix(tvix/eval): remove overlapping instancesAdam Joseph1-12/+0
I broke the build. I didn't understand that before hitting "submit" you need to re-test your changes on latest HEAD (and that CI doesn't do that for you); I failed to re-test cl/6912 following the merge of cl/6907. This commit fixes the build by removing the overlapping instances. Change-Id: I2a720d2c60cc7103b350f78102a8998f93bac828 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6965 Tested-by: BuildkiteCI Autosubmit: Adam Joseph <adam@westernsemico.com> Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-12 r/5110 feat(tvix/eval): From<Into<NixString>> for ValueAdam Joseph1-0/+9
Signed-off-by: Adam Joseph <adam@westernsemico.com> Change-Id: I27615baf4104ab96600708b12a780599a3714bae Reviewed-on: https://cl.tvl.fyi/c/depot/+/6912 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-10 r/5097 fix(tvix/eval): Actually trace spans for thunksGriffin Smith1-18/+30
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/5095 feat(tvix/eval): Implement builtins.readFileGriffin Smith1-0/+7
Change-Id: If3fd0b087009a2bfbad8bb7aca0aa20de906eb12 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6921 Tested-by: BuildkiteCI Reviewed-by: Adam Joseph <adam@westernsemico.com> Reviewed-by: kanepyork <rikingcoding@gmail.com> Autosubmit: grfn <grfn@gws.fyi> Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-10 r/5094 refactor(tvix/eval): after calling, the caller has to popVincent Ambo2-5/+5
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 Smith3-7/+14
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-0/+6
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/5084 feat(tvix/eval): Implement builtins.concatStringsSepGriffin Smith2-0/+21
Change-Id: I6e46bcdbf3b5258b60edb017709fee577eb8ec74 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6907 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-10 r/5083 feat(tvix/eval): Support builtins.readDirWilliam Carroll1-0/+6
Co-authored-by: Griffin Smith <root@gws.fyi> Change-Id: I5ff19efbe87d8f571f22ab0480500505afa624c5 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6552 Autosubmit: wpcarro <wpcarro@gmail.com> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-10 r/5080 feat(tvix/eval): Implement builtins.elemGriffin Smith1-0/+6
Change-Id: Id99c1d33f87ad9866990d3483d3531e9e48f861f Reviewed-on: https://cl.tvl.fyi/c/depot/+/6916 Autosubmit: grfn <grfn@gws.fyi> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-10 r/5075 feat(tvix/eval): Implement builtins.genListGriffin Smith1-0/+6
Change-Id: Iabe28656229f508226b244d81382e517961eb3cf Reviewed-on: https://cl.tvl.fyi/c/depot/+/6901 Autosubmit: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-08 r/5067 refactor(tvix/eval): Encapsulate Value::Attrs constructionGriffin Smith2-2/+10
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-08 r/5066 fix(tvix/eval): Force thunks when comparing against ground valsGriffin Smith1-2/+8
Thunks correctly force when comparing for equality against other thunks, but weren't being forced correctly when comparing against non-thunk values, in either direction. Change-Id: Ia03702895ec4d70aed3445c1b0a9a7a641d1a300 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6897 Autosubmit: grfn <grfn@gws.fyi> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-06 r/5040 refactor(tvix/eval): builtins now contain closuresVincent Ambo1-7/+15
For some upcoming builtins (notably, import) we need to capture arguments in the builtin's implementation. To allow this, we can no longer use function pointers for builtins, but must use a reference-counted closure object instead. Unfortunately this adds an extra pointer operation to every builtin call. We should benchmark this later against having a split builtin representation. Change-Id: I109d98d0e25998870542f47573eb1ec2e546f2a2 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6856 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-10-03 r/5021 refactor(tvix/eval): implement IntoIterator for NixListVincent Ambo1-4/+9
This is the same code as before, just moved into a trait impl to gain access to stuff that needs IntoIterator Change-Id: Iff9375cd05593dd2681fa85ccc7f4554bf944a02 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6842 Autosubmit: tazjin <tazjin@tvl.su> Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-29 r/4990 chore(tvix/eval): remove existing nested key implementationVincent Ambo2-79/+2
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-20 r/4943 refactor(tvix/eval): add VM::call_value helper methodVincent Ambo1-17/+3
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-20 r/4942 test(tvix/eval): Make proptests a bit smallerGriffin Smith2-4/+4
Generate smaller recursive values for generated Values, and run fewer cases for the attrs proptests which are particularly egregious. Change-Id: Ia35c7c120270feaf045be1deb440c87ebb185c27 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6716 Autosubmit: grfn <grfn@gws.fyi> Reviewed-by: tazjin <tazjin@tvl.su> Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-20 r/4940 chore(tvix/eval): clean up this one weird trick ...Vincent Ambo1-3/+0
... Rust developers don't want you to know about! Change-Id: Ic43b670ac0982c726bfb8cd27cb57b17934e4b70 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6715 Reviewed-by: grfn <grfn@gws.fyi> Reviewed-by: sterni <sternenseemann@systemli.org> Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
2022-09-19 r/4918 refactor(tvix/eval): handle forcing in Builtin::applysterni1-4/+12
Instead of arity, we pass a array reference to Builtin::new that describes how many arguments there are and which of them need to be forced, eliminating the need to force manually. Note that this change doesn't fix some of the instances where the the Builtin doesn't consider that the value could be a Thunk. Change-Id: Iadb58bb79886c30dc6b09dcf0ffad8abf28036a1 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6662 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-09-18 r/4914 refactor(tvix/eval): Simplify forcing in builtinsGriffin Smith1-0/+33
Refactor the `force!` macro to a method on `Value` which returns a smart-pointer-esque type, which simplifies the callsite and eliminates rightward drift, especially for high-arity builtins. Change-Id: I97a7837580accfb4bbd03b24f2acdbd38645efa5 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6656 Autosubmit: grfn <grfn@gws.fyi> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-09-18 r/4910 fix(tvix/eval): Force thunks during equality comparisonGriffin Smith1-5/+8
Thunks might be encountered deep in equality comparison (eg nested inside a list or attr-set), at which point we need to force them in order to compare them for equality (or else we panic when trying to get at their value). Fixes: b/192 Change-Id: I912151085f8298f30d5214c7965251c9266443f2 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6652 Autosubmit: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-09-18 r/4909 chore(tvix/eval): Pass in VM to nix_eqGriffin Smith4-19/+54
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 Smith7-113/+191
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-18 r/4906 test(tvix/eval): Add Eq-laws tests for NixAttrsGriffin Smith1-0/+11
As before, this limits the cases to a relatively small number because otherwise things get quite large. Change-Id: I5371dc56418fca52e1dd1d905b20868f647091ba Reviewed-on: https://cl.tvl.fyi/c/depot/+/6649 Autosubmit: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-09-18 r/4905 test(tvix/eval): Add tests for the Eq laws of ValueGriffin Smith1-0/+16
Only running 20 cases for now, since Value can get quite big if you let it run for a while. Change-Id: I09ef19da22c789c4869793836c98937c44595340 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6648 Autosubmit: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-09-18 r/4904 fix(tvix/value): Properly match on Path for PartialEqGriffin Smith1-0/+1
this was found by proptests! Change-Id: I16d6a6ece3b20cdddd6f78c94cc87befb1b651e6 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6647 Autosubmit: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-09-18 r/4902 test(tvix/eval): impl Arbitrary for ValueGriffin Smith4-0/+128
Impl Arbitrary for Value (and NixAttrs and NixList) in the same way we did for NixString. Value currently only generates non-"internal" values (no thunks, AttrNotFound, etc.) and can't generate functions (builtins or closures), because those'd require full generation of tvix bytecode, which is a bit more work than I'd like to do now - there's a `todo!` left in the code for a place where we could allow opting-in to internal values and functions later. Change-Id: I07a59e2b1d89cfaa912d4ecebd642caf4ddb040a Reviewed-on: https://cl.tvl.fyi/c/depot/+/6627 Autosubmit: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-09-17 r/4894 test(tvix/eval): Add proptests covering trait impls for StringGriffin Smith1-0/+11
Add a suite of proptests covering the laws of the handwritten stdlib trait impls (Eq, Ord, and Hash) for String, generated from a new set of macros for generating those tests which can be applied to other types. Change-Id: Ib3276c9e96fca497aece094e5612707d3dc77ccd Reviewed-on: https://cl.tvl.fyi/c/depot/+/6626 Autosubmit: grfn <grfn@gws.fyi> Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-17 r/4893 test(tvix/eval): Test StringRepr::Smol as wellGriffin Smith1-1/+8
The From<String> impl for NixString only generates StringRepr::Heap strings, but we want to make sure we're testing StringRepr::Smol too Change-Id: I6d04b9cf12ef8462fe2788e0c6414b165f40311d Reviewed-on: https://cl.tvl.fyi/c/depot/+/6629 Autosubmit: grfn <grfn@gws.fyi> Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-17 r/4892 test(tvix/eval): impl Arbitrary for NixStringGriffin Smith1-0/+17
Change-Id: I3fe2d410c789429493a1278d571ca8fe74c2a69d Reviewed-on: https://cl.tvl.fyi/c/depot/+/6625 Autosubmit: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-09-16 r/4881 refactor(tvix/eval): fix current clippy lintssterni1-8/+5
Change-Id: I88482453a62955515a0dcc0b243351b2bbac5236 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6618 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-09-15 r/4867 feat(tvix/eval): Support builtins.bitAndWilliam Carroll1-0/+1
Bitwise `and` on integers. Change-Id: I9f2a9182a057af26906683acd97a40dfabbdded8 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6548 Reviewed-by: wpcarro <wpcarro@gmail.com> Autosubmit: wpcarro <wpcarro@gmail.com> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-09-15 r/4865 feat(tvix/eval): implement Value::coerce_to_path()sterni2-1/+10
This function is necessary for all builtins that expect some form of path as an argument. It is merely a wrapper around coerce_to_string that can shortcut if we already have a path. The absolute path check is done in the same way as in C++ Nix for compatibility, although it should probably be revised in the long term (think about Windows, for example). Since coercing to a path is not an operation possible in the language directly, this function can live in the builtins module as the only place it is required. Change-Id: I69ed5455c00d193fea88b8fa83e28907a761cab5 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6574 Autosubmit: sterni <sternenseemann@systemli.org> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-09-15 r/4864 feat(tvix/eval): Support builtins.attrNamesWilliam Carroll1-2/+10
Define `.len()` method on `NixAttrs` to preallocate the capacity of the result vector. Also anchor an errant comment to its context (I think). Change-Id: I268f15025d453d7b3ae1146558c80e51433dd2a8 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6546 Reviewed-by: wpcarro <wpcarro@gmail.com> Reviewed-by: sterni <sternenseemann@systemli.org> Autosubmit: wpcarro <wpcarro@gmail.com> Tested-by: BuildkiteCI
2022-09-15 r/4862 feat(tvix/eval): Support builtins.headWilliam Carroll1-0/+4
TL;DR: - support `builtins.head` - define `ErrorKind::IndexOutOfBounds` and canonical error code - support basic unit tests Change-Id: I859107ffb4e220cba1be8c2ac41d1913dcca37ff Reviewed-on: https://cl.tvl.fyi/c/depot/+/6544 Reviewed-by: wpcarro <wpcarro@gmail.com> Autosubmit: wpcarro <wpcarro@gmail.com> Reviewed-by: sterni <sternenseemann@systemli.org> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-09-15 r/4857 feat(tvix/eval): implement correct toString behaviorsterni1-0/+142
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/4845 fix(tvix/eval): add branch for directly comparing two thunksVincent Ambo1-0/+1
Pointed out by sterni in cl/6370 Change-Id: I324d8049a2702ced8f30ad43a64d63ae79dd0eab Reviewed-on: https://cl.tvl.fyi/c/depot/+/6569 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-13 r/4839 fix(tvix/eval): address current clippy lintsVincent Ambo1-4/+4
Change-Id: I5288849d0e93511b0b5664fa92f1c6882e4a1356 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6563 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-13 r/4836 feat(tvix/eval): implement initial fancy formatting for errorsVincent Ambo1-5/+1
This very closely follows the way it's done for warnings, but errors have a lot more information available in some cases which we do not surface yet. Note also that due to requiring the `CodeMap`, this is not yet called from eval.rs as the way that is threaded through needs to be refactored, so only the method for reporting these errors as strings is implemented so far. Next steps for this will be to add a generic diagnostics module that reduces some of the boilerplate for this between warnings & errors, and which will also give us a good point in the future to switch to a fancier diagnostics crate. Change-Id: If6bb209f8e7a568d866e516a90335b9b2afbf66d Reviewed-on: https://cl.tvl.fyi/c/depot/+/6534 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-09-11 r/4800 refactor(tvix/eval): introduce Upvalues struct in closures & thunksVincent Ambo2-12/+20
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-10 r/4787 fix(tvix/eval): reintroduce 'InvalidAttribuetName' error variantVincent Ambo1-1/+5
As pointed out by sterni in cl/6205, this is actually possible in syntactically valid expressions like { ${12 + 13} = 12; } Change-Id: Id8a1e3aceb551f288f9050c4eea563eb6572f1a7 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6461 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-10 r/4786 fix(tvix/eval): fix doc comment syntax where applicableVincent Ambo3-31/+34
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-08 r/4769 fix(tvix/eval): hold thunk borrow as shortly as possibleVincent Ambo1-7/+6
At the point where control flow exits Thunk::force (which may be due to recursing), it is vital that there is no longer a borrow to the inner thunk representation, otherwise this can cause accidental infinite recursion (which will be detected, but cause failures on valid code). Change-Id: I2846f3142830ae3110a4f5d2299e9d7928634504 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6436 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-08 r/4749 fix(tvix/eval): don't panic when printing a black holeVincent Ambo1-2/+6
This could occur when the disassembler is enabled and tracing the runtime while a thunk is being evaluated, as it would not be possible for the *tracer* to borrow the thunk at this exact moment. However, we know that if the borrowing fails here we are dealing with a not-fully evaluated thunk (blackhole), which should just print the internal representation. Change-Id: I4bdb4f17818d55795368e3d28842048f488f0a91 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6416 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-2/+2
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-62/+42
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-4/+14
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/4742 fix(tvix/eval): thread thunk forcing errors through correctlyVincent Ambo1-3/+3
With this, if an error occurs while forcing a thunk (which is very likely) it is threaded through to the top by wrapping it in the ErrorKind::ThunkForce variant. We could use this to generate "stacktrace-like" error output if we wanted, or simply jump through and discard everything except the innermost error. Change-Id: I3c1c8708c2f73ae062815adf490ce935b1979da8 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6409 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI