about summary refs log tree commit diff
path: root/tvix/eval/src/compiler/mod.rs (follow)
AgeCommit message (Collapse)AuthorFilesLines
2022-10-13 r/5121 fix(tvix/eval): fix Compiler::new on wasmVincent Ambo1-1/+3
This path normalisation business causes runtime panics on WebAssembly because those operations are unsupported. Maybe this shouldn't be happening in the compiler anyways, not sure, but for now this commit adds a workaround based on the target to disable the normalisation if we're compiling for wasm. Change-Id: I908a84fbdffc3401f8d443e2c73ec673e9f397ff Reviewed-on: https://cl.tvl.fyi/c/depot/+/7004 Autosubmit: tazjin <tazjin@tvl.su> Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-10-13 r/5120 fix(tvix/eval): src/compiler: ensure root_dir is absoluteAdam Joseph1-8/+16
Cppnix immediately absolutizes pathnames at parse time; if you write `./foo`, it is immediately converted to `$(pwd)/foo` and manipulated as an absolute path at all times. To avoid having to introduce filesystem access operations in the implementation of otherwise-pure builtins, let's guarantee that the `root_dir` of the VM is always an absolute path. Signed-off-by: Adam Joseph <adam@westernsemico.com> Change-Id: I7cbbae2cba4b2716ff3f5ff7c9ce0ad529358c8a Reviewed-on: https://cl.tvl.fyi/c/depot/+/6995 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-10-13 r/5119 refactor(tvix/eval): factor out all calls to canon_pathAdam Joseph1-2/+1
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-10 r/5097 fix(tvix/eval): Actually trace spans for thunksGriffin Smith1-1/+1
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/5090 fix(tvix/eval): Thunk `if` exprGriffin Smith1-1/+3
Since the body of an `if` expr can refer to deferred upvalues, it needs to be thunked so when we actually compile those deferred upvalues we have something for the finalize op to point at. Without this all sorts of weird things can happen due to the finalize op being run in the wrong lambda context, up to and including a panic. Change-Id: I040d5e1a7232fd841cfa4953539898fa49cbbb83 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6929 Reviewed-by: sterni <sternenseemann@systemli.org> Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-10-10 r/5087 feat(tvix/eval): Initial resolution of `<...>` pathsGriffin Smith1-6/+17
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/5082 refactor(tvix/eval): Compile OpAssert using conditional jumpsGriffin Smith1-4/+22
In order to behave nicely with tryEval, asserts need to leave the instruction pointer in a reasonable place even if they fail - whereas with the previous implementation catching a failed assert would still end up running the op for the *body* of the assert. With this change, we compile asserts much more like an `if` expression with conditional jumps rather than having an OpAssert op. Change-Id: I1b266c3be90185c84000da6b1995ac3e6fd5471b Reviewed-on: https://cl.tvl.fyi/c/depot/+/6925 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-08 r/5057 refactor(tvix/eval): move `spans` module to crate rootVincent Ambo1-2/+7
This is also useful for error-handling related logic, outside of just the compiler module. Change-Id: I5c386e2b4c31cda0a0209b31136ca07f00e39e45 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6869 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-10-07 r/5048 feat(tvix/eval): insert `import` into the builtins itselfVincent Ambo1-5/+9
Adding `import` to builtins causes causes a bootstrap cycle because the `import` builtin needs to be initialised with the set of globals before being inserted into the globals, which also must contain itself. To break out of the cycle this hack wraps the builtins passed to the compiler in an `Rc` (probably sensible anyways, as they will end up getting cloned a bunch), containing a RefCell which gives us mutable access to the builtins. This opens up a potentially dangerous footgun in which we could mutate the builtins at runtime leading to different compiler invocations seeing different builtins, so it'd be nice to have some kind of "finalised" status for them or some such, but I'm not sure how to represent that atm. Change-Id: I25f8d4d2a7e8472d401c8ba2f4bbf9d86ab2abcb Reviewed-on: https://cl.tvl.fyi/c/depot/+/6867 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-10-04 r/5034 refactor(tvix/eval): remove unnecessary clones in compilerVincent Ambo1-128/+124
There's basically nothing that needs *ownership* of an AST node (which is just a little box full of references to other things anyways), so we can thread this through as references all the way. Change-Id: I35a1348a50c0e8e07d51dfc18847829379166fbf Reviewed-on: https://cl.tvl.fyi/c/depot/+/6853 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-10-04 r/5033 refactor(tvix/eval): split observer traits in twoVincent Ambo1-4/+4
There are actually two different types of observers, the ones that observe the compiler (and emitted chunks from different kinds of expressions), and the ones that trace runtime execution. Use of the NoOpObserver is unchanged, it simply implements both traits. Change-Id: I4277b82674c259ec55238a0de3bb1cdf5e21a258 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6852 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-09-29 r/4994 refactor(tvix/eval): emit OpAttrs inside of compile_bindingsVincent Ambo1-1/+1
This needs to move here so that we can reuse compile_bindings for the nested attribute sets we're about to start constructing. Change-Id: Ie83f52f7e1d128886e96a1da47792211fa826f21 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6796 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-29 r/4991 chore(tvix/eval): fix all current clippy lintsVincent Ambo1-2/+2
Change-Id: I28d6af8cb408f8427a75d30b9120aaa809a1ea40 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6784 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-29 r/4989 refactor(tvix/eval): merge all bindings creation logicVincent Ambo1-1/+0
As of this commit, all three types of bindings scopes are compiled the same way (i.e. compilation of non-recursive attribute sets has been switched over to the new code paths). This sets us up for doing the final implementation of nested attribute sets. HOWEVER, this breaks the existing implementation of nested attributes in non-recursive attribute sets. That implementation is flawed and unworkable in practice, so we need to do this dance to be able to implement it correctly. Change-Id: Iba2545c0d1d6b51f5e1a31a5d005b8d01da546d3 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6782 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-28 r/4972 refactor(tvix/eval): bye compiler::attrs, hello compiler::bindingsVincent Ambo1-438/+121
Changes the module structure of the compiler to have a module dedicated to the logic of setting up bindings. This logic is in the process of being merged between attribute sets and `let`-expressions, and the structure of the modules makes more sense when ecapsulating that specifically. (Other bits of code related to e.g. attribute sets are pretty straightforward and can just live in the main compiler module). Change-Id: I9469b73a7034e5b5f3bb211694d97260c4c9ef54 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6766 Autosubmit: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-22 r/4962 fix(tvix/eval): manually count entries in recursive scopesVincent Ambo1-2/+10
The previous version had a bug where we assumed that the number of entries in an attribute set AST node would be equivalent to the number of entries in the runtime attribute set, but due to inherit nodes containing a variable number of entries, this did not work out. Fixes b/199 Change-Id: I6f7f7729f3512b297cf29a2e046302ca28477854 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6749 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-22 r/4958 fix(tvix/eval): support string identifiers in inheritsVincent Ambo1-61/+89
This updates rnix-parser to a version where inherits provide an iterator over `ast::Attr` instead of `ast::Ident`, which mirrors the behaviour of Nix (inherits can have (statically known) strings as their identifiers). This actually required some fairly significant code reshuffling in the compiler, as there was an implicit assumption in many places that we would have an `ast::Ident` node available when dealing with variable access (which is then explicitly only not true in this case). Change-Id: I12f1e786c0030c85107b1aa409bd49adb5465546 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6747 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-20 r/4939 refactor(tvix/eval): Define a Compiler::new functionGriffin Smith1-27/+39
Change-Id: I6b9283d16447c83dd3978371d9a6ac1beb985926 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6657 Autosubmit: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-09-20 r/4937 fix(tvix/eval): force condition of an assertsterni1-0/+1
Change-Id: I3ad2234e8a8e4280e498c6d7af8ea0733ed4c7ea Reviewed-on: https://cl.tvl.fyi/c/depot/+/6699 Autosubmit: sterni <sternenseemann@systemli.org> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-09-18 r/4907 refactor(tvix/eval): clone the Arc<codemap::File> for the compilerVincent Ambo1-5/+6
This disconnects ownership of the `File` reference in a compiler from the calling scope, which is required for when we implement `import`. `import` will need to carry an `Rc<RefCell<CodeMap>>` (or maybe, in the future, Arc) to give us the ability to add new detected code files at runtime. Note that the choice of `Arc` over `Rc` here is not ours - it's the codemap crate's. Change-Id: I3aeca4ffc167acbd1701846a332d93550b56ba7d Reviewed-on: https://cl.tvl.fyi/c/depot/+/6630 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-09-18 r/4898 fix(tvix/eval): Emit errors for invalid integersGriffin Smith1-1/+4
Invalid integers (eg integers that're too long) end up as error returns on the `.value()` returned from the literal in the AST - previously we'd unwrap this error, causing it to panic the compiler, but now we've got a nice error variant for it (which just unwraps the underlying std::num::ParseIntError). Change-Id: I50c3c5ba89407d86659e20d8991b9658415f39a0 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6635 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-17 r/4889 refactor(tvix/eval): clean up implementation of `compile_literal`Vincent Ambo1-10/+7
Suggested by sterni in cl/6231 Change-Id: I58bbc8a922d360ea79a4dacb76cf8aa1fad93757 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6622 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-17 r/4884 refactor(tvix/eval): use new ToSpan trait wherever possibleVincent Ambo1-24/+16
... it would be nice if we could thread it through to the `Scope` stuff (declaring phantoms & locals). Change-Id: Id3b84e79032b8fbb12138b719e657565355fbc79 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6616 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-17 r/4883 feat(tvix/eval): introduce `ToSpan` trait in compiler moduleVincent Ambo1-18/+12
This trait can be used to convert most structures from rnix-parser into a codemap::Span. It uses a macro to implement the trait for the various expression types in the rnix AST, as Rust's silly semantic versioning restriction stops us from doing a blanket implementation. This will be used in the next commit to clean up the span handling in the compiler a bit. Change-Id: I0a437034e5fa203b5a49c6f25c45932a9f3b2bca Reviewed-on: https://cl.tvl.fyi/c/depot/+/6615 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-16 r/4877 feat(tvix/eval): implement legacy let syntaxVincent Ambo1-4/+12
... and emit a warning if anyone decides to use. Change-Id: Iaa6fe9fa932340e6d0fa9f357155e78823702576 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6611 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-16 r/4876 feat(tvix/eval): implement recursive attribute setsVincent Ambo1-22/+67
Yep. This is kind of ugly right now. The idea is that the recursive_scope compilation function is used for recursive sets as well by emitting the keys. The rest of the logic is pretty much identical. There is quite a lot of code here that can be cleaned up (duplication between attrs and let, duplication inside of the recursive scope compilation function etc.), but I'd like to get it working first and then make it nice. Note that nested keys are *not* supported yet. Change-Id: I45c7cdd5f0e1d35fd94797093904740af3a97134 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6610 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-16 r/4875 refactor(tvix/eval): introduce a type for tracking bindingsVincent Ambo1-24/+64
This type is used in the list temporarily populated by the *second* pass over all identifiers in a recursive scope. This first pass only serves to make all bindings known to the compiler, without populating their values yet. Having a type here is going to be useful once we implement `rec`, which needs to thread through slightly more information. Change-Id: Ie33e0f096c5fcb6c864c991255466748b6f0d1eb Reviewed-on: https://cl.tvl.fyi/c/depot/+/6609 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-16 r/4874 refactor(tvix/eval): extract recursive scope logic into a helperVincent Ambo1-7/+14
This needs to be reused between let & `rec` attrs. Change-Id: I4a3bb90af4be32771b0f9e405c19370e105c0fef Reviewed-on: https://cl.tvl.fyi/c/depot/+/6608 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-16 r/4873 refactor(tvix/eval): move compile_inherit_attrs to compiler::attrsVincent Ambo1-62/+0
Plain move, no other changes. Change-Id: Ic4f89709f5c2cbc03182a848af080c820e39a0fd Reviewed-on: https://cl.tvl.fyi/c/depot/+/6607 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-16 r/4872 refactor(tvix/eval): explicitly construct attrs in phasesVincent Ambo1-0/+1
This makes the phases of attribute set construction that Nix has very explicit (inherits, static keys, dynamic keys). This change focuses on the split between dynamic/static keys by collecting all dynamic ones while compiling the static ones, and then phasing them in afterwards. It's possible we also need to do some additional splitting inside of the inherits. Change-Id: Icae782e2a5c106e3ce0831dda47ed81c923c0a42 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6530 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-15 r/4861 refactor(tvix/eval): don't move parts Vec in compile_str_partssterni1-7/+5
This allows us to get rid of the count local variable which was a bit confusing. Calling parts.len() multiple times is fine, since the length doesn't need to be computed. Change-Id: I4f626729ad1bf23a93cb701385c3f4b50c57456d Reviewed-on: https://cl.tvl.fyi/c/depot/+/6584 Autosubmit: sterni <sternenseemann@systemli.org> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-09-15 r/4860 fix(tvix/eval): coerce string interpolation parts to stringsterni1-35/+45
With this puzzle piece of string compilation in place, `compile_str` becomes less redundant, as every part now needs to be compiled the same. The thunking logic becomes a bit trickier, since we need to thunk even in the case of `count == 1` if the single part is interpolating. Splitting the inner (shared) code in a separate function turned out to be easier for making rustc content. Change-Id: I6a554ca599926ae5907d7acffce349c9616f568f Reviewed-on: https://cl.tvl.fyi/c/depot/+/6582 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-09-15 r/4859 fix(tvix/eval): thunk string interpolationsterni1-18/+32
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-13 r/4847 fix(tvix/eval): force exprs inside string interpolationsterni1-1/+5
The expression inside ${…} may return arbitrary values, including thunks, so we need to make sure to force them just in case. Change-Id: Ic11ba00c4c92a10a83becd91233db5f57f6e59c8 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6541 Autosubmit: sterni <sternenseemann@systemli.org> Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-09-13 r/4843 refactor(tvix/eval): point `OpPushWith` span at namespaceVincent Ambo1-1/+1
Pointed out by sterni in cl/6395 Change-Id: I2dda2bb11fef702df05fd7a4fd93b9e717a85dad Reviewed-on: https://cl.tvl.fyi/c/depot/+/6567 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-13 r/4842 refactor(tvix/eval): point `OpAssert` span at conditionVincent Ambo1-1/+1
This is more useful than pointing it at the entire assert expression, as that includes the body as well which is not going to be relevant in the error. Pointed out by sterni in cl/6391 Change-Id: I95a5d1edf90df65e7fa53d4d04502afd6e99e89a Reviewed-on: https://cl.tvl.fyi/c/depot/+/6566 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-13 r/4840 refactor(tvix/eval): encapsulate scope_depth in compiler::scopeVincent Ambo1-12/+6
This field no longer needs to be directly accessible by the compiler. Addresses a sterni lint from cl/6466 Change-Id: I5e6791943d7f0ab3d9b7a30bb1654c4a6a435b1f Reviewed-on: https://cl.tvl.fyi/c/depot/+/6564 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-13 r/4836 feat(tvix/eval): implement initial fancy formatting for errorsVincent Ambo1-10/+6
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/4824 chore(tvix/eval): address current clippy lintsVincent Ambo1-2/+1
Change-Id: I76326c20a525044e89d3cd1392a29faa3414ca04 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6529 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-11 r/4823 refactor(tvix/eval): remove `todo!()` calls in compilerVincent Ambo1-3/+18
It is impossible for tvixbolt to recover from panics, so the user experience of typing an expression using an unsupported feature was that it would get sad and stop responding to input. Instead, raise a normal value-level error of a new variant and continue where possible. Change-Id: Ibe016c92cacb87b85095c0f83758eddc6468053e Reviewed-on: https://cl.tvl.fyi/c/depot/+/6528 Autosubmit: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-11 r/4813 fix(tvix/eval): reduce scope depth in scope moduleVincent Ambo1-2/+0
Change-Id: If32f9e4c9212f20a39baa15d479ff1387c17570d Reviewed-on: https://cl.tvl.fyi/c/depot/+/6500 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-11 r/4811 refactor(tvix/eval): refactor methods for parsing static identsVincent Ambo1-22/+39
Refactors the methods used for determining whether an identifier in a binding (i.e. an `rnix::Attr` node) is a static string, and extracting it. Previously all uses of this logic were for `let`-expressions, where dynamic attributes are always an error. However, we need the same logic to properly implement the phase separation of attribute set compilation. To facilitate this, the actual core logic of these methods now return `Option`, and are only converted to errors in cases where the errors are actually required. Change-Id: Iad7826eff2cb428182521c6f92276310edeae1eb Reviewed-on: https://cl.tvl.fyi/c/depot/+/6498 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-11 r/4810 refactor(tvix/eval): move attrset-related code to compiler::attrsVincent Ambo1-202/+1
There's about to be a lot more code for attrsets (hopefully temporarily as part of an expand&contract cycle), while nested attribute logic is being refactored in preparation for recursive attribute sets. This does not change any functionality. Change-Id: I667565cd810ca7d9046120d1721c2ceb9095550b Reviewed-on: https://cl.tvl.fyi/c/depot/+/6497 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-11 r/4809 fix(tvix/eval): place plain inherits in correct stack slotssterni1-27/+41
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/4807 fix(tvix/eval): wrap asserts in a thunksterni1-1/+3
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/4805 fix(tvix/eval): declare let inherit (from) locals before compilingsterni1-47/+49
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 Ambo1-1/+3
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/4801 refactor(tvix/eval): capture entire with_stack in upvaluesVincent Ambo1-85/+66
This completely rewrites the handling of "dynamic upvalues" to, instead of resolving them at thunk/closure instantiation time (which forces some values too early), capture the entire with stack of parent contexts if it exists. There are a couple of things in here that could be written more efficiently, but I'm first working through this to get to a bug related to with + recursion and the code complexity of some of the optimisations is distracting. Change-Id: Ia538e06c9146e3bf8decb9adf02dd726d2c651cf Reviewed-on: https://cl.tvl.fyi/c/depot/+/6486 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-11 r/4798 fix(tvix/eval): correctly account for slots during list constructionVincent Ambo1-1/+18
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/4797 refactor(tvix/eval): cut down one iteration over locals arrayVincent Ambo1-1/+2
Caught by sterni in cl/6339 Change-Id: I2f2cd746114f14854cf599a7223a42a3e8ebe4fc Reviewed-on: https://cl.tvl.fyi/c/depot/+/6469 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>