about summary refs log tree commit diff
path: root/tvix/eval/src/tests
AgeCommit message (Collapse)AuthorFilesLines
2022-09-20 r/4936 fix(tvix/eval): make sure to deref thunk in type predicate builtinssterni2-0/+35
Previously we only matched the outer constructor after forcing which would mean that we would always return `false` if the inspected value was a thunk, regardless what value would be present inside. Change-Id: I361ea6e855e23ef8e5b59098a50b9cd59253803f Reviewed-on: https://cl.tvl.fyi/c/depot/+/6692 Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-20 r/4933 fix(tvix/eval): make sure to force before selecting in catAttrssterni2-0/+11
Previously, this would almost always crash because list items are thunked more often nowadays and selecting from a thunk would fail. Also we no longer pop from args, accessing it by index should avoid an unnecessary clone here. Change-Id: I4410c4c2e28cc255a2c7cf2a5322db3d2c556a0e Reviewed-on: https://cl.tvl.fyi/c/depot/+/6693 Reviewed-by: grfn <grfn@gws.fyi> Autosubmit: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-19 r/4928 feat(tvix/eval): Support builtins.getAttrWilliam Carroll2-0/+7
Support looking-up values from attrsets by their keys. Change-Id: Ib37a472a511dab145f99ebc849879b3494e8e89f Reviewed-on: https://cl.tvl.fyi/c/depot/+/6554 Reviewed-by: wpcarro <wpcarro@gmail.com> Reviewed-by: sterni <sternenseemann@systemli.org> Autosubmit: wpcarro <wpcarro@gmail.com> Tested-by: BuildkiteCI
2022-09-19 r/4927 feat(tvix/eval): Support builtins.hasAttrWilliam Carroll2-0/+10
See unit tests for examples :) Change-Id: Ieec51d780a7762cc455ca03a9dc1648a0711924a Reviewed-on: https://cl.tvl.fyi/c/depot/+/6553 Reviewed-by: wpcarro <wpcarro@gmail.com> Autosubmit: wpcarro <wpcarro@gmail.com> Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-19 r/4920 fix(tvix/eval): move compareVersions test so cargo test finds themsterni2-0/+13
And insert the missing newline the C++ Nix test script needs. Change-Id: I04ddd7268f9caa1414fd23314c281bb7c1e854cf Reviewed-on: https://cl.tvl.fyi/c/depot/+/6689 Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-19 r/4917 feat(tvix/eval): Support builtins.tailWilliam Carroll2-0/+5
TL;DR: - support `builtins.tail` - define `ErrorKind::TailEmptyList` and canonical error code - support basic unit tests Unsure whether or not the error should be a dedicated `ErrorKind`... Change-Id: Iae90fda1bb21ce7bdb1aaa2aeb2b8c1e6dcb0f05 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6545 Reviewed-by: wpcarro <wpcarro@gmail.com> Autosubmit: wpcarro <wpcarro@gmail.com> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-09-19 r/4916 feat(tvix/eval): Support builtins.substringWilliam Carroll2-0/+19
Nix's `builtins.substring`: - doesn't accept negative indexes for `beg` or `end`. Compare the error messages for: - `builtins.substring -3 5 "testing"` - `builtins.substring 3 -5 "testing"` - `builtins.substring -3 -5 "testing"` - returns an empty string when `beg >= end` - allows `end` to exceed the length of the input string Change-Id: I83af7a099d81b6d537ebe5029d946c7031cb7113 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6555 Reviewed-by: wpcarro <wpcarro@gmail.com> Autosubmit: wpcarro <wpcarro@gmail.com> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-09-18 r/4913 fix(tvix/eval): ensure all thunks are forced in nested selectsVincent Ambo2-0/+6
Previously only the first one was guaranteed to be forced, but we need to do this for all of them. Fixes b/190 Change-Id: I76b5667dbfb2f3fde3587e7b91d268cbf32aca00 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6645 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: tazjin <tazjin@tvl.su>
2022-09-18 r/4911 refactor(tvix/eval): use Clap for arg+env parsingGriffin Smith1-2/+4
Refactor the environment variable and argument parsing for the tvix repl to use Clap instead of doing things ad-hoc, and thread through options obtained from environment variables via explicit arguments rather than obtaining them from the environment as they're needed. This makes adding more flags more sustainable, and also makes the binary fully self-documenting, including supported env vars, via `--help`. Change-Id: Ib1f6a0cd20056e8c9196760ff755fa5729667760 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6653 Autosubmit: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-09-18 r/4910 fix(tvix/eval): Force thunks during equality comparisonGriffin Smith2-0/+12
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-17 r/4888 test(tvix/eval): add a test for lazy evaluation of with namespaceVincent Ambo2-0/+7
Suggested by grfn in cl/6222 Change-Id: I8413af6404786bd43e5cb462a307cc236c4fb9ae Reviewed-on: https://cl.tvl.fyi/c/depot/+/6621 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-17 r/4882 feat(tvix/eval): Support builtins.elemAtWilliam Carroll2-0/+6
(Attempt to) index into a list. Change-Id: I3592e60a79e64d265e34100d4062041b0b410e00 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6551 Reviewed-by: wpcarro <wpcarro@gmail.com> Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: wpcarro <wpcarro@gmail.com> Tested-by: BuildkiteCI
2022-09-16 r/4879 test(tvix/eval): add tests for legacy let syntaxVincent Ambo4-0/+15
Change-Id: I42d6240056c93cf90986ef3fec88b42f804b06ef Reviewed-on: https://cl.tvl.fyi/c/depot/+/6613 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-16 r/4878 test(tvix/eval): add some tests for recursive attribute setsVincent Ambo4-0/+19
Change-Id: I6a4fbe6d5aa6d2ce223bf77330d6c0f512d4300d Reviewed-on: https://cl.tvl.fyi/c/depot/+/6612 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-15 r/4869 feat(tvix/eval): Support builtins.bitXorWilliam Carroll2-0/+13
Bitwise "exclusive-or" on integers. Change-Id: I90a0a15afb3a58662d70e82ea14e48b877476e04 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6550 Autosubmit: wpcarro <wpcarro@gmail.com> Reviewed-by: wpcarro <wpcarro@gmail.com> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-09-15 r/4868 feat(tvix/eval): Support builtins.bitOrWilliam Carroll2-0/+11
Bitwise `or` on integers. Change-Id: I45b0897331d1a9b6840f9d0feedcf10acc67fcec Reviewed-on: https://cl.tvl.fyi/c/depot/+/6549 Autosubmit: wpcarro <wpcarro@gmail.com> Reviewed-by: wpcarro <wpcarro@gmail.com> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-09-15 r/4867 feat(tvix/eval): Support builtins.bitAndWilliam Carroll2-0/+11
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/4866 feat(tvix/eval): Support builtins.attrValuesWilliam Carroll2-0/+5
:) Change-Id: Idf170b506ed6fab035b1e6f61055fee02e5c9be8 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6547 Reviewed-by: wpcarro <wpcarro@gmail.com> Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: wpcarro <wpcarro@gmail.com> Tested-by: BuildkiteCI
2022-09-15 r/4864 feat(tvix/eval): Support builtins.attrNamesWilliam Carroll2-0/+14
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 Carroll2-0/+5
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/4859 fix(tvix/eval): thunk string interpolationsterni2-0/+8
If we have multiple string parts, we need to thunk assembling the string. If we have a single literal, it is strict (like all literals), but a single interpolation part may compile to a thunk, depending on how the expression inside is compiled – we can avoid forcing to early here compared to the previous behavior. Note that this CL retains the bug that `"${x}"` is erroneously translated to `x`, implying e.g. `"${12}" == 12`. The use of `parts.len()` is unproblematic, since normalized_parts() builds a `Vec` instead of returning an iterator. Change-Id: I3aecbfefef65cc627b1b8a65be27cbaeada3582b Reviewed-on: https://cl.tvl.fyi/c/depot/+/6580 Autosubmit: sterni <sternenseemann@systemli.org> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-09-15 r/4857 feat(tvix/eval): implement correct toString behaviorsterni2-7/+24
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/4849 fix(tvix/eval): force left argument of `?` before checking for attrssterni2-0/+27
OpAttrsIsSet and OpAttrsTrySelect will fail silently if the attribute set value on the stack is actually a thunk, so we need to make sure to force at every step of the way. Emitting the force instructions in the compiler because it is easier to add, but maybe the VM should do this when handling the relevant opcodes? Comments welcome. Change-Id: I65c5ef348d59b2d07c9bb06abb24f9f3e6a0fdb2 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6540 Reviewed-by: grfn <grfn@gws.fyi> Autosubmit: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-09-13 r/4848 fix(tvix/eval): force value in builtins.typeOfsterni2-0/+23
This prevents Nix programs to observe the "internal" type of thunks. Possibly .type_of() is also an area of the runtime where we should panic if "internal" would ever be returned. Change-Id: I9f358044c48ad64896fb6a1b1a42f00a29efac00 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6539 Autosubmit: sterni <sternenseemann@systemli.org> Reviewed-by: wpcarro <wpcarro@gmail.com> Reviewed-by: grfn <grfn@gws.fyi> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-09-11 r/4809 fix(tvix/eval): place plain inherits in correct stack slotssterni2-0/+21
We need to make sure that we compile all plain inherits in a let expression before declaring any other locals. Plain inherits are special in the sense that they can never be recursive, instead resolving to a higher scope. Thus we need to compile their value, before declaring them. If we don't do that, before any other local can be declared, we cause a situation where the plain inherits' values are placed into other locals' stack slots. Note that we can't integrate the plain inherit compilation into the regular 2-3 phase model where we defer the compilation of the value or we'd compile `let inherit x; in …` as `let x = x; in …`. Change-Id: I951d5df3c9661a054e12401546875f4685b5bf08 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6496 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su> Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-11 r/4808 test(tvix/eval): add test for mutually recursive let bindingssterni2-0/+15
This test shows that let bindings' dependencies can form a cyclical graph, so we need to use thunking to break this cycle. Change-Id: I2a4de71fd7024f3d3d1166154784139a82f39411 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6495 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su> Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-11 r/4807 fix(tvix/eval): wrap asserts in a thunksterni2-0/+9
As the new test case demonstrates, asserts need to be evaluated lazily. Change-Id: I808046722c5a504e9497855ca5026d255c7a4c34 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6494 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su> Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-11 r/4806 test(tvix/eval): test “useful” plain inheritssterni2-0/+10
Change-Id: Ic4700f0618a393e45a2ee7c70045edff97e30c49 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6493 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su> Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-11 r/4805 fix(tvix/eval): declare let inherit (from) locals before compilingsterni4-2/+20
The recent change that split declaration of let based locals and the compilation of their values did not touch locals bound by inherit in let. These were previously declared and compiled immediately before starting to work on the other locals introduced in a let. In the case of plain inherits, this behavior is kept in this change, because there's nothing wrong with it: The value of a plain inherit will always resolve to a higher scope, either statically or dynamically. Since inherit (from) expression might refer to other locals bound in the same let, we need to handle them in the same three steps as ordinary let based locals: 1. We need to declare the (uninitialised) locals. 2. We need to compile the expression that obtains their value. For this, we create a new thunk, since the from expression may very well return a thunk which we need to force before selecting the value we are interested in. 3. Thunks need to be finalised. For 1., we create an extra pass over the inherits that already declares and initialises plain inherits and notes inherit (from) expressions in the entries vector after declaring them. 2. only needs a bit of adapting to create the thunks for selecting if appropriate, the rest of the existing code can be reused. Change-Id: Ie4ac1c0f9ffcbf7c07c452036aa8e577443af773 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6490 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org> Reviewed-by: tazjin <tazjin@tvl.su>
2022-09-11 r/4802 fix(tvix/eval): thunk all uses of `with`Vincent Ambo4-0/+19
With this all other "weird scope" logic starts working for `with` as well. Change-Id: I0ea1d8c5fbd9cec5084bd574224f77b71ff2b487 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6487 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-11 r/4798 fix(tvix/eval): correctly account for slots during list constructionVincent Ambo2-0/+14
Similarly to attribute sets, list elements can be arbitrary expressions and their (temporary) stack slots during construction must be accounted for by the compiler. Change-Id: I3b6f7927860627fd867c64d0cab9104fd636d4f5 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6470 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-11 r/4796 fix(tvix/eval): account for attrset temporaries during constructionVincent Ambo4-0/+21
The temporaries left on the stack as operands to `OpAttrs` must be accounted for in the locals array in order for operations within them to receive correct slots. Some test cases that were previously broken have been added. Change-Id: Ib52b629bbdf7931f63fd45a45af1073022da923c Reviewed-on: https://cl.tvl.fyi/c/depot/+/6468 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-08 r/4770 feat(tvix/eval): thunk binary operations and select expressionsVincent Ambo2-0/+8
With this, most cases of `fix` in attribute sets will work correctly. A simple test exercising both has been added. Change-Id: I70fd431177bb6e48ecb33a87518b050c4c3d1c09 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6437 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-08 r/4764 test(tvix/eval): add test for stack slot accounting edge-caseVincent Ambo2-0/+18
This was fixed by some of the previous commits around scopes. It's somewhat similar to a few other tests, but I had this one failing earlier and everything else succeeding, so it is useful to keep it around for sure. Change-Id: Ie6cf372b5c805daf992cd87aeb3dfe91542c381c Reviewed-on: https://cl.tvl.fyi/c/depot/+/6431 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-08 r/4762 fix(tvix/eval): consider local depth when deciding to deferVincent Ambo2-0/+11
Deferred local upvalues can *only* occur at the same depth as the thing that is closing over them, but there are various situations with scope nesting where the actual stack indexes of the local and the closer look like a deferred value is being accessed. To fix this, simply compare the depth as well. Change-Id: Ice77424cc87ab0a2c4f01379e68d4399a917b12b Reviewed-on: https://cl.tvl.fyi/c/depot/+/6429 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-08 r/4761 chore(tvix/eval): remove accidentally duplicated testVincent Ambo2-3/+0
This is the same as `eval-okay-attrs-simple-inherit`. Change-Id: I23878accc6cd62c16ec96601239838a385d31306 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6428 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-08 r/4743 fix(tvix/eval): inherit scope poisoning data in nested contextsVincent Ambo2-0/+5
Scope poisoning must be inherited across lambda context boundaries, e.g. if an outer scope has a poisoned `null`, any lambdas defined on the same level must reference that poisoned identifier correctly. Change-Id: I1aac64e1c048a6f3bacadb6d78ed295fa439e8b4 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6410 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-07 r/4740 feat(tvix/eval): Support builtins.lengthWilliam Carroll2-0/+6
Get the length of a list Change-Id: I41d91e96d833269541a1b3c23b7cc879f96d1e5a Reviewed-on: https://cl.tvl.fyi/c/depot/+/6407 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-07 r/4739 feat(tvix/eval): Support builtins.{mul,div}William Carroll4-0/+18
Multiplication and division :) Change-Id: I79e76f3b55b7e9b8d1eb09b3d9741140b4d75742 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6406 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-07 r/4738 feat(tvix/eval): Support builtins.{add,sub}William Carroll4-0/+16
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/4705 feat(tvix/eval): semi-strictly evaluate output values of the VMVincent Ambo2-3/+1
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/4699 feat(tvix/eval): construct attribute sets lazilyVincent Ambo2-1/+3
This thunks the construction of attribute sets. Because Tvix does not currently have a "strict output" mode, a test had to be disabled that now displays a thunk representation. The test will be re-enabled once that is available. Change-Id: I360332be64cd5c154f9caea21828f6f1b37a265c Reviewed-on: https://cl.tvl.fyi/c/depot/+/6363 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-07 r/4690 test(tvix/eval): test very simple late-bound identifier accessVincent Ambo2-0/+5
This is the simplest kind of thunk that can be created (and so far the only one the compiler knows how to create), in which an identifier inside a `let` encounters a value that is bound *after* it is initialised. Change-Id: I6ea4408a3baef1e7d5137365d70804283f2dbf8e Reviewed-on: https://cl.tvl.fyi/c/depot/+/6354 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-07 r/4689 feat(tvix/eval): always emit OpForce as the last instructionVincent Ambo2-0/+8
Change-Id: Id70c987f654dc5d9b47db74e395281309762b468 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6353 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-06 r/4671 test(tvix/eval): add a test case for nested, deferred upvaluesVincent Ambo2-0/+7
This test case was previously broken by the bug introduced by confusing local and stack indexes. Change-Id: Ibef299dad266c6105deac1da5dde112fe9f640b1 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6341 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-06 r/4669 feat(tvix/eval): emit OpFinalise when local scopes are completeVincent Ambo2-0/+5
With this change, the runtime can correctly capture deferred upvalues. Change-Id: I1e43b7b1ac2553b1812424adfc8bd08ef77bf1ea Reviewed-on: https://cl.tvl.fyi/c/depot/+/6339 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-06 r/4659 fix(tvix/eval): instantiate *new* closures from blueprints each timeVincent Ambo2-0/+8
The previous closure refactoring introduced a bug in which the same closure object would get mutated constantly for each instance of a closure, which is incorrect behaviour. This commit instead introduces an explicit new Value variant for the internal "blueprint" that the compiler generates (essentially just the lambda) and uses this variant to construct the closure at runtime. If the blueprint ever leaks out to a user somehow that is a critical bug and tvix-eval will panic. As a ~treat~ test for this, the fibonacci function is being used as it is a self-recursive closure (i.e. different instantiations of the same "blueprint") getting called with different values and it's good to have it around. Change-Id: I485de675e9bb0c599ed7d5dc0f001eb34ab4c15f Reviewed-on: https://cl.tvl.fyi/c/depot/+/6323 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-06 r/4658 fix(tvix/eval): correctly thread through dynamic upvaluesVincent Ambo2-0/+19
This puts together the puzzle pieces for threading dynamic upvalues (that is, upvalues resolved from the `with`-stack) all the way through. Reading the test case enclosed in this commit and walking through it is recommended to understand what problem is being tackled here. In short, because the compiler can not statically know *which* with-scope a dynamic argument is resolved from it needs to lay the groundwork for resolving from *all* possible scopes. There are multiple different approaches to doing this. The approach chosen in this commit is that if a dynamic upvalue is detected, the compiler will emit instructions to close over this dynamic value in *all* enclosing lambda contexts. It uses a new instruction for this that will leave around a sentinel value in case an identifier could not be resolved, and wire the location of this found value (or sentinel) up through the upvalues to the next level of nesting. In this tradeoff, tvix potentially closes over more upvalues than are needed (but in practice, how often do people create *really* deep `with`-stacks? and in *this* kind of code situation? maybe we should even warn for this!) but avoids keeping the entire attribute sets themselves around. Looking at the test case, each surrounding closure will close over *all* dynamic identifiers that are referenced later on visible to it, but only the last one for each identifier will actually end up being used. This also covers our bases for an additional edge-case this creates, in which an identifier potentially resolves to a dynamic upvalue *and* to a dynamic value within the function's own scope (again, would anyone really do this?) by introducing a resolution instruction for that particular case. There is likely some potential for cleaning up this code which is quite ugly in some parts, but as this implementation is now carefully calibrated to work I decided it is time to commit it and clean it up in subsequent commits. Change-Id: Ib701e3e6da39bd2c95938d1384036ff4f9fb3749 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6322 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-06 r/4655 fix(tvix/eval): correctly resolve dynamic upvalues one scope upVincent Ambo4-0/+22
This does not yet correctly resolve them if they are more than one scope up, however. Change-Id: I6687073c60aee0282f2b6ffc98b34c1e96a60f20 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6319 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-06 r/4653 feat(tvix/eval): implement capture of self-recursive upvaluesVincent Ambo2-0/+5
With this change, it becomes possible for functions to call themselves as they are being defined in local bindings. Change-Id: Ib46a39ba17b1452b5673d96fa729d633d237241a Reviewed-on: https://cl.tvl.fyi/c/depot/+/6314 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
generated by cgit-pink 1.4.1 (git 2.44.2) at 2024-12-29T17·24+0000