Commit graph

1093 commits

Author SHA1 Message Date
Aspen Smith
98b89e53e2 test(tvix/eval): Add test asserting size of Value
Now that I've done a ton of things to make sure Value is small on the
stack (16 bytes, which is a perfectly reasonable size for a programming
language Value enum), add a test asserting it stays that way.

These size improvements have a measurable impact, too - here's the
`hello outpath` benchmark compared between canon (as of r/7495) and this
commit:

hello outpath time:   [990.56 ms 995.83 ms 1.0070 s]
              change: [-7.1397% -6.1302% -5.1651%] (p = 0.00 < 0.05)
              Performance has improved.

Change-Id: If99a0976eab28eb5e516fcd2f4a0e068145af23e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10799
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
Autosubmit: aspen <root@gws.fyi>
Reviewed-by: sterni <sternenseemann@systemli.org>
2024-02-13 16:49:53 +00:00
Aspen Smith
7e286aab1a feat(tvix/eval): Box Value::Catchable
This is now the only enum variant for Value that is larger than 8
bytes (it's 16 bytes), so boxing it (especially since it's not
perf-critical) allows us to get the Value size down to only 16 bytes!

Change-Id: I98598e2b762944448bef982e8ff7da6d6683c4aa
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10798
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Autosubmit: aspen <root@gws.fyi>
2024-02-13 16:49:53 +00:00
Aspen Smith
dd26177319 revert(tvix/eval): Don't double-box Path values
This reverts commit d3d41552cf.

This was well-intentioned, but now the boxed Path values are actually
the *largest* Value enum variants, at 16 bytes (because they're
fat-pointers, with a len) instead of 8 bytes like all the other values.
Having the double reference is a reasonable price to pay (it seems; more
benchmarks may end up disagreeing) for a smaller Value repr.

