about summary refs log tree commit diff
path: root/tvix
AgeCommit message (Collapse)AuthorFilesLines
2022-10-31 r/5226 fix(tvix/eval): nix_eq() must recurseAdam Joseph5-3/+5
The current implementation of nix_eq will force one level of thunks and then switch to the (non-forcing) rust Eq::eq() method. This gives incorrect results for lists-of-thunks. This commit changes nix_eq() to be recursive. A regression test (which fails prior to this commit) is included. This fix also causes nix_tests/eval-okay-fromjson.nix to pass, so it is moved out of notyetpassing. Change-Id: I655fd7a5294208a7b39df8e2c3c12a8b9768292f Signed-off-by: Adam Joseph <adam@westernsemico.com> Reviewed-on: https://cl.tvl.fyi/c/depot/+/7142 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-31 r/5225 docs(tvix/eval): builtins.md: note implementation statusAdam Joseph1-116/+123
We're getting close to the finish line, folks. I went through the list of builtins and there are only 33 that remain unimplemented. I've marked them, and indicated which are ready to be implemented vs which are waiting for other things. We can delete this column from the table once everything is implemented. Signed-off-by: Adam Joseph <adam@westernsemico.com> Change-Id: Idfaef93283536288b12e59aef5c3e1cd139bd133 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7140 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-30 r/5224 docs(tvix/eval): builtins.md: mark impureAdam Joseph1-7/+7
I believe that the currentTime, findFile, hashFile, pathExists, readDir, path (unless ?sha256), and readFile builtins are impure. This commit marks them as such in docs/builtins.md. Change-Id: Ib1b59fe643dde73cb2b00050b4ef9d3401ad22eb Signed-off-by: Adam Joseph <adam@westernsemico.com> Reviewed-on: https://cl.tvl.fyi/c/depot/+/7139 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-29 r/5223 test(tvix/eval): builtins.sort must preserve order of equal elementssterni2-0/+8
Change-Id: I59a0756940d1e5360a2ab4e886cf0bc9af7b8901 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7133 Autosubmit: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-29 r/5222 feat(tvix/eval): Implement builtins.sortGriffin Smith4-3/+46
This is a bit tricky because the comparator can throw errors, so we need to propagate them out if they exist and try to avoid sorting forever by returning a reasonable ordering in this case (as short-circuiting is not available). Co-Authored-By: Vincent Ambo <tazjin@tvl.su> Change-Id: Icae1d30f43ec1ae64b2ba51e73ee467605686792 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7072 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-10-29 r/5221 feat(tvix/eval): Implement comparison for listsGriffin Smith7-3/+45
Lists are compared lexicographically in C++ nix as of [0], and our updated nix test suites depend on this. This implements comparison of list values in `Value::nix_cmp` using a very similar algorithm to what C++ does - similarly to there, this requires passing in the VM so we can force thunks in the list elements as we go. [0]: https://github.com/NixOS/nix/commit/09471d2680292af48b2788108de56a8da755d661# Change-Id: I5d8bb07f90647a1fec83f775243e21af856afbb1 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7070 Autosubmit: grfn <grfn@gws.fyi> Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-10-28 r/5220 feat(tvix/eval): builtins.replaceStrings: don't clone() N timesAdam Joseph1-8/+10
CL/7034 looks great, except that for a length-N target string it will perform N deep copies of each of the from and to-lists. Let's use references instead of clones. Signed-off-by: Adam Joseph <adam@westernsemico.com> Change-Id: Icd341213a9f0e728f9c8453cec6d23af5e1dea91 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7095 Reviewed-by: wpcarro <wpcarro@gmail.com> Reviewed-by: j4m3s <james.landrein@gmail.com> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-28 r/5219 feat(tvix/eval): add builtins.replaceStringsJames Landrein5-0/+77
Change-Id: I93dcdaeb101364ee2273bcaeb19acb57cf6b9e7d Reviewed-on: https://cl.tvl.fyi/c/depot/+/7034 Autosubmit: j4m3s <james.landrein@gmail.com> Reviewed-by: Adam Joseph <adam@westernsemico.com> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
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 Landrein7-0/+13
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/5170 test(tvix/verify-lang-tests): also check notyetpassing testssterni1-12/+12
Adding these to the C++ Nix CI ensures that it is possible for the tests in that directory to pass at all. Mainly this would catch situations like fixed in a previous CL when moving the tests around would break them so that they wouldn't even pass in C++ Nix. For this to work, we need to track skips by basename to be directory-independent (assuming that every skipped test name is unique is hopefully okay). Change-Id: If6cb63ebdef0fc19b082b6a04e79ada2e47c658e Reviewed-on: https://cl.tvl.fyi/c/depot/+/7048 Autosubmit: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
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 mastersterni23-32/+94
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.11sterni2-120/+183
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>
generated by cgit-pink 1.4.1 (git 2.44.2) at 2024-11-16T23·28+0000