about summary refs log tree commit diff
path: root/tvix/eval/src/value
AgeCommit message (Collapse)AuthorFilesLines
2022-12-29 r/5542 refactor(tvix/eval): remove extra Rc<..> around Value::AttrsVincent Ambo2-3/+14
The `im::OrdMap` is already small and cheap to copy while sharing memory, so this is not required anymore. Only the `KV` variant may have slightly larger content, but in practice this doesn't seem to make a difference when comparing the two variants and this one is less complicated. Change-Id: I64a563b209a2444125653777551373cb2989ca7d Reviewed-on: https://cl.tvl.fyi/c/depot/+/7677 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-12-29 r/5541 refactor(tvix/eval): persistent, memory-sharing OrdMap for NixAttrsVincent Ambo4-56/+55
This uses the `im::OrdMap` for `NixAttrs` to enable sharing of memory between different iterations of a map. This slightly speeds up eval, but not significantly. Future work might include benchmarking whether using a `HashMap` and only ordering in cases where order is actually required would help. This switches to a fork of `im` that fixes some bugs with its OrdMap implementation. Change-Id: I2f6a5ff471b6d508c1e8a98b13f889f49c0d9537 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7676 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-12-29 r/5540 refactor(tvix/eval): use im::Vector directly where possibleVincent Ambo2-13/+13
The conversion from im::Vector -> Vec is cheaper for NixList construction (of course), so where possible we should make use of that. This updates most builtins dealing with lists to use Vector directly, and marks the function constructing NixList from Vec as deprecated so that we get appropriate warnings in places where it's still in use. These places are currently inside of JSON serialisation logic which is in flux right now, so lets leave them as-is until it's stabilised. Change-Id: I037f12a2800f2576db4d9526bd935efd079163f0 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7671 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-12-29 r/5534 refactor(tvix/eval): use im::Vector for NixList representationVincent Ambo1-29/+29
This is a persistent, structurally sharing data structure which is more efficient in some of our use-cases. I have verified the efficiency improvement using `hyperfine` repeatedly over expressions on nixpkgs. Lists are not the most performance-critical structure in Nix (that would be attribute sets), but we can already see a small (~5-10%) improvement. Note that there are a handful of cases where we still go via `Vec` that need to be fixed, most notable for `builtins.sort` which can not currently be implemented directly using `im::Vector` because of a restrictive type bound. Change-Id: I237cc50cbd7629a046e5a5e4601fbb40355e551d Reviewed-on: https://cl.tvl.fyi/c/depot/+/7670 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-12-25 r/5486 fix(tvix/eval): fix current clippy warningsVincent Ambo5-10/+9
It's been a while since the last time, so quite a lot of stuff has accumulated here. Change-Id: I0762827c197b30a917ff470fd8ae8f220f6ba247 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7597 Reviewed-by: grfn <grfn@gws.fyi> Autosubmit: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-12-25 r/5485 refactor(tvix/eval): non-recursive thunk forcingAdam Joseph1-29/+115
Introduces continuation-passing-based trampolining of thunk forcing to avoid recursing when forcing deeply nested expressions. This is required for evaluating large expressions. This change was extracted out of cl/7362. Co-authored-by: Vincent Ambo <tazjin@tvl.su> Co-authored-by: Griffin Smith <grfn@gws.fyi> Change-Id: Ifc1747e712663684b2fff53095de62b8459a47f3 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7551 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-12-25 r/5484 feat(tvix/eval): implement From<f64> for ValueRyan Lahfa1-0/+6
Change-Id: I287282a195d6f752260242739332b2357791974a Reviewed-on: https://cl.tvl.fyi/c/depot/+/7625 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-12-22 r/5477 feat(tvix/eval): display function names in documentationVincent Ambo1-1/+8
... if they are known. We currently do not propagate names correctly for curried functions. Change-Id: I19d57fb30a5c0000ccdf690b91076f6b2191de23 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7596 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-12-22 r/5475 feat(tvix/eval): add Value::explain methodVincent Ambo1-0/+33
This value creates a human-readable explanation of a value. This can be used to implement documentation related functionality. For some values, the amount of information displayed can be expanded quite a bit. Change-Id: Ie8c400feae909e7680af163596f99060262e4241 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7592 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-12-21 r/5467 feat(tvix/eval): use `EvalIO::import_path` when coercing pathsVincent Ambo1-1/+2
This "ties the knot" of importing files into a store when referring to them through path literals, e.g. inside of strings. I'm not yet sure if this interface is sufficient for builtins.path (which we haven't implemented at all yet), but it's enough to wire up eval & store initially. In the default implementations nothing interesting happens in this function at all. Change-Id: Ie01ff4161617d1e743a68dbd1a5e54c1b40c0990 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7582 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-12-21 r/5459 feat(tvix/eval): add EvalIO to public crate APIVincent Ambo2-7/+7
This lets users set the `io_handle` field on an `Evaluation`, which is then propagated to the VM. Change-Id: I616d7140724fb2b4db47c2ebf95451d5303a487a Reviewed-on: https://cl.tvl.fyi/c/depot/+/7566 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-12-21 r/5457 refactor(tvix/eval): add a LightSpan type for lighter span trackingVincent Ambo1-9/+12
This type carries the information required for calculating a span (i.e. the chunk and offset), instead of the span itself. The span is then only calculated in cases where it is required (when throwing errors). This reduces the eval time for `builtins.length (builtins.attrNames (import <nixpkgs> {}))` by *one third*! The data structure in chunks that carries span information reduces in-memory size by trading off the speed of retrieving span information. This is because the span information is only actually required when throwing errors (or emitting warnings). However, somewhere along the way we grew a dependency on carrying span information in thunks (for correctly reporting error chains). Hitting the code paths for span retrieval was expensive, and carrying the spans in a different way would still be less cache-efficient. This change is the best tradeoff I could come up with. Refs: b/229. Change-Id: I27d4c4b5c5f9be90ac47f2db61941e123a78a77b Reviewed-on: https://cl.tvl.fyi/c/depot/+/7558 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-12-21 r/5455 feat(tvix/eval): add thunks with suspended native Rust codeAdam Joseph1-1/+45
Having thunks which, when forced, execute native Rust code rather than interpreted opcodes lets us avoid having to bundle `src/libexpr/primops/derivation.nix` like cppnix does by implementing it in Rust instead. Change-Id: If91d77a6736234321eee87ba4b4777eed5a3fe1c Reviewed-on: https://cl.tvl.fyi/c/depot/+/7450 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-12-21 r/5453 feat(tvix/eval): remove `derive(Copy)` from UpvaluesAdam Joseph2-8/+22
Change-Id: I0fa069fbeff6718a765ece948c2c1bce285496f7 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7449 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-12-21 r/5452 feat(tvix/eval): wrap Closure in Rc<> to match cppnix semanticsAdam Joseph3-56/+29
Change-Id: I595087eff943d38a9fc78a83d37e207bb2ab79bc Reviewed-on: https://cl.tvl.fyi/c/depot/+/7443 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-12-03 r/5379 feat(tvix/eval): Continue removing leakage of BTreeMap.Lyle Mantooth2-10/+3
Fixes b/212. Based on feedback in https://cl.tvl.fyi/c/depot/+/7492, all uses of `NixAttrs::from_map` have been removed. Only `from_iter` and `from_kv` remain. Change-Id: I52e25f73018c2aa1843197427516b7a852503e2c Reviewed-on: https://cl.tvl.fyi/c/depot/+/7500 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Autosubmit: IslandUsurper <lyle@menteeth.us>
2022-12-02 r/5375 feat(tvix/eval): impl FromIterator for NixAttrsLyle Mantooth1-0/+18
Allows for the removal of some BTreeMap usage when constructing NixAttrs by allowing any iterator over 2-tuples to build a NixAttrs. Some instances of BTreeMap didn't have anything to do with making NixAttrs, and some were just the best tool for the job, so they are left using the old `from_map` interface. Change-Id: I668ea600b0d93eae700a6b1861ac84502c968d78 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7492 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-12-01 r/5355 feat(tvix/eval): impl Default for AttrsRepAdam Joseph1-1/+7
Change-Id: I3a55413e5004777b90c06cd8655f26abb2faf39b Reviewed-on: https://cl.tvl.fyi/c/depot/+/7448 Autosubmit: Adam Joseph <adam@westernsemico.com> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-11-30 r/5354 feat(tvix/eval): From<Rc<Vec<Value>>> for NixListAdam Joseph1-0/+6
Change-Id: I2ab53453ed7370b520bb929ef7285e4f23eec65b Reviewed-on: https://cl.tvl.fyi/c/depot/+/7453 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Autosubmit: Adam Joseph <adam@westernsemico.com>
2022-11-28 r/5349 feat(tvix/eval): add CoercionKind::ThunksOnlyAdam Joseph1-4/+11
Signed-off-by: Adam Joseph <adam@westernsemico.com> Change-Id: I92acb7e6099a4796d953b2d4d02cca4076ed0fb1 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7426 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-11-27 r/5346 fix(tvix/eval): implement function/thunk ptr-equality for list ordAdam Joseph1-1/+1
With this change, the test introduced by cl/7370 passes. Change-Id: Ie7d2f02a59d61151f14ebd328e6cfa5892cacfb0 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7375 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org> Autosubmit: Adam Joseph <adam@westernsemico.com>
2022-11-27 r/5345 feat(tvix/eval): non-recursive implementation of nix_eq()Adam Joseph2-26/+10
This passes all the function/thunk-pointer-equality tests in cl/7369. Change-Id: Ib47535ba2fc77a4f1c2cc2fd23d3a879e21d8b4c Signed-off-by: Adam Joseph <adam@westernsemico.com> Reviewed-on: https://cl.tvl.fyi/c/depot/+/7358 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-11-26 r/5326 feat(tvix/eval): declare function-pointer-equality dealt withAdam Joseph1-1/+0
Change-Id: If81ff414dba10a0448b905eec373730a68795376 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7376 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Autosubmit: Adam Joseph <adam@westernsemico.com>
2022-11-26 r/5325 feat(tvix/eval): add Closure::ptr_eq()Adam Joseph1-0/+6
See cl/7368 Change-Id: I97630994c3d65f4d16414a0da236ce000a5b6d33 Signed-off-by: Adam Joseph <adam@westernsemico.com> Reviewed-on: https://cl.tvl.fyi/c/depot/+/7374 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-11-26 r/5324 feat(tvix/eval): wrap Closure::upvalues in RcAdam Joseph2-5/+9
See cl/7372; Nix equality semantics require the ability to track pointer equality of upvalue-sets. Signed-off-by: Adam Joseph <adam@westernsemico.com> Change-Id: I82ba517499cf370189a80355e4e46a5caaab7153 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7373 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-11-23 r/5305 feat(tvix/eval): ExactSizeIterator for Iter<KeyValue<'a>> and KeysAdam Joseph1-0/+20
Signed-off-by: Adam Joseph <adam@westernsemico.com> Change-Id: Ia373eb30d8516a056f1349f9011dee9816593d6f Reviewed-on: https://cl.tvl.fyi/c/depot/+/7357 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-11-23 r/5303 feat(tvix/eval): improve panic!() messages in Thunk::value()Adam Joseph1-4/+4
Change-Id: I3b1284e28c350bfed84d643ae7f922f3487e1f2a Reviewed-on: https://cl.tvl.fyi/c/depot/+/7355 Autosubmit: Adam Joseph <adam@westernsemico.com> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-11-23 r/5302 feat(tvix/eval): add NixAttrs::into_iter()Adam Joseph1-0/+52
Signed-off-by: Adam Joseph <adam@westernsemico.com> Change-Id: Ib813d794177c623bf2f12fc2e6a6f304089607d1 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7356 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-11-23 r/5301 feat(tvix/eval): make NixList::clone() cheapAdam Joseph1-24/+20
When we start unrecursivifying (sp?) things, Rust's borrow checker is going to be a headache; its magic only works when you use the CPU stack as your call stack. Fixing the borrow checker issues usually involves adding lots of `clone()`s. Right now `NixList` is the only variant of `Value` that isn't cheap to clone() -- all the others are either a wrapper around Rc or else are of bounded size. Note that this requires dropping the `DerefMut for NixList` instance and using `Vec<Value>` instead in those situations. Change-Id: I5a47df66855342aa2064f8f3cb7934ff422d26bd Signed-off-by: Adam Joseph <adam@westernsemico.com> Reviewed-on: https://cl.tvl.fyi/c/depot/+/7359 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-11-21 r/5299 fix(tvix/eval): aggressively fix a borrow error in nix_eqVincent Ambo1-3/+9
When comparing Nix values for equality, an issue can occur where recursive values contain thunks to themselves which causes borrow errors when forcing them for comparison later down the line. To work around this we clone the values for now. There might be some optimisations possible like checking for thunk equality directly and short-circuiting on that (we have to check what Nix does). Change-Id: I7e75c992ea68f100058f52b4b46168da7d671994 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7314 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-11-21 r/5297 fix(tvix/eval): ensure callable is forced when using call_withVincent Ambo1-0/+4
When passing multiple arguments, every intermediate callable needs to be forced as this is expected by the VM's call_value function. Also adds a debug assertion for this which makes it easier to spot exactly what went wrong. Change-Id: I3aa519cb6cdaab713bd18282bef901c4cd77c535 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7312 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-11-08 r/5269 feat(tvix/eval): Add docstrings as documentation for builtinsGriffin Smith1-0/+8
Add a new `documentation: Option<&'static str>` field to Builtin, and populate it in the `#[builtins]` macro with the docstring of the builtin function, if any. Change-Id: Ic68fdf9b314d15a780731974234e2ae43f6a44b0 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7205 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-11-08 r/5268 feat(tvix/eval): Give names to builtin argumentsGriffin Smith2-10/+16
Refactor the arguments of a Builtin to be a vec of a new BuiltinArgument struct, which contains the old strictness boolean and also a static `name` str - this is automatically determined via the ident for the corresponding function argument in the proc-macro case, and passed in in the cases where we're still manually calling Builtin::new. Currently this name is unused, but in the future this can be used as part of a documentation system for builtins. Change-Id: Ib9dadb15b69bf8c9ea1983a4f4f197294a2394a6 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7204 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-11-08 r/5262 feat(tvix/eval): add helper for selecting required attributesVincent Ambo1-0/+7
Change-Id: Idd4ae78ef55891d89b72b5c2f3afc8b697b4b26e Reviewed-on: https://cl.tvl.fyi/c/depot/+/7189 Reviewed-by: grfn <grfn@gws.fyi> Reviewed-by: Adam Joseph <adam@westernsemico.com> Tested-by: BuildkiteCI Autosubmit: tazjin <tazjin@tvl.su>
2022-11-05 r/5250 refactor(tvix/eval): rename Opcode::DataLocalIdx to DataStackIdxAdam Joseph1-1/+1
It is very confusing that this opcode is called DataLocalIdx, but it carries a StackIdx rather than a LocalIdx. It seems like this really ought to be called DataStackIdx, but maybe I've misunderstood; if so please explain it to me. Change-Id: I91f6ffa759412beef0b91d3c19ec0d873fe51b99 Signed-off-by: Adam Joseph <adam@westernsemico.com> Reviewed-on: https://cl.tvl.fyi/c/depot/+/7088 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-11-04 r/5241 fix(tvix/eval): quote keys which are not valid identifiersAdam Joseph1-1/+24
The impl Display for NixAttrs needs to wrap double quotes around any keys which are not valid Nix identifiers. This commit does that, and adds a test (which fails prior to this commit and passes after this commit). Change-Id: Ie31ce91e8637cb27073f23f115db81feefdc6424 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7084 Autosubmit: Adam Joseph <adam@westernsemico.com> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-11-04 r/5236 fix(tvix/eval): remove impl PartialEq for ValueAdam Joseph6-22/+29
It isn't possible to implement PartialEq properly for Value, because any sensible implementation needs to force() thunks, which cannot be done without a `&mut VM`. The existing derive(PartialEq) has false negatives, which caused the bug which cl/7142 fixed. Fortunately that bug was easy to find, but a silent false negative deep within the bowels of nixpkgs could be a real nightmare to hunt down. Let's just remove the PartialEq impl for Value, and the other derive(PartialEq)'s that depend on it. Signed-off-by: Adam Joseph <adam@westernsemico.com> Change-Id: Iacd3726fefc7fc1edadcd7e9b586e04cf8466775 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7144 Reviewed-by: kanepyork <rikingcoding@gmail.com> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-31 r/5226 fix(tvix/eval): nix_eq() must recurseAdam Joseph1-3/+3
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-29 r/5222 feat(tvix/eval): Implement builtins.sortGriffin Smith1-1/+7
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 Smith2-1/+24
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/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-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-25 r/5198 feat(tvix/eval): add builtins.{floor,ceil}James Landrein1-0/+1
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 Smith1-0/+21
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 Smith1-0/+10
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/5189 feat(nix/eval): Implement builtins.groupByGriffin Smith2-0/+23
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/5186 feat(tvix/eval): initial attempt at setting lambda namesVincent Ambo1-9/+7
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/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/5175 feat(tvix/eval): Implement builtins.deepSeqGriffin Smith3-6/+78
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-19 r/5159 feat(tvix/eval): deduplicate overlap between Closure and ThunkAdam Joseph2-74/+99
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>
generated by cgit-pink 1.4.1 (git 2.44.2) at 2024-11-22T19·19+0000