about summary refs log tree commit diff
path: root/tvix/eval/src
AgeCommit message (Collapse)AuthorFilesLines
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/5754 feat(tvix/eval): implement builtins.fromTOMLFlorian Klink4-0/+22
This allows parsing TOML from Tvix. We can enable the eval-okay-fromTOML testcase from nix_tests. It uses the `toml` crate, and the serde integration it brings with it. Change-Id: Ic6f95aacf2aeb890116629b409752deac49dd655 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7920 Autosubmit: flokli <flokli@flokli.de> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2023-01-25 r/5753 feat(tvix/eval): use lexical-core to format floatFlorian Klink3-4/+88
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-23 r/5747 chore(tvix/eval): delete "useless parenthesis" warning/optimisationVincent Ambo2-30/+0
Two main reasons: 1. Traversing the structure to do this optimisation is actually *slower* than not optimising it. 2. There are literally hundreds of thousands of incidences of this in nixpkgs, and with some of the weird code there some of these (functionally) useless parens are actually required for readability reasons. Change-Id: I1044b1c5f9fe20df4b6085851fc3b191277c65dc Reviewed-on: https://cl.tvl.fyi/c/depot/+/7917 Autosubmit: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de>
2023-01-23 r/5746 fix(tvix/eval): force functors before applying themVincent Ambo3-0/+12
call_value in the VM expects the callable to be forced when calling it, which was not the case for functors. Change-Id: Id55a2fe32a9573be42aef8669e268df519a989cd Reviewed-on: https://cl.tvl.fyi/c/depot/+/7909 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2023-01-22 r/5735 feat(tvix/eval): support builtins implemented in Nix itselfVincent Ambo4-17/+108
This makes it possible to inject builtins into the builtin set that are written in Nix code, and which at runtime are represented by a thunk that will compile them the first time they are used. Change-Id: Ia632367328f66fb2f26cb64ae464f8f3dc9c6d30 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7891 Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de>
2023-01-22 r/5734 docs(tvix/eval): update some outdated commentsVincent Ambo1-8/+3
These don't apply anymore since the "antidote-CL". Change-Id: I40ee73ef43d44bbfc650a8fe6c2b33263dd06959 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7890 Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de>
2023-01-21 r/5721 refactor(tvix/eval): administer antidote for poisonAdam Joseph7-133/+47
The codebase contains a lot of complexity and odd roundabout handling for shadowing globals. I'm pretty sure none of this is necessary, and all of it disappears if you simply make the globals part of the ordinary identifier resolution chain, with their own scope up above the root scope. Then the ordinary shadowing routines do the right thing, and no special cases or new terminology are required. This commit does that. Note by tazjin: This commit was originally abandoned when Adam decided not to take away reviewer bandwidth for this at the time (eval was still in a much earlier stage). As we've recently done some significant refactoring of globals initialisation this came up again, and it seems we can easily cover the use-cases of the poison tracking in other ways now, so I've rebased, updated and resurrected the CL. Co-Authored-By: Vincent Ambo <tazjin@tvl.su> Signed-off-by: Adam Joseph <adam@westernsemico.com> Change-Id: Ib3309a47a7b31fa5bf10466bade0d876b76ae462 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7089 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de>
2023-01-20 r/5715 refactor(tvix/eval): keep globals alive through VM structVincent Ambo4-10/+59
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/5714 docs(tvix/eval): add doc comments to VM fieldsVincent Ambo1-0/+8
Change-Id: Ia4857c217de15aec8b61e1abd39e22c50e2d816a Reviewed-on: https://cl.tvl.fyi/c/depot/+/7876 Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI Autosubmit: tazjin <tazjin@tvl.su>
2023-01-20 r/5707 refactor(tvix/eval): directly return builtin tuples from macroVincent Ambo3-21/+4
All invocations of the builtin macro had to previously filter through the `builtin_tuple` function, but it's more sensible to directly return these from the macro. Change-Id: I45600ba84d56c9528d3e92570461c319eea595ce Reviewed-on: https://cl.tvl.fyi/c/depot/+/7825 Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de>
2023-01-20 r/5706 feat(tvix/eval): add error contexts to annotate error kindsVincent Ambo5-34/+99
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 Ambo3-7/+5
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/5674 feat(tvix/eval): add builtins to builtinsVincent Ambo3-2/+34
This is a somewhat terrifying hack that enables us to support `builtins.builtins`, by running a "fake compilation" inside of a suspended native thunk that can resolve the weak pointer to the globals. With this implementation, the thunk at `builtins.builtins` actually resolves to the "real" `builtins` (verified with a new test). This is kind of ugly, and it's something users shouldn't use, but bubbling a warning out of this is difficult at the moment due to a little bit of trickery with how the spans in suspended native thunks work (they don't) (see b/237, b/238) Change-Id: I67d0e93246dd5b279c960aeda00402031aa12af3 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7748 Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de>
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/5665 feat(tvix/eval): add error variant for threading through errorsVincent Ambo1-2/+17
This variant is required for external builtins (which in our case includes `derivation`) to thread through reasonable error messages. This has some potential for improvement, but it's an improvement over the status quo of panicking in the external builtins when no appropriate error is available. Change-Id: I7e4bdb0a156c7717092dde30aa4785192182dc66 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7841 Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
2023-01-16 r/5664 feat(tvix/eval): implement builtins.toXMLVincent Ambo7-0/+157
Change-Id: I009efc53a8e98f0650ae660c4decd8216e8a06e7 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7835 Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
2023-01-16 r/5663 chore(tvix/eval): add other required items to public APIVincent Ambo2-3/+7
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 Ishtyaq3-1/+13
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 Ambo11-10/+75
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/5642 fix(tvix/eval): address useless_format clippy warnAaqa Ishtyaq1-13/+6
This CL address clippy warnings related to use of 'format!' macro to return unmodified 'String'. Change-Id: I88726e59d8f39f6a455a8c1f48075b52d167e489 Signed-off-by: Aaqa Ishtyaq <aaqaishtyaq@gmail.com> Reviewed-on: https://cl.tvl.fyi/c/depot/+/7804 Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
2023-01-10 r/5640 feat(tvix/eval): implement serde::Deserialize for ValueRyan Lahfa7-47/+96
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-10 r/5639 refactor(tvix/eval): impl Display for ErrorKindVincent Ambo1-2/+8
This just shuffles the Display implementations around so that ErrorKind itself is displayable, which is useful in some situations where errors under construction need to be type-converted. Change-Id: I7b633d03d0dc34f345c4f20676e0023ecb1db0c4 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7802 Autosubmit: tazjin <tazjin@tvl.su> Reviewed-by: edef <edef@edef.eu> Tested-by: BuildkiteCI
2023-01-08 r/5629 fix(tvix/eval): fix last uses of Vec<Value> -> NixList in builtinsVincent Ambo2-22/+21
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-07 r/5626 fix(tvix/eval): fix typo'd function name in testsVincent Ambo1-1/+1
Caught by sterni on cl/7783. Change-Id: I15d57b893ef22538fdd7e809f3b92861dd2bc1af Reviewed-on: https://cl.tvl.fyi/c/depot/+/7789 Autosubmit: tazjin <tazjin@tvl.su> Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
2023-01-06 r/5620 refactor(tvix/eval): use builtins macro for placeholdersVincent Ambo1-67/+45
Change-Id: I30bc475e3e36a163fa169083481cdd4b4d0ca456 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7785 Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI Autosubmit: tazjin <tazjin@tvl.su>
2023-01-06 r/5619 refactor(tvix/eval): move mocked builtins.derivation to testsVincent Ambo2-36/+38
This placeholder should not live in the main crate anymore as we will be injecting the real one from outside of eval, but there are still language tests that depend on a (simple, mockable) version of it. Change-Id: I68ea169db15cbdbeed320930d3069e21e376c90d Reviewed-on: https://cl.tvl.fyi/c/depot/+/7783 Reviewed-by: flokli <flokli@flokli.de> Autosubmit: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2023-01-06 r/5607 feat(tvix/eval): skip & warn for useless parenthesisVincent Ambo2-0/+32
Change-Id: I567ca0682012b9d09f1217e57a104ac5671f8d82 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7771 Tested-by: BuildkiteCI Reviewed-by: raitobezarius <tvl@lahfa.xyz> Reviewed-by: flokli <flokli@flokli.de>
2023-01-06 r/5606 feat(tvix/eval): warn on empty let-bindingsVincent Ambo2-1/+10
Change-Id: Ib6ef7ce514abbd3e372dfe9df7137aa36dbda9d4 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7770 Reviewed-by: raitobezarius <tvl@lahfa.xyz> Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
2023-01-06 r/5605 refactor(tvix/eval): short-circuit on empty attrs in compilerVincent Ambo1-0/+9
This is marginally more efficient and has simpler bytecode. Change-Id: Iad37c9aeef24583e8f696911bcd83d43639f2e36 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7769 Reviewed-by: raitobezarius <tvl@lahfa.xyz> Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de>
2023-01-06 r/5604 feat(tvix/eval): warn about empty `inherit`sVincent Ambo2-0/+11
Change-Id: I82bec6fe2210bcb88c46fd2fdf3e26bd613d1c1f Reviewed-on: https://cl.tvl.fyi/c/depot/+/7768 Reviewed-by: flokli <flokli@flokli.de> Reviewed-by: raitobezarius <tvl@lahfa.xyz> Tested-by: BuildkiteCI
2023-01-06 r/5603 fix(tvix/eval): compile but don't emit dead codeVincent Ambo2-9/+33
This adds a mechanism to the compiler to compile an expression without emitting any code. This allows for detected dead code to still be compiled to detect errors & warnings inside of it. Change-Id: Ie78479173570e9c819d8f32ae683ce34234a4c5d Reviewed-on: https://cl.tvl.fyi/c/depot/+/7767 Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
2023-01-06 r/5602 feat(tvix/eval): implement initial compiler::optimiser moduleVincent Ambo5-0/+165
This optimiser can rewrite some expressions into more efficient forms, and warn users about those cases. As a proof-of-concept, only some simple boolean comparisons are supported for now. Change-Id: I7df561118cfbad281fc99523e859bc66e7a1adcb Reviewed-on: https://cl.tvl.fyi/c/depot/+/7766 Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de>
2023-01-06 r/5601 refactor(tvix/eval): take owned ast::Expr in Compiler::compileVincent Ambo2-34/+34
This adds a very minimal amount of additional Rc-increments (~1 per compilation), but makes it a lot easier to add an AST-optimising compiler pass without incurring a lot of extra cost. Change-Id: I57208bdfc8882e3ae21c5850e14aa380d3ccea36 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7765 Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
2023-01-06 r/5599 feat(tvix/eval): add Evaluation::compile_only methodVincent Ambo2-47/+99
This would make it possible to implement something like a linter based on the tvix-eval compiler warnings. Change-Id: I1feb4e7c4a44be7d1204b0a962ab522fd32b93c6 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7763 Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de>
2023-01-06 r/5598 fix(tvix/eval): don't increase `with_stack_size` in scope inheritsVincent Ambo1-1/+1
There was probably a misunderstanding somewhere about the with_stack_size being related to how far away it is from the with, but it is about whether there is a with at all. This broke a warning (`UselessInherit`), and may actually have let to more inefficient codegen in some cases. Change-Id: I08338ea59ae39dad01ca8a4e09d934a936cdea2f Reviewed-on: https://cl.tvl.fyi/c/depot/+/7762 Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
2023-01-06 r/5597 fix(tvix/eval): VM & Builtin* types have to be publicVincent Ambo1-9/+2
... without them, using the new Builtins API is basically impossible for library consumers. Change-Id: Ice0557a2e55e12d812f51bf5a99e6b8c91ad1b91 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7755 Autosubmit: tazjin <tazjin@tvl.su> Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
2023-01-06 r/5595 feat(tvix/eval): add builtins.{null,true,false}sterni1-0/+3
Code probably rarely relies on these, but it's not hard to support them. Change-Id: I8499fec34efaf031f9c013bbd370a13db929a2a3 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7772 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Autosubmit: sterni <sternenseemann@systemli.org>
2023-01-06 r/5594 test(tvix/eval): add test for builtins paritysterni3-0/+41
This will eventually force us to have a base builtins set in common with C++ Nix, i.e. all 2.3 builtins except the controversial builtins.valueSize. Change-Id: I2c767f07d6a14711911658e87da9f18ede57a143 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7747 Autosubmit: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2023-01-04 r/5585 feat(tvix/serde): implement enum deserialisationVincent Ambo1-1/+1
Implements externally tagged enum deserialisation. Other serialisation methods are handled by serde internally using the existing methods. See the tests for examples. Change-Id: Ic4a9da3b5a32ddbb5918b1512e70c3ac5ce64f04 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7721 Tested-by: BuildkiteCI Autosubmit: tazjin <tazjin@tvl.su> Reviewed-by: flokli <flokli@flokli.de>
2023-01-04 r/5583 refactor(tvix/eval): avoid unnecessary pop/push in OpForceVincent Ambo1-6/+1
Change-Id: Ic7559eaa43aa0dcc97babb7669770c0f7f959f1b Reviewed-on: https://cl.tvl.fyi/c/depot/+/7754 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 identifierssterni3-1/+32
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>
2023-01-04 r/5581 refactor(tvix/eval): streamline construction of globals/builtinsVincent Ambo6-218/+269
Previously the construction of globals (a compiler-only concept) and builtins (a (now) user-facing API) was intermingled between multiple different modules, and kind of difficult to understand. The complexity of this had grown in large part due to the implementation of `builtins.import`, which required the notorious "knot-tying" trick using Rc::new_cyclic (see cl/7097) for constructing the set of globals. As part of the new `Evaluation` API users should have the ability to bring their own builtins, and control explicitly whether or not impure builtins are available (regardless of whether they're compiled in or not). To streamline the construction and allow the new API features to work, this commit restructures things by making these changes: 1. The `tvix_eval::builtins` module is now only responsible for exporting sets of builtins. It no longer has any knowledge of whether or not certain sets (e.g. only pure, or pure+impure) are enabled, and it has no control over which builtins are globally available (this is now handled in the compiler). 2. The compiler module is now responsible for both constructing the final attribute set of builtins from the set of builtins supplied by a user, as well as for populating its globals (that is identifiers which are available at the top-level scope). 3. The `Evaluation` API now carries a `builtins` field which is populated with the pure builtins by default, and can be extended by users. 4. The `import` feature has been moved into the compiler, as a special case. In general, builtins no longer have the ability to reference the "fix point" of the globals set. This should not change any functionality, and in fact preserves minor differences between Tvix/Nix that we already had (such as `builtins.builtins` not existing). Change-Id: Icdf5dd50eb81eb9260d89269d6e08b1e67811a2c Reviewed-on: https://cl.tvl.fyi/c/depot/+/7738 Reviewed-by: sterni <sternenseemann@systemli.org> Autosubmit: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de>
2023-01-02 r/5565 chore(tvix/eval): implement std::error::Error for tvix_eval::ErrorVincent Ambo2-1/+20
This makes it easier to interface this error with other crates. Change-Id: I4947ea6097608f8c0427fb94a819ef748d94ea4b Reviewed-on: https://cl.tvl.fyi/c/depot/+/7711 Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de>
2022-12-29 r/5542 refactor(tvix/eval): remove extra Rc<..> around Value::AttrsVincent Ambo5-21/+24
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 Ambo6-73/+72
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 Ambo3-28/+31
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 Ambo3-34/+38
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 Ambo16-100/+58
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
generated by cgit-pink 1.4.1 (git 2.44.2) at 2024-11-30T12·32+0000