Change-Id: I0d3e84f646c8f5ffd0b7259c4e456637eea360f7
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10797
Tested-by: BuildkiteCI
Autosubmit: aspen <root@gws.fyi>
Reviewed-by: sterni <sternenseemann@systemli.org>
2024-02-13 16:49:53 +00:00
Aspen Smith
e3c92ac3b4 fix(tvix/eval): Replace inner NixString repr with Box<Bstr>
Storing a full BString here incurs the extra overhead of the capacity
for the inner byte-vector, which we basically never use as Nix strings
are immutable (and we don't do any mutation / sharing analysis).
Switching to a Box<BStr> cuts us from 72 bytes to 64 bytes per
string (and there are a lot of strings!)

Change-Id: I11f34c14a08fa02759f260b1c78b2a2b981714e4
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10794
Autosubmit: aspen <root@gws.fyi>
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
2024-02-13 16:49:53 +00:00
Aspen Smith
5d72d3980f refactor(tvix/eval): Box the strings in CatchableErrorKind
These strings are allocated once and never changed, so they don't need
the additional overhead of a capacity given by String - instead, we can
use Box<str> and save on 16 bytes for each of these, *and* for each
Value since this is currently the largest Value variant.

Change-Id: I3e5cb070fe6c5bf82114c92d04f6bae775663a7e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10796
Autosubmit: aspen <root@gws.fyi>
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
2024-02-10 20:35:28 +00:00
Aspen Smith
5d2ae840f1 refactor(tvix/eval): Box the inside of Value::Json
serde_json::Value is pretty large, and is contributing (albeit not
exclusively) to the large size of the Value repr. Putting it in a box
is *especially* cheap (since it's rarely used) and allows us
to (eventually) cut down on the size of Value.

Change-Id: I005a802d8527b639beb4e938e3320b11ffa1ef23
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10795
Reviewed-by: sterni <sternenseemann@systemli.org>
Autosubmit: aspen <root@gws.fyi>
Tested-by: BuildkiteCI
2024-02-10 20:34:25 +00:00
Ryan Lahfa
7b1632ec71 feat(tvix/eval): strengthen significantly catchable test suite
Adds a bunch (notably certain overlapping) tests for catchable situations.

This should cover many scenarios, argument is catchable, element in argument is catchable, function returns
catchable in the middle of the processing, etc.

Co-authored-by: Aspen Smith <root@gws.fyi>
Change-Id: Icd722cf8dbc91a24f45cd540a328711e5826f76c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10621
Reviewed-by: aspen <root@gws.fyi>
Tested-by: BuildkiteCI
2024-02-10 15:34:15 +00:00
Aspen Smith
3fb0697a71 fix(tvix/eval): Propagate catchables in NixAttrs::construct
Correctly propagate the case where the *key* of an attrset is a
Value::Catchable (eg { "${builtins.throw "c"}" = "b"; }) in
`NixAttrs::construct`, by converting the return type to
`Result<Result<Self, CatchableErrorKind>, ErrorKind>` (ugh!!) and
correctly handling that everywhere (including an `expect` in the
Deserialize impl for NixAttrs, since afaict this is impossible to hit
when deserializing from stuff like JSON).

Change-Id: Ic4bc611fbfdab27c0bd8a40759689a87c4004a17
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10786
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
2024-02-09 19:11:09 +00:00
Aspen Smith
8446cd1c8b fix(tvix/eval): Inline List.sort_by, and propagate errors
In order to correctly propagate errors in the comparator passed to
builtins.sort, we need to do all the sorting in a context where we can
short-circuit return `Value`s (because catchables are Values on the `Ok`
side of the Result , not `Err`s). Unfortunately this means we have
to *inline* the List `sort_by` implementation into the builtin_sort
function - fortunately this is the only place that was called so this is
relatively low cost. This does that, and adds the requisite `try_value!`
invocation to allow us to propagate comparator errors here.

As before, this doesn't include tests, primarily since those are coming
in the next commit.

Change-Id: I8453c3aa2cd82299eae89828e2a2bb118da4cd48
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10754
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2024-02-08 22:50:27 +00:00
Aspen Smith
ddb7bc8d18 fix(tvix): Catch errors for generator in some builtins
Nix doesn't propagate errors for the function argument to some builtins,
like genList and map:

    ❯ nix repl
    Welcome to Nix version 2.3.17. Type :? for help.

    nix-repl> (builtins.tryEval (builtins.genList (builtins.throw "a") 10)).success
    true

    nix-repl> (builtins.tryEval (builtins.map (builtins.throw "a") [ "" ])).success
    true

Note that this is untested as of this particular commit, only because a
big test suite covering all sorts of catchable error propagation issues
is coming next

Change-Id: I48c8eb390a541204b1a6d438c753fa1ca9b3877e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10753
Autosubmit: aspen <root@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2024-02-08 21:58:53 +00:00
Aspen Smith
b77a103a2a fix(tvix/eval): Propagate catchables in a few more places
Propagate catchables that we get from forcing thunks in builtins in a
few more places using the new try_value! macro

Change-Id: I95fd41a231f877ff153f4adbabd944372d4cc7eb
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10738
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
2024-02-08 19:59:21 +00:00
Aspen Smith
780b47193a refactor(tvix/eval): Generalize propagation of catchable values
Rather than explicitly checking for Value::Catchable in all builtins,
make the #[builtin] proc macro insert this for all strict arguments by
default, with support for a #[catch] attribute on the argument to
disable this behavior. That attribute hasn't actually been *used*
anywhere here, primarily because the tests pass without it, even for
those builtins which weren't previously checking for Value::Catchable -
if some time passes without this being used I might get rid of support
for it entirely.

There's also a `try_value` macro in builtins directly for the places
where builtins were eg forcing something, then explicitly propagating a
catchable value.

Change-Id: Ie22037b9d3e305e3bdb682d105fe467bd90d53e9
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10732
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2024-02-08 19:59:21 +00:00
Florian Klink
172173d744 docs(eval/docs): toJSON is done
Change-Id: I67381b19eedb651f4049e20e7b1e43641a116ae5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10739
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
2024-02-04 17:13:52 +00:00
Aspen Smith
5f0f4ea374 refactor(tvix/eval): Box Value::String
NixString is *quite* large - like 80 bytes - because of the extra
capacity value for BString and because of the context. We want to keep
Value small since we're passing it around a lot, so let's box the
NixString inside Value::String to save on some memory, and make cloning
ostensibly a little cheaper

Change-Id: I343c8b4e7f61dc3dcbbaba4382efb3b3e5bbabb2
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10729
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
2024-02-02 16:16:56 +00:00
Aspen Smith
8caa097ba8 feat(tvix/eval): Don't emit OpForce for non-thunk constants
In the compiler, skip emitting an OpForce if the last op was an
OpConstant for a non-thunk constant. This gives a small (~1% on my
machine) perf boost, eg when evaluating hello.outPath:

    ❯ hyperfine \
        "./before --no-warnings -E '(import <nixpkgs> {}).hello.outPath'" \
        "./after --no-warnings -E '(import <nixpkgs> {}).hello.outPath'"
    Benchmark 1: ./before --no-warnings -E '(import <nixpkgs> {}).hello.outPath'
      Time (mean ± σ):      1.151 s ±  0.022 s    [User: 1.003 s, System: 0.151 s]
      Range (min … max):    1.123 s …  1.184 s    10 runs

    Benchmark 2: ./after --no-warnings -E '(import <nixpkgs> {}).hello.outPath'
      Time (mean ± σ):      1.140 s ±  0.022 s    [User: 0.989 s, System: 0.152 s]
      Range (min … max):    1.115 s …  1.175 s    10 runs

    Summary
      ./after --no-warnings -E '(import <nixpkgs> {}).hello.outPath' ran
        1.01 ± 0.03 times faster than ./before --no-warnings -E '(import <nixpkgs> {}).hello.outPath'

Change-Id: I2105fd431d4bad699087907e16c789418e9a4062
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10714
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
2024-02-01 21:08:35 +00:00
Aspen Smith
d3d41552cf refactor(tvix/eval): Don't double-box Path values
PathBuf internally contains a heap pointer (an OsString), so we were in
effect double-boxing here. Removing the extra layer by making
Tvix::Value represented by a Box<Path> rather than a Box<PathBuf> saves
us an indirection, while still avoiding the extra memory overhead of the
capacity which was the reason we were boxing PathBuf in the first place.

Change-Id: I8c185b9d4646161d1921917f83e87421496a3e24
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10725
Reviewed-by: sterni <sternenseemann@systemli.org>
Autosubmit: aspen <root@gws.fyi>
Tested-by: BuildkiteCI
2024-02-01 17:25:42 +00:00
Aspen Smith
201173afac fix(tvix): Represent strings as byte arrays
C++ nix uses C-style zero-terminated char pointers to represent strings
internally - however, up to this point, tvix has used Rust `String` and
`str` for string values. Since those are required to be valid utf-8, we
haven't been able to properly represent all the string values that Nix
supports.

To fix that, this change converts the internal representation of the
NixString struct from `Box<str>` to `BString`, from the `bstr` crate -
this is a wrapper around a `Vec<u8>` with extra functions for treating
that byte vector as a "morally string-like" value, which is basically
exactly what we need.

Since this changes a pretty fundamental assumption about a pretty core
type, there are a *lot* of changes in a lot of places to make this work,
but I've tried to keep the general philosophy and intent of most of the
code in most places intact. Most notably, there's nothing that's been
done to make the derivation stuff in //tvix/glue work with non-utf8
strings everywhere, instead opting to just convert to String/str when
passing things into that - there *might* be something to be done there,
but I don't know what the rules should be and I don't want to figure
them out in this change.

To deal with OS-native paths in a way that also works in WASM for
tvixbolt, this also adds a dependency on the "os_str_bytes" crate.

Fixes: b/189
Fixes: b/337
Change-Id: I5e6eb29c62f47dd91af954f5e12bfc3d186f5526
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10200
Reviewed-by: tazjin <tazjin@tvl.su>
Reviewed-by: flokli <flokli@flokli.de>
Reviewed-by: sterni <sternenseemann@systemli.org>
Autosubmit: aspen <root@gws.fyi>
Tested-by: BuildkiteCI
2024-01-31 14:51:49 +00:00
Aspen Smith
6f9e25943f feat(tvix/eval/observer): Allow capturing timing of events
Add a new --trace-runtime-timing flag (probably a better bikeshed for
this) that enables capturing the time, relative to the last event, of
each event recorded with the tracing observer.

This probably isn't *super* useful yet, but I'd like to start here in
adding new profiling tools to the VM, specifically based on the runtime
observer

Change-Id: Id7f12077291c39bf3eef42ab6744bfba53687a65
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10713
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
2024-01-31 04:41:37 +00:00
Florian Klink
e0a1c03b24 feat(tvix/eval/tvix_tests): add some more xml tests
https: //cl.tvl.fyi/c/depot/+/10686/comment/ea582dae_574d6c3f/
Change-Id: Iac74abbf2f2e0327bc9ddf9dcc6bb43f918a1c63
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10689
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
2024-01-25 12:06:44 +00:00
Florian Klink
850e50cd3f feat(tvix/eval/tests): compare .xml outputs
In case a .exp.xml file is provided alongside the test, compare its
output with the desired state.

Also, add some function .exp.xml that were presumably moved out of the
way back to src/tests/nix_tests, as they now produce the correct XML
output.

Change-Id: Ibd8123f3e6ed7bae3a44407d2284a2b2c8ce9a28
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10687
Reviewed-by: sterni <sternenseemann@systemli.org>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
2024-01-25 11:37:35 +00:00
Florian Klink
023e372583 feat(tvix/eval): track pattern binding names
These need to be preserved at least for builtins.toXML.

Also, we incorrectly only wrote an <attrspat> in case ellipsis was true,
but that's not the case.

Change-Id: I6bff9c47c2922f878d5c43e48280cda9c9ddb692
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10686
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: aspen <root@gws.fyi>
2024-01-25 11:37:35 +00:00
Florian Klink
e1d2589163 fix(tvix/eval/value/function): use BTreeMap for function arg names
At least toXML wants to get these out in a sorted fashion.

Change-Id: I6373d7488fff7c40dc2ddeeecd03ba537c92c4af
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10685
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
2024-01-24 14:00:44 +00:00
Florian Klink
adff7be4d1 feat(tvix/eval): expose value_to_xml for test cases
It's debateable on whether the serialization code should be exposed a
bit more prominently or not.

Change-Id: Iff7a28f884b1490b12b145dfdadbedacb84fd387
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10684
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: tazjin <tazjin@tvl.su>
2024-01-24 14:00:44 +00:00
Florian Klink
8e517bc8d0 fix(tvix/eval/tests): fix eval-okay-getenv
This relies on TEST_VAR=foo being set to "foo".

Nix does this in tests/functional/lang.sh, we do it in the test suite.

Change-Id: I7ffa9ed27124530b7758aeadf07c79477656f34f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10683
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
2024-01-24 14:00:44 +00:00
Florian Klink
e8061fc619 docs(tvix/eval/tests): update comment
There's no `expected_failures` feature, we run them unconditionally.

Change-Id: Ibe1c93497e040d0d5b6cbfcaa043027814c191f1
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10681
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
2024-01-23 12:16:14 +00:00
Florian Klink
56ba7a72d8 docs(tvix/eval): builtins.toXML is done
Our docs say it's still todo, but that's wrong, it landed in cl/7835
and cl/7962.

Change-Id: Ice374ae47ab653146be18c02dce27145e7488397
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10674
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: sterni <sternenseemann@systemli.org>
2024-01-20 20:09:07 +00:00
sterni
526295a71d chore(3p/sources): Bump channels & overlays
- Adjust to ecl 23.9.9 release

- Regenerate go protos after protoc-gen-go update

- Drop dhall fork which hasn't kept up with 1.42.*

- Address new clippy warnings:

  - Variant naming of Error::ValidationError
  - Simplify .try_into().unwrap()
  - Drop unnecessary identity function
  - Test module must be last in file
  - Drop unused `pub use`

- Update agenix to 0.15.0. Current master has a installCheckPhase that
  doesn't work with C++ Nix 2.3.*:
  a23aa271be (commitcomment-137185861)

Change-Id: Ic29eef20d6fd1362ce1031364a5ca6b4edf195bd
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10615
Reviewed-by: aspen <root@gws.fyi>
Tested-by: BuildkiteCI
Autosubmit: sterni <sternenseemann@systemli.org>
2024-01-19 21:47:32 +00:00
Florian Klink
43b9e25025 refactor(tvix/eval): move Evaluation::{default,new_pure}() again
Have a Evaluation::new() function that's used to set up the Evaluation
struct initially - which is also used by both new_pure and new_impure
internally.

It's generic over the exact type of IO, making it easier to instantiate
Evaluation with non-tvix-eval EvalIO implementations, that might not be
in a Box.

Change-Id: Ibf728da24aca59639c5b6df58d00ae98c99a63f5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10640
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
2024-01-18 14:24:40 +00:00
Florian Klink
e0a867cabf refactor(tvix/eval): generalize EvalIO container
Don't restrict to a Box<dyn EvalIO>.

There's still one or two places where we do restrict, this will be
solved by b/262.

Change-Id: Ic8d927d6ea81fa12d90b1e4352f35ffaafbd1adf
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10639
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2024-01-18 09:09:15 +00:00
Ryan Lahfa
44d24852c3 fix(tvix/eval): catchable-aware throw
`throw (throw "a")` should work and propagate the internal throw.
Before this commit, it didn't work.

Change-Id: Id5d46f74e484dba99e912ad9fa211f3bf1617bac
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10600
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
2024-01-17 17:39:32 +00:00
Ryan Lahfa
e5e33611d7 fix(tvix/eval): catchable-aware elem
`elem` did not catch the list being a catchable.
This surfaced during Nixpkgs evaluation.

Change-Id: Icf19b94e914e35a435c4412d769ee63ba59ab7b0
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10599
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
2024-01-17 17:39:28 +00:00
Ryan Lahfa
f71bb351d2 feat(tvix/glue): introduce test suite for context strings
This is an additional test suite on the top of the Nix ones
for context strings matters.

It already smoked out multiple mistakes and potential bugs and non-deterministic result from the evaluator.

It uses a similar technology as the one in the tvix-eval albeit we instantiate a fully fledged evaluator
with in-memory store.

We copy the files instead of symlinking them because crates are built in
isolation, so symlinks cannot work.

Change-Id: I63ae225ce4f83c6e2c8ccd60d779c2f8eb9d08fb
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10619
Autosubmit: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
2024-01-17 17:31:07 +00:00
Ryan Lahfa
75cc52ddb1 fix(tvix/eval): getContext merges underlying values
Previously, we were assembling very naively an attribute set composed of context we saw.

But it was forgetting that `"${drv}${drv.drvPath}"` would contain 2 contexts with the same key, but
with different values, one with `outputs = [ "out" ];` and `allOutputs = true;`.

Following this reasoning and comparing with what Nix does, we ought to merge underlying values systematically.

Hence, I bring `itertools` to perform a group by on the key and merge everything on the fly, it's not
beautiful but it's the best I could find, notice that I don't use
`group_by` but I talk about group by, that is, because `group_by` is a
`group_by_consecutive`, see
https://github.com/rust-itertools/itertools/issues/374.

Initially, I tried to do it without a `into_grouping_map_by`, it was akin to assemble the final `NixAttrs` directly,
it was less readable and harder to pull out because we don't have a lot of in-place mutable functions on
our data structures.

Change-Id: I9933c9bd88ffe04de50dda14f21879b60d8b8cd4
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10620
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
2024-01-17 17:25:24 +00:00
Ryan Lahfa
97e6c39dcd fix(tvix/eval): context-aware dirOf
`dirOf` forgot to accepts contextful strings, e.g. derivations and propagates this context
further.

Change-Id: I6c05944a3ce5073e243e7676c9be56c48407d657
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10618
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
2024-01-17 07:57:43 +00:00
Ryan Lahfa
c2d0e245e2 fix(tvix/eval): context-aware… hasContext
Yes, `hasContext e` should work where `e` is a contextful strings, otherwise, it is really useless.

Change-Id: I5eb071fc257217d6e8a63fe519132ebd98186696
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10617
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
2024-01-17 07:57:38 +00:00
Ryan Lahfa
bc8fb825c7 feat(tvix/eval): move away from test_generator to rstest
`test-generator` has not been updated in the past 2 years.
`rstest` has not been updated in the past 5 months.

This is an improvement in the maintenance state… I guess?
We get also new features, it changes the name of the tests with numbers too.

Change-Id: I5376104c7704f525dba7524da78daa09867cc669
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10623
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
2024-01-17 07:34:02 +00:00
edef
b624edb2ae fix(tvix/eval): lift VM ops over Catchable
We want to handle bottoms in a consistent fashion. Previously this was
handled by repetitive is_catchable checks, which were not consistently
present.

Change-Id: I9614c479cc6297d1f64efba22b620a26e2a96802
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10485
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
2024-01-16 19:20:28 +00:00
Ryan Lahfa
2750e1e640 fix(tvix/eval): catchable-aware builtins
A bunch of operations in Tvix are not aware of catchable values
and does not propagate them.

In the meantime, as we wait for a better solution, we just offer this
commit for moving the needle.

Change-Id: Ic3f0e1550126b0847b597dfc1402c35e0eeef469
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10473
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
2024-01-14 03:37:17 +00:00
Florian Klink
82540717d6 feat(tvix/eval): make into_json public
Allow other crates (like tvix-glue) to look at a Value in JSON, which is
used by the structured attrs feature.

Change-Id: Iba02ace6e11a74c3f9b19dcbef4b008b76dec046
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10602
Reviewed-by: tazjin <tazjin@tvl.su>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
2024-01-12 22:25:35 +00:00
Florian Klink
9cdb5e17a4 fix(tvix/eval): fix JSON error types
The error message is misleading. The errors we return can happen both
during serialization or deserialization, though the messages suggested
the latter only.

Change-Id: I2dafe17ec78ee75cab5937a3a81540fda3175eac
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10603
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
2024-01-12 09:22:37 +00:00
Florian Klink
c9c95f4ef3 chore(tvix): bump itertools
Change-Id: I2c798a30d3e69789c7d9120422bfdd67b1560dd5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10565
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2024-01-06 15:33:12 +00:00
Aspen Smith
e66dcba195 fix(tvix/eval): Update eval benches for new API
cl/10475 updated the API of tvix_eval::Evaluation, but didn't update
this benchmark to use that new API.

Also, fixes the docstring to no longer specify that there is a "given
snippet".

Change-Id: Ibb8285731849dbeec814e2585bbaa36f22368afe
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10542
Autosubmit: aspen <root@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Reviewed-by: flokli <flokli@flokli.de>
2024-01-05 16:30:51 +00:00
Ryan Lahfa
f44ac2a594 feat(tvix/eval): context-aware split
Nix does something like:

```cpp
        NixStringContext context;
        const auto str = state.forceString(*args[1], context, pos, "while evaluating the second argument passed to builtins.split");
```

And then do nothing with that context, therefore, we follow them and
make `split` aware of the context but still do nothing with it.

Change-Id: I4fee1936600ce86d99d00893ca3f64013213935b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10428
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: raitobezarius <tvl@lahfa.xyz>
2024-01-03 23:24:07 +00:00
Ryan Lahfa
556e52c9cb feat(tvix/eval): impl unsafeDiscardStringContext
Change-Id: I7f0cc42cbebfe5cd27bf6d4f58a4af927b83646a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10423
Autosubmit: raitobezarius <tvl@lahfa.xyz>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
2024-01-03 23:24:07 +00:00
Ryan Lahfa
20c894e232 feat(tvix/glue): context-aware toFile
This removes the reference tracking and uses instead the context for
references and returns some.

Change-Id: Ic359ca6b903b63f1a9c679c566004c617b792442
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10435
Autosubmit: raitobezarius <tvl@lahfa.xyz>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
2024-01-03 19:43:10 +00:00
Ryan Lahfa
cc098b9aaa feat(tvix/eval): contextful coercion of files
In the past reference tracking system, `tvix-io` glue was appending
plain paths in the known path state.

Now, we make up for this by just making contextful coercion of file
imports.

Change-Id: Ieb9b04dd83302c77909252d5f7733857ac3cf8fd
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10443
Tested-by: BuildkiteCI
Autosubmit: raitobezarius <tvl@lahfa.xyz>
Reviewed-by: tazjin <tazjin@tvl.su>
2024-01-03 18:50:29 +00:00
Ryan Lahfa
951854defc feat(tvix/eval): contextful string coercion
String with contexts are always coerced to a string with the same
context.

Change-Id: I224814febd9cad196bb28876793e76bed564dc72
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10440
Tested-by: BuildkiteCI
Autosubmit: raitobezarius <tvl@lahfa.xyz>
Reviewed-by: sterni <sternenseemann@systemli.org>
2024-01-03 18:49:57 +00:00
Ryan Lahfa
9d43c26576 feat(tvix/eval): contextful == of derivations
Otherwise, you just fail because they are not... contextless strings!

Change-Id: I0b8f63a18cd89c3841b613d41c12ec4ee336f953
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10442
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
2024-01-03 18:49:57 +00:00
Ryan Lahfa
207f6fed46 feat(tvix/eval): match DO NOT propagate context
`match` silently ignore the input context and do not propagate
it and successful matches.

The why is unclear but nixpkgs does rely implicitly on this behavior
because dynamic attribute selection cannot be done with contextful
strings.

Change-Id: I5167fa9b2c2db8ecab0c2fb3e9895c9cfce6eeb2
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10441
Autosubmit: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
2024-01-03 18:49:57 +00:00
Ryan Lahfa
09ec8b6fcf feat(tvix/eval): implement getContext primop
Change-Id: I2c5068a28f9883a01b0ff80a5e5ab32ba18bfc1a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10437
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
2024-01-03 18:20:43 +00:00