about summary refs log tree commit diff
path: root/tvix
AgeCommit message (Collapse)AuthorFilesLines
2022-09-07 r/4713 feat(tvix/eval): track source spans for literalsVincent Ambo1-3/+3
Change-Id: Icfe77f85c4f65b6bf28b8752c2795419e8e396ce Reviewed-on: https://cl.tvl.fyi/c/depot/+/6380 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-07 r/4712 feat(tvix/eval): add methods for emitting code with tracked spansVincent Ambo2-80/+111
These are not actually used yet; this is in preparation for a multi-commit chain for emitting all the right spans in the right locations. Change-Id: Ie99d6add2696c1cc0acb9ab928917a10237159de Reviewed-on: https://cl.tvl.fyi/c/depot/+/6379 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-07 r/4711 chore(tvix/eval): thread a codemap::File reference to the compilerVincent Ambo2-5/+21
This instantiates a codemap outside of the compiler and passes a reference to the file currently under compilation to it. Note that the "file" might just be a REPL line. Change-Id: I131ae1ddb6d718e1374750da9ba0b99608c6058d Reviewed-on: https://cl.tvl.fyi/c/depot/+/6378 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-07 r/4710 refactor(tvix/eval): add and use Compiler::push_op methodVincent Ambo1-60/+65
This is currently just a wrapper around Chunk::push_op, but will gain the span resolution logic in a moment. Change-Id: I862bf9ecff0932f8da6708401ea044b9442c5d5b Reviewed-on: https://cl.tvl.fyi/c/depot/+/6377 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-07 r/4709 feat(tvix/eval): add data structures for tracking spans in chunksVincent Ambo1-0/+50
This adds a new vector to the chunk data structure which tracks spans into a codemap. The compiler will emit this information to the chunk when adding instructions. The internal representation of the spans is slightly optimised to avoid storing duplicate spans, as there are cases where many instructions might be derived from the same span. Change-Id: I336f8c912e7eb50ea02ed71e6164f651ca3ca790 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6376 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-07 r/4708 chore(tvix/eval): bump all dependenciesVincent Ambo2-9/+9
Updated the rnix hash manually, and ran `cargo update` for the rest. Change-Id: I457262625d648e25d745efa4d33ae44cb8f21326 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6375 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-07 r/4707 chore(tvix/eval): add codemap dependencyVincent Ambo2-0/+8
This will be used to track source spans when emitting bytecode. The codemap is a data structure which tracks *all* the source files visited by an evaluation, and makes it possible to represent locations across all of the files using a simple span (i.e. pair of offsets). When reporting errors, this even contains enough information to reconstruct the rnix AST to create fancier reporting in certain cases if desired. Change-Id: I4ae98620b9b150fb5a389bd7f1e12670e3192c62 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6374 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-07 r/4706 fix(tvix/eval): address current clippy lintsVincent Ambo4-5/+5
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 Ambo3-4/+36
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/4704 feat(tvix/eval): implement NixList::iterVincent Ambo1-0/+4
This does not require a custom iterator type (for now?) Change-Id: I5beb194bd8629571bd4040c69c977c27149807fa Reviewed-on: https://cl.tvl.fyi/c/depot/+/6371 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-07 r/4703 fix(tvix/eval): thread Display & PartialEq through to thunk valuesVincent Ambo2-1/+21
If a thunk is already evaluated, there are cases where due to the memoisation implementation something might observe a value wrapped in a thunk. In these cases, the implementation of `Display` and `PartialEq` must delegate to the underlying value. Note that there are a handful of other cases like these which we need to cover. It is a little tricky to write integration tests for these directly, especially as some of the open-upvalue optimisations coming down the pipe will reduce the number of observable thunks. One test that covers a part of this behaviour is currently disabled (needs some more machinery), but it's being brought back in the next commits. Change-Id: Iaa8cd338c12236af844bbc99d8cec2205f0d0095 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6370 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-07 r/4702 feat(tvix/eval): implement NixAttrs::iter()Vincent Ambo2-1/+121
Implementing iteration over NixAttrs requires a custom iterator type in order to encapsulate the different representations. The BTreeMap for example has its own iterator type which needs to be encapsulated. This is mostly boilerplate code, but for a change some simple unit tests have been added in. Change-Id: Ie13b063241d461b810876f95f53878388e918ef2 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6367 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-07 r/4701 chore(tvix/eval): provide 'static references to "name"/"value"Vincent Ambo1-0/+3
These static strings show up a bunch when dealing with the internals of attribute sets, and having them available as static references is required. Due to the way const expressions are evaluated, taking a reference to the existing NixString::NAME / NixString::VALUE items does not work and the references themselves need to be const-evaluated. Change-Id: If6e75847af978118a3b266fe6a3242321722434d Reviewed-on: https://cl.tvl.fyi/c/depot/+/6366 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-07 r/4700 refactor(tvix/eval): encapsulate all thunk-forcing logic in moduleVincent Ambo2-22/+44
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/4699 feat(tvix/eval): construct attribute sets lazilyVincent Ambo3-3/+13
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/4698 chore(tvix/eval): add release-with-debug cargo profileVincent Ambo1-0/+4
This lets us create a release build with debug info, for use with e.g. perf + hotspot Change-Id: I03897de36c872d318abf1332ca0c1aeabe344ec6 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6362 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-07 r/4697 fix(tvix/eval): avoid recomputing width in disassemble_op constantlyVincent Ambo1-4/+4
As noticed by sterni in cl/6195 Change-Id: Ie9c1e80e2e709284fa8412334af9188d999f64dc Reviewed-on: https://cl.tvl.fyi/c/depot/+/6361 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-07 r/4696 fix(tvix/eval): correctly resolve identifiers in inheritVincent Ambo1-21/+4
This should not have grown a second implementation of the identifier resolution logic, but it somehow did. This implementation ended up being incorrect because it did not account for upvalues inside of thunks. Change-Id: Ieb1364d8fe43c96aaf4b125fd4b8a522aedff167 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6360 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-07 r/4695 chore(tvix/eval): implement Debug for compiler::scope::ScopeVincent Ambo1-1/+2
Change-Id: I112b0119bd0511f26bb72f7e73d867d1b7144a36 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6359 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-07 r/4694 feat(tvix/eval): insert strictness points for attribute set keysVincent Ambo1-2/+10
All attribute set *key* related operations strictly evaluate all key fragments, including during construction of an attribute set. Change-Id: I3519e5e9b0886c2cdc8615ea7dcb5f7be0c59b3f Reviewed-on: https://cl.tvl.fyi/c/depot/+/6358 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-07 r/4693 feat(tvix/eval): insert strictness points for unary/binary operatorsVincent Ambo1-1/+17
The arguments of all unary/binary operators that are built in to Nix are forced when encountered. This emits the necessary OpForce operations. Change-Id: I691fcdbebfe7586cfe217c68d44b10b1192f82d1 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6357 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-07 r/4692 chore(tvix/eval): thread `slot` value through all compiler methodsVincent Ambo1-54/+58
With this change any compilation of an expression is aware of its own stack slot if it is leaving identifiers on the stack. Change-Id: I0c9f148ae06b078a46b25180c4961686d5f2e166 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6356 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-07 r/4691 chore(tvix/eval): thread `slot` value through Compiler:compile_attrVincent Ambo1-17/+23
This threads the current local slot through the `compile_attr` function and all of its callers. At the moment this does not improve any user-facing behaviour, just internally changes the way in which some correct expressions would fail to run. Eventually this slot will need to reach everywhere ... Change-Id: Iba73123dd1ced421093d8fc18ebeeffc16efacf8 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6355 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 Ambo3-0/+14
Change-Id: Id70c987f654dc5d9b47db74e395281309762b468 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6353 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-07 r/4688 feat(tvix/eval): implement OpForce in VMVincent Ambo4-2/+46
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>
2022-09-06 r/4682 fix(tvix/eval): allocate Thunk::upvalues with known capacityVincent Ambo1-1/+1
The capacity (i.e. number of builtins) is known from the lambda, so we can size it correctly right away. Change-Id: Iab0b5a3f47d450fa9866c091ebbbed935b934907 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6351 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-06 r/4681 feat(tvix/eval): implement OpThunk for runtime thunk constructionVincent Ambo1-2/+17
Implements an operation very similar to `OpClosure` which populates a thunk's upvalues and leaves it on the stack. Change-Id: I753b4dfeeaae6919316c7028ec361aaa13d87646 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6350 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-06 r/4679 refactor(tvix/eval): extract VM::populate_upvalues functionVincent Ambo1-30/+42
This function is reusable between thunks & closures. Change-Id: I44d5f9897b087a385c8e75027d2ff39c48a096f0 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6349 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-06 r/4678 refactor(tvix/eval): Carry lambda & upvalues directly in CallFrameVincent Ambo1-13/+25
CallFrame has to work for both thunks & closures (as a thunk is basically a "weird 0-argument closure"). We opt to store the common, relevant fields directly in the frame to avoid having to dereference through the nested structures constantly (which would be especially annoying in the case of thunks). Change-Id: I47781597b84ec5cd55502dba1713e92cf2592af3 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6348 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-06 r/4677 refactor(tvix/eval): introduce UpvalueCarrier traitVincent Ambo5-18/+81
This trait abstracts over the commonalities of upvalue handling between closures and thunks. It allows the VM to simplify the code used for setting up upvalues, without duplicating between the two different types. Note that this does not yet refactor the VM code to optimally make use of this. Change-Id: If8de5181f26ae1fa00d554f1ae6ea473ee4b6070 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6347 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-06 r/4676 refactor(tvix/eval): simplify thunk representationsVincent Ambo2-8/+8
For now, do not distinguish between closing and non-closing thunks, it will make the initial implementation easier. See Knuth etc. Change-Id: I0bd51e0f89f2c77e90bac63b507e5027b649e3d8 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6346 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-06 r/4675 feat(tvix/eval): emit thunks for recursive local scope resolutionVincent Ambo1-6/+13
When resolving a value on the same level that is known but not yet defined, emit a thunk. Consider for example: let # v--- requires a thunk a = 1 * b; b = 10; in a Change-Id: I922cb50973ebe05e335a7bc7cb851960cf34733b Reviewed-on: https://cl.tvl.fyi/c/depot/+/6345 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-06 r/4674 feat(tvix/eval): Add Compiler::thunk method for emitting thunksVincent Ambo3-1/+41
The logic in this method is *very* similar to `compile_lambda`. It is intended to be called around any expression that should be thunked (such as function applications, attribute set values, etc.). Change-Id: Idfbb2daa9f4b735095378fb9c39a2fd07c8cff91 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6344 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-06 r/4673 feat(tvix/eval): introduce Value::Thunk variantVincent Ambo2-1/+61
Introduces the representation of runtime thunks, that is lazily evaluated values. Their representation is very similar to closures. Change-Id: I24d1ab7947c070ae72ca6260a7bbe6198bc8c7c5 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6343 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-06 r/4672 refactor(tvix/eval): extract compiler's upvalue logic into helperVincent Ambo1-14/+20
This exact same logic is reused for thunk creation. Change-Id: I731db9cc659a1f2ca87db55d58d6ff632f417812 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6342 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/4670 fix(tvix/eval): distinguish statically between StackIdx and LocalIdxVincent Ambo2-94/+115
Previously the functions in the scope module returned usize values, which - sometimes from the same function - were either indexes into the runtime stack *or* indexes into the compiler's local stack. This is extremely confusing because it requires the caller to be aware of the difference, and it actually caused subtle bugs. To avoid this, there is now a new LocalIdx wrapper type which is used by the scope module to return indexes into the compiler's stack, as well as helpers for accounting for the differences between these indexes and the runtime indexes. Change-Id: I58f0b50ad94b28a304e3372fd9731b6590b3fdb8 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6340 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-06 r/4669 feat(tvix/eval): emit OpFinalise when local scopes are completeVincent Ambo3-0/+13
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/4668 feat(tvix/eval): track whether locals needs to be finalisedVincent Ambo2-0/+12
When encountering a deferred local upvalue, the compiler will now mark the corresponding local as needing a finaliser which makes it possible to emit the OpFinalise instruction for this stack slot a little bit down the line. Change-Id: I3962066f10fc6c6e1472722b8bdb415a811e0740 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6338 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-06 r/4667 feat(tvix/eval): implement OpFinalise instructionVincent Ambo3-1/+28
This instruction finalises the initialisation of deferred upvalues in closures (and soon, thunks). The compiler does not yet emit this instruction, some more accounting is needed for that. Change-Id: Ic4181b26e19779e206f51e17388559400da5f93a Reviewed-on: https://cl.tvl.fyi/c/depot/+/6337 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-06 r/4666 feat(tvix/eval): set up deferred upvalues at runtimeVincent Ambo2-3/+7
This will leave a sentinel value in the upvalue slot in which the actual value is to be captured after resolution once a scope is fully set up. Change-Id: I12b37b0dc8d32603b03e675c3bd039468e70b354 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6336 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-06 r/4665 feat(tvix/eval): detect deferred upvalue capturingVincent Ambo3-3/+22
Uses the threaded through slot offset to determine whether initialisation of a captured local upvalue must be defered to a later point where all values of a scope are available. This adds a new data representation to the opcode for this situation, but the equivalent runtime handling is not yet implemented. This is in part because there is more compiler machinery needed to find the resolution point. Change-Id: Ifd0c393f76abfe6e2d91483faf0f58947ab1dedc Reviewed-on: https://cl.tvl.fyi/c/depot/+/6329 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-06 r/4664 chore(tvix/eval): thread current slot index through compilerVincent Ambo1-34/+34
While compiling local bindings, we now know the stack slot of the value currently being compiled. This will let us determine whether an upvalue can be captured directly or whether it needs to wait for a synchronisation point at which the upvalue can be instantiated. This machinery lets us avoid unnecessary work at runtime when instantiating closures that actually do not need complicated recursive resolution. This change itself introduces no new functionality, but since the threading is noisy it is split out as a separate change. Change-Id: I847c677ee8f6725fda1d2efd689b6a58bdccb779 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6328 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-06 r/4663 refactor(tvix/eval): optimise initialisation of localsVincent Ambo1-23/+19
Instead of looking up the local to be initialised by its name again, we can simply track the index at which it was declared from the point where the declaration was made. This reduces some string cloning and removes unnecessary logic. It also theoretically makes the *current* index available during locals compilation, which can be used to optimise some recursion cases. Change-Id: I06f403603d4f86c3d319debfe74b5a52eec00990 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6327 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-06 r/4662 refactor(tvix/eval): declare all locals before compiling themVincent Ambo1-1/+12
This actually makes things full-circle, as this tree already had this implementation once before all the other required components were in place. With this commit, the compiler can resolve recursive upvalues within the same scope (though they will not yet work at runtime). Change-Id: I6267e477d08f367257c3a6dde054b880d7b47211 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6326 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-06 r/4661 refactor(tvix/eval): decouple local depth & initialisation trackingVincent Ambo2-56/+33
In order to resolve recursive references correctly, these two can not be initialised the same way as a potentially large number of (nested!) locals can be declared without initialising their depth. This would lead to issues with detecting things like shadowed variables, so making both bits explicit is preferable. Change-Id: I100cdf1724faa4a2b5a0748429841cf8ef206252 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6325 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-06 r/4660 fix(tvix/eval): correct runtime error for missing dynamic upvalueVincent Ambo1-0/+6
Change-Id: I75c351619780fdc5186a54f3df9b244ada984069 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6324 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-06 r/4659 fix(tvix/eval): instantiate *new* closures from blueprints each timeVincent Ambo6-17/+33
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 Ambo6-12/+112
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>
generated by cgit-pink 1.4.1 (git 2.44.2) at 2024-11-10T20·28+0000