about summary refs log tree commit diff
path: root/tvix/eval/src/compiler
AgeCommit message (Collapse)AuthorFilesLines
2022-09-06 r/4659 fix(tvix/eval): instantiate *new* closures from blueprints each timeVincent Ambo1-3/+4
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 Ambo1-4/+20
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/4657 refactor(tvix/eval): thread dynamic upvalues through all contextsVincent Ambo2-9/+65
With this change, encountering a dynamic upvalue will thread through all contexts starting from the lowest context that has a non-empty `with`-stack. The additional upvalues are not actually used yet, so the effective behaviour remains mostly the same. This is done in preparation for an upcoming change, which will implement proper dynamic resolution for complex cases of nested dynamic upvalues. Yes, this whole upvalue + dynamic values thing is a little bit mind-bending, but we would like to not give up being able to resolve a large chunk of the scoping behaviour statically. Change-Id: Ia58cdd47d79212390a6503ef13cef46b6b3e19a2 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6321 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-06 r/4656 feat(tvix/eval): allow ignoring locals by prefixing with _Vincent Ambo1-1/+2
This is a common idiom in both Nix and other languages when a local is declared without actually being used. Since Tvix warns for unused locals, having this available is useful and can be included in the final error message as a suggestion if an unused variable is intentional. Change-Id: Ia85f704ba183499a3bae657c58166e2e29f9bde5 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6320 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-06 r/4655 fix(tvix/eval): correctly resolve dynamic upvalues one scope upVincent Ambo1-6/+7
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 Ambo1-4/+8
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>
2022-09-06 r/4651 refactor(tvix/eval): encapsulate internal mutability within ClosureVincent Ambo1-2/+1
This is required to efficiently construct the upvalue array at runtime, as there are situations where during Closure construction multiple things already have a reference to the closure (e.g. a self-reference). Change-Id: I35263b845fdc695dc873de489f5168d39b370f6a Reviewed-on: https://cl.tvl.fyi/c/depot/+/6312 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-04 r/4643 refactor(tvix/eval): track with stack size as a simple integerVincent Ambo2-13/+24
The `With` struct no longer contained any internals after the cleanup logic for the stack had been moved into Compiler::compile_with, leaving the `Vec<With>` to essentially act as a counter for the number of things on the with stack. That's inefficient of course, so with this commit it actually becomes an integer (with an encapsulated API within scope::Scope). Change-Id: I67a00987fc8b46b30d369a96d41e83c8af5b1998 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6311 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-04 r/4642 refactor(tvix/eval): move compiler's scope logic to separate moduleVincent Ambo2-172/+196
The compiler module is getting quite long and this will help keep some order. Right now the scope internals are not very well encapsulated; this paves a way to reducing the API surface of the `scope` type to the things that are actually used by the compiler instead of giving access to its internals. Change-Id: I8c16c26d263f018baa263f395c9cd80715199241 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6310 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-04 r/4641 fix(tvix/eval): declare locals before marking them initialisedVincent Ambo1-1/+1
This has no effect yet, other than changing the way in which some upvalue captures break (that are already not working correctly). However, after this change the compiler correctly detects self-recursion and can start emitting the instructions to deal with this at runtime. Change-Id: Id3b0ac206c0204739597a4325bcc66f9c806c242 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6309 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-04 r/4640 fix(tvix/eval): Account for uninitialised variables in with_idxVincent Ambo1-1/+11
Calculating the with_idx (i.e. the stack offset of the "phantom" variable from which a `with` dynamically reads at runtime) needs to account for unitialised variables the same way as the resolution of normal locals does. Change-Id: I9ffe404535bf1c3cb5dfe8d9e005798c857fff94 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6308 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-04 r/4639 fix(tvix/eval): open/close additional scope around `with`Vincent Ambo1-0/+2
This is required to correctly clean up the `with` values. At the moment, the attrset from which identifiers are being taken is always pushed on the stack. This means that it must also be removed again, otherwise in an expression like with { a = 15; }; a The final stack is `[ { a = 15; } 15 ]` *after the last operation*, which means that the attrset is still on there as garbage. This has little practical impact right now because it is always shadowed by the fact that the actual expression value is at the right location, but becomes relevant when accounting for upvalue captures. Change-Id: I69e9745bfaa4d6bbcb60ee71f4dc3f8d8695d16a Reviewed-on: https://cl.tvl.fyi/c/depot/+/6303 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-04 r/4638 refactor(tvix/eval): extend resolve_local logic for self-recursionVincent Ambo1-22/+89
This extends the logic of `Scope::resolve_local` to detect cases where self-recursion is occuring (i.e. an identifier is being accessed in its own identifier). These cases are not yet handled specially, and the logic of when things are marked initialised (which was previously always at the same spot as their declaration) has not changed, making this commit a runtime no-op for now. Change-Id: I3179642a7c55869ad4465fdd2678b0cd51a20f15 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6302 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-04 r/4637 feat(tvix/eval): detect illegally shadowed variablesVincent Ambo1-0/+31
Nix does not allow things like `let a = 1; a = 2; in a`, but doing it across depths is allowed. Change-Id: I6a259f8b01a254b433b58c736e245c9c764641b6 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6301 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-04 r/4636 refactor(tvix/eval): introduce Depth enum to track variable statusVincent Ambo1-4/+26
This does not yet change anything semantically, but will be useful for resolving simple cases of self-recursion etc. Change-Id: I139ecb7e4a8a81193774392a96e73e0ea6b9f85d Reviewed-on: https://cl.tvl.fyi/c/depot/+/6300 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-04 r/4635 feat(tvix/eval): implement upvalue resolution in `with` scopesVincent Ambo1-1/+17
These need to be handled specially by the runtime if the compiler determines that a given local must be resolved via `with`. Note that this implementation has a bug: It currently allows `with` inside of nested lambdas to shadow statically known identifiers. This will be cleaned up in the next commit. Change-Id: If196b99cbd1a0f2dbb4a40a0e88cdb09a009c6b9 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6299 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-04 r/4633 fix(tvix/eval): pop with stack immediately after processing bodyVincent Ambo1-13/+5
Instead of tying the popping of the with stack to scope depth, clean up the stack immediately after processing a with body. The previous behaviour was actually incorrect, as it would leave things on the with-stack longer than they were supposed to be there. This could lead to false positive resolutions in some situations involving closures. Change-Id: I7b0638557503f1f71eb602e3d5ff193cdfcb67cc Reviewed-on: https://cl.tvl.fyi/c/depot/+/6297 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-03 r/4625 feat(tvix/eval): compile creation of closure objectsVincent Ambo1-3/+21
Fully implements the instructions for compiling closure objects (without runtime handling yet). Closure (and thunk) objects are created at runtime by capturing all known upvalues. To represent this, the instructions for creating them need to have a variable number of arguments. Due to that, this commit introduces new variants in OpCode that are not actually operations, but data. If the VM is implemented correctly, the instruction pointer should never point at these. Due to this, the VM will panic if it sees a data operand during an execution run. Change-Id: Ic56b49b3a42736dc437751e76df0e89c8d0619c6 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6291 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-09-03 r/4623 feat(tvix/eval): implement compilation of upvalue accessVincent Ambo1-3/+55
This adds a new upvalue tracking structure in the compiler to resolve upvalues and track their positions within a function when compiling a closure. The compiler will emit runtime upvalue access instructions after this commit, but the creation of the runtime closure object etc. is not yet wired up. Change-Id: Ib0c2c25f686bfd45f797c528753068858e3a770d Reviewed-on: https://cl.tvl.fyi/c/depot/+/6289 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-09-03 r/4622 refactor(tvix/eval): add opcode::Count type for less ambiguityVincent Ambo1-6/+6
Change-Id: Ibde0b2baa1128a74c1364ee9a6330b62db3da699 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6288 Autosubmit: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-03 r/4621 refactor(tvix/eval): add opcode::StackIdx type for less ambiguityVincent Ambo1-4/+4
Change-Id: I9b9de1f681972c205d4d20bc5731d2ce79858edb Reviewed-on: https://cl.tvl.fyi/c/depot/+/6287 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-03 r/4619 refactor(tvix/eval): add opcode::JumpOffset type for less ambiguityVincent Ambo1-9/+12
This adds a transparent wrapper around `usize` used for jump offsets in the opcodes. This is a step towards getting rid of ambiguous plain `usize` usage in the opcode. Change-Id: I21e35e67d94b32d68251908b96c7f62b6f56a8bb Reviewed-on: https://cl.tvl.fyi/c/depot/+/6282 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-03 r/4618 refactor(tvix/eval): rename CompilationResult -> CompilationOutputVincent Ambo1-3/+3
grfn pointed out in cl/6174 that `Result` might cause developers to believe that this behaves like std::Result, which it does not. Change-Id: Ia30ab0dcb7e8da7bf842777ee3fe17bcf35cb0c1 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6281 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-03 r/4613 refactor(tvix/eval): rename Value::NotFound & OpAttrOrNotFoundVincent Ambo1-2/+2
grfn suggested clearer naming for these in cl/6166. Change-Id: I83164bf1d1902ebd42272e9d5d63819a0f6a72c5 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6277 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-09-03 r/4612 docs(tvix/eval): add doc comment on `compiler::patch_jump`Vincent Ambo1-0/+6
Change-Id: Ifdd7b99223d239d955ac7eeeae95db97eb742bf0 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6276 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-09-03 r/4608 fix(tvix/eval): address all current clippy lintsVincent Ambo1-4/+3
Change-Id: I758fc4f3b9078de7ca6228a75a4351c3e085c4cf Reviewed-on: https://cl.tvl.fyi/c/depot/+/6272 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-09-03 r/4606 refactor(tvix/eval): move resolve_local to Scope structVincent Ambo1-15/+18
This is a more sensible place for this function to live and makes upvalue resolution easier down the line. Change-Id: I48ee39bdcdb4f96a16a327f7015aff60db5b15fb Reviewed-on: https://cl.tvl.fyi/c/depot/+/6270 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-09-03 r/4605 refactor(tvix/eval): introduce Closure struct in Value typeVincent Ambo1-2/+4
This struct will carry the upvalue machinery in addition to the lambda itself. For now, all lambdas are wrapped in closures (though technically analysis of the environment can later remove innermost Closure wrapper, but this optimisation may not be worth it). Change-Id: If2b68549ec1ea4ab838fdc47a2181c694ac937f2 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6269 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-09-02 r/4602 chore(tvix/eval): move compiler module to a new folderVincent Ambo1-0/+1093
Change-Id: I76157f9cf1369cd17506de1b1ded1a4fd06f004a Reviewed-on: https://cl.tvl.fyi/c/depot/+/6268 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
generated by cgit-pink 1.4.1 (git 2.44.2) at 2024-11-20T06·57+0000