about summary refs log tree commit diff
path: root/tvix/eval/src
AgeCommit message (Collapse)AuthorFilesLines
2024-11-04 r/8889 test(tvix/eval): test throw in __toString with toJSONBob van der Linden2-1/+4
Change-Id: Ia4a9a04c7e157b6add94dc8901ffab35486fe344 Reviewed-on: https://cl.tvl.fyi/c/depot/+/12731 Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de>
2024-11-02 r/8881 feat(tvix/eval): introduce ErrorKind::InvalidHashBob van der Linden1-0/+5
The nixhash errors were wrapped in a generic TvixError. Now it has its own TvixError with unique error code. The nixhash error is passed along as a string. The errors looked like: error[E997]: invalid encoded digest length '51' for algo sha256 Now they look like: error[E041]: Invalid hash: invalid encoded digest length '51' for algo sha256 Change-Id: I5c420815538ba4c6567c95f5d44d60c4d48f43fd Reviewed-on: https://cl.tvl.fyi/c/depot/+/12718 Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
2024-10-27 r/8869 fix(tvix/eval): fix unused variable warnings being invertedYureka1-1/+1
Thanks to lexi for finding this Change-Id: Ic248af55426630b5e07183e4eac1596d52954478 Reviewed-on: https://cl.tvl.fyi/c/depot/+/12696 Tested-by: BuildkiteCI Autosubmit: yuka <yuka@yuka.dev> Reviewed-by: tazjin <tazjin@tvl.su>
2024-10-16 r/8813 feat(tvix/eval/io): impl From<std::fs::FileType> for FileTypeFlorian Klink1-0/+14
Change-Id: If92ddaf3b469c4635c234b193f8d7716e11887f6 Reviewed-on: https://cl.tvl.fyi/c/depot/+/12630 Tested-by: BuildkiteCI Reviewed-by: Ilan Joselevich <personal@ilanjoselevich.com> Autosubmit: flokli <flokli@flokli.de>
2024-10-12 r/8797 refactor(tvix/eval): remove usage of lazy_staticVincent Ambo3-32/+26
Equivalent logic is now in the standard library, and this dependency is no longer needed for eval. Change-Id: Iaa4410d89fdaa5b84cbd9e6bc6ae479c659d92f2 Reviewed-on: https://cl.tvl.fyi/c/depot/+/12602 Autosubmit: tazjin <tazjin@tvl.su> Reviewed-by: edef <edef@edef.eu> Tested-by: BuildkiteCI
2024-10-12 r/8796 refactor(tvix/eval): Make `strict` an EvalMode enumAspen Smith3-22/+33
Refactor the `strict` boolean passed into evaluation at the top-level to be a (two-variant, so far) EvalMode enum of Lazy and Strict. This is more explicit than a boolean, and if we ever add more EvalModes it's a simple extension of the enum. Change-Id: I3de50e74ec971011664f6cd0999d08b792118410 Reviewed-on: https://cl.tvl.fyi/c/depot/+/12186 Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de> Autosubmit: aspen <root@gws.fyi>
2024-09-25 r/8713 feat(tvix/eval): Use thiserror for ErrorKind and CatchableErrorKindIlan Joselevich1-242/+94
thiserror is much more easier to maintain than manually implementing Error and Display. Change-Id: Ibf13e2d8a96fba69c8acb362b7515274a593dfd6 Reviewed-on: https://cl.tvl.fyi/c/depot/+/12452 Reviewed-by: flokli <flokli@flokli.de> Autosubmit: Ilan Joselevich <personal@ilanjoselevich.com> Tested-by: BuildkiteCI
2024-09-05 r/8653 chore(3p/sources): bump channels & overlays (2024-09-01)Vincent Ambo1-2/+2
Included changes: * users/aspen: explicitly use open-source nvidia driver This now has to be specified explicitly, otherwise evaluation fails with an error. * users/aspen: nixfmt -> nixfmt-classic * users/aspen: fixes for renamed packages & options * users/tazjin: fixes for renamed packages & options * 3p/overlays: remove cbtemulator patch (merged upstream) * tvix/shell: remove unnecessary patches (merged upstream) * 3p/rust-crates: mark libgit2_sys as broken * users/Profpatsch: mark git-db as broken * 3p/overlays: pick `mypaint` from stable channel * tvix: fix comments that clippy doesn't like anymore * tvix/glue: disable a misfiring clippy lint (applying its suggestion breaks code below) Change-Id: I6d3fc027694bbe7425a2d25dc53d65467a44f3b0 Reviewed-on: https://cl.tvl.fyi/c/depot/+/12403 Tested-by: BuildkiteCI Reviewed-by: aspen <root@gws.fyi> Reviewed-by: Profpatsch <mail@profpatsch.de> Autosubmit: tazjin <tazjin@tvl.su>
2024-08-28 r/8605 feat(tvix/eval): allow external users to construct native thunksVincent Ambo1-0/+6
This interface is the only way to construct lazy builtins outside of eval, which is actually a useful feature to have. Change-Id: I386323af20aa3134bb4f669fa66fbb21e9b05fd4 Reviewed-on: https://cl.tvl.fyi/c/depot/+/12386 Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI Autosubmit: tazjin <tazjin@tvl.su> Reviewed-by: Yury Shvedov <yury.shvedov@kaspersky.com>
2024-08-28 r/8604 refactor(tvix/eval): remove useless impl blockVincent Ambo1-8/+5
There's no need to duplicate this. Change-Id: If3d930211a1d625d6c7ef129b05034e7a915da83 Reviewed-on: https://cl.tvl.fyi/c/depot/+/12385 Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI Autosubmit: tazjin <tazjin@tvl.su> Reviewed-by: Yury Shvedov <yury.shvedov@kaspersky.com>
2024-08-19 r/8521 refactor(tvix/eval): remove use of imbl::OrdMapVincent Ambo4-113/+91
Removes imbl::OrdMap in favour of an Rc over the standard library's BTreeMap, which allows us to drop the imbl dependency completely. In my local tests this is actually slightly faster for `hello` and `firefox`. Change-Id: Ic9597ead4e98bf9530f290c6a94a3c5c3efd0acc Reviewed-on: https://cl.tvl.fyi/c/depot/+/12201 Reviewed-by: aspen <root@gws.fyi> Tested-by: BuildkiteCI
2024-08-19 r/8520 refactor(tvix/eval): remove use of imbl::VectorVincent Ambo4-47/+46
This vector type has served us well for now, but it contains internal refcounts which are incompatible with upcoming changes related to garbage collection. The performance impact of this change within all benchmarks I ran was within the margin of error: [nix-shell:/tmp/perf]$ hyperfine "./before -E '(import <nixpkgs> {}).firefox.outPath' --log-level ERROR --no-warnings" Benchmark 1: ./u64 -E '(import <nixpkgs> {}).firefox.outPath' --log-level ERROR --no-warnings Time (mean ± σ): 7.528 s ± 0.272 s [User: 6.578 s, System: 0.631 s] Range (min … max): 7.160 s … 8.012 s 10 runs nix-shell:/tmp/perf]$ hyperfine "./std-vec -E '(import <nixpkgs> {}).firefox.outPath' --log-level ERROR --no-warnings" Benchmark 1: ./std-vec -E '(import <nixpkgs> {}).firefox.outPath' --log-level ERROR --no-warnings Time (mean ± σ): 7.515 s ± 0.178 s [User: 6.508 s, System: 0.652 s] Range (min … max): 7.276 s … 7.861 s 10 runs Change-Id: Ib95f871956e336a1e5771f6293583854b1efb276 Reviewed-on: https://cl.tvl.fyi/c/depot/+/12197 Reviewed-by: aspen <root@gws.fyi> Tested-by: BuildkiteCI
2024-08-19 r/8519 refactor(tvix/eval): ensure VM operations fit in a single byteVincent Ambo9-572/+748
This replaces the OpCode enum with a new Op enum which is guaranteed to fit in a single byte. Instead of carrying enum variants with data, every variant that has runtime data encodes it into the `Vec<u8>` that a `Chunk` now carries. This has several advantages: * Less stack space is required at runtime, and fewer allocations are required while compiling. * The OpCode doesn't need to carry "weird" special-cased data variants anymore. * It is faster (albeit, not by much). On my laptop, results consistently look approximately like this: Benchmark 1: ./before -E '(import <nixpkgs> {}).firefox.outPath' --log-level ERROR --no-warnings Time (mean ± σ): 8.224 s ± 0.272 s [User: 7.149 s, System: 0.688 s] Range (min … max): 7.759 s … 8.583 s 10 runs Benchmark 2: ./after -E '(import <nixpkgs> {}).firefox.outPath' --log-level ERROR --no-warnings Time (mean ± σ): 8.000 s ± 0.198 s [User: 7.036 s, System: 0.633 s] Range (min … max): 7.718 s … 8.334 s 10 runs See notes below for why the performance impact might be less than expected. * It is faster while at the same time dropping some optimisations we previously performed. This has several disadvantages: * The code is closer to how one would write it in C or Go. * Bit shifting! * There is (for now) slightly more code than before. On performance I have the following thoughts at the moment: In order to prepare for adding GC, there's a couple of places in Tvix where I'd like to fence off certain kinds of complexity (such as mutating bytecode, which, for various reaons, also has to be part of data that is subject to GC). With this change, we can drop optimisations like retroactively modifying existing bytecode and *still* achieve better performance than before. I believe that this is currently worth it to pave the way for changes that are more significant for performance. In general this also opens other avenues of optimisation: For example, we can profile which argument sizes actually exist and remove the copy overhead of varint decoding (which does show up in profiles) by using more adequately sized types for, e.g., constant indices. Known regressions: * Op::Constant is no longer printing its values in disassembly (this can be fixed, I just didn't get around to it, will do separately). Change-Id: Id9b3a4254623a45de03069dbdb70b8349e976743 Reviewed-on: https://cl.tvl.fyi/c/depot/+/12191 Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de>
2024-08-18 r/8515 refactor(tvix/eval): Pull context out into its own moduleAspen Smith2-156/+165
I'm gonna be doing some poking around in the internals of Context, so in preparation this pulls it out into its own module. Change-Id: I72ea7df80b5f36f838934ee07bdba66874c334c9 Reviewed-on: https://cl.tvl.fyi/c/depot/+/12189 Autosubmit: aspen <root@gws.fyi> Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
2024-08-11 r/8480 chore(tvix/eval): Update module comment for value::stringAspen Smith1-3/+2
This sentence is a little stale; let's just link to NixString directly for the authoritative source of truth. Change-Id: I64e065c4148d29702b09820a0e7724a65fae7c67 Reviewed-on: https://cl.tvl.fyi/c/depot/+/12181 Autosubmit: aspen <root@gws.fyi> Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de>
2024-08-11 r/8479 feat(tvix/value): Print derivation values speciallyAspen Smith1-0/+12
Just like tvix-repl does (except we don't force values when printing them, so... not entirely like tvix-repl does). But it's something. Change-Id: I2e69b08d7d82b0b2d337f1d4c5d87ed28475fa84 Reviewed-on: https://cl.tvl.fyi/c/depot/+/12180 Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI Autosubmit: aspen <root@gws.fyi>
2024-08-10 r/8475 chore(tvix/eval): Drop obsolete todoAspen Smith1-1/+0
the answer is at https://cl.tvl.fyi/c/depot/+/10798 Change-Id: I5f0ed51a3954c7241ef15a8268e0e51695e994c6 Reviewed-on: https://cl.tvl.fyi/c/depot/+/12175 Autosubmit: aspen <root@gws.fyi> Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de>
2024-08-10 r/8474 feat(tvix/eval): Store hash in key of internerAspen Smith1-7/+20
Rather than storing the leaked allocation for the string as the key in the interner, store the hash (using NoHashHashBuilder). I thought this would improve performance, but it doesn't: hello outpath time: [736.85 ms 748.42 ms 760.42 ms] change: [-2.0754% +0.4798% +2.7096%] (p = 0.72 > 0.05) No change in performance detected. but it at least doesn't *hurt* performance, and it *does* avoid an `unsafe`, so it's probably net good. Change-Id: Ie413955bdb6f04b1f468f511e5ebce56e329fa37 Reviewed-on: https://cl.tvl.fyi/c/depot/+/12049 Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de> Autosubmit: aspen <root@gws.fyi>
2024-08-09 r/8467 feat(tvix/eval): Re-enable testing for eval-okay-readDirIlan Joselevich6-2/+0
When we moved to building tvix with crate2nix we had to disable the readDir tests for the same reasons mentioned in https://cl.tvl.fyi/c/depot/+/12131. But now with the 12131 CL, we can re-enable the readDir tests. In this change I re-enabled the readDir tests in nix_tests by moving them out of notyetpassing and I also deleted them from tvix_tests because we don't need them in both places. Change-Id: I82ac39605299a8b22d80f8b51fc8ec2476d21dc9 Reviewed-on: https://cl.tvl.fyi/c/depot/+/12133 Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de> Autosubmit: Ilan Joselevich <personal@ilanjoselevich.com>
2024-08-09 r/8466 feat(tvix/eval): Implement builtins.readFileTypeIlan Joselevich9-2/+46
builtins.readFileType was added to Nix back in version 2.14. The tests were also moved out of notyetpassing in addition to the readDir fixtures they depend on. I caught a bug where we previously used std::fs::metadata (via the .metadata() method on File) which follows symlinks so it would always return false for is_symlink(). Instead we now use std::fs::symlink_metadata directly which does not follow symlinks, so tests now pass. This wasn't an issue for builtins.readDir as it uses walkdir and walkdir doesn't follow symlinks either. Change-Id: I58eb97bdb5ec95df4f6882f495f8c572fe7c6793 Reviewed-on: https://cl.tvl.fyi/c/depot/+/12130 Reviewed-by: flokli <flokli@flokli.de> Autosubmit: Ilan Joselevich <personal@ilanjoselevich.com> Tested-by: BuildkiteCI
2024-08-09 r/8463 feat(tvix/eval): Implement Display for io::FileTypeIlan Joselevich2-7/+14
In newer versions of Nix there's a builtins.readFileType builtin, we should try to avoid duplicating the enum -> string conversion by implementating Display before we implement builtins.readFileType. Change-Id: I579e95949a76eb33d2e7bda0000ed84859df765e Reviewed-on: https://cl.tvl.fyi/c/depot/+/12129 Reviewed-by: flokli <flokli@flokli.de> Autosubmit: Ilan Joselevich <personal@ilanjoselevich.com> Tested-by: BuildkiteCI
2024-08-08 r/8455 feat(tvix/eval): Put interner in a thread-local RefCellAspen Smith1-9/+8
Rather than making the interner be a global lazy_static mutex, put it in a thread-local RefCell. This doesn't change anything in terms of sharing (since we're currently actually just single threaded), but avoids the overhead of a mutex, for a nice performance boost (compared to the mutex version): hello outpath time: [726.71 ms 729.79 ms 735.69 ms] change: [-5.7277% -3.9733% -2.1144%] (p = 0.00 < 0.05) Performance has improved. Change-Id: I240b238dcbaf854ebafc3017b4425fb7d7b91b03 Reviewed-on: https://cl.tvl.fyi/c/depot/+/12048 Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de> Autosubmit: aspen <root@gws.fyi>
2024-08-08 r/8454 feat(tvix/eval): Intern (and leak) small strings, behind a mutexAspen Smith1-5/+82
This is the most naive version of string interning possible - we store a map from the string itself to the pointer behind a global mutex, and memoize the allocation of all strings below a threshold length (16 bytes, for now) into that map. This requires leaking /all/ strings, since it's not easy to know just from the pointer that a string has been interned - so interning is disabled if string leaking is also disabled. In the case where we're leaking strings (the default), even the naive version of this gets us a pretty nice perfomance boost: hello outpath time: [742.54 ms 745.89 ms 749.14 ms] change: [-2.8722% -2.0135% -1.0654%] (p = 0.00 < 0.05) Performance has improved. However, in the case where we're not leaking strings, we have to keep track of which strings have and haven't been interned, which makes this a little worse: hello outpath time: [779.30 ms 792.82 ms 808.74 ms] change: [+2.5258% +4.0884% +5.8931%] (p = 0.00 < 0.05) Performance has regressed. Hopefully we can close the gap here a bit with some clever tricks (coming next). Change-Id: If08cb48ede703c7fe3bdd8d617443f8a561ad09b Reviewed-on: https://cl.tvl.fyi/c/depot/+/12047 Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de> Autosubmit: aspen <root@gws.fyi>
2024-08-07 r/8453 feat(tvix/eval): Forbid Hash{Map,Set}, use Fx insteadAspen Smith8-30/+37
Per https://nnethercote.github.io/perf-book/hashing.html, we have basically no reason to use the default hasher over a faster, non-DoS-resistant hasher. This gives a nice perf boost basically for free: hello outpath time: [704.76 ms 714.91 ms 725.63 ms] change: [-7.2391% -6.1018% -4.9189%] (p = 0.00 < 0.05) Performance has improved. Change-Id: If5587f444ed3af69f8af4eead6af3ea303b4ae68 Reviewed-on: https://cl.tvl.fyi/c/depot/+/12046 Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de> Reviewed-by: Ilan Joselevich <personal@ilanjoselevich.com> Autosubmit: aspen <root@gws.fyi>
2024-08-05 r/8443 feat(tvix/eval): Leak strings (with flag to disable)Aspen Smith1-3/+15
Default to always leaking strings, and copying strings by copying pointers rather than cloning the underlying allocation. This (somewhat bafflingly) doesn't seem to affect any benchmarks, but paves the way for some tricks around string allocation that do. Unfortunately, we can't do this (yet?) for contextful strings, for reasons I don't currently understand but which I will address later, when I address contextful strings more holistically. I've left a flag in here to disable this, both to test the cloning logic for strings for when/if we decide to bring this back, and to allow people who care more about memory usage than perf to disable leaking. Change-Id: Iec44bcbfe9b3d20389d2450b9a551792a79b9b26 Reviewed-on: https://cl.tvl.fyi/c/depot/+/12045 Autosubmit: aspen <root@gws.fyi> Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de>
2024-08-01 r/8435 chore(3p/sources): Bump channels & overlays (2024-07-28)Aspen Smith2-8/+6
* Treewide: re-run depotfmt * //third_party/nixpkgs:html5validator: build with Python 3.11, dependency openstackdocstheme doesn't support 3.12 * //users/sterni/machines/ingeborg: adapt to poorly handled fcgiwrap module API change: https://github.com/NixOS/nixpkgs/pull/318599 * //tvix/*-go: regenerate protobuf files * //third_party/nixpkgs:treefmt: Remove patch for merged pull request * //users/flokli/ipu6-softisp: rebase, drop upstreamed kernel patches Change-Id: Ie4e0df007c287e8cd6207683a9a25838aa5bd39a Reviewed-on: https://cl.tvl.fyi/c/depot/+/11971 Autosubmit: sterni <sternenseemann@systemli.org> Reviewed-by: sterni <sternenseemann@systemli.org> Reviewed-by: flokli <flokli@flokli.de> Reviewed-by: aspen <root@gws.fyi> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su> Reviewed-by: Ilan Joselevich <personal@ilanjoselevich.com>
2024-07-27 r/8418 feat(tvix/eval): ConstantIdx expansion for more opsMatthew Tromp1-7/+19
Previously, OpConstant would display some detail about its ConstantIdx: whether it's a thunk or closure, and what its address is. This has been expanded to also show when the ConstantIdx is a blueprint, along with the blueprint's address, and to the other opcodes that use a ConstantIdx. Currently, it seems like blueprint addresses don't correspond to the address of the thunk listed in the bytecode output, but it's still useful to see that the constant being grabbed is a blueprint, and maybe this pointer can be made to make more sense in the future. Change-Id: Ia212b0d52b004c87051542c093274e7106ee08e4 Reviewed-on: https://cl.tvl.fyi/c/depot/+/12044 Tested-by: BuildkiteCI Reviewed-by: aspen <root@gws.fyi> Autosubmit: chickadee <matthewktromp@gmail.com>
2024-07-27 r/8416 fix(tvix/eval): don't bubble up io errors from path_existsMatthew Tromp3-1/+4
path_exists was returning an error when certain common IO errors were encountered. e.g. in the path "/dev/null/.", path_exists would throw an error because the underlying call to Path::try_exists threw an error because null isn't a directory. But if null isn't a directory, then the path is invalid, so this should really be returning false. That's what nix's behavior is and that's what makes sense. The trait function isn't being changed because some other implementers (e.g. tvix_store_io) have actual errors they can throw. Fixes: b/411 Change-Id: I9e810e7a198bffe61365697c6d3d7e71f264c20b Reviewed-on: https://cl.tvl.fyi/c/depot/+/12042 Tested-by: BuildkiteCI Autosubmit: chickadee <matthewktromp@gmail.com> Reviewed-by: aspen <root@gws.fyi>
2024-07-07 r/8355 fix(tvix/repl): Share globals and sourcemap across evaluationsAspen Smith2-38/+115
Now that we can bind (potentially lazy, potentially lambda-containing) values in the REPL and then reference them in subsequent evaluations, it's important that the values to which we construct shared references are shared across those subsequent evaluations - otherwise, we get panics due to unknown source map locations, or dropped weak references to globals. This change assigns both the globals and the source map as fields on the Repl after the first evaluation, and then passes those in (to the EvaluationBuilder) on subsequent evaluations. On the EvaluationBuilder side, there's some panicking introduced - this is intentional, as my intent is for the builder to be configured statically enough that panicking is the best way to report errors here (it's always a bug to misconfigure an Evaluation, and we'd never want to handle it dynamically). Change-Id: I37225697235c22b683ca48a17d30fa8fedd12d1b Reviewed-on: https://cl.tvl.fyi/c/depot/+/11960 Reviewed-by: flokli <flokli@flokli.de> Autosubmit: aspen <root@gws.fyi> Tested-by: BuildkiteCI
2024-07-06 r/8352 refactor(tvix/eval): Construct globals in EvaluationBuilder::buildAspen Smith2-58/+47
Construct the Rc<GlobalsMap> for the evaluation as part of EvaluiationBuilder::build, rather than deferring it until we actually compile. This changes nothing functionally, but gets us one step closer to sharing this globals map across evaluations. Change-Id: Id92e9fb88d974d763056d4f15ce61962ab776e84 Reviewed-on: https://cl.tvl.fyi/c/depot/+/11957 Tested-by: BuildkiteCI Autosubmit: aspen <root@gws.fyi> Reviewed-by: flokli <flokli@flokli.de>
2024-07-06 r/8351 refactor(tvix/eval): Builderize EvaluationAspen Smith3-65/+216
Make constructing of a new Evaluation use the builder pattern rather than setting public mutable fields. This is currently a pure refactor (no functionality has changed) but has a few advantages: - We've encapsulated the internals of the fields in Evaluation, meaning we can change them without too much breakage of clients - We have type safety that prevents us from ever changing the fields of an Evaluation after it's built (which matters more in a world where we reuse Evaluations). More importantly, this paves the road for doing different things with the construction of an Evaluation - notably, sharing certain things like the GlobalsMap across subsequent evaluations in eg the REPL. Fixes: b/262 Change-Id: I4a27116faac14cdd144fc7c992d14ae095a1aca4 Reviewed-on: https://cl.tvl.fyi/c/depot/+/11956 Tested-by: BuildkiteCI Autosubmit: aspen <root@gws.fyi> Reviewed-by: flokli <flokli@flokli.de>
2024-07-05 r/8345 feat(tvix/eval): Allow passing in an env to evaluationAspen Smith7-15/+99
Allow passing in a top-level env, a map from name to value, to evaluation. The intent is to support bound identifiers in the REPL just like upstream nix does. Getting this working involves mucking around a bit with internals - most notably, locals now only optionally have a Span (since locals don't have an easy span we can use) - and getting that working requires propagating some minor hacks to places where we currently *need* a span (and which would require too much changing now to make spans optional; my guess is that that would essentially end up making spans optional throughout the codebase). Also, some extra care has to be taken to close out the scope in the case that we do pass in an env, to avoid breaking our assumptions about the size of the stack when we return from the toplevel Change-Id: Ie475b2d3dfc72ccbf298d2a3ea28c63ac877d653 Reviewed-on: https://cl.tvl.fyi/c/depot/+/11953 Tested-by: BuildkiteCI Autosubmit: aspen <root@gws.fyi> Reviewed-by: flokli <flokli@flokli.de>
2024-07-05 r/8343 refactor(tvix/eval): Drop LightSpan entirelyAspen Smith8-175/+109
This was made unnecessary in c92d06271 (feat(tvix/eval): drop LightSpan::Delayed, 2023-12-08) because it didn't improve benchmarks as much as expected and has been vestigial since; this continues the cleanup by just removing it altogether Change-Id: I21ec7ae9b52a5cccd2092696a5a87f658194d672 Reviewed-on: https://cl.tvl.fyi/c/depot/+/11949 Autosubmit: aspen <root@gws.fyi> Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de>
2024-06-26 r/8314 refactor(tvix/eval): prefix ctx iterators in NixStringFlorian Klink1-3/+15
NixString::iter_plain() didn't make much sense, especially without a docstring. Rename it to iter_ctx_plain(), and copy the docstring from NixContext. Do the same for the two other context element types too. Change-Id: I1bbfcb967d8d9b14487d069bfe3a1f762253ef4d Reviewed-on: https://cl.tvl.fyi/c/depot/+/11882 Tested-by: BuildkiteCI Reviewed-by: Ilan Joselevich <personal@ilanjoselevich.com> Reviewed-by: flokli <flokli@flokli.de>
2024-06-26 r/8309 feat(tvix/eval): add ErrorKind::UnexpectedArgumentBuiltinFlorian Klink1-0/+9
Change-Id: Ieb091b32aad566719fbe8604c4a589f5ccaaf6b3 Reviewed-on: https://cl.tvl.fyi/c/depot/+/11877 Tested-by: BuildkiteCI Reviewed-by: Connor Brewster <cbrewster@hey.com>
2024-06-26 r/8308 refactor(tvix/eval): rename UnexpectedArgument{,Formals}Florian Klink2-6/+6
There's other places where unexpected arguments can be provided, like in builtins. Make space for that error type. Change-Id: Ic831497a3a1dd36a3a184bedadcf1374bf0ae6db Reviewed-on: https://cl.tvl.fyi/c/depot/+/11876 Reviewed-by: Connor Brewster <cbrewster@hey.com> Tested-by: BuildkiteCI
2024-06-26 r/8304 feat(tvix/eval): add file_type methodFlorian Klink1-0/+25
This allows peeking of the type at a given path. It's necessary, as an open() might not fail until you try to read() from it, and generally, stat'ing can be faster in some cases. Change-Id: Ib002da3194a3546ca286de49aac8d1022ec5560f Reviewed-on: https://cl.tvl.fyi/c/depot/+/11871 Tested-by: BuildkiteCI Reviewed-by: Ilan Joselevich <personal@ilanjoselevich.com> Reviewed-by: Connor Brewster <cbrewster@hey.com>
2024-06-22 r/8303 refactor(tvix/eval): drop placeholder impls of builtins in glueIlan Joselevich1-16/+0
The builtins `placeholder` and `filterSource` are now implemented in tvix-glue and so we no longer need their placeholder "implementations" in tvix-eval. Change-Id: Ibf161ccf1ad40103e9fbb688e9f6be58e7772af1 Reviewed-on: https://cl.tvl.fyi/c/depot/+/11867 Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de> Autosubmit: Ilan Joselevich <personal@ilanjoselevich.com>
2024-06-11 r/8247 fix(tvix/eval): handle builtins.split matching the empty stringbinarycat3-0/+14
This prevents the following statements from looping endlessly: ``` builtins.split "(.*)" "" builtins.split "([abc]*)" "abc" builtins.split "(.*)" "abc" builtins.split ".*" "" ``` Cover these (and some more examples) in the test suite. Co-Authored-By: Florian Klink <flokli@flokli.de> Change-Id: Ibd339f971e0f4e3e5c229816e2be5a8e3836fec9 Reviewed-on: https://cl.tvl.fyi/c/depot/+/11743 Autosubmit: flokli <flokli@flokli.de> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2024-06-05 r/8217 feat(tvix/glue): Implement builtins.storePathAspen Smith1-0/+6
This one's relatively simple - we just check if the store path exists, and if it does we make a new contextful string containing the store path as its only context element. Automatic testing seems tricky for this (I think?) so I tested it manually: tvix-repl> builtins.storePath /nix/store/yn46i4xx5alh7gs6fpkxk430i34rp2q9-hello-2.12.1 => "/nix/store/yn46i4xx5alh7gs6fpkxk430i34rp2q9-hello-2.12.1" :: string Change-Id: I8a0d9726e4102ab872c53c2419679c2c855a5a18 Reviewed-on: https://cl.tvl.fyi/c/depot/+/11696 Tested-by: BuildkiteCI Autosubmit: aspen <root@gws.fyi> Reviewed-by: flokli <flokli@flokli.de>
2024-05-30 r/8184 fix(tvix/eval): nix_tests.rs's eval_test requires impure flagIlan Joselevich1-0/+1
This fixes the following command: 'cargo test --no-default-features --features nix_tests' Change-Id: I9883c39e1e428c72a0e7e0b75a73c8ed734abd3b Reviewed-on: https://cl.tvl.fyi/c/depot/+/11740 Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
2024-05-30 r/8183 fix(tvix/eval): proptests require arbitrary featureFlorian Klink2-2/+2
`cargo test --no-default-features` fails, if we don't conditionalize this on the `arbitrary` feature too. Change-Id: I81a277810119fed0cfc37c942c422f731aa14b2e Reviewed-on: https://cl.tvl.fyi/c/depot/+/11726 Autosubmit: flokli <flokli@flokli.de> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Reviewed-by: Connor Brewster <cbrewster@hey.com>
2024-05-30 r/8182 fix(tvix/eval/tests/one_offs): test_source_builtin can be pureFlorian Klink1-1/+1
`Evaluation::new_impure()` would require the test to be impure, but there's nothing in this test specifically requiring us to make use of impure features. Change-Id: Idb24981195d1a94f51053ae04403eb5f0e27f3d9 Reviewed-on: https://cl.tvl.fyi/c/depot/+/11725 Autosubmit: flokli <flokli@flokli.de> Tested-by: BuildkiteCI Reviewed-by: Connor Brewster <cbrewster@hey.com>
2024-05-30 r/8180 refactor(tvix/eval): move nix_tests to separate moduleFlorian Klink2-198/+207
Gate this behind the nix_tests feature flag (which wasn't applied consistently). Also, at least right now all of these use StdIO internally, either by creating it on their own, or through the `eval_test` helper. Once we have some proper classification system for tests we can probably split this up further, but for now only run them if the impure feature is enabled. Change-Id: I3236cf09b3391585df99073367c4e4832c5e7898 Reviewed-on: https://cl.tvl.fyi/c/depot/+/11723 Tested-by: BuildkiteCI Autosubmit: flokli <flokli@flokli.de> Reviewed-by: Connor Brewster <cbrewster@hey.com>
2024-05-30 r/8179 fix(tvix/eval/vm/generators): allow unused rq_{path_exists,read_dir}Florian Klink1-0/+2
These VM requests are only emitted by code gated behind the impure feature. To prevent warning from popping up when building without default features, allow these to be unused if `impure` is not enabled. Change-Id: Id871a5215e9a0f09aa78edecdd111369ee7ffe34 Reviewed-on: https://cl.tvl.fyi/c/depot/+/11722 Reviewed-by: Connor Brewster <cbrewster@hey.com> Tested-by: BuildkiteCI Autosubmit: flokli <flokli@flokli.de>
2024-05-26 r/8176 fix(tvix/eval/nix_search_path): gate tests on impure featureFlorian Klink1-0/+2
These use StdIO, which is only available if the impure feature is enabled. Change-Id: I18d8e191a7eba6ba5bd59f43631973eaa796c7bb Reviewed-on: https://cl.tvl.fyi/c/depot/+/11721 Tested-by: BuildkiteCI Autosubmit: flokli <flokli@flokli.de> Reviewed-by: tazjin <tazjin@tvl.su>
2024-05-26 r/8175 fix(tvix/eval/io): OsStringExt and std::fs::File import are impure-onlyFlorian Klink1-2/+4
These are only needed when building with the impure feature enabled. This removes some warnings when building with --no-default-features. Change-Id: I3139d9133d4846aeb1b1b5f3830c0d078d047292 Reviewed-on: https://cl.tvl.fyi/c/depot/+/11720 Autosubmit: flokli <flokli@flokli.de> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2024-05-25 r/8168 fix(tvix/eval): drop superfluous string context checkedef1-4/+1
cl/11712 simultaneously introduced this check and made it unnecessary, since NixString::context should never return `Some` for empty contexts now. Change-Id: I41a655ff33910e8326cbb7d7526eb91bd19e9585 Reviewed-on: https://cl.tvl.fyi/c/depot/+/11713 Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
2024-05-25 r/8167 fix(tvix/eval): disallow empty but allocated string contextsedef1-3/+25
Both `Some(NixContext::new())` and `None` represent empty contexts, but the former trips up `NixString::has_context`, and seems likely to trip up other things. We could hide the difference in the accessors, but we don't really *want* the distinction to exist, since heap-allocating a null value is pretty much always a mistake. Change-Id: Ie84d26fb0d4b59e68354891ba13bde3bae40ab6e Reviewed-on: https://cl.tvl.fyi/c/depot/+/11712 Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de>
2024-05-23 r/8164 feat(tvix/eval): rm NixContext::join, add take_context & IntoIteratorFlorian Klink5-58/+55
In places where we want to extend context with that from another NixString, use take_context() to split it off, then call .extend(), making use of IntoIterator to avoid a bunch of clones. Change-Id: I2460141a3ed776c64c36132b2203b6a1d710b922 Reviewed-on: https://cl.tvl.fyi/c/depot/+/11705 Tested-by: BuildkiteCI Autosubmit: flokli <flokli@flokli.de> Reviewed-by: edef <edef@edef.eu>