about summary refs log tree commit diff
path: root/tvix/eval/src/value
AgeCommit message (Collapse)AuthorFilesLines
2023-02-03 r/5828 fix(tvix/eval): ensure all evaluated thunks are correctly memoizedVincent Ambo1-45/+185
This fixes a very complicated bug (b/246). Evaluation progresses *much* further after this, leading to several less complicated bugs likely being uncovered by this What was the problem? ===================== Previously, when evaluating a thunk, we had a code path that looked like this: match *thunk { ThunkRepr::Evaluated(Value::Thunk(ref inner_thunk)) => { let inner_repr = inner_thunk.0.borrow().clone(); drop(thunk); self.0.replace(inner_repr); } /* ... */ } This code path created a copy of the inner `ThunkRepr` of a nested thunk, and moved that copy into the `ThunkRepr` of the parent. The effect of this was that the original `ThunkRepr` (unforced!) lived on in the original thunk, without the memoization of the subsequent forcing applying to it. This had the result that Tvix would repeatedly evaluate these thunks without ever memoizing them, if they occured repeatedly as shared inner thunks. Most notably, this would *always* occur when builtins.import was used. What's the solution? ==================== I have completely rewritten `Thunk::force_trampoline_self` to make all flows that can occur in it explicit. I have also removed the outer loop inside of that function, and resorted to more use of trampolining instead. The function is now well-commented and it should be possible to read it from top-to-bottom and get a general sense of what is going on, though the trampolining itself (which is implemented in the VM) needs to be at least partially understood for this. What's the new problem(s)? ========================== One new (known) problem is that we have to construct `Error` instances in all error types here, but we do not have spans available in some thunk-related situations. Due to b/238 we cannot ask the VM for an arbitrary span from the callsite leading to the force. This means that there are now code paths where, under certain conditions, causing an evaluation error during thunk forcing will panic. To fix this we will need to investigate and fix b/238, and/or add a span tracking mechanism to thunks themselves. What other impacts does this have? ================================== With this commit, eval of nixpkgs mostly succeeds (things like stdenv evaluate to the same hashes for us and C++ Nix, meaning we now construct identical derivations without eval breaking). Due to this we progress much further into nixpkgs, which lets us uncover more additional bugs. For example, after this commit we can quickly see that cl/7949 introduces some kind of behavioural issue and should not be merged as-is (this was not apparent before). Additionally, tvix-eval is now seemingly very fast. When doing performance analysis of a nixpkgs eval, we now mostly see the code path for shelling out to C++ Nix to add things to the store in there. We still need those code paths, so we can not (yet) do a performance analysis beyond that. Change-Id: I738525bad8bc5ede5d8c737f023b14b8f4160612 Reviewed-on: https://cl.tvl.fyi/c/depot/+/8012 Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de>
2023-02-02 r/5821 chore(tvix/eval): elaborate on internal types in Value::type_ofVincent Ambo1-6/+8
This aids in debugging quite substantially. Change-Id: Ic43232aa6165ae1c3db7ac2701938e1dfeeb418c Reviewed-on: https://cl.tvl.fyi/c/depot/+/8013 Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de> Autosubmit: tazjin <tazjin@tvl.su>
2023-01-26 r/5763 chore(tvix/eval): remove dead commentVincent Ambo1-1/+1
Change-Id: Icedb7f272e5067569b8dbf1c2d8b0fdd352b8e12 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7936 Tested-by: BuildkiteCI Autosubmit: tazjin <tazjin@tvl.su> Reviewed-by: flokli <flokli@flokli.de>
2023-01-25 r/5757 test(tvix/eval): add test for total_fmt_floatFlorian Klink1-0/+28
Change-Id: If6c478ee3d2e4ecf5ef92289614f86535ad05cb7 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7927 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: flokli <flokli@flokli.de>
2023-01-25 r/5756 refactor(tvix/eval): extract float formatting into a helperVincent Ambo1-71/+75
This keeps the actual TotalDisplay implementation readable, as this float formatting code suddenly made up the majority of its implementation. Change-Id: I2c0d00e4a691e0b8ffbc72680f680e16feef4bee Reviewed-on: https://cl.tvl.fyi/c/depot/+/7925 Reviewed-by: flokli <flokli@flokli.de> Autosubmit: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2023-01-25 r/5753 feat(tvix/eval): use lexical-core to format floatFlorian Klink1-2/+86
Apparently our naive implementation of float formatting, which simply used {:.5}, and trimmed trailing "0" strings not sufficient. It wrongly trimmed numbers with zeroes but no decimal point, like `10000` got trimmed to `1`. Nix uses `std::to_string` on the double, which according to https://en.cppreference.com/w/cpp/string/basic_string/to_string is equivalent to `std::sprintf(buf, "%f", value)`. https://en.cppreference.com/w/cpp/io/c/fprintf mentions this is treated like this: > Precision specifies the exact number of digits to appear after > the decimal point character. The default precision is 6. In the > alternative implementation decimal point character is written even if > no digits follow it. For infinity and not-a-number conversion style > see notes. This doesn't seem to be the case though, and Nix uses scientific notation in some cases. There's a whole bunch of strategies to determine which is a more compact notation, and which notation should be used for a given number. https://github.com/rust-lang/rust/issues/24556 provides some pointers into various rabbit holes for those interested. This gist seems to be that currently a different formatting is not exposed in rust directly, at least not for public consumption. There is the [lexical-core](https://github.com/Alexhuszagh/rust-lexical) crate though, which provides a way to format floats with various strategies and formats. Change our implementation of `TotalDisplay` for the `Value::Float` case to use that. We still need to do some post-processing, because Nix always adds the sign in scientific notation (and there's no way to configure lexical-core to do that), and lexical-core in some cases keeps the trailing zeros. Even with all that in place, there as a difference in `eval-okay- fromjson.nix` (from tvix-tests), which I couldn't get to work. I updated the fixture to a less problematic number. With this, the testsuite passes again, and does for the upcoming CL introducing builtins.fromTOML, and enabling the nix testsuite bits for it, too. Change-Id: Ie6fba5619e1d9fd7ce669a51594658b029057acc Reviewed-on: https://cl.tvl.fyi/c/depot/+/7922 Tested-by: BuildkiteCI Autosubmit: flokli <flokli@flokli.de> Reviewed-by: tazjin <tazjin@tvl.su>
2023-01-20 r/5715 refactor(tvix/eval): keep globals alive through VM structVincent Ambo2-7/+42
This forces users to pass the fully constructed set of globals to the VM, making it harder to accidentally "lose" the set while weak references to it still exist. This doesn't modify any functionality, but is laying the foundation for simplifying some of the builtins behaviour that has grown more complex again. Change-Id: I5120f97861c65dc46d90b8a4e2c92ad32cc53e03 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7877 Autosubmit: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de>
2023-01-20 r/5706 feat(tvix/eval): add error contexts to annotate error kindsVincent Ambo1-4/+2
This makes it possible for users to add additional context to an error, which will then be rendered as an additional secondary span in the formatted error output. We should strive to do this basically anywhere errors are raised that can occur multiple times, *especially* during type casts. This was triggered by me debugging a type cast error attached to a fairly large-ish span (a builtin invocation). Change-Id: I51be41fabee00cf04de973935daf34fe6424e76f Reviewed-on: https://cl.tvl.fyi/c/depot/+/7849 Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de>
2023-01-17 r/5676 refactor(tvix/eval): non-hacky suspended native thunksVincent Ambo1-55/+35
Instead of having a representation of suspended native thunks that involves constructing a fake code chunk, make these thunks a first-class part of the internal thunk representation. The previous code was not that simple to understand, and actually contained a critical bug which could lead to Tvix crashes. This version fixes the particular instance of that bug, but instead uncovers another (b/238) which can still lead to Tvix crashes. Fixes: b/237. Change-Id: I771d03864084d63953bdbb518fec94487481f839 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7750 Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
2023-01-17 r/5675 refactor(tvix/eval): remove `Box` in new_suspended_nativeVincent Ambo1-3/+1
This is unnecessary, Rc already provides all the boxing we need. Change-Id: I08cf0939c48da43f04c847526c7e5dae5336d528 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7749 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi> Reviewed-by: sterni <sternenseemann@systemli.org>
2023-01-17 r/5670 refactor(tvix/value): use proptest strategies from imbl crateVincent Ambo4-70/+39
Instead of going through Vec/BTreeMap for generating our internal types, use the proptest strategies from imbl. The one thing I couldn't figure out in the previous implementation is where the ranges/sizes of generated collections came from. The strategies in proptest use different types (Range, with an unknown default value, and SizeRange with 0..100). I've opted to specify 0..100 directly, but we can probably make it configurable. Change-Id: I749bc4c703fe424099240cab822b1642e5216361 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7791 Autosubmit: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de>
2023-01-16 r/5663 chore(tvix/eval): add other required items to public APIVincent Ambo1-2/+2
External implementors of builtins must be able to force values, which necessitates publishing a bunch more items from the crate. Change-Id: I8f6b8ae88156aae417dbe630a698d123d0c1c8d4 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7830 Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de>
2023-01-12 r/5654 fix(tvix/eval): len_without_is_empty clippy warnAaqa Ishtyaq2-0/+12
This CL addresses clippy warning len_without_is_empty which expects `.is_empty()` method to be present when implementing `.len()` method for an item. Change-Id: I8878db630b9ef5853649a906b764a33299bb5dc8 Signed-off-by: Aaqa Ishtyaq <aaqaishtyaq@gmail.com> Reviewed-on: https://cl.tvl.fyi/c/depot/+/7806 Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
2023-01-12 r/5652 feat(tvix/eval): implement builtins.toJSONVincent Ambo5-9/+41
Implements `Serialize` for `tvix_eval::Value`. Special care is taken with serialisation of attribute sets, and forcing of thunks. The tests should cover both cases well. Change-Id: I9bb135bacf6f87bc6bd0bd88cef0a42308e6c335 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7803 Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI Autosubmit: tazjin <tazjin@tvl.su>
2023-01-10 r/5640 feat(tvix/eval): implement serde::Deserialize for ValueRyan Lahfa4-44/+92
Co-Authored-By: Vincent Ambo <tazjin@tvl.su> Change-Id: Ib6f7d1f4f4faac36b44f5f75cccc57bf912cf606 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7626 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2023-01-08 r/5629 fix(tvix/eval): fix last uses of Vec<Value> -> NixList in builtinsVincent Ambo1-11/+6
Change-Id: I0d71b82eb7ddc1e457b0996b0668006f55f56751 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7790 Autosubmit: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2023-01-04 r/5582 fix(tvix/eval): ' is allowed in nonfirst position in Nix identifierssterni1-1/+1
With this is_valid_nix_identifier should line up with the upstream lexer definition: ID [a-zA-Z\_][a-zA-Z0-9\_\'\-]* While we're working on this, add a simple test checking the various formatting rules. Interestingly, it would not be suitable as an identity test, since you have to write { "assert" = null; } in order to avoid an evaluation error, but C++ Nix is happy to print this as { assert = null; } – maybe should be considered to be a bug. Change-Id: I0a4e1ccb5033a80f3767fb8d1c4bba08d303c5d8 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7744 Autosubmit: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
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