about summary refs log tree commit diff
path: root/tvix/eval/src
AgeCommit message (Collapse)AuthorFilesLines
2023-12-12 r/7169 fix(tvix/eval): preserve catchables in nix_cmp_ordering(), fix b/338Adam Joseph5-9/+15
This commit fixes b/338 by properly propagating catchables through comparison operations. Change-Id: I6b0283a40f228ecf9a6398d24c060bdacb1077cf Reviewed-on: https://cl.tvl.fyi/c/depot/+/10221 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Autosubmit: Adam Joseph <adam@westernsemico.com>
2023-12-12 r/7168 test(tvix/eval): test for b/338 catchable hygiene problemAdam Joseph2-0/+2
Commit 05f42519b53575ad3235b5e0a0cd7d71f04076a5 fixed b/281 by establishing a hygiene regimen to partition *catchable* errors (i.e. those which tryEval can detect) from all other errors, like internal VM failures or I/O errors (which Nix must not be allowed to detect, since these errors are fundamentally impure). Unfotunately there are still cases where tvix assumes that anything other than Value::Bool means it should panic!(). I found another one, and added a test case for it in: eval_okay_src_tests_tvix_tests_eval_okay_compare_ordering_catchable_nix Not yet passing. Change-Id: I69c62ed9ea5c8f81870e8de5c5fe12dcde849763 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10220 Autosubmit: Adam Joseph <adam@westernsemico.com> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2023-12-12 r/7167 feat(tvix/eval): nonrecursive nix_cmp_ordering(), fixes b/339Adam Joseph7-44/+75
This commit rewrites Value::nix_cmp_ordering() into an equivalent nonrecursive form. Except for calls to Thunk::force(), the new form no longer uses generators, and is async only because of the fact that it calls Thunk::force(). I originally believed that this commit would make evaluation faster. In fact it is slightly slower. I believe this is due to the added vec![] allocation. I am investigating. Prev-Nixpkgs-Benchmark: {"attrpath":"pkgsCross.aarch64-multiplatform.hello.outPath","peak-kbytes":"460048","system-seconds":"0.68","user-seconds":"5.73"} This-Nixpkgs-Benchmark: {"attrpath":"pkgsCross.aarch64-multiplatform.hello.outPath","peak-kbytes":"460224","system-seconds":"0.67","user-seconds":"5.84"} Change-Id: Ic627bc220d9c5aa3c5e68b9b8bf199837cd55af5 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10212 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Autosubmit: Adam Joseph <adam@westernsemico.com>
2023-12-12 r/7166 fix(tvix/eval): never use partial_cmp() (partial fix b/338)Adam Joseph3-23/+16
This is part of a fix for b/338. We should never use PartialOrd::partial_cmp(). All Nix types except floats are obviously totally-ordered. In addition, it turns out that because Nix treats division by zero rather than producing a NaN, and because it does not support "negative zero", even floats are in fact totally ordered in Nix. Therefore, every call to PartialOrd::partial_cmp() in tvix is an error. We have to *implement* this function, but we should never call it on built-in types. Moreover, nix_cmp_ordering() currently returns an Option<Ordering>. I'm not sure what was going on there, since it's impossible for it to return None. This commit fixes it to return simply Ordering rather than Option<Ordering>. Change-Id: If5c084164cf19cfb38c5a15554c0422faa5f895d Reviewed-on: https://cl.tvl.fyi/c/depot/+/10218 Autosubmit: Adam Joseph <adam@westernsemico.com> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2023-12-12 r/7165 test(tvix/eval): add test case for b/339Adam Joseph2-0/+2
Not yet passing. Change-Id: I1de3f72d8b3f46567fdba010fc3ab4bace3f1699 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10219 Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: Adam Joseph <adam@westernsemico.com> Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2023-12-12 r/7164 feat(tvix/eval): nonrecursive nix_eq()Adam Joseph4-137/+159
This commit rewrites Value::nix_eq() into an equivalent. Except for calls to Thunk::force(), the new form no longer uses generators, and is async only because of the fact that it calls Thunk::force(). I believed that the nonrecursive form would be faster. It is, in fact, slightly slower. I believe this is due to the vec![] allocation; I am investigating. Prev-Nixpkgs-Benchmark: {"attrpath":"pkgsCross.aarch64-multiplatform.hello.outPath","peak-kbytes":"459068","system-seconds":"0.71","user-seconds":"5.39"} This-Nixpkgs-Benchmark: {"attrpath":"pkgsCross.aarch64-multiplatform.hello.outPath","peak-kbytes":"460048","system-seconds":"0.68","user-seconds":"5.73"} Change-Id: I10f4868891e4b7475df13f0cbc41ec78dd985dd8 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10118 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Autosubmit: Adam Joseph <adam@westernsemico.com>
2023-12-09 r/7134 refactor(tvix/eval): address clippy lintsFlorian Klink1-1/+1
Change-Id: Ic2bd4e8291b30ceac9fa0e88a4f56e61ae99b603 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10227 Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
2023-12-09 r/7130 feat(tvix/eval): impl DoubleEndedIter for OwnedAttrsIteratorAdam Joseph1-0/+10
Change-Id: I4bd85dbe9c27047f4abbdeff4e2b796e9bcab3a1 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10211 Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: Adam Joseph <adam@westernsemico.com> Tested-by: BuildkiteCI
2023-12-06 r/7120 feat(tvix/eval): rewrite Thunk::force() in nonrecursive formAdam Joseph3-64/+97
This commit rewrites Thunk::force() so that it is not (directly) self-recursive. It maintains a Vec of all the previously-encountered thunks which point to the one it is currently forcing, rather than recursively calling itself. Benefits: - Short term: This commit saves the cost of a round-trip through the generator machinery for the generators::request_force() which is removed by this commit. - Medium term: Once a similar transformation has been applied to nix_cmp(), nix_add(), nix_eq(), and coerce_to_string(), those four functions, along with Thunk::force(), will make non-tail calls only to each other. They can then be merged into a single tail-recursive function which does not use the generator machinery at all: enum Task { Cmp, Add, Eq, CoerceToString, Force}; fn Value::walk(task:Task, v1:Value, v2:Value) { // ... - Long term: The long-term goal here is to use generators **only for builtins** and [Marionette]-style remote control of the VM. In other words: use `async` for things that actually involve concurrency. Calls from the VM to builtins can then be blocking calls, because even cppnix will overflow the stack if you make a MAX_STACK_DEPTH-deep recursive call which passes through a builtin at every stack frame (e.g. `{ func = builtins.sort (a: b: ... func ...) ...}`). This way the inner "tight loop" of the interpreter doesn't pay the costs of `async` and generators. These costs manifest in terms of: performance, complex nonlocal control flow, and language impediments (async Rust is a restricted subset of real Rust, and is missing things like traits). [Marionette]: https://firefox-source-docs.mozilla.org/testing/marionette/Intro.html Change-Id: I6179b8abb2ea0492180fcb347f37595a14665777 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10039 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2023-12-05 r/7119 fix(tvix/eval): Return error rather than panicking on bad substringAspen Smith2-1/+10
If builtins.substring is invoked with (byte!!) offsets that aren't at codepoint boundaries, return an error rather than panicking. This is still incorrect (see b/337) but pushes the incorrectness forward a step. Change-Id: I5a4261f2ff250874cd36489ef598dcf886669d04 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10199 Tested-by: BuildkiteCI Autosubmit: grfn <grfn@gws.fyi> Reviewed-by: sterni <sternenseemann@systemli.org>
2023-11-25 r/7065 refactor(tvix/eval): use `or_default` helper in entry APIVincent Ambo1-3/+1
This fixes a future clippy lint. Change-Id: Ic830e94ef23595580c1037f10878c76bbb546dd9 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10110 Tested-by: BuildkiteCI Reviewed-by: Adam Joseph <adam@westernsemico.com>
2023-11-25 r/7064 fix(tvix): ensure PartialOrd/Ord agree for StorePath & NixStringVincent Ambo1-1/+1
This fixes a *future* clippy lint: https://rust-lang.github.io/rust-clippy/master/index.html#/incorrect_partial_ord_impl_on_ord_type In essence, because the implementation of *both* Ord and PartialOrd implies that ordering is not partial, all results of PartialOrd should simply be those of Ord. This is to avoid subtle bugs in future refactorings. Change-Id: I8fc6694010208752dd47746a2aaaeca0c788d574 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10109 Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de>
2023-11-25 r/7055 refactor(tvix/eval): add ThunkRepr::is_forced()Adam Joseph1-5/+9
Change-Id: I4eab5c81fb82337da06327248845cd2f3a4490d3 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10038 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Autosubmit: Adam Joseph <adam@westernsemico.com>
2023-11-25 r/7054 feat(tvix/eval): add Thunk::unwrap_or_clone()Adam Joseph1-1/+34
This commit adds Thunk::unwrap_or_clone(), which uses Rc::try_unwrap() to avoid cloning the Value out of a an Rc which has only one strong reference. Change-Id: Icacefe0c823dcddf046d90c0c5cd5ed59fe976d4 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10037 Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: Adam Joseph <adam@westernsemico.com> Tested-by: BuildkiteCI
2023-11-06 r/6963 docs(tvix/eval): document where EvalIO methods are usedVincent Ambo1-7/+34
Change-Id: I335f2ba4420973861c2a22125995b45a34d3608c Reviewed-on: https://cl.tvl.fyi/c/depot/+/9969 Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI Autosubmit: tazjin <tazjin@tvl.su>
2023-11-05 r/6958 chore(tvix): add missing clippy attributes & configVincent Ambo2-0/+2
For cases where clippy lints don't apply to us, or something is misfiring, add appropriate configuration. Relates to b/321. Change-Id: I0af453910b4a4112bf685b2a8e9a73de10ec87ea Reviewed-on: https://cl.tvl.fyi/c/depot/+/9965 Reviewed-by: flokli <flokli@flokli.de> Autosubmit: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2023-11-05 r/6957 refactor(tvix/eval): use IntoIterator trait for owned NixAttrs iterVincent Ambo1-15/+22
Uses the standard library IntoIterator trait for the construction of our iterators. Clippy complains about duplicating this. While doing this, I opted to rename the `IntoIter` type into something that is more useful to users, in case somebody ends up working with these manually. This fixes a clippy lint, and is related to b/321. Change-Id: I851fde0d7b8b38d182343a0fd6d9f8dd2a33ee11 Reviewed-on: https://cl.tvl.fyi/c/depot/+/9963 Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de> Autosubmit: tazjin <tazjin@tvl.su>
2023-11-05 r/6955 chore(tvix): fix trivial clippy lintsVincent Ambo9-26/+25
Relates to b/321. Change-Id: I37284f89b186e469eb432e2bbedb37aa125a6ad4 Reviewed-on: https://cl.tvl.fyi/c/depot/+/9961 Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de> Autosubmit: tazjin <tazjin@tvl.su>
2023-11-03 r/6932 refactor(tvix/eval): delay allocation when comparing attr valuesVincent Ambo1-4/+4
Delays allocation (through cloning) of the values to be compared until *after* the keys have been compared. Change-Id: I7d68c27d7a0fbcdcc387db7c092bce50ca4b94ea Reviewed-on: https://cl.tvl.fyi/c/depot/+/9900 Autosubmit: tazjin <tazjin@tvl.su> Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
2023-11-03 r/6931 chore(tvix/eval): add a marker for sorted borrowed attrs iterationVincent Ambo1-0/+6
Similar to `into_iter_sorted`, add a marker function for call sites that want *borrowed* sorted iteration. Change-Id: I7c6f14e1ac43fdb14b861b3da183eb5d12bba139 Reviewed-on: https://cl.tvl.fyi/c/depot/+/9899 Autosubmit: tazjin <tazjin@tvl.su> Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
2023-11-03 r/6930 refactor(tvix/eval): more efficiently intersect attributesVincent Ambo1-9/+70
builtins.intersectAttrs is used a _lot_ in nixpkgs eval, for whatever reason. We previously had a very inefficient implementation that would allocate for each comparison. It stuck out like a sore thumb in perf analysis. This moves to a custom algorithm with two iterators, one for the left and one for the right side, advancing them along the (borrowed) map keys until a match is found and allocation is required. I've not made any effort to reduce the verbosity of this code, I don't think it's worth it. On my machine this reduces the mean runtime of evaluating `nixpkgs.emacs.outPath` by ~8%. Change-Id: Ie506d82cb8d5f45909628f771a6b73e0eca16b27 Reviewed-on: https://cl.tvl.fyi/c/depot/+/9898 Autosubmit: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de>
2023-09-26 r/6662 feat(tvix/eval): test case for b/281Adam Joseph2-0/+2
This commit adds a test case for b/281. Change-Id: I8dfbfc0ff636184d7882530d8aefb329a3af9e5c Reviewed-on: https://cl.tvl.fyi/c/depot/+/9288 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Autosubmit: Adam Joseph <adam@westernsemico.com> Reviewed-by: flokli <flokli@flokli.de>
2023-09-24 r/6650 fix(tvix/eval): fix b/281 by adding Value::CatchableAdam Joseph14-206/+244
This commit makes catchable errors a variant of Value. The main downside of this approach is that we lose the ability to use Rust's `?` syntax for propagating catchable errors. Change-Id: Ibe89438d8a70dcec29e016df692b5bf88a5cad13 Reviewed-on: https://cl.tvl.fyi/c/depot/+/9289 Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: Adam Joseph <adam@westernsemico.com> Tested-by: BuildkiteCI
2023-09-24 r/6649 refactor(tvix/eval): factor CatchableErrorKind out of ErrorKindAdam Joseph5-29/+57
This commit creates a separate enum for "catchable" errors (the kind that `builtins.tryEval` can detect). Change-Id: Ie81d1112526d852255d9842f67045f88eab192af Reviewed-on: https://cl.tvl.fyi/c/depot/+/9287 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: Adam Joseph <adam@westernsemico.com>
2023-09-22 r/6624 docs(tvix/eval): fix some broken docstr referencesFlorian Klink5-10/+11
There's some more left, but they've been renamed/refactored out of sight. Change-Id: I41579dedc74342b4c5f8cb39d2995b5b0c90b0f4 Reviewed-on: https://cl.tvl.fyi/c/depot/+/9372 Tested-by: BuildkiteCI Reviewed-by: Connor Brewster <cbrewster@hey.com> Autosubmit: flokli <flokli@flokli.de>
2023-09-15 r/6589 fix(tvix/eval): update identifier quoting to match cppnix 2.17Adam Joseph12-3/+35
In cppnix 2.17, commit b72bc4a972fe568744d98b89d63adcd504cb586c, the libexpr pretty-printing routine was fixed so that it would no longer pretty-print attrsets with keywords in their attrnames incorrectly. This commit implements the corresponding fix for tvix, fixes our tests to work with cppnix>=2.17 oracles, and expands our test cases to cover all the keywords. Change-Id: I4b51389cd3a9c44babc8ab2a84b383b7b0b116ca Reviewed-on: https://cl.tvl.fyi/c/depot/+/9283 Autosubmit: Adam Joseph <adam@westernsemico.com> Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2023-09-08 r/6571 docs(tvix/eval): document remaining opcodesVincent Ambo1-3/+48
Change-Id: Iad8a5f78930872719b6481ddf1bbad8532bfa888 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7981 Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
2023-09-08 r/6570 docs(tvix/eval): document attribute set related opcodesVincent Ambo1-0/+11
Change-Id: Ib98a9fe8c9aa3f0e61c27d10285c0926cda7a969 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7979 Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de>
2023-09-08 r/6569 docs(tvix/eval): add documentation strings for some OpCode variantsVincent Ambo1-2/+37
Change-Id: I42e610740b3687e1fd897d36694cce425751a8bc Reviewed-on: https://cl.tvl.fyi/c/depot/+/7975 Tested-by: BuildkiteCI Autosubmit: tazjin <tazjin@tvl.su> Reviewed-by: flokli <flokli@flokli.de>
2023-08-24 r/6523 fix(tvix/eval): off-by-one in replaceStringsLinus Heckemann3-2/+3
replaceStrings would previously fail to replace the last character in a string. Change-Id: I43a7c960945350b2e7a5b731b7fdb617723eb38f Reviewed-on: https://cl.tvl.fyi/c/depot/+/9151 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2023-08-20 r/6509 refactor(tvix/eval): don't use `format!` in `write!` argsFlorian Klink1-1/+1
Change-Id: I0b2fdb418c2e36280d5c551a81634e1742193903 Reviewed-on: https://cl.tvl.fyi/c/depot/+/9105 Tested-by: BuildkiteCI Autosubmit: flokli <flokli@flokli.de> Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2023-08-20 r/6508 refactor(tvix/eval/vm): don't put HashMap in a BoxFlorian Klink1-1/+1
HashMap already is on the heap. Change-Id: I53763e17469359e85862f297b5c2e7c0d8c3a980 Reviewed-on: https://cl.tvl.fyi/c/depot/+/9104 Autosubmit: flokli <flokli@flokli.de> Tested-by: BuildkiteCI Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2023-08-20 r/6507 refactor(tvix/eval): impl Default for SourceCodeFlorian Klink2-6/+8
… instead of new(). Suggested by clippy. Change-Id: Iac7be733392afefc2b4ff2e38386eee95f3bce94 Reviewed-on: https://cl.tvl.fyi/c/depot/+/9103 Tested-by: BuildkiteCI Autosubmit: flokli <flokli@flokli.de> Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2023-08-20 r/6506 refactor(tvix/eval/observer): cargo clippyFlorian Klink1-1/+1
Change-Id: I11d7d55f31ddd27952f201a0abbed7445013f434 Reviewed-on: https://cl.tvl.fyi/c/depot/+/9102 Autosubmit: flokli <flokli@flokli.de> Tested-by: BuildkiteCI Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2023-08-20 r/6505 refactor(tvix/eval): cargo clippyFlorian Klink1-1/+4
This passes a unit value to the function. Change-Id: I4df3ad8fb0f35c0f110cee3349971ae28ce2878c Reviewed-on: https://cl.tvl.fyi/c/depot/+/9101 Autosubmit: flokli <flokli@flokli.de> Reviewed-by: raitobezarius <tvl@lahfa.xyz> Tested-by: BuildkiteCI
2023-08-20 r/6504 refactor(tvix/eval/io): cargo clippy &PathFlorian Klink1-1/+1
Change-Id: Ifec2a4fc93c482e41ff5993183643b5126a7728b Reviewed-on: https://cl.tvl.fyi/c/depot/+/9100 Autosubmit: flokli <flokli@flokli.de> Reviewed-by: raitobezarius <tvl@lahfa.xyz> Tested-by: BuildkiteCI
2023-08-20 r/6503 refactor(tvix/eval): cargo clippy (len() is usize)Florian Klink1-1/+1
Change-Id: I9d931ffcc03c6df7c0392dbc1c9a4ae0e3804213 Reviewed-on: https://cl.tvl.fyi/c/depot/+/9099 Autosubmit: flokli <flokli@flokli.de> Reviewed-by: raitobezarius <tvl@lahfa.xyz> Tested-by: BuildkiteCI
2023-08-20 r/6502 refactor(tvix/eval): cargo clippy &GenCoFlorian Klink2-5/+5
Change-Id: I6b1b902ccbc12bf2acdb0fdf406d6ef336ff0b2f Reviewed-on: https://cl.tvl.fyi/c/depot/+/9098 Reviewed-by: raitobezarius <tvl@lahfa.xyz> Tested-by: BuildkiteCI Autosubmit: flokli <flokli@flokli.de>
2023-08-20 r/6501 refactor(tvix/eval): derive default for value::AttrsRep enumFlorian Klink1-7/+2
Change-Id: Iaeb9ed7024c2ce85373f8aec0d223f52e1a3a539 Reviewed-on: https://cl.tvl.fyi/c/depot/+/9097 Autosubmit: flokli <flokli@flokli.de> Reviewed-by: raitobezarius <tvl@lahfa.xyz> Tested-by: BuildkiteCI
2023-08-13 r/6482 fix(tvix/eval): fix a comment position in value::jsonVincent Ambo1-2/+2
Change-Id: Ic58653254b36694f1991a18db9ceea0d532c2e31 Reviewed-on: https://cl.tvl.fyi/c/depot/+/9068 Autosubmit: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de>
2023-08-11 r/6480 test(tvix/eval): check truncation direction of builtins.divsterni2-1/+24
builtins.div ought to truncate towards zero so that -(builtins.div a b) == builtins.div (-a) b -(builtins.div a b) == builtins.div a (-b) Change-Id: I8b7c08cd7f4fa8a1363c786d42c8d484f6cd133d Reviewed-on: https://cl.tvl.fyi/c/depot/+/9006 Autosubmit: sterni <sternenseemann@systemli.org> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2023-07-22 r/6439 feat(tvix/store/proto): use Bytes instead of Vec<u8>Florian Klink3-8/+10
Makes use of https://github.com/tokio-rs/prost/pull/341, which makes our bytes field cheaper to clone. It's a bit annoying to configure due to https://github.com/hyperium/tonic/issues/908, but the workaround does get the job done. Change-Id: I25714600b041bb5432d3adf5859b151e72b12778 Reviewed-on: https://cl.tvl.fyi/c/depot/+/8975 Reviewed-by: raitobezarius <tvl@lahfa.xyz> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: flokli <flokli@flokli.de>
2023-07-21 r/6436 refactor(tvix/store): use bytes for node names and symlink targetsFlorian Klink3-9/+12
Some paths might use names that are not valid UTF-8. We should be able to represent them. We don't actually need to touch the PathInfo structures, as they need to represent StorePaths, which come with their own harder restrictions, which can't encode non-UTF8 data. While this doesn't change any of the wire format of the gRPC messages, it does however change the interface of tvix_eval::EvalIO - its read_dir() method does now return a list of Vec<u8>, rather than SmolStr. Maybe this should be OsString instead? Change-Id: I821016d9a58ec441ee081b0b9f01c9240723af0b Reviewed-on: https://cl.tvl.fyi/c/depot/+/8974 Autosubmit: flokli <flokli@flokli.de> Reviewed-by: raitobezarius <tvl@lahfa.xyz> Tested-by: BuildkiteCI
2023-07-11 r/6405 docs(tvix): document when pointer equality is preserved in C++ Nixsterni4-5/+39
This explicitly documents behavior of C++ Nix that goes against the intuition you'd gather from this document: that e.g. a simple select from an attribute set causes a value to no longer be pointer equal to its former self. The point of documenting this is that we can show in a to be written section on the use of pointer equality in nixpkgs that pointer equality is only needed in a limited sense for evaluating it (C++ Nix's exterior pointer equality). Tvix's pointer equality is far more powerful since value identity preserving operations also preserve pointer equality, generally speaking (this is because we implement interior pointer equality in my made up terminology). This should eventually also be documented. Change-Id: I6ce7ef2d67b012f5ebc92f9e81bba33fb9dce7d0 Reviewed-on: https://cl.tvl.fyi/c/depot/+/8856 Tested-by: BuildkiteCI Autosubmit: sterni <sternenseemann@systemli.org> Reviewed-by: tazjin <tazjin@tvl.su>
2023-07-11 r/6404 fix(tvix/eval): use byte, not codepoint index for slicing in escapesterni3-1/+12
This fixes a subtle issue which would occasionally lead to a crash (e.g. when evaluating (pkgs.systemd.outPath with --trace-runtime): With each character in the string that has a multi byte representation in UTF-8, the actual byte position and what tvix thought it was would get out of sync. This could either lead to * Tvix swallowing characters or jumbling characters if multi byte characters would cause the tracked index to become out of sync with the byte position before the first character to be escaped, or * Tvix crashing if (in the same situation) the out of sync index would be within a UTF-8 byte sequence. Luckily, std's `char_indices()` iterator implements exactly what `nix_escape_char()`'s original author had in mind with `.chars().enumerate()`. Using `i + 1` for continuing is safe, since all characters that need (in fact, can) to be escaped in Nix are represented as a single byte in UTF-8. Change-Id: I1c836f70cde3d72db1c644e9112852f0d824715e Reviewed-on: https://cl.tvl.fyi/c/depot/+/8952 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Autosubmit: sterni <sternenseemann@systemli.org>
2023-06-30 r/6372 chore(tvix/eval/vm): drop unused importFlorian Klink1-1/+1
Change-Id: Ia04778391c198fde21da217bf697aa40157898b0 Reviewed-on: https://cl.tvl.fyi/c/depot/+/8846 Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
2023-06-22 r/6344 feat(tvix/eval): allow extending builtins outside of tvix_evalEvgeny Zemtsov3-0/+3
The change allows applications that use tvix_serde for parsing nix-based configuration to extend the language with domain-specific set of features. Change-Id: Ia86612308a167c456ecf03e93fe0fbae55b876a6 Reviewed-on: https://cl.tvl.fyi/c/depot/+/8848 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2023-06-21 r/6341 fix(tvix/eval): use realpaths for import cachesterni8-3/+51
I've noticed this behavior when writing the admittedly cursed test case included in this CL. Alternatively we could use some sort of machinery using `builtins.trace`, but I don't think we capture stderr anywhere. I've elected to put this into the eval cache itself while C++ Nix does it in builtins.import already, namely via `realisePath`. We don't have an equivalent for this yet, since we don't support any kind of IfD, but we could revise that later. In any case, it seems good to encapsulate `ImportCache` in this way, as it'll also allow using file hashes as identifiers, for example. C++ Nix also does our equivalent of canon_path in `builtins.import` which we still don't, but I suspect it hardly makes a difference. Change-Id: I05004737ca2458a4c67359d9e7d9a2f2154a0a0f Reviewed-on: https://cl.tvl.fyi/c/depot/+/8839 Autosubmit: sterni <sternenseemann@systemli.org> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2023-06-20 r/6336 fix(tvix/eval): only finalise formal arguments if defaultingsterni8-52/+221
When dealing with a formal argument in a function argument pattern that has a default expression, there are two different things that can happen at runtime: Either we select its value from the passed attribute successfully or we need to use the default expression. Both of these may be thunks and both of these may need finalisers. However, in the former case this is taken care of elsewhere, the value will always be finalised already if necessary. In the latter case we may need to finalise the thunk resulting from the default expression. However, the thunk corresponding to the expression may never end up in the local's stack slot. Since finalisation goes by stack slot (and not constants), we need to prevent a case where we don't fall back to the default expression, but finalise anyways. Previously, we worked around this by making `OpFinalise` ignore non-thunks. Since finalisation of already evaluated thunks still crashed, the faulty compilation of function pattern arguments could still cause a crash. As a new approach, we reinstate the old behavior of `OpFinalise` to crash whenever encountering something that is either not a thunk or doesn't need finalisation. This can also help catching (similar) miscompilations in the future. To then prevent the crash, we need to track whether we have fallen back or not at runtime. This is done using an additional phantom on the stack that holds a new `FinaliseRequest` value. When it comes to finalisation we check this value and conditionally execute `OpFinalise` based on its value. Resolves b/261 and b/265 (partially). Change-Id: Ic04fb80ec671a2ba11fa645090769c335fb7f58b Reviewed-on: https://cl.tvl.fyi/c/depot/+/8705 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Autosubmit: sterni <sternenseemann@systemli.org>
2023-06-15 r/6314 test(tvix/eval): update nix_tests suite to C++ Nix mastersterni26-7/+384
Adds new tests for foldl', intersectAttrs as well as fills in missing .exp files. New test cases we don't pass: - fromTOML timestamp capabilities - path antiquotation - replaceStrings is lazier on C++ Nix master The C++ Nix revision used is 7066d21a0ddb421967980094222c4bc1f5a0f45a. Change-Id: Ic619c96e2d41e6c5ea6fa93f9402b12e564af3c5 Reviewed-on: https://cl.tvl.fyi/c/depot/+/8778 Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
generated by cgit-pink 1.4.1 (git 2.44.2) at 2024-11-04T11·48+0000