about summary refs log tree commit diff
path: root/tvix/eval
AgeCommit message (Collapse)AuthorFilesLines
2022-10-28 r/5218 docs(tvix/eval): warn that AttrsRep::KV is not for Key-Value pairsAdam Joseph1-1/+8
I assumed that AttrsRep::KV represented attrsets with a single attribute as a Key-Value pair. That is not the case. Let's warn other people about this. Signed-off-by: Adam Joseph <adam@westernsemico.com> Change-Id: Ie3d2765fcc1ab705c153ab94ffe77bbd6d4ab39e Reviewed-on: https://cl.tvl.fyi/c/depot/+/7093 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-28 r/5217 docs(tvix/eval): add "intern literals" to future optimisationsAdam Joseph1-0/+6
Signed-off-by: Adam Joseph <adam@westernsemico.com> Change-Id: I460230863de853ca5248733bc977d4780b216f36 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7096 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-10-27 r/5215 fix(tvix/eval): correct wasm32-unknown-unknown to wasm32-noneAdam Joseph1-0/+11
Rustc uses wasm32-unknown-unknown, which is rejected by config.sub, for wasm-in-the-browser environments. Rustc should be using wasm32-unknown-none, which config.sub accepts. Hopefully the rustc people will change their triple before stabilising this triple. In the meantime, we fix it here in order to unbreak tvixbolt. https://doc.rust-lang.org/beta/nightly-rustc/rustc_target/spec/wasm32_unknown_unknown/index.html Signed-off-by: Adam Joseph <adam@westernsemico.com> Change-Id: I941fd8d6f3db4e249901772fd79321ad88cd9cc6 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7107 Reviewed-by: tazjin <tazjin@tvl.su> Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-10-27 r/5214 refactor(tvix/eval): search-and-replace changesAdam Joseph1-5/+5
This commit contains two search-and-replace renames which are broken out from I04131501029772f30e28da8281d864427685097f in order to reduce the noise in that CL: - `is_thunk -> is_suspended_thunk`, since there are now OpThunkClosure and OpThunkSuspended - `compile_lambda_or_thunk` -> `compile_lambda_or_suspension` Change-Id: I7cc5bbb75ef6605e3428c7be27e812f41a10c127 Signed-off-by: Adam Joseph <adam@westernsemico.com> Reviewed-on: https://cl.tvl.fyi/c/depot/+/7037 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-27 r/5213 feat(tvix/eval): builtins.import without RefCellAdam Joseph5-137/+154
CL/6867 added support for builtins.import, which required a cyclic reference import->globals->builtins->import. This was implemented using a RefCell, which makes it possible to mutate the builtins during evaluation. The commit message for CL/6867 expressed a desire to eliminate this possibility: 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. This CL replaces the RefCell with Rc::new_cyclic(), making the globals/builtins immutable once again. At VM runtime (once opcodes start executing) everything is the same as before this CL, except that the Rc<RefCell<>> introduced by CL/6867 is turned into an rc::Weak<>. The function passed to Rc::new_cyclic works very similarly to overlays in nixpkgs: a function takes its own result as an argument. However instead of laziness "breaking the cycle", Rust's Rc::new_cyclic() instead uses an rc::Weak. This is done to prevent memory leaks rather than divergence. This CL also resolves the following TODO from CL/6867: // TODO: encapsulate this import weirdness in builtins The main disadvantage of this CL is the fact that the VM now must ensure that it holds a strong reference to the globals while a program is executing; failure to do so will cause a panic when the weak reference in the builtins is upgrade()d. In theory it should be possible to create strong reference cycles the same way Rc::new_cyclic() creates weak cycles, but these cycles would cause a permanent memory leak -- without either an rc::Weak or RefCell there is no way to break the cycle. At some point we will have to implement some form of cycle collection; whatever library we choose for that purpose is likely to provide an "immutable strong reference cycle" primitive similar to Rc::new_cyclic(), and we should be able to simply drop it in. Signed-off-by: Adam Joseph <adam@westernsemico.com> Change-Id: I34bb5821628eb97e426bdb880b02e2097402adb7 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7097 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-26 r/5201 docs(tvix/eval): StackIdx, LocalIdx UpvalueIdxAdam Joseph2-5/+7
This adds a comment noting that StackIdx is an offset relative to the base of the current CallFrame, whereas UpvalueIdx is an absolute index into the upvalues array. It also removes the confusing mention of StackIdx in the descriptive comment for LocalIdx. They index into totally different structures; one exists at runtime and the other exists at compile time. Change-Id: Ib932b1b0679734c15001e8c5c95a08293fa016b4 Signed-off-by: Adam Joseph <adam@westernsemico.com> Reviewed-on: https://cl.tvl.fyi/c/depot/+/7017 Reviewed-by: grfn <grfn@gws.fyi> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-26 r/5200 feat(tvix/eval): add NixList::force_elements()Adam Joseph1-0/+5
This adds a function NixList::force_elements() which forces each element of the list shallowly. This behavior is needed for `builtins.replaceStrings`, and probably a few other builtins as well. Signed-off-by: Adam Joseph <adam@westernsemico.com> Change-Id: I3f0681acbbfe50e781b5f07b6a441647f5e6f8da Reviewed-on: https://cl.tvl.fyi/c/depot/+/7094 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-26 r/5199 feat(tvix/eval): include filename of failing test when failingAdam Joseph1-5/+5
Unfortunately we have to mangle test case filenames into rust-valid symbols, since test-generator doesn't use `r#"..."` (deliberately?). This means that when a test fails, there's nothing on the console you can copy-and-paste in order to view/edit the code of the failing test case. This commit (partially) fixes it by including the unmangled name in the panic!() string. However failures due to panic!()s inside the vm (including deliberate panics due to panic!()-debugging) still won't display an unmangled filename. Maybe we should reconsider the use of test-generator? Change-Id: I2208a859ffab1264f17f48fd303ff5e19675967e Signed-off-by: Adam Joseph <adam@westernsemico.com> Reviewed-on: https://cl.tvl.fyi/c/depot/+/7092 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-25 r/5198 feat(tvix/eval): add builtins.{floor,ceil}James Landrein6-0/+11
Change-Id: I4e6c4f96f6f5097a5c637eb3dbbd7bb8b34b7d52 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7032 Autosubmit: j4m3s <james.landrein@gmail.com> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su> Reviewed-by: grfn <grfn@gws.fyi>
2022-10-24 r/5193 refactor(tvix/eval): Implement value comparison with a methodGriffin Smith3-26/+47
Rather than implementing all of the interesting semantics of value comparison with a macro bound to the VM, implement the bulk of the logic with a method on Value itself that returns an Ordering, and then use the macro to implement the comparison against that Ordering. This has no functional change, but paves the way to implementing lexicographic comparison of list values, which is supported in the latest version of upstream nix. Change-Id: I8af1a020b41577021af5939f5edc160c407d4a9e Reviewed-on: https://cl.tvl.fyi/c/depot/+/7069 Autosubmit: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-24 r/5192 feat(tvix/eval): Implement builtins.mapAttrsGriffin Smith4-0/+23
I played around a little bit with doing this in-place, but ended up going with this perhaps slightly clone-heavy approach for now because ideally most clones on Value are cheap - but later we should benchmark alternate approaches that get to reuse allocations better if necessary or possible. Change-Id: If998eb2056cedefdf2fb480b0568ac8329ccfc44 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7068 Autosubmit: grfn <grfn@gws.fyi> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-24 r/5191 feat(tvix/eval): add builtins.langVersionVincent Ambo1-0/+2
The last bump in langVersion (5->6) in C++ Nix was due to making lists comparable (commit `09471d2680292af48b2788108de56a8da755d661`), which we support in Tvix with cl/7070. Change-Id: Id3beed5150b8fb6e0a46a4d1b7e3942022a65346 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7074 Autosubmit: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-10-24 r/5190 feat(tvix/eval): implement builtins.currentSystemAdam Joseph6-0/+360
This commit implements builtins.currentSystem, by capturing the cargo environment variable `TARGET` and exposing it to rustc as `TVIX_CURRENT_SYSTEM` so it can be inserted into the source code using `env!()`. The resulting value needs to be massaged a bit, since it is an "LLVM triple". The current code should work for all the platforms for which cppnix works (thanks qyliss for generating the list!). It does *not* reject all of the triples that cppnix's configure.ac rejects -- it is much more forgiving. We can tighten this up in a future commit. Signed-off-by: Adam Joseph <adam@westernsemico.com> Change-Id: I947f504b2af5a7fee8cf0cb301421d2fc9174ce1 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6986 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-24 r/5189 feat(nix/eval): Implement builtins.groupByGriffin Smith3-0/+34
Change-Id: I3e0aa017a7100cbeb86d2e5747471b36affcc102 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7038 Autosubmit: grfn <grfn@gws.fyi> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-23 r/5187 fix(tvix/eval): Use natural arg order for call_withGriffin Smith2-2/+3
Since we push arguments onto a stack when calling multi-argument functions, we actually were ending up calling `call_with` with the arguments in the *reverse order* - we patched around this by passing the arguments in the reverse order for `foldl'`, but it makes more sense to have them just be the order that the function would be called with in user surface code instead. Change-Id: Ifddb98f46970ac89872383709c3ce758dc965c65 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7067 Reviewed-by: sterni <sternenseemann@systemli.org> Autosubmit: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-10-23 r/5186 feat(tvix/eval): initial attempt at setting lambda namesVincent Ambo4-15/+39
When compiling a lambda, take the name of the outer slot (if available) and store it as the name on the lambda. These names are then shown in the observer, and nowhere else (so far). It is of course common for these things to thread through many different context levels (e.g. `f = a: b: c: ...`), in this setup only the outermost closure or thunk gains the name, but it's better than nothing. Change-Id: I681ba74e624f2b9e7a147144a27acf364fe6ccc7 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7065 Reviewed-by: grfn <grfn@gws.fyi> Autosubmit: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-23 r/5185 test(tvix/eval): add a test case for nested sibling accessVincent Ambo2-0/+8
Change-Id: I3898284bc4871e5483ead0c27e0f2832d66f2b29 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7061 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-10-23 r/5184 fix(tvix/eval): thunk let-expressionVincent Ambo1-1/+5
There are some rare scope cases with deferred access where this doesn't behave correctly otherwise. Change-Id: I6c774f5e62c1cb50b598026c54727017a52cd22d Reviewed-on: https://cl.tvl.fyi/c/depot/+/7064 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-10-23 r/5183 fix(tvix/eval): fix condition for useless inherit warningVincent Ambo1-2/+2
The warning needs to consider whether it is occuring inside of a thunk, i.e. the dynamic ancestry chain of lambda contexts must be inspected and not just the current scope. Change-Id: I5cf5482d67a8bbb9f03b0ecee7a62f58754f8e59 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7063 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org> Reviewed-by: grfn <grfn@gws.fyi>
2022-10-23 r/5182 refactor(tvix/eval): simplify check for deferring upvalue resolutionVincent Ambo1-9/+4
This check is now actually simply equivalent to checking whether the target has been initialised or not. Change-Id: I30660d11073ba313358f3a64234a90ed81abf74c Reviewed-on: https://cl.tvl.fyi/c/depot/+/7062 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-10-23 r/5181 feat(tvix/eval): add mechanism for placeholder builtinsVincent Ambo1-0/+19
These are builtins which can be basically implemented as the identity function from the perspective of pure evaluation, and which help us get closer to evaluating nixpkgs. For now, builtins added here will be "usable" and just emit a warning about not being implemented yet. Change-Id: I0fce94677f01c98c0392aeefb7ab353c7dc7ec82 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7060 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-10-23 r/5180 refactor(tvix/eval): simplify self-reference checkVincent Ambo1-1/+1
Checking the computed depth and stack slot against the computed depth and stack slot is equivalent to just checking the indices into the locals vector against each other (i.e. "is the slot we're compiling into the slot we're accessing?") Change-Id: Ie85a68df073e3b2e3d9aba7fe8634c48eada81fc Reviewed-on: https://cl.tvl.fyi/c/depot/+/7059 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-10-23 r/5179 chore(tvix/eval): return detailed TvixBug if an upvalue is missingVincent Ambo1-1/+17
When capturing an upvalue, return a detailed TvixBug error that contains metadata about what exactly was missing. This particular thing helps with debugging some scope issues we still seem to have. Change-Id: I1089a1df4b3bbc63411a4907c3641a5dc3fad984 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7058 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-10-23 r/5178 fix(tvix/eval): detect cycles when printing infinite valuesVincent Ambo4-24/+45
Using the same method as in Thunk::deep_force, detect cycles when printing values by maintaining a set of already seen thunks. With this, display of infinite values matches that of Nix: > nix-instantiate --eval --strict -E 'let as = { x = 123; y = as; }; in as' { x = 123; y = { x = 123; y = <CYCLE>; }; } > tvix-eval -E 'let as = { x = 123; y = as; }; in as' => { x = 123; y = { x = 123; y = <CYCLE>; }; } :: set Change-Id: I007b918d5131d82c28884e46e46ff365ef691aa8 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7056 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-10-22 r/5177 feat(tvix/eval): add `TvixBug` error kindVincent Ambo1-1/+23
This can be raised in cases where some panics lurk (e.g. indexed accesses). Change-Id: I93f4d60568277e9c5635aa52f378645626d68c5e Reviewed-on: https://cl.tvl.fyi/c/depot/+/7057 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-10-22 r/5176 test(tvix/eval): add a test for observing an infinite attribute setVincent Ambo2-0/+5
Note that this test (ironically) fails if the observer is used (e.g. with --trace-runtime), but that's a separate issue. Change-Id: I952eaeac8b5a7acce9c66cd4744ec570280748e7 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7055 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-10-22 r/5175 feat(tvix/eval): Implement builtins.deepSeqGriffin Smith10-62/+100
This is done via a new `deepForce` function on Value. Since values can be cyclical (for example, see the test-case), we need to do some extra work to avoid RefCell borrow errors if we ever hit a graph cycle: While deep-forcing values, we keep a set of thunks that we have already seen and avoid doing any work on the same thunk twice. The set is encapsulated in a separate type to stop potentially invalid pointers from leaking out. Finally, since deep_force is conceptually similar to `VM::force_for_output` (but more suited to usage in eval since it doesn't clone the values) this removes the latter, replacing it with the former. Co-Authored-By: Vincent Ambo <tazjin@tvl.su> Change-Id: Iefddefcf09fae3b6a4d161a5873febcff54b9157 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7000 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi> Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-22 r/5174 fix(tvix/eval): use top-level span for `force_with_output`Vincent Ambo3-6/+29
When forcing thunks in `force_with_output`, the call stack of the VM is actually empty (as the calls are synthetic and no longer part of the evaluation of the top-level expression). This means that Tvix crashed when constructing error spans for the `fallible` macro, as the assumption of there being an enclosing span was violated. To work around this, we instead pass the span for the whole top-level expression to force_for_output and set this as the span for the enclosing error chain. Existing output logic will already avoid printing the entire expression as an error span. This fixes b/213. Change-Id: I93978e0deaf5bcb0f47a6fa95b3f5bebef5bad4c Reviewed-on: https://cl.tvl.fyi/c/depot/+/7052 Autosubmit: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-10-21 r/5172 fix(tvix): distinguish search- and relative path resolution errorssterni4-11/+19
Failures to resolve a nix search path lookup in angle brackets can be caught using tryEval (if it reaches the runtime). Resolving relative paths (either to the current directory or the current user's home) can never be caught, even if they happen inside a thunk at runtime (which is currently the case for home-relative paths). Change-Id: I7f73221df66d82a381dd4063358906257826995a Reviewed-on: https://cl.tvl.fyi/c/depot/+/7025 Autosubmit: sterni <sternenseemann@systemli.org> Reviewed-by: Adam Joseph <adam@westernsemico.com> Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-10-20 r/5168 fix(tvix/eval): restore .exp.xml files and skip in test suitesterni3-0/+7
Change-Id: Iebda5e0d99925a0a8c1d6ae1d7a35397d127bf31 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7047 Autosubmit: sterni <sternenseemann@systemli.org> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-20 r/5167 fix(tvix/eval): fix path in notyetpassing testssterni4-4/+4
cl/7036 moved these paths around, but neglected to update the relative paths they contain. Without these updated, they will never start passing! Change-Id: Ib16468611af59729883e501be8486f43d850fd58 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7046 Autosubmit: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-20 r/5166 test(tvix/eval): eval-okay-eq.nix can actually be testedsterni3-1/+1
eval-okay-eq.nix is actually an ancient test case that used the ATerm syntax for the test result. It was disabled for a while, since the behavior got reverted for a bit, but works on any reasonably modern Nix implementation. This change matches my [C++ Nix PR]. [C++ Nix PR]: https://github.com/NixOS/nix/pull/7196 Change-Id: I602fd7c83a0bc104ab502c8b6a74e4591272be1a Reviewed-on: https://cl.tvl.fyi/c/depot/+/7045 Autosubmit: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-20 r/5165 fix(tvix/eval): eval-okay-pathexists test can be executedsterni2-0/+0
Change-Id: Ibdcaa165024584370ce9578e67985a3526e44f77 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7044 Autosubmit: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-20 r/5164 test(tvix/eval): update C++ Nix test suite to current mastersterni22-31/+85
The language test suite actually doesn't require flakes and the new features are mostly sensible (added builtins) as well as some tests for regressions the C++ implementation experienced. The path interpolation test is not included in this update because there is no way to construct an location-independent .exp file for it (the C++ repo also doesn't have one). We may still want to implement that feature eventually (in case rnix adds support for it). The C++ Nix revision used is ac0fb38e8a5a25a84fa17704bd31b453211263eb. Change-Id: I75f1e780ddeeee6f6b1f28cf3c66c288dca2c20c Reviewed-on: https://cl.tvl.fyi/c/depot/+/7043 Autosubmit: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-20 r/5163 test(tvix/eval): enable nix_tests by defaultsterni1-1/+1
Since cl/7036 we have a mechanism for dealing with the nix_tests we do not pass yet. Change-Id: I246c52963ae7f2500253f4035a77d7006dd35307 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7049 Autosubmit: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-20 r/5162 test(tvix): nix-planned test verification using C++ Nix 2.3 and 2.11sterni1-120/+0
Reimplement the test discovery of the lang tests script in Nix which allows for a more flexible skipping logic that can e.g. react to the C++ Nix version used. This allows us to run the test suite against both C++ Nix 2.3 and the latest C++ Nix version 2.11. The latter is mainly useful, so we can implement newer Nix features and still verify them against the C++ implementation. Change-Id: I30c802844133b86b5e49f5e4f4fefacdb6215e0e Reviewed-on: https://cl.tvl.fyi/c/depot/+/7042 Autosubmit: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-19 r/5161 feat(tvix/eval): expect not-yet-passing tests to failAdam Joseph74-14/+49
It is helpful to be able to use the test suite as a regression test: make a change to the compiler/vm, re-run the tests, and if there are any failures you know it's your fault. Right now we can't do that, because the expected-to-fail tests are mixed in with the expected-to-pass tests. So we can't use them as a regression test. Change-Id: Ied606882b9835a7effd7e75bfcf3e5f827e0a2c8 Signed-off-by: Adam Joseph <adam@westernsemico.com> Reviewed-on: https://cl.tvl.fyi/c/depot/+/7036 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-19 r/5159 feat(tvix/eval): deduplicate overlap between Closure and ThunkAdam Joseph9-149/+203
This commit deduplicates the Thunk-like functionality from Closure and unifies it with Thunk. Specifically, we now have one and only one way of breaking reference cycles in the Value-graph: Thunk. No other variant contains a RefCell. This should make it easier to reason about the behavior of the VM. InnerClosure and UpvaluesCarrier are no longer necessary. This refactoring allowed an improvement in code generation: `Rc<RefCell<>>`s are now created only for closures which do not have self-references or deferred upvalues, instead of for all closures. OpClosure has been split into two separate opcodes: - OpClosure creates non-recursive closures with no deferred upvalues. The VM will not create an `Rc<RefCell<>>` when executing this instruction. - OpThunkClosure is used for closures with self-references or deferred upvalues. The VM will create a Thunk when executing this opcode, but the Thunk will start out already in the `ThunkRepr::Evaluated` state, rather than in the `ThunkRepr::Suspeneded` state. To avoid confusion, OpThunk has been renamed OpThunkSuspended. Thanks to @sterni for suggesting that all this could be done without adding an additional variant to ThunkRepr. This does however mean that there will be mutating accesses to `ThunkRepr::Evaluated`, which was not previously the case. The field `is_finalised:bool` has been added to `Closure` to ensure that these mutating accesses are performed only on finalised Closures. Both the check and the field are present only if `#[cfg(debug_assertions)]`. Change-Id: I04131501029772f30e28da8281d864427685097f Signed-off-by: Adam Joseph <adam@westernsemico.com> Reviewed-on: https://cl.tvl.fyi/c/depot/+/7019 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-19 r/5158 feat(tvix/eval): NixList::concat(): avoid an unnecessary moveAdam Joseph1-4/+3
In `a++b`, the previous implementation would move `b` (i.e. memcpy its elements) twice. Let's do that only once. We sure do call NixList.clone() a whole lot. At some point in the future we probably want to do a SmolStr-type split for NixList into a two-variant enum where one side is an Rc<Vec<Value>> for lists longer than a certain length. Signed-off-by: Adam Joseph <adam@westernsemico.com> Change-Id: I32154d18785a1f663454a8b9d4afd3e78bffdf9c Reviewed-on: https://cl.tvl.fyi/c/depot/+/7040 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-18 r/5157 docs(tvix/eval): upvalues.rs: define "upvalue", comment with_stackAdam Joseph1-9/+30
This commit adds a comment for the benefit of non-Lua-users explaining what an upvalue is. It also adds a comment explaining what `with_stack` does, including a brief reminder of nix's slightly-unusual-but-well-justified handling of this construct. Signed-off-by: Adam Joseph <adam@westernsemico.com> Change-Id: If6d0e133292451af906e1c728e34010f99be8c7c Reviewed-on: https://cl.tvl.fyi/c/depot/+/7007 Reviewed-by: j4m3s <james.landrein@gmail.com> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-18 r/5156 fix(tvix/eval): wrap dynamic resolution in an extra thunkVincent Ambo4-3/+16
Without this change it was possible to cause situations (see the new test) in which a `with`-namespace was forced prematurely. Change-Id: I879ea7763b43edc693feace2c73c890d426fafd3 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7031 Autosubmit: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Reviewed-by: Adam Joseph <adam@westernsemico.com>
2022-10-17 r/5155 feat(nix/eval): Implement builtins.functionArgsGriffin Smith4-0/+100
Now that we're tracking formals on Lambda this ends up being quite easy; we just pull them off of the Lambda for the argument closure and use them to construct the result attribute set. Change-Id: I811cb61ec34c6bef123a4043000b18c0e4ea0125 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7003 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-17 r/5154 feat(tvix/eval): Validate closed formalsGriffin Smith7-13/+133
Validate "closed formals" (formal parameters without an ellipsis) via a new ValidateClosedFormals op, which checks the arguments (in an attr set at the top of the stack) against the formal parameters on the Lambda in the current frame, and returns a new UnexpectedArgument error (including the span of the formals themselves!!) if any arguments aren't allowed Change-Id: Idcc47a59167a83be1832a6229f137d84e426c56c Reviewed-on: https://cl.tvl.fyi/c/depot/+/7002 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-17 r/5153 feat(tvix/eval): Record formals on lambdaGriffin Smith4-13/+42
In preparation for both implementing the `functionArgs` builtin and adding support for validating closed formals, record information about the formal arguments to a function *on the Lambda itself*. This may seem a little odd for the purposes of just closed formal checking, but is something we have to have anyway for builtins.functionArgs so I figured I'd do it this way to kill both birds with one stone. Change-Id: Ie3770a607bf352a1eb395c79ca29bb25d5978cd8 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7001 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-17 r/5152 feat(tvix/eval): Implement builtins.seqGriffin Smith4-0/+8
Since we already have infra for forcing arguments to builtins, this ends up being almost *too* simple - we just return the second argument! Change-Id: I070d3d0b551c4dcdac095f67b31e22e0de90cbd7 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6999 Reviewed-by: kanepyork <rikingcoding@gmail.com> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-17 r/5151 docs(tvix/eval) comments for various fieldsAdam Joseph3-5/+27
Change-Id: I8dcddf2b419761e475e71215c199eef2f7dc61dc Reviewed-on: https://cl.tvl.fyi/c/depot/+/7028 Autosubmit: Adam Joseph <adam@westernsemico.com> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-16 r/5150 refactor(tvix/eval): unify compile_lambda() with thunk()Adam Joseph1-61/+59
This resolves a TODO. Signed-off-by: Adam Joseph <adam@westernsemico.com> Change-Id: If4d2124648ac88094e547e1ad7f1b446feb26182 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7010 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-16 r/5149 fix(tvix/eval): more faithfully serialise ast::LiteralVincent Ambo1-10/+9
The previous serialisation format kind of lost the information about what AST node we're dealing with (e.g. `1234` would serialise to an AST with a literal `1234`). That's great for pretty-printing the _code_, but we explicitly want to serialise how rnix-parser parses something. To that end, literals are now instead serialised into a structure like all the other ones (`kind: literal` and appropriate value fields). Change-Id: I586c95d7db41820b8ec43565ba4016ed3834d1b5 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7030 Autosubmit: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Reviewed-by: j4m3s <james.landrein@gmail.com> Reviewed-by: Adam Joseph <adam@westernsemico.com>
2022-10-16 r/5148 feat(tvix/eval): implement builtins.partitionJames Landrein3-0/+39
Change-Id: I8b591f3057c68c1542046fc5a771973f2238c9df Reviewed-on: https://cl.tvl.fyi/c/depot/+/7020 Autosubmit: j4m3s <james.landrein@gmail.com> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-16 r/5147 fix(tvix/eval): resolve home relative paths at runtimesterni3-12/+31
Home relative paths depend on the environment to be resolved. We have elected to do everything that depends on the environment, e.g. resolving SPATH expressions using NIX_PATH, at runtime, so tvix evaluation would continue to behave correctly even if we separated the compilation and execution phases more, e.g. via serializing bytecode. Then the value of HOME, NIX_PATH etc. could reasonably change in the time until execution, yielding wrong results if the resolution results were cached in the bytecode. We also take the opportunity to fix the broken path concatenation previously found in the compiler, fixing b/205. Another thing we could consider is emitting a warning for home relative path literals, as they are by nature relatively fragile. One sideeffect of this change is that home path resolution errors become catchable which is not the case in C++ Nix. This will need to be fixed up in a subsequent change. Change-Id: I30bd69b575667c49170a9fdea23a020565d0f9ec Reviewed-on: https://cl.tvl.fyi/c/depot/+/7024 Autosubmit: sterni <sternenseemann@systemli.org> Reviewed-by: Adam Joseph <adam@westernsemico.com> Tested-by: BuildkiteCI
generated by cgit-pink 1.4.1 (git 2.44.2) at 2024-11-10T01·17+0000