about summary refs log tree commit diff
path: root/tvix/eval/src/tests/tvix_tests
AgeCommit message (Collapse)AuthorFilesLines
2023-12-12 r/7178 fix(tvix/eval): fix branching on catchable defaults (b/343)Adam Joseph2-0/+0
This commit adds Opcode::OpJumpIfCatchable, which can be inserted ahead of most VM operations which expect a boolean on the stack, in order to handle catchables in branching position properly. Other than remembering to patch the jump, no other changes should be required. This commit also fixes b/343 by emitting this new opcode when compiling if-then-else. There are probably other places where we need to do the same thing. Change-Id: I48de3010014c0bbeba15d34fc0d4800e0bb5a1ef Reviewed-on: https://cl.tvl.fyi/c/depot/+/10288 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: Adam Joseph <adam@westernsemico.com>
2023-12-12 r/7177 test(tvix/eval): test branching on catchable defaults (b/343)Adam Joseph2-0/+2
This is a test case for b/343, wherein tvix dies if you try to branch on an argument whose defaulted value is a catchable. Change-Id: I891ca825e39ad14dda9f220f06d9591874fcd45d Reviewed-on: https://cl.tvl.fyi/c/depot/+/10287 Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: Adam Joseph <adam@westernsemico.com> Tested-by: BuildkiteCI
2023-12-12 r/7173 test(tvix/eval): nested assertions (b/340)Adam Joseph2-0/+2
Change-Id: I898d7056877a6370d5720b633df416f54e7cf65f Reviewed-on: https://cl.tvl.fyi/c/depot/+/10222 Tested-by: BuildkiteCI Autosubmit: Adam Joseph <adam@westernsemico.com> Reviewed-by: tazjin <tazjin@tvl.su>
2023-12-12 r/7169 fix(tvix/eval): preserve catchables in nix_cmp_ordering(), fix b/338Adam Joseph2-0/+0
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 Joseph2-0/+0
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/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-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-15 r/6589 fix(tvix/eval): update identifier quoting to match cppnix 2.17Adam Joseph11-2/+23
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-08-24 r/6523 fix(tvix/eval): off-by-one in replaceStringsLinus Heckemann2-1/+2
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-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-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 escapesterni2-0/+7
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-21 r/6341 fix(tvix/eval): use realpaths for import cachesterni5-0/+11
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 defaultingsterni2-0/+21
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/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)sterni6-0/+55
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/+2
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-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-07 r/6243 fix(tvix/eval): type check function argument with set patternsterni1-0/+2
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-05-26 r/6207 fix(tvix): don't call function eagerly in genList, map & mapAttrssterni2-0/+32
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-03-17 r/6022 fix(tvix/eval): use coerce_to_string in builtins.substringVincent Ambo2-0/+6
This actually uses coercion under the hood in C++ Nix. See the test for an example. Change-Id: Id56b364acf269225b6829d0b600e0222f8b3608d Reviewed-on: https://cl.tvl.fyi/c/depot/+/8322 Reviewed-by: andi <andi@notmuch.email> Tested-by: BuildkiteCI
2023-03-13 r/5977 fix(tvix/eval): handle toJSON on attribute sets with `outPath`Vincent Ambo4-0/+15
These are serialised as the serialisation of the value of that field. Change-Id: Ida51708b1f43ce09b0ec835f4e265918aa31dd09 Reviewed-on: https://cl.tvl.fyi/c/depot/+/8205 Reviewed-by: raitobezarius <tvl@lahfa.xyz> Tested-by: BuildkiteCI
2023-03-13 r/5976 fix(tvix/eval): handle `__toString` when JSON-serialising attrsetsVincent Ambo4-0/+20
These must be serialised to a JSON string of the *result* of coercing the function application to a string. Change-Id: Ib7f49ccd950503ddbdbf99643cd59565e26b50da Reviewed-on: https://cl.tvl.fyi/c/depot/+/8204 Reviewed-by: raitobezarius <tvl@lahfa.xyz> Tested-by: BuildkiteCI
2023-03-13 r/5970 fix(tvix/eval): correctly thunk deferred formals accessVincent Ambo2-0/+7
Formals can be initialised with deferred default values (see the test cases), in which case they need an extra thunk to have something that can be finalised appropriately when the setup is done. Fixes: b/255 Change-Id: I380e3770be68eaa83ace96d450c7cead32dacc9f Reviewed-on: https://cl.tvl.fyi/c/depot/+/8196 Tested-by: BuildkiteCI Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2023-03-13 r/5965 test(tvix/eval): add test for infinite recursion detectionVincent Ambo1-0/+1
Change-Id: Ief20544a44c3542fe40a5c09f81d0f064a346f44 Reviewed-on: https://cl.tvl.fyi/c/depot/+/8149 Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2023-01-31 r/5784 test(tvix/eval): add tests for internal formals dependenciesVincent Ambo4-0/+8
Formals can depend on each other when using another formal as a default value. This test ensures that the compiler's declaration and initialisation order of formals is consistent with what actually happens in the VM. Change-Id: Ibdabe262554e8066d67fac1ebc3b5a48ef626e18 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7948 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de>
2023-01-25 r/5753 feat(tvix/eval): use lexical-core to format floatFlorian Klink2-2/+2
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/5746 fix(tvix/eval): force functors before applying themVincent Ambo2-0/+8
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-17 r/5674 feat(tvix/eval): add builtins to builtinsVincent Ambo2-0/+2
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-12 r/5652 feat(tvix/eval): implement builtins.toJSONVincent Ambo4-0/+22
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 Lahfa2-1/+2
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-06 r/5602 feat(tvix/eval): implement initial compiler::optimiser moduleVincent Ambo2-0/+22
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/5594 test(tvix/eval): add test for builtins paritysterni2-0/+36
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/5582 fix(tvix/eval): ' is allowed in nonfirst position in Nix identifierssterni2-0/+31
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/5541 refactor(tvix/eval): persistent, memory-sharing OrdMap for NixAttrsVincent Ambo1-5/+5
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-15 r/5420 refactor(tvix): build Rust projects using crate2nixVincent Ambo1-0/+0
Introduces granular dependency builds using crate2nix, bootstrapped off the generated configuration from the newly introduced workspace (see cl/7533). This commit checks in the generated Cargo.nix file which can be regenerated with a parameterless invocation of `crate2nix generate` in `//tvix`. I tried generating this in IFD, but it turned out to be harder than what seemed worthwhile for now. In this setup, the various build targets for Rust projects end up being attributes of the imported `Cargo.nix` file at the `tvix.crates` attribute. These still lack configuration, however, which has been fixed in the various `default.nix` files of individual projects. Note that we (temporarily) lose the ability to build tvix-eval's benchmarks in CI. I haven't figured out what magic incantation summons them from the void again ... The `eval-okay-readDir` tests from both test suites have been disabled because they fail for unknown reasons when run in this new derivation. Somebody will have to debug it! Change-Id: I2014614ccb9c8951aedbd71df7966ca191a13695 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7538 Autosubmit: tazjin <tazjin@tvl.su> Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
2022-12-02 r/5374 test(tvix/eval): check inner forcing despite declaring pointersterni2-0/+12
Maybe counter-intuitively the inner elements of a list or the attribute values of an attribute set will be forced despite pointer equality (but only one layer deep). Change-Id: I485d96452fb56f5fb342d39039c9137725b33d3f Reviewed-on: https://cl.tvl.fyi/c/depot/+/7371 Reviewed-by: Adam Joseph <adam@westernsemico.com> Tested-by: BuildkiteCI
2022-12-02 r/5373 test(tvix/eval): add a test for repeated keys in listToAttrsVincent Ambo2-0/+14
This came up in the Nix Language channel today and I thought it warranted a test case. We did actually implement this correctly. Change-Id: I4b37c92d06eb6e3a7f59ea3d10af38f2b0a93d53 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7493 Reviewed-by: sterni <sternenseemann@systemli.org> Autosubmit: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-12-02 r/5369 test(tvix/eval): verify pointer equality checkssterni2-0/+47
Change-Id: I9baf2810fbd5b6ee8bfe10fce5b64801a35c1d67 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7369 Reviewed-by: tazjin <tazjin@tvl.su> Reviewed-by: Adam Joseph <adam@westernsemico.com> Tested-by: BuildkiteCI
2022-12-02 r/5368 test(tvix/eval): verify pointer equality in list comparisonssterni2-0/+7
Change-Id: I617d402c8ecc7aaf607c4bdcd58a06ebddb71fac Reviewed-on: https://cl.tvl.fyi/c/depot/+/7370 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Reviewed-by: Adam Joseph <adam@westernsemico.com>
2022-11-26 r/5339 fix(tvix/eval): OpAdd must weakly stringify if either arg is stringAdam Joseph2-0/+8
Tests included. Change-Id: I7a4905d6103813373e383e2e8629c5fd243d6bca Reviewed-on: https://cl.tvl.fyi/c/depot/+/7377 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: Adam Joseph <adam@westernsemico.com>
2022-11-25 r/5310 test(tvix/eval): add eval-okay-closure-pointer-compareAdam Joseph2-0/+15
This test case checks two things: * A sanity check that "pointer equality for functions" means not just the lambda, but also the upvalues. * To be pointer-equal, it is not enough for the upvalues to be normal-form equal (i.e. `nix_eq()`-equal); the upvalues must be *pointer*-equal. The second part of the test case checks for this. Signed-off-by: Adam Joseph <adam@westernsemico.com> Change-Id: I4e59327a6f199b8212e97197b212e3c3934bb3f0 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7372 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-11-24 r/5308 test(tvix/eval): test limits of builtins.seq's forcingsterni2-2/+2
Change-Id: I6dfc9108220762ef3372cd2739e91d79c01a55e1 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7366 Tested-by: BuildkiteCI Autosubmit: sterni <sternenseemann@systemli.org> Reviewed-by: tazjin <tazjin@tvl.su>
2022-11-04 r/5242 test(tvix/eval): add a test case for groupBy with thunksAdam Joseph2-0/+7
We have to be careful implementing `builtins.groupBy`, since the list may contain thunks, and tvix's to_xxx() functions do not work on thunks. Signed-off-by: Adam Joseph <adam@westernsemico.com> Change-Id: I182b6fc2d4296f864ed16744ef70b153e8e6978a Reviewed-on: https://cl.tvl.fyi/c/depot/+/7039 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-11-04 r/5241 fix(tvix/eval): quote keys which are not valid identifiersAdam Joseph2-0/+2
The impl Display for NixAttrs needs to wrap double quotes around any keys which are not valid Nix identifiers. This commit does that, and adds a test (which fails prior to this commit and passes after this commit). Change-Id: Ie31ce91e8637cb27073f23f115db81feefdc6424 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7084 Autosubmit: Adam Joseph <adam@westernsemico.com> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-31 r/5226 fix(tvix/eval): nix_eq() must recurseAdam Joseph2-0/+2
The current implementation of nix_eq will force one level of thunks and then switch to the (non-forcing) rust Eq::eq() method. This gives incorrect results for lists-of-thunks. This commit changes nix_eq() to be recursive. A regression test (which fails prior to this commit) is included. This fix also causes nix_tests/eval-okay-fromjson.nix to pass, so it is moved out of notyetpassing. Change-Id: I655fd7a5294208a7b39df8e2c3c12a8b9768292f Signed-off-by: Adam Joseph <adam@westernsemico.com> Reviewed-on: https://cl.tvl.fyi/c/depot/+/7142 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-29 r/5223 test(tvix/eval): builtins.sort must preserve order of equal elementssterni2-0/+8
Change-Id: I59a0756940d1e5360a2ab4e886cf0bc9af7b8901 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7133 Autosubmit: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-29 r/5221 feat(tvix/eval): Implement comparison for listsGriffin Smith2-0/+18
Lists are compared lexicographically in C++ nix as of [0], and our updated nix test suites depend on this. This implements comparison of list values in `Value::nix_cmp` using a very similar algorithm to what C++ does - similarly to there, this requires passing in the VM so we can force thunks in the list elements as we go. [0]: https://github.com/NixOS/nix/commit/09471d2680292af48b2788108de56a8da755d661# Change-Id: I5d8bb07f90647a1fec83f775243e21af856afbb1 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7070 Autosubmit: grfn <grfn@gws.fyi> Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
generated by cgit-pink 1.4.1 (git 2.44.2) at 2024-11-12T22·56+0000