about summary refs log tree commit diff
AgeCommit message (Collapse)AuthorFilesLines
2023-12-14 r/7218 fix(tvix/eval): remove incorrect imports when coercingsterni8-54/+207
The default behavior of string coercion in C++ Nix is to weakly coerce and import to store if necessary. There is a flag to make it strongly coerce (coerceMore) and a flag that controls whether path values have the corresponding file/directory imported into the store before returning the (store) path as a string (copyToStore). We need to implement our equivalent to the copyToStore (import_paths) flag for the benefit of weak coercions that don't import into the store (dirOf, baseNameOf, readFile, ...) and strong coercions that don't import into the store (toString). This makes coerce_to_string as well as CoercionKind weirder and more versatile, but prevents us from reimplementing parts of the coercion logic constantly as can be seen in the case of baseNameOf. Note that it is not possible to test this properly in //tvix/eval tests due to the lack of an appropriate EvalIO implementation being available. Tests should be added to //tvix/glue down the line. Change-Id: I8fb8ab99c7fe08e311d2ba1c36960746bf22f566 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10361 Autosubmit: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI Reviewed-by: Adam Joseph <adam@westernsemico.com>
2023-12-14 r/7217 fix(tvix/eval): determine meaning of `+` exprs based on first typesterni1-5/+11
This matches the behavior of C++ Nix more closely where the decision is made based on the first type based to ExprConcatStrings: https://github.com/NixOS/nix/blob/1f93fa2ed29ff0ba92bfa58995232668187fb0d0/src/libexpr/eval.cc#L1967-L2025 Note that this doesn't make a difference in any successful evaluation (at least to my knowledge), but ensures that our error messages will match C++ Nix more closely, e.g. in the case of `1 + "string"`. Change-Id: I8059930788f9c8d98baf98e3d93d8a060ef961f2 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10360 Tested-by: BuildkiteCI Autosubmit: sterni <sternenseemann@systemli.org> Reviewed-by: Adam Joseph <adam@westernsemico.com>
2023-12-13 r/7216 docs(tvix/castore/directorysvc): update commentFlorian Klink1-3/+3
This comment didn't make a lot of sense before. Change-Id: Ie057a133ca4b1a099ed3c885e32316b0d87c5eb0 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10339 Tested-by: BuildkiteCI Autosubmit: flokli <flokli@flokli.de> Reviewed-by: Connor Brewster <cbrewster@hey.com>
2023-12-13 r/7215 docs(tvix/castore): document expectations about DirectoryServiceFlorian Klink1-1/+7
Namely, all trait implementations should reject invalid data being fed, and detect invalid data being returned. b/355 tracks writing some more tests for this, to ensure we're compliant with this. Change-Id: I3b05752932837ce208785efb21ffc21508b4b33a Reviewed-on: https://cl.tvl.fyi/c/depot/+/10338 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi> Autosubmit: flokli <flokli@flokli.de>
2023-12-13 r/7214 docs(tvix/castore): fix docstringsFlorian Klink1-2/+2
There's been some copypasta errors. Change-Id: I8fcad6cfc951ead6c789e0dce823c798adbfcf97 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10337 Autosubmit: flokli <flokli@flokli.de> Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2023-12-13 r/7213 feat(tvix/glue): add single-file import testsFlorian Klink1-0/+23
This ensures importing these paths also behave the same way as Nix. Change-Id: Icaa507bbe3d9867a301fc7a300c5d2b3f9feb911 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10355 Reviewed-by: raitobezarius <tvl@lahfa.xyz> Tested-by: BuildkiteCI Autosubmit: flokli <flokli@flokli.de>
2023-12-13 r/7212 feat(tvix/glue): add some import testsFlorian Klink4-42/+301
This creates a directory with a .keep file inside, and uses "${path/to/there}" to coerce it to a string (and import it into the store), ensuring it calculates the same store paths as Nix does. Change-Id: Ie14ae075104ce278bc4f2cce93aab5762a2734d1 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10343 Tested-by: BuildkiteCI Autosubmit: flokli <flokli@flokli.de> Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2023-12-12 r/7211 fix(3p/rust-crates): fix build of remaining Rust cratesVincent Ambo1-3/+2
Change-Id: If3aec8efa6ea5a1402af4dbdb662cd4c0d184322 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10341 Tested-by: BuildkiteCI Autosubmit: tazjin <tazjin@tvl.su> Reviewed-by: flokli <flokli@flokli.de>
2023-12-12 r/7210 fix(tvix/glue/tvix_store_io): don't unwrap ingest_path errorsFlorian Klink1-1/+1
Fixes b/344. Change-Id: I1446726e3be3a8fc20801d466a964c4d6b8cbc02 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10331 Autosubmit: flokli <flokli@flokli.de> Reviewed-by: raitobezarius <tvl@lahfa.xyz> Tested-by: BuildkiteCI
2023-12-12 r/7209 fix(tvix/castore/import): don't unwrap entryFlorian Klink1-2/+8
If the path specified doesn't exist, construct a proper error instead of panicking. Part of b/344. Change-Id: Id5c6a91248b0a387f3e8f138f8e686e402009e8f Reviewed-on: https://cl.tvl.fyi/c/depot/+/10330 Autosubmit: flokli <flokli@flokli.de> Reviewed-by: raitobezarius <tvl@lahfa.xyz> Tested-by: BuildkiteCI
2023-12-12 r/7208 feat(tvix/castore/import): log returned errorsFlorian Klink1-1/+1
This will emit a log event / trace in case this function returns an error-y type. Change-Id: I48db6807f3e42304357c422a2b6e177cb8b95228 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10329 Autosubmit: flokli <flokli@flokli.de> Reviewed-by: raitobezarius <tvl@lahfa.xyz> Tested-by: BuildkiteCI
2023-12-12 r/7207 refactor(tvix/castore/blobservice): use io::Result in traitFlorian Klink7-56/+52
For all these calls, the caller has enough context about what it did, so it should be fine to use io::Result here. We pretty much only constructed crate::Error::StorageError before anyways, so this conveys *more* information. Change-Id: I5cabb3769c9c2314bab926d34dda748fda9d3ccc Reviewed-on: https://cl.tvl.fyi/c/depot/+/10328 Reviewed-by: raitobezarius <tvl@lahfa.xyz> Tested-by: BuildkiteCI Autosubmit: flokli <flokli@flokli.de>
2023-12-12 r/7206 refactor(tvix/eval): vm::add_values(): be less verboseAdam Joseph1-12/+8
Change-Id: Icf328649fd70bdf0fc3ba6cd7754ae29735f11f7 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10035 Autosubmit: Adam Joseph <adam@westernsemico.com> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2023-12-12 r/7205 fix(tvix/eval): substring: propagate catchablesAdam Joseph3-4/+7
Change-Id: Ia9b7858c817fbc9c95a3d1c2855b2445f7830e8d Reviewed-on: https://cl.tvl.fyi/c/depot/+/10326 Tested-by: BuildkiteCI Autosubmit: Adam Joseph <adam@westernsemico.com> Reviewed-by: tazjin <tazjin@tvl.su>
2023-12-12 r/7204 fix(tvix/eval): baseNameOf: propagate catchablesAdam Joseph3-0/+3
Change-Id: Id8dc772ea8f338dfd243210f4108f79072570c3b Reviewed-on: https://cl.tvl.fyi/c/depot/+/10324 Autosubmit: Adam Joseph <adam@westernsemico.com> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2023-12-12 r/7203 fix(tvix/eval): builtins.length: propagate catchablesAdam Joseph3-0/+5
This commit fixes out builtins.length so it propagates catchables like cppnix does. Change-Id: I7670bec5eee1d4cd3f67a04c9a6808979fb56a8d Reviewed-on: https://cl.tvl.fyi/c/depot/+/10315 Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: Adam Joseph <adam@westernsemico.com> Tested-by: BuildkiteCI
2023-12-12 r/7202 fix(3p/rust-crates): remove remaining `crateName` overridesVincent Ambo1-10/+0
These are broken on whitby after GC, because crates.io changed the upstream URLs to always match the package name. Change-Id: I96d4d7bcc13e21a883c7b9b8ca657db64b74f716 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10332 Autosubmit: tazjin <tazjin@tvl.su> Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
2023-12-12 r/7201 fix(tvix/eval): builtins.filter: propagate catchablesAdam Joseph3-2/+7
This commit fixes builtins.filter so it propagates catchables correctly. Change-Id: Ib23a383bc5e272e42052205ffd1e94649a0ebc47 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10313 Autosubmit: Adam Joseph <adam@westernsemico.com> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2023-12-12 r/7200 feat(tvix/eval): builtins.hashString: add placeholderAdam Joseph1-0/+12
This adds an unimplemented placeholder for builtins.hashString. Change-Id: Ibc770103acf5dbc3ea7589ab5ca23fe6e07bd91a Reviewed-on: https://cl.tvl.fyi/c/depot/+/10311 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Autosubmit: Adam Joseph <adam@westernsemico.com>
2023-12-12 r/7199 fix(tvix/eval): builtins.getAttr: propagate catchablesAdam Joseph3-0/+8
Change-Id: I84b6b8f8568d57614a03aff0d6069e0bc27357bf Reviewed-on: https://cl.tvl.fyi/c/depot/+/10310 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Autosubmit: Adam Joseph <adam@westernsemico.com>
2023-12-12 r/7198 fix(tvix/eval): builtins.elemAt: propagate catchablesAdam Joseph3-0/+8
This commit fixes builtins.elemAt so it propagates catchables like cppnix does. Change-Id: Ieca5e128da17e78af0b14dae4a28a1ff8796e4f2 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10308 Autosubmit: Adam Joseph <adam@westernsemico.com> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2023-12-12 r/7197 feat(tvix/glue): make unimplemented-structuredAttrs catchableAdam Joseph1-1/+3
This commit adjusts the error produced by STRUCTURED_ATTRS so that it is catchable. This way we are able to enumerate the release packageset, and the enumeration process will simply treat the few derivations using structured attributes as being broken, rather than killing the whole eval session. Change-Id: I2e17638b8e3227f88543c3718aaf505deaec22ae Reviewed-on: https://cl.tvl.fyi/c/depot/+/10306 Autosubmit: Adam Joseph <adam@westernsemico.com> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2023-12-12 r/7196 fix(tvix/eval): propagate catchables through builtins.splitVersionAdam Joseph3-0/+5
This fixes our implementation of builtins.splitVersion so it propagates catchables like cppnix does. Change-Id: Id5d83ea76229f8c8f202aa42353cb609e67de43f Reviewed-on: https://cl.tvl.fyi/c/depot/+/10305 Autosubmit: Adam Joseph <adam@westernsemico.com> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2023-12-12 r/7195 feat(tvix/eval): builtins.hasContext: placeholder implementationAdam Joseph1-0/+8
Currently this just `throw`s a message explaining that it is not implemented. This is necessary in order to allow enumerating the nixpkgs release attrset (afaict only one package uses this builtin). Change-Id: I45266d46af579ddb5856b192b6be4b481369543c Reviewed-on: https://cl.tvl.fyi/c/depot/+/10302 Autosubmit: Adam Joseph <adam@westernsemico.com> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2023-12-12 r/7194 fix(tvix/eval): baseNameOf should not coerce paths into stringsAdam Joseph1-4/+12
... since this may import them to the store which changes their basename. Fixes b/350. Change-Id: Iabd08ff4d6a424c66d6d7784d7a96b0c078f0a91 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10298 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Autosubmit: Adam Joseph <adam@westernsemico.com>
2023-12-12 r/7193 fix(tvix/eval): add unimplemented __curPos and builtins.filterSourceAdam Joseph3-0/+19
This commit adds __curPos (to the global scope, yuck) and builtins.filterSource. These are not implemented; forcing them will produce the same result as `throw "message"`. Unfortunately these two post-2.3 features are used throughout nixpkgs. Since an unresolved indentifier is a catchable error, this breaks the entire release eval. With this commit, it simply causes those broken packages that use these features to appear as they are: broken. Change-Id: Ib43dea571f6a9fab4d54869349f80ee4ec5424c2 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10297 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Autosubmit: Adam Joseph <adam@westernsemico.com>
2023-12-12 r/7192 fix(tvix/eval): propagate catchables through `&&`Adam Joseph3-0/+4
Change-Id: I7bb5ac1ef47b41c47269e64cee0e90eb64c6fcce Reviewed-on: https://cl.tvl.fyi/c/depot/+/10322 Autosubmit: Adam Joseph <adam@westernsemico.com> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2023-12-12 r/7191 fix(tvix/eval): make `||` propagate catchablesAdam Joseph3-0/+4
Change-Id: I42f994d7c9228368d5f6c30c4730c24666f7bc69 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10320 Autosubmit: Adam Joseph <adam@westernsemico.com> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2023-12-12 r/7190 fix(tvix/eval): fix nested assertions b/340Adam Joseph3-0/+2
This commit fixes our handling of `throw` within an `assert` condition. Fixes: b/340 Change-Id: I40a383639ec266da50a853f16216b1b7868495da Reviewed-on: https://cl.tvl.fyi/c/depot/+/10318 Autosubmit: Adam Joseph <adam@westernsemico.com> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2023-12-12 r/7189 fix(tvix/eval): `?`: propagate catchablesAdam Joseph3-8/+18
This commit fixes out `?` operator so it correctly propagates catchables. Change-Id: Iebaa153a8492101ee3ddd29893c98730ff331547 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10317 Autosubmit: Adam Joseph <adam@westernsemico.com> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2023-12-12 r/7188 feat(tvix/eval): OpAttrsSelect should propagate catchablesAdam Joseph1-11/+19
Previously, using a catchable as either argument of OpAttrsSelect would result in an unrecoverable error. This commit matches cppnix behavior by propagating the catchable. Change-Id: I4877f4068ec2b823225f185290693c101d0b9c9e Reviewed-on: https://cl.tvl.fyi/c/depot/+/10303 Tested-by: BuildkiteCI Autosubmit: Adam Joseph <adam@westernsemico.com> Reviewed-by: tazjin <tazjin@tvl.su>
2023-12-12 r/7187 fix(tvix/eval): calling a catchable is catchableAdam Joseph3-0/+8
When attempting to call a Value, if it is a Value::Catchable we must not cause an uncatchable failure. This commit simply reuses the Value::Catchable as the result of attempting to call it. This is safe because nix is designed so that nix code cannot distinguish between different catchable failures -- they all look the same to the interpreted code. This fixes b/351. Change-Id: Ibf763a08753e541843626182ff59fdbf15ea2959 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10300 Autosubmit: Adam Joseph <adam@westernsemico.com> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2023-12-12 r/7186 fix(tvix/eval): fix catchables in named formalsAdam Joseph3-11/+27
Fixes b/348. Change-Id: I5e8d56b5fd26a19eac32ec5e11baf93765691dc8 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10296 Autosubmit: Adam Joseph <adam@westernsemico.com> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2023-12-12 r/7185 test(tvix/eval): test catchables in named formalsAdam Joseph2-0/+2
Relates to b/348. $ /git/depot/result/bin/tvix -E '(builtins.tryEval (({ fred }: "bob") (throw "3"))).success' note: while evaluating this Nix code --> [code]:1:1 | 1 | (builtins.tryEval (({ fred }: "bob") (throw "3"))).success | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ note: while evaluating this as native code (force) --> [code]:1:1 | 1 | (builtins.tryEval (({ fred }: "bob") (throw "3"))).success | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ note: while evaluating this as native code (tryEval) --> [code]:1:2 | 1 | (builtins.tryEval (({ fred }: "bob") (throw "3"))).success | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ note: while evaluating this Nix code --> [code]:1:20 | 1 | (builtins.tryEval (({ fred }: "bob") (throw "3"))).success | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error[E006]: expected value of type 'set', but found a 'internal[catchable]' --> [code]:1:21 | 1 | (builtins.tryEval (({ fred }: "bob") (throw "3"))).success | ^^^^^^^^ Change-Id: I730fdd996f7e1b81dbbf83dc1524104a8cad2f78 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10295 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Autosubmit: Adam Joseph <adam@westernsemico.com>
2023-12-12 r/7184 fix(tvix/eval): fix testing catchables for inequalityAdam Joseph3-2/+7
Fixes b/347. Change-Id: Icad0251884d4d8adcdf8d690b91385bf4896f9c8 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10294 Tested-by: BuildkiteCI Autosubmit: Adam Joseph <adam@westernsemico.com> Reviewed-by: tazjin <tazjin@tvl.su>
2023-12-12 r/7183 test(tvix/eval): testing catchable for inequalityAdam Joseph2-0/+2
Relates to b/347. $ /git/depot/result/bin/tvix -E '(builtins.tryEval (throw "bob" != 3)).success' note: while evaluating this Nix code --> [code]:1:1 | 1 | (builtins.tryEval (throw "bob" != 3)).success | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ note: while evaluating this as native code (force) --> [code]:1:1 | 1 | (builtins.tryEval (throw "bob" != 3)).success | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ note: while evaluating this as native code (tryEval) --> [code]:1:2 | 1 | (builtins.tryEval (throw "bob" != 3)).success | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error[E006]: expected value of type 'bool', but found a 'internal[catchable]' --> [code]:1:20 | 1 | (builtins.tryEval (throw "bob" != 3)).success | ^^^^^^^^^^^^^^^^ Change-Id: Ide19b3ddaf314ef310efc2fe5ac36667e43011dc Reviewed-on: https://cl.tvl.fyi/c/depot/+/10293 Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: Adam Joseph <adam@westernsemico.com> Tested-by: BuildkiteCI
2023-12-12 r/7182 fix(tvix/eval): handle catchables in attribute set updatesAdam Joseph3-4/+11
Fixes b/346. Change-Id: I277121d2363e605ebe09651ed9440fe1bc126c8c Reviewed-on: https://cl.tvl.fyi/c/depot/+/10292 Tested-by: BuildkiteCI Autosubmit: Adam Joseph <adam@westernsemico.com> Reviewed-by: tazjin <tazjin@tvl.su>
2023-12-12 r/7181 test(tvix/eval): test for catchable in attribute mergesAdam Joseph2-0/+2
Relates to b/346. $ /git/depot/result/bin/tvix -E '(builtins.tryEval (throw "bob" // { })).success' note: while evaluating this Nix code --> [code]:1:1 | 1 | (builtins.tryEval (throw "bob" // { })).success | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ note: while evaluating this as native code (force) --> [code]:1:1 | 1 | (builtins.tryEval (throw "bob" // { })).success | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ note: while evaluating this as native code (tryEval) --> [code]:1:2 | 1 | (builtins.tryEval (throw "bob" // { })).success | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error[E006]: expected value of type 'set', but found a 'internal[catchable]' --> [code]:1:20 | 1 | (builtins.tryEval (throw "bob" // { })).success | ^^^^^^^^^^^^^^^^^^ Change-Id: Ib84c4ec6d2612d4f1f6066e66c3dd1bf04369b6e Reviewed-on: https://cl.tvl.fyi/c/depot/+/10291 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Autosubmit: Adam Joseph <adam@westernsemico.com>
2023-12-12 r/7180 fix(tvix/eval): fix recovering from throws in implicationsAdam Joseph3-0/+2
This fixes b/345. Change-Id: Ic0d3b6ffacd2a5e0050d22354d08320b69a4fe13 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10290 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: Adam Joseph <adam@westernsemico.com>
2023-12-12 r/7179 test(tvix/eval): test recovering from throw in implicationsAdam Joseph2-0/+2
error[E006]: expected value of type 'bool', but found a 'internal[catchable]' --> src/tests/tvix_tests/notyetpassing/eval-okay-test-catchables-in-implications.nix:1:43 | 1 | (builtins.tryEval (({ foo ? throw "up" }: foo -> true) { })).success | ^^^^^^^^^^^ Relates to b/345 Change-Id: Ic331c32ea59bf67ae775f485b444dc6804ca13d5 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10289 Tested-by: BuildkiteCI Autosubmit: Adam Joseph <adam@westernsemico.com> Reviewed-by: tazjin <tazjin@tvl.su>
2023-12-12 r/7178 fix(tvix/eval): fix branching on catchable defaults (b/343)Adam Joseph6-0/+20
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/7176 feat(tvix/eval): nonrecursive coerce_to_string()Adam Joseph6-121/+138
After this commit, the only non-builtins uses of generators are: - coerce_to_string() uses generators::request_enter_lambda() - Thunk::force() uses generators::request_enter_lambda() That's it! Once those two are taken care of, GenCo can become an implementation detail of `builtins::BuiltinGen`. No more crazy nonlocal flow control within the interpreter: if you've got a GenCo floating around in your code it's because you're writing a builtin, which isn't part of the core interpreter. The interpreter won't need GenCos to talk to itself anymore. Technically generators::request_path_import() is also used by coerce_to_string(), but that's just because the io_handle happens to be part of the VM. There's no recursion-depth issue there, so the call doesn't need to go through the generator mechanism (request_path_import() doesn't call back to the interpreter!) Change-Id: I83ce5774d49b88fdafdd61160975b4937a435bb0 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10256 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Autosubmit: Adam Joseph <adam@westernsemico.com>
2023-12-12 r/7175 feat(tvix/eval): nonrecursive deep_force()Adam Joseph5-80/+82
This commit implements deep_force() nonrecursively, by maintaining an explicit stack rather than using the call stack for recursion. As an added bonus, we don't need to pass around the SharedThunkSet anymore, and can in fact completely eliminate SharedThunkSet. Change-Id: I7c4f59f37834d451a28bf6be317eb0a90eac4ee6 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10252 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: Adam Joseph <adam@westernsemico.com>
2023-12-12 r/7174 docs(tvix/eval): clarify difference between ThunkSet and BlackholeAdam Joseph1-2/+5
The comment explaining ThunkSet makes it seem like it does the same think as ThunkRepr::Blackhole. In fact neither one is a substitute for the other. Let's explain the difference. Change-Id: I89ceaaa9d3c499edbc7d48f70ca5d11f97666c43 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10250 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: Adam Joseph <adam@westernsemico.com>
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/7172 feat(tvix/eval): drop LightSpan::DelayedAdam Joseph2-13/+1
LightSpan::Delayed was introduced in commit bf286a54bc2ac5eeb78c3d5c5ae66e9af24d74d4 which claimed that "This reduces the eval time for `builtins.length (builtins.attrNames (import <nixpkgs> {}))` by *one third*!" I am unable to reproduce this result. In fact, dropping the LightSpan::Delayed variant of the enum makes eval of the same expression slightly faster! I also tried a large evaluation (pkgsCross...hello) and got similar results: slightly faster, slightly less memory. See git footers. I suspect that there was some unrelated horrific inefficiency that has since been fixed. The avoided computation in `get_span()` is nothing more than a binary search! If this were in fact a major performance issue we could simply precompute the mapping from CodeIdx to Span when the Chunk becomes immutable (i.e. at the end of the compilation process, when compiler backtracking is no longer a concern). Since a Span is just 64 bits this is not a space issue, and since binary search is much simpler than compiling Nix expressions it isn't a performance issue either. Technically there is no longer any reason to have LightSpan since it is now a single-variant enum. However there is no rush to remove it, since Rust will optimize its representation into the same thing you'd get if you replaced LightSpan by Span. Prev-Benchmark: {"nixpkgs-attrnames":{"kbytes":"233824","system":"0.32","user":"2.02"}} This-Benchmark: {"nixpkgs-attrnames":{"kbytes":"230192","system":"0.29","user":"2.00"}} Prev-Benchmark: {"pkgsCross.aarch64-multiplatform.hello.outPath":{"kbytes":"458936","system":"0.73","user":"5.36"}} This-Benchmark: {"pkgsCross.aarch64-multiplatform.hello.outPath":{"kbytes":"451808","system":"0.53","user":"5.10"}} Change-Id: Ib9e04806850aa1fc4e66e2a042703986440a7b4e Reviewed-on: https://cl.tvl.fyi/c/depot/+/10254 Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: Adam Joseph <adam@westernsemico.com> Tested-by: BuildkiteCI
2023-12-12 r/7171 feat(tvix/cli): add benchmark for bf286a54bcAdam Joseph1-0/+2
cl/7558 used this expression as a benchmark to justify the introduction of LightSpan::Delayed: builtins.length (builtins.attrNames (import ${pkgs.path} {})) Let's add it as a benchmark case so it can be referenced easily. Benchmark: {"nixpkgs-attrnames":{"kbytes":"233824","system":"0.32","user":"2.02"}} Change-Id: Idb6c69ddd284605dd3b5fd9ac5c79a69b9a470b7 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10253 Autosubmit: Adam Joseph <adam@westernsemico.com> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2023-12-12 r/7170 refactor(tvix): use io::Result for EvalIOFlorian Klink3-22/+22
This is just a alias for Result<_, io::Error>, but shorter. Change-Id: I7c22f61b85e3014885a747b5c1e5abd11b0ef17d Reviewed-on: https://cl.tvl.fyi/c/depot/+/10327 Tested-by: BuildkiteCI Autosubmit: flokli <flokli@flokli.de> Reviewed-by: raitobezarius <tvl@lahfa.xyz>
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>
generated by cgit-pink 1.4.1 (git 2.44.2) at 2024-11-03T19·10+0000