about summary refs log tree commit diff
path: root/tvix/eval/src
AgeCommit message (Collapse)AuthorFilesLines
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
2023-06-15 r/6309 test(tvix/eval): genericClosure (pointer) comparison supportsterni4-0/+37
genericClosure has very limited support for pointer equality: It relies on comparison (not equality!) in C++ Nix, so as soon as C++ Nix supports comparing lists (langVersion >= 6) we can rely on pointer equality for key. Since Tvix uses equality, not comparison for the insert, our behavior is currently different, as documented by the notyetpassing tests. Change-Id: Ifcd741ed4fc3ccc3825f7038875d56a9918b786a Reviewed-on: https://cl.tvl.fyi/c/depot/+/8720 Tested-by: BuildkiteCI Autosubmit: sterni <sternenseemann@systemli.org> Reviewed-by: tazjin <tazjin@tvl.su>
2023-06-15 r/6308 fix(tvix/eval): make tvix display values like nix-instantiate(1)sterni8-3/+59
In order for the test suite we have currently to be comparable to C++ Nix, we need to display values in the same way. This was largely the case except in some weird cases. * <CODE> for thunks and <CYCLE> for repeated thunks (?) are already in use. <CODE> formatting is tested by the oracle test suite already. * Instead of lambda, we need to use <LAMBDA> * <<primop>> and <<primop-app>> (a formatting C++ Nix uses nowhere) now are <PRIMOP> and <PRIMOP-APP>. We'll probably want to have a fancier display of values (in a separate trait) down the line. This could be used for interactive usage, e.g. the REPL or a potential debugger. There is a peculiarity with C++ Nix 2.3 formatting primops: import is considered a <<PRIMOP-APP>>, since it is internally implemented by means of scopedImport. This implementation detail no longer leaks in C++ Nix 2.13 nor in Tvix. <CYCLE> display is untested at the moment, since we exhibit a discrepancy to C++ Nix 2.3. Our current detection is more similar to C++ Nix 2.13—luckily it is also the more consistent of the two. See also b/245. Change-Id: I1d534434b02e470bf5475b3758920ea81e3420dc Reviewed-on: https://cl.tvl.fyi/c/depot/+/8760 Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2023-06-14 r/6300 test(tvix/eval): move division by zero tests into tvix_testssterni2-0/+0
These were added by us in r/5276, so they should go into our test suite. Change-Id: I6dc74fc242f33c22a17e0b4aee546ccae886ac85 Reviewed-on: https://cl.tvl.fyi/c/depot/+/8774 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Autosubmit: sterni <sternenseemann@systemli.org>
2023-06-14 r/6299 test(tvix/eval): add test case for builtins set pointer equalitysterni2-0/+21
Unsupported by Tvix at the moment. Documents b/280. Change-Id: I48844feeefa9da8ed7e5d85300d52bb5650f82d2 Reviewed-on: https://cl.tvl.fyi/c/depot/+/8772 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Autosubmit: sterni <sternenseemann@systemli.org>
2023-06-14 r/6298 test(tvix/eval): re-enable blackhole teststerni1-0/+0
Change-Id: Id933f3bd708aa3342b9fd6a5584e65ee11751ff8 Reviewed-on: https://cl.tvl.fyi/c/depot/+/8773 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: sterni <sternenseemann@systemli.org>
2023-06-14 r/6297 fix(tvix/eval): don't thunk home relative pathssterni1-11/+10
C++ Nix resolves home relative paths at [parse] time. This is not an option for us, since it prevents being able to separate the compilation and execution phase later (e.g. precompiled nix expressions). However, a practical consequence of this is that paths expressions are always literals (strict) and never thunks. [parse]: https://github.com/NixOS/nix/blob/7066d21a0ddb421967980094222c4bc1f5a0f45a/src/libexpr/parser.y#L518-L527 Change-Id: Ie4b9dc68f62c86d6c7fd5f1c9460c850d97ed1ca Reviewed-on: https://cl.tvl.fyi/c/depot/+/7041 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2023-06-12 r/6268 test(tvix/eval): builtins.substring's behavior with negative argssterni3-0/+9
Change-Id: Ie8b97d174e9d58e33bf08c9b9e0afeeddd089ba8 Reviewed-on: https://cl.tvl.fyi/c/depot/+/8700 Autosubmit: sterni <sternenseemann@systemli.org> Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
2023-06-12 r/6267 fix(tvix/eval): allow negative substring lengthsLinus Heckemann2-21/+5
Nix uses string::substr without checking the sign of the length[1]. The NixOS testing infrastructure relies on this[2], and on the implicit conversion of that to the maximum possible value for a size_t. [1]: https://github.com/NixOS/nix/blob/ecae62020b64914d9859a71ce197d03688c6133c/src/libexpr/primops.cc#L3597 [2]: https://github.com/NixOS/nixpkgs/blob/c7c298471676ac1c7789ab3c424fbcebecaa6791/nixos/lib/testing/driver.nix#L29 Change-Id: I6d0caf6830b6bda3fdf44c40c81de6a1befeca7b Reviewed-on: https://cl.tvl.fyi/c/depot/+/8746 Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
2023-06-11 r/6266 fix(tvix/eval): emit only warnings on shadowed outputsLinus Heckemann2-1/+8
Unfortunately, nixpkgs has at least one case[1] where the out environment variable is shadowed -- though it doesn't cause a problem, since it's shadowed with the correct value, odd as this may be! [1]: https://github.com/NixOS/nixpkgs/blob/c7c298471676ac1c7789ab3c424fbcebecaa6791/pkgs/development/python-modules/pybind11/default.nix#L19 Change-Id: Ibf6790d2484dc9cce8e424feeb5886664d498dc3 Reviewed-on: https://cl.tvl.fyi/c/depot/+/8696 Autosubmit: tazjin <tazjin@tvl.su> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2023-06-07 r/6244 fix(tvix/eval): use normal thunking behavior for default in formalssterni1-9/+2
When comparing to C++ Nix, we notice that the thunking of default expressions in function formals corresponds to their normal thunking, e.g. literals are not thunked. This means that we can just invoke compile() without much of a care and trust that it will sort it out correctly. If function formals blow up as a result of this, it likely indicates that the expression is treated incorrectly by compile(), not compile_param_pattern(). Change-Id: I64acbff2f251423eb72ce43e56a0603379305e1d Reviewed-on: https://cl.tvl.fyi/c/depot/+/8704 Autosubmit: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2023-06-07 r/6243 fix(tvix/eval): type check function argument with set patternsterni5-0/+18
C++ Nix forces and typechecks the passed argument even if it is not necessary in order to compute the return value of the function. I discovered this when I thought our formals miscompilation might be that we are too strict, but doesn't look like it in this case. Change-Id: Ifb3c92592293052c489d1e3ae8c7c54e4b6b4dc6 Reviewed-on: https://cl.tvl.fyi/c/depot/+/8701 Tested-by: BuildkiteCI Autosubmit: sterni <sternenseemann@systemli.org> Reviewed-by: tazjin <tazjin@tvl.su>
2023-06-07 r/6242 refactor(tvix/eval): don't track idx twice in compile_param_patternsterni1-9/+7
Change-Id: I27f9105ddb20d84342550b2a73b479a7764ee3fe Reviewed-on: https://cl.tvl.fyi/c/depot/+/8699 Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2023-05-29 r/6217 fix(tvix/eval): thunk lambda expressionssterni1-5/+3
As cl/8658 and b/274 reveal, lambda expressions are also wrapped in thunks. Resolves b/274. Change-Id: I02fe5c8730ac76748d940e4f4427116587875275 Reviewed-on: https://cl.tvl.fyi/c/depot/+/8662 Autosubmit: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2023-05-29 r/6216 fix(tvix/eval): thunk HasAttr expressionssterni1-1/+3
HasAttrs was weird because with longer attribute paths it would sometimes not turn out to be a thunk. If it was a thunk, it'd usually still do some eval strictly which we'll want to avoid. Verified against C++ Nix using a new test suite introduced in a later CL. Change-Id: I6d047ccc68d046bb268462f170a3c4f3c5ddeffe Reviewed-on: https://cl.tvl.fyi/c/depot/+/8656 Autosubmit: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2023-05-29 r/6215 fix(tvix/eval): thunk legacy let to match regular onesterni1-1/+3
Probably no real world code broken by this overzealous evaluation, but let's be thorough! Change-Id: Ib405a677182eab7940ace940c68e107573473a54 Reviewed-on: https://cl.tvl.fyi/c/depot/+/8655 Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2023-05-29 r/6214 fix(tvix/eval): thunk unary operator applicationssterni1-1/+1
Unary operator applications are thunked which can easily be observed by nix-instantiate --eval -E '[ (!true) (-1) ]' Unfortunately, there are few simple expressions where this makes a difference in the end result. Thus it only cropped up when using nixpkgs for cross compilation: Here we would compile the expression !(stdenv.cc.isGNU or false) to assemble python3Minimal's passthru attribute set (at least this seems to be the most likely explanation from the backtraces I've studied). This means that an unthunked <stdenv.cc.isGNU or false> OpForce OpInvert would be performed in order to assemble this attribute set, causing stdenv.cc to be evaluated too early, causing an infinite recursion. Resolves b/273. It seems that having a test suite that doesn't use --strict and relies on thunks rendered as <CODE> would be beneficial for catching such issues. I've not been able to find a test case with --strict that demonstrates the problem fixed in this CL. Change-Id: I640a5827b963f5b9d0f86fa2142e75e3a6bbee78 Reviewed-on: https://cl.tvl.fyi/c/depot/+/8654 Tested-by: BuildkiteCI Autosubmit: sterni <sternenseemann@systemli.org> Reviewed-by: tazjin <tazjin@tvl.su>
2023-05-26 r/6207 fix(tvix): don't call function eagerly in genList, map & mapAttrssterni4-5/+88
mapAttrs, map and genList call Nix functions provided by the caller and store the result of applying them in a Nix data structure that does not force all of its contents when forced itself. This means that when such a builtin application is forced, the Nix function calls performed by the builtin should not be forced: They may be forced later, but it is also possible that they will never be forced, e.g. in builtins.length (builtins.map (builtins.add 2) [ 1 2 3 ]) it is not necessary to compute a single application of builtins.add. Since request_call_with immediately performs the function call requested, Tvix would compute function applications unnecessarily before this change. Because this was not followed by a request_force, the impact of this was relatively low in Nix code (most functions return a new thunk after being applied), but it was enough to cause a lot of bogus builtins.trace applications when evaluating anything from `lib.modules`. The newly added test includes many cases where Tvix previously incorrectly applied a builtin, breaking a working expression. To fix this we add a new helper to construct a Thunk performing a function application at runtime from a function and argument given as `Value`s. This mimics the compiler's compile_apply(), but does itself not require a compiler, since the necessary Lambda can be constructed independently. I also looked into other builtins that call a Nix function to verify that they don't exhibit such a problem: - Many builtins immediately use the resulting value in a way that makes it necessary to compute all the function calls they do as soon as the outer builtin application is forced: * all * any * filter * groupBy * partition - concatMap needs to (shallowly) force the returned list for concatenation. - foldl' is strict in the application of `op` (I added a comment that makes this explicit). - genericClosure needs to (shallowly) force the resulting list and some keys of the attribute sets inside. Resolves b/272. Change-Id: I1fa53f744bcffc035da84c1f97ed25d146830446 Reviewed-on: https://cl.tvl.fyi/c/depot/+/8651 Autosubmit: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2023-05-25 r/6205 feat(tvix/eval): unthunk empty lists and attribute setsVincent Ambo2-0/+8
Change-Id: Ie66cb1b163a544d45d113fd0f866286f230b0188 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7960 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2023-05-25 r/6204 feat(tvix/eval): implement unthunking in compilerVincent Ambo1-1/+22
This feature allows the compiler to detect situations where the created thunk is useless and can be merged into the parent chunk instead. The only case where the compiler does this initially is when statically optimising a select expression. For example, previously the expression `builtins.length` compiled into two thunks: 1. An "inner" thunk which contained an `OpConstant` that had the optimised `length` builtin in it. 2. An "outer" thunk which contained an `OpConstant` to access the inner thunk, and the trailing OpForce of the top-level program. With this change, the inner thunk is skipped completely and the outer chunk directly contains the `length` builtin access. This can be applied in several situations, some easier than others, and we will add them in as we go along. Change-Id: Ie44521445fce1199f99b5b17712833faea9bc357 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7959 Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de>
2023-05-25 r/6203 feat(tvix/eval): implement Chunk::extend methodVincent Ambo1-1/+104
This method extends the contents of one chunk with that of another, effectively merging the thunks together. This will be used for the upcoming "unthunking" functionality. Change-Id: I6ad74232cd7f3eca198ed921e455205e00d76e6b Reviewed-on: https://cl.tvl.fyi/c/depot/+/7958 Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
2023-05-25 r/6202 refactor(tvix/eval): stop borrowing &mut selfFlorian Klink1-12/+12
This does undo cl/8571. Change-Id: Ib14b4e7404f906e346304b6113860ae811afc94a Reviewed-on: https://cl.tvl.fyi/c/depot/+/8631 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: flokli <flokli@flokli.de>
2023-05-22 r/6173 refactor(tvix/eval): use &Path instead of PathBufFlorian Klink3-13/+13
This allows getting rid of some clones in eval/src/vm/generators.rs. Change-Id: I330390307d3bcfeef19c98954c753ee55b1ccee3 Reviewed-on: https://cl.tvl.fyi/c/depot/+/8604 Autosubmit: flokli <flokli@flokli.de> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
generated by cgit-pink 1.4.1 (git 2.44.2) at 2024-12-03T06·29+0000