Commit graph

1185 commits

Author SHA1 Message Date
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
Ryan Lahfa
96d06031af feat(tvix/eval): context-aware replaceStrings
And it also preserve the original context if it exists.

Change-Id: I904f7c13b7f003a267aace6301723780fccaafb7
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10434
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: raitobezarius <tvl@lahfa.xyz>
2024-01-03 18:16:09 +00:00
Ryan Lahfa
cbd22af2b5 chore(tvix/eval): note on context-aware hashString
It must propagate context too.

Change-Id: If57c22c9723ea02aa013f69d3dcf96054476d8de
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10433
Tested-by: BuildkiteCI
Autosubmit: raitobezarius <tvl@lahfa.xyz>
Reviewed-by: tazjin <tazjin@tvl.su>
2024-01-03 18:15:36 +00:00
Ryan Lahfa
e372b1d1a5 feat(tvix/eval): ${} propagates contexts
We just perform union of contexts of every pieces.

Change-Id: Ief925c1818cd8bbec0503e9c625b0630feebfdda
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10432
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Autosubmit: raitobezarius <tvl@lahfa.xyz>
2024-01-03 18:15:36 +00:00
Ryan Lahfa
0e6285ec8b feat(tvix/eval): context-aware concatStringsSep
Change-Id: Id0e169084d26dc598091d157563c4d959b66279b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10431
Autosubmit: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
2024-01-03 18:15:04 +00:00
Ryan Lahfa
88fcab68d4 chore(tvix/eval): note on context-aware toString
Change-Id: Ie26ebd16e95e6a7b6f81051d8269169842978058
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10430
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: raitobezarius <tvl@lahfa.xyz>
2024-01-03 18:14:02 +00:00
Ryan Lahfa
c7d6d08799 feat(tvix/eval): context-aware throw
Change-Id: Ie552dabe4cf93cc396c883268a3bee67796dbbd8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10429
Autosubmit: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
2024-01-03 18:09:30 +00:00
Ryan Lahfa
7dcb37fc52 feat(tvix/eval): context-aware substring
`substring` has a very funny behavior when it comes to empty strings,
it propagates the context too, this is used in nixpkgs to attach context
to strings without using any builtin: `lib.addContextFrom`.

Change-Id: Id655356799b3485f7519b3d1914c630f9d8416c3
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10448
Autosubmit: raitobezarius <tvl@lahfa.xyz>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
2024-01-03 18:08:58 +00:00
Ryan Lahfa
375f7eaa59 feat(tvix/eval): context-aware abort
Change-Id: Id5a435961ce3a2a2240b3936ea48515650d445d6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10427
Autosubmit: raitobezarius <tvl@lahfa.xyz>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
2024-01-03 18:07:56 +00:00
Ryan Lahfa
e259a5c42b feat(tvix/eval): context-aware dirOf
Change-Id: If73a82a7106de9b479c950741efb70bffabd470a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10424
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
2024-01-03 16:57:35 +00:00
Ryan Lahfa
db748741da feat(tvix/eval): context-aware baseNameOf
Change-Id: I9f0a8143070805b85276f721bdfbdf7ede2cf615
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10421
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
2024-01-03 16:54:33 +00:00
Ryan Lahfa
e15b6580cc chore(tvix/eval): notes on coercion and contexts
We make a case for adding a `reject_context` `CoercionKind` here.
It does happen during concatenation actually for path concats.

Change-Id: I0c196aad917550b9bcd0896cd2127a94f8181ffb
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10444
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: raitobezarius <tvl@lahfa.xyz>
2024-01-03 16:54:00 +00:00
Ryan Lahfa
743c362049 feat(tvix/eval): context-aware coerce_to_string
I am still undecided whether we need a CoercionKind to control
the coerced context, here's a simple attempt.

Change-Id: Ibe59d09ef26c519a6acfdfe392014446646dd6d8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10426
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Autosubmit: raitobezarius <tvl@lahfa.xyz>
2024-01-03 16:45:26 +00:00
Florian Klink
6b136dfd23 feat(tvix/glue): emit a warning in case of bad SRI hashes
And include a test to ensure we show the warning.

Change-Id: Ib6a436dbba2592b398b54e44f15a48d1aa345099
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10470
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2024-01-03 13:01:02 +00:00
Ryan Lahfa
d5aa75bbcf feat(tvix/eval): emit warnings with kinds and spans
In the past, we had `emit_warning` be no-op and we used `push_warnings` exclusively
but as we have consumers of this function, we need it to work somewhat.

Change-Id: I78a5ece199a473dec9ef5ea1fae60b36e35137b8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10477
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
2024-01-03 13:01:02 +00:00
Florian Klink
12f9b95a2c feat(tvix/eval): accept impl AsRef<str> for code
We're also happy to consume strings, or other owned stringy types.

Change-Id: I5bead4407976134815d8f879f9f70468e6af1dc4
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10476
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
2023-12-31 13:15:24 +00:00
Florian Klink
4fba57c2c9 refactor(tvix/eval): remove code and location from struct
Instead, it's passed in the evaluate/compile_only functions, which feels
more naturally. It lets us set up the Evaluation struct long before
we actually feed it with data to evaluate.

Now that Evaluation::new() would be accepting an empty list of
arguments, we can simply implement Default, making things a bit more
idiomatic.

Change-Id: I4369658634909a0c504fdffa18242a130daa0239
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10475
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: flokli <flokli@flokli.de>
2023-12-31 13:15:24 +00:00
Ryan Lahfa
3e63d7be42 feat(tvix/eval): context-aware casting to strings
By default, we don't want contextful strings and we almost always want contextless strings.

To this end, we make taking a contextful string a very explicit operation under `to_contextful_str`
and we implement manually the `to_str` cast which requires a `if !s.has_context()` guard that
the macro cannot cover.

Change-Id: I7aae8e57a7d73e547e62b1edb0b1cc7e8c0c69b6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10425
Autosubmit: raitobezarius <tvl@lahfa.xyz>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
2023-12-29 22:00:04 +00:00
Ryan Lahfa
343ee0de57 feat(tvix/eval): implement hasContext primop
`hasContext` is now functional.

Change-Id: I23b128afc9150b833bc0d9b042d31fee35badadb
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10422
Tested-by: BuildkiteCI
Autosubmit: raitobezarius <tvl@lahfa.xyz>
Reviewed-by: tazjin <tazjin@tvl.su>
2023-12-29 21:58:32 +00:00
Ryan Lahfa
5dfb15d2c8 feat(tvix/eval): introduce NixContext
This prepares the data structures to implement string contexts
in Nix.

Change-Id: Idd913c9c881daeb8d446907f4b940e462e730978
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10420
Autosubmit: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
2023-12-29 21:57:30 +00:00
Ryan Lahfa
fc182061c6 docs(tvix/eval): why context strings now
Explain why we had to take the bullet on context strings now
and move the historical approach in a historical section.

Change-Id: Ie3bcc2213b391c6ba06547cc05c850891a41d06b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10446
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
2023-12-29 21:55:58 +00:00
Adam Joseph
4b3f27ebc9 fix(tvix/eval): propagate catchables through builtins.attrNames
Change-Id: Id14e39543239272aed041998fd9a78465c9cb8b2
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10359
Autosubmit: Adam Joseph <adam@westernsemico.com>
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
2023-12-29 21:34:45 +00:00
Adam Joseph
9cb3cd10f5 fix(tvix/eval): propagate catchables through builtins.intersectAttrs
Change-Id: I4ada8cf10611e98519cb1b1da11bfd06815f2932
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10358
Autosubmit: Adam Joseph <adam@westernsemico.com>
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
2023-12-29 21:34:45 +00:00
Adam Joseph
2883b2d4bd fix(tvix/eval): propagate catchables in string interpolations
Change-Id: I13d7ce0c7328a8e6fbc6d2c4ff5c4fe6095b96ea
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10357
Autosubmit: Adam Joseph <adam@westernsemico.com>
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
2023-12-29 21:34:45 +00:00
Adam Joseph
7ddea7340f fix(tvix/eval): catchable in type field of nix_eq()
Change-Id: I165ff77764e272cc94d18cb03ad6cbc9a8ebefde
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10348
Autosubmit: Adam Joseph <adam@westernsemico.com>
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
2023-12-29 21:34:45 +00:00
Adam Joseph
2af8174e2e fix(tvix/eval): builtins.match: propagate catchables
Change-Id: I14c9e625c91369e10d0c00380dca992811ae9059
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10346
Reviewed-by: sterni <sternenseemann@systemli.org>
Autosubmit: Adam Joseph <adam@westernsemico.com>
Tested-by: BuildkiteCI
2023-12-29 21:34:45 +00:00
Adam Joseph
11e35a77a6 refactor(tvix/eval): let OpCoerceToString select the CoercionKind
Change-Id: I92d58ef216d7e0766af70f019b3dcd445284a95d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10344
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
2023-12-29 21:34:45 +00:00
Adam Joseph
0c22454bb9 fix(tvix/eval): add stack depth assertion to OpReturn
I'm still trying to work out the exact stack invariants for tvix.
We really should add assertions for them; getting the stack messed
up is no fun.  This commit adds one simple assertion.  It also adds
a missing stack-push (my mistake) in one place, which was uncovered
by the assertion.

Change-Id: I9d8b4bd1702d954e325832c5935b0d7e3eb68422
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10369
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
2023-12-29 21:34:45 +00:00
sterni
d07600dbca fix(tvix/eval/value): correctly emit spaces when coercing lists
r/7176 introduced an incorrect assumption was the benefit of the
nonrecursive coercion algorithm, namely that a coercion operation always
returns a non empty string. This allows to detect whether we are
coercing a list or not by checking if the intermediate result is empty
or not. Unfortunately, coercing null and false yields an empty string,
so we need to explicitly track whether we are coercing a list.

Updated the test case to hopefully catch similar bugs in the future. I'm
not a hundred percent certain I have not introduced a new edge case with
this, so it may be interesting to add a prop test case for this to
nix_oracle down the line. At least lists are the only nested data
structures that can be serialized as nested data structures, so the
problem is kind of limited.

Change-Id: Ia41e904356f1c41a9d35e4e65ec02f2fe5a4100e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10418
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Autosubmit: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
2023-12-25 15:06:23 +00:00
sterni
7165ebc43b fix(tvix/eval): remove incorrect imports when coercing
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 13:15:23 +00:00
sterni
a30dd0905a fix(tvix/eval): determine meaning of + exprs based on first type
This matches the behavior of C++ Nix more closely where the decision is
made based on the first type based to ExprConcatStrings:
1f93fa2ed2/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-14 02:59:58 +00:00
Adam Joseph
91456c3520 refactor(tvix/eval): vm::add_values(): be less verbose
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 18:00:37 +00:00
Adam Joseph
243a4b5699 fix(tvix/eval): substring: propagate catchables
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 17:51:29 +00:00
Adam Joseph
c5f58d4af3 fix(tvix/eval): baseNameOf: propagate catchables
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 17:50:56 +00:00
Adam Joseph
2565a21aa9 fix(tvix/eval): builtins.length: propagate catchables
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 17:49:54 +00:00
Adam Joseph
1ddfed9825 fix(tvix/eval): builtins.filter: propagate catchables
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 17:35:39 +00:00
Adam Joseph
25dd1458b9 feat(tvix/eval): builtins.hashString: add placeholder
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 17:35:06 +00:00
Adam Joseph
8c8409c0d2 fix(tvix/eval): builtins.getAttr: propagate catchables
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 17:34:34 +00:00
Adam Joseph
afba150036 fix(tvix/eval): builtins.elemAt: propagate catchables
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 17:34:02 +00:00
Adam Joseph
29464fc65b fix(tvix/eval): propagate catchables through builtins.splitVersion
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 17:21:53 +00:00
Adam Joseph
012b44244b feat(tvix/eval): builtins.hasContext: placeholder implementation
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 17:21:53 +00:00
Adam Joseph
531f0c0c42 fix(tvix/eval): baseNameOf should not coerce paths into strings
... 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 17:21:21 +00:00
Adam Joseph
8d4aa2c15c fix(tvix/eval): add unimplemented __curPos and builtins.filterSource
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 17:20:50 +00:00
Adam Joseph
bb1e79e5d1 fix(tvix/eval): propagate catchables through &&
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 17:04:07 +00:00
Adam Joseph
bcc1ea8552 fix(tvix/eval): make || propagate catchables
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 17:04:07 +00:00
Adam Joseph
7b9eea71d0 fix(tvix/eval): fix nested assertions b/340
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 17:03:03 +00:00
Adam Joseph
fc963033ae fix(tvix/eval): ?: propagate catchables
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 16:57:55 +00:00
Adam Joseph
990c4e727f feat(tvix/eval): OpAttrsSelect should propagate catchables
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 16:09:47 +00:00
Adam Joseph
2949ee08f1 fix(tvix/eval): calling a catchable is catchable
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 16:09:15 +00:00
Adam Joseph
52b68c0539 fix(tvix/eval): fix catchables in named formals
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 15:55:06 +00:00
Adam Joseph
e516046ed2 test(tvix/eval): test catchables in named formals
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 15:54:04 +00:00
Adam Joseph
663362f3df fix(tvix/eval): fix testing catchables for inequality
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 15:53:32 +00:00
Adam Joseph
1b2a1892cb test(tvix/eval): testing catchable for inequality
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 15:49:28 +00:00
Adam Joseph
289663cac4 fix(tvix/eval): handle catchables in attribute set updates
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 15:48:25 +00:00
Adam Joseph
e54533518b test(tvix/eval): test for catchable in attribute merges
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 15:48:25 +00:00
Adam Joseph
edf411a86c fix(tvix/eval): fix recovering from throws in implications
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 15:47:53 +00:00
Adam Joseph
24ff74d346 test(tvix/eval): test recovering from throw in implications
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 15:47:21 +00:00
Adam Joseph
9792920f8c fix(tvix/eval): fix branching on catchable defaults (b/343)
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 14:55:48 +00:00
Adam Joseph
e54eeda0ff test(tvix/eval): test branching on catchable defaults (b/343)
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 14:55:48 +00:00
Adam Joseph
1ac57b0d1c feat(tvix/eval): nonrecursive coerce_to_string()
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 14:54:46 +00:00
Adam Joseph
3701379745 feat(tvix/eval): nonrecursive deep_force()
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 14:36:02 +00:00
Adam Joseph
c956138ee6 docs(tvix/eval): clarify difference between ThunkSet and Blackhole
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 14:35:00 +00:00
Adam Joseph
ddba4d4e17 test(tvix/eval): nested assertions (b/340)
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 14:35:00 +00:00
Adam Joseph
c92d06271d feat(tvix/eval): drop LightSpan::Delayed
LightSpan::Delayed was introduced in commit
bf286a54bc 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 14:34:28 +00:00
Florian Klink
27c07b72c6 refactor(tvix): use io::Result for EvalIO
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 14:28:50 +00:00
Adam Joseph
ad566999ca fix(tvix/eval): preserve catchables in nix_cmp_ordering(), fix b/338
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 14:26:46 +00:00
Adam Joseph
ae28dc3ca6 test(tvix/eval): test for b/338 catchable hygiene problem
Commit 05f42519b5 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 14:26:46 +00:00
Adam Joseph
edbd5055a1 feat(tvix/eval): nonrecursive nix_cmp_ordering(), fixes b/339
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 14:26:46 +00:00
Adam Joseph
8a40f75c2d fix(tvix/eval): never use partial_cmp() (partial fix b/338)
This is part of a fix for b/338.

We should never use PartialOrd::partial_cmp().

All Nix types except floats are obviously totally-ordered.  In
addition, it turns out that because Nix treats division by zero
rather than producing a NaN, and because it does not support
"negative zero", even floats are in fact totally ordered in Nix.

Therefore, every call to PartialOrd::partial_cmp() in tvix is an
error.  We have to *implement* this function, but we should never
call it on built-in types.

Moreover, nix_cmp_ordering() currently returns an Option<Ordering>.
I'm not sure what was going on there, since it's impossible for it
to return None.  This commit fixes it to return simply Ordering
rather than Option<Ordering>.

Change-Id: If5c084164cf19cfb38c5a15554c0422faa5f895d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10218
Autosubmit: Adam Joseph <adam@westernsemico.com>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
2023-12-12 14:26:46 +00:00
Adam Joseph
19d13eb070 test(tvix/eval): add test case for b/339
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-12-12 14:26:46 +00:00
Adam Joseph
72ece2e518 feat(tvix/eval): nonrecursive nix_eq()
This commit rewrites Value::nix_eq() into an equivalent.  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 believed that the nonrecursive form would be faster.  It is, in
fact, slightly slower.  I believe this is due to the vec![]
allocation; I am investigating.

Prev-Nixpkgs-Benchmark: {"attrpath":"pkgsCross.aarch64-multiplatform.hello.outPath","peak-kbytes":"459068","system-seconds":"0.71","user-seconds":"5.39"}
This-Nixpkgs-Benchmark: {"attrpath":"pkgsCross.aarch64-multiplatform.hello.outPath","peak-kbytes":"460048","system-seconds":"0.68","user-seconds":"5.73"}
Change-Id: I10f4868891e4b7475df13f0cbc41ec78dd985dd8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10118
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Autosubmit: Adam Joseph <adam@westernsemico.com>
2023-12-12 14:26:46 +00:00
Florian Klink
01a3c87a3a chore(tvix): bump criterion to 0.5
Change-Id: I28904ca23437b4bb745c0eb1f4eb9ae33e09eb5a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10244
Reviewed-by: grfn <grfn@gws.fyi>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
2023-12-11 03:59:30 +00:00
Florian Klink
d3ecef1a5b refactor(tvix/eval): address clippy lints
Change-Id: Ic2bd4e8291b30ceac9fa0e88a4f56e61ae99b603
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10227
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
2023-12-09 12:58:39 +00:00
Adam Joseph
5fd42a9e70 feat(tvix/eval): impl DoubleEndedIter for OwnedAttrsIterator
Change-Id: I4bd85dbe9c27047f4abbdeff4e2b796e9bcab3a1
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10211
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: Adam Joseph <adam@westernsemico.com>
Tested-by: BuildkiteCI
2023-12-09 11:35:19 +00:00
Adam Joseph
49b34183e3 feat(tvix/eval): rewrite Thunk::force() in nonrecursive form
This commit rewrites Thunk::force() so that it is not (directly)
self-recursive.  It maintains a Vec of all the
previously-encountered thunks which point to the one it is currently
forcing, rather than recursively calling itself.

Benefits:

- Short term:

  This commit saves the cost of a round-trip through the generator
  machinery for the generators::request_force() which is removed by
  this commit.

- Medium term:

  Once a similar transformation has been applied to nix_cmp(),
  nix_add(), nix_eq(), and coerce_to_string(), those four functions,
  along with Thunk::force(), will make non-tail calls only to each
  other.  They can then be merged into a single tail-recursive
  function which does not use the generator machinery at all:

    enum Task { Cmp, Add, Eq, CoerceToString, Force};

    fn Value::walk(task:Task, v1:Value, v2:Value) {
      // ...

- Long term:

  The long-term goal here is to use generators **only for builtins**
  and [Marionette]-style remote control of the VM.  In other words:
  use `async` for things that actually involve concurrency.  Calls
  from the VM to builtins can then be blocking calls, because even
  cppnix will overflow the stack if you make a MAX_STACK_DEPTH-deep
  recursive call which passes through a builtin at every stack frame
  (e.g. `{ func = builtins.sort (a: b: ... func ...) ...}`).

  This way the inner "tight loop" of the interpreter doesn't pay the
  costs of `async` and generators.  These costs manifest in terms
  of: performance, complex nonlocal control flow, and language
  impediments (async Rust is a restricted subset of real Rust, and
  is missing things like traits).

[Marionette]: https://firefox-source-docs.mozilla.org/testing/marionette/Intro.html

Change-Id: I6179b8abb2ea0492180fcb347f37595a14665777
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10039
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
2023-12-06 06:53:01 +00:00
Aspen Smith
8135a8d38c fix(tvix/eval): Return error rather than panicking on bad substring
If builtins.substring is invoked with (byte!!) offsets that aren't at
codepoint boundaries, return an error rather than panicking. This is
still incorrect (see b/337) but pushes the incorrectness forward a step.

Change-Id: I5a4261f2ff250874cd36489ef598dcf886669d04
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10199
Tested-by: BuildkiteCI
Autosubmit: grfn <grfn@gws.fyi>
Reviewed-by: sterni <sternenseemann@systemli.org>
2023-12-05 23:12:23 +00:00
Vincent Ambo
e65fa82d74 refactor(tvix/eval): use or_default helper in entry API
This fixes a future clippy lint.

Change-Id: Ic830e94ef23595580c1037f10878c76bbb546dd9
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10110
Tested-by: BuildkiteCI
Reviewed-by: Adam Joseph <adam@westernsemico.com>
2023-11-25 15:48:46 +00:00
Vincent Ambo
5ffb997864 fix(tvix): ensure PartialOrd/Ord agree for StorePath & NixString
This fixes a *future* clippy lint:
https://rust-lang.github.io/rust-clippy/master/index.html#/incorrect_partial_ord_impl_on_ord_type

In essence, because the implementation of *both* Ord and PartialOrd
implies that ordering is not partial, all results of PartialOrd should
simply be those of Ord. This is to avoid subtle bugs in future
refactorings.

Change-Id: I8fc6694010208752dd47746a2aaaeca0c788d574
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10109
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
2023-11-25 15:48:46 +00:00
Adam Joseph
512346ba0b refactor(tvix/eval): add ThunkRepr::is_forced()
Change-Id: I4eab5c81fb82337da06327248845cd2f3a4490d3
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10038
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Autosubmit: Adam Joseph <adam@westernsemico.com>
2023-11-25 02:55:57 +00:00
Adam Joseph
b5a15989cd feat(tvix/eval): add Thunk::unwrap_or_clone()
This commit adds Thunk::unwrap_or_clone(), which uses
Rc::try_unwrap() to avoid cloning the Value out of a an Rc which has
only one strong reference.

Change-Id: Icacefe0c823dcddf046d90c0c5cd5ed59fe976d4
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10037
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: Adam Joseph <adam@westernsemico.com>
Tested-by: BuildkiteCI
2023-11-25 02:55:56 +00:00
sterni
c5961e7774 docs(tvix/eval): optimization potential for inherit (from) exprs
Change-Id: Ibddaa111a5b7a86c42dbe153ae8e53f9a5601a54
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10112
Tested-by: BuildkiteCI
Reviewed-by: Adam Joseph <adam@westernsemico.com>
2023-11-24 23:36:15 +00:00
Vincent Ambo
7c32d85455 docs(tvix/eval): document where EvalIO methods are used
Change-Id: I335f2ba4420973861c2a22125995b45a34d3608c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9969
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Autosubmit: tazjin <tazjin@tvl.su>
2023-11-06 09:06:26 +00:00
Vincent Ambo
f325456cba chore(tvix): bump proptest dependency
This *might* contain a fix for a clippy lint thrown by that crate.

Relates to b/321.

Change-Id: Ia7ebd3e26e0feb8bcc7a6c811b1e583f9016fd9e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9966
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
Autosubmit: tazjin <tazjin@tvl.su>
2023-11-05 20:28:38 +00:00
Vincent Ambo
87d3fac676 chore(tvix): add missing clippy attributes & config
For cases where clippy lints don't apply to us, or something is
misfiring, add appropriate configuration.

Relates to b/321.

Change-Id: I0af453910b4a4112bf685b2a8e9a73de10ec87ea
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9965
Reviewed-by: flokli <flokli@flokli.de>
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
2023-11-05 20:28:38 +00:00
Vincent Ambo
2e1399698b refactor(tvix/eval): use IntoIterator trait for owned NixAttrs iter
Uses the standard library IntoIterator trait for the construction of
our iterators. Clippy complains about duplicating this.

While doing this, I opted to rename the `IntoIter` type into something
that is more useful to users, in case somebody ends up working with
these manually.

This fixes a clippy lint, and is related to b/321.

Change-Id: I851fde0d7b8b38d182343a0fd6d9f8dd2a33ee11
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9963
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
Autosubmit: tazjin <tazjin@tvl.su>
2023-11-05 20:28:38 +00:00
Vincent Ambo
b3b1f649d6 chore(tvix): fix trivial clippy lints
Relates to b/321.

Change-Id: I37284f89b186e469eb432e2bbedb37aa125a6ad4
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9961
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
Autosubmit: tazjin <tazjin@tvl.su>
2023-11-05 20:28:37 +00:00
Vincent Ambo
99f618bcb4 refactor(tvix/eval): delay allocation when comparing attr values
Delays allocation (through cloning) of the values to be compared
until *after* the keys have been compared.

Change-Id: I7d68c27d7a0fbcdcc387db7c092bce50ca4b94ea
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9900
Autosubmit: tazjin <tazjin@tvl.su>
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
2023-11-03 09:24:31 +00:00
Vincent Ambo
2dd2b844c7 chore(tvix/eval): add a marker for sorted borrowed attrs iteration
Similar to `into_iter_sorted`, add a marker function for call sites
that want *borrowed* sorted iteration.

Change-Id: I7c6f14e1ac43fdb14b861b3da183eb5d12bba139
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9899
Autosubmit: tazjin <tazjin@tvl.su>
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
2023-11-03 09:24:31 +00:00
Vincent Ambo
d91fcd4f28 refactor(tvix/eval): more efficiently intersect attributes
builtins.intersectAttrs is used a _lot_ in nixpkgs eval, for whatever
reason. We previously had a very inefficient implementation that would
allocate for each comparison. It stuck out like a sore thumb in perf
analysis.

This moves to a custom algorithm with two iterators, one for the left
and one for the right side, advancing them along the (borrowed) map
keys until a match is found and allocation is required.

I've not made any effort to reduce the verbosity of this code, I don't
think it's worth it.

On my machine this reduces the mean runtime of evaluating
`nixpkgs.emacs.outPath` by ~8%.

Change-Id: Ie506d82cb8d5f45909628f771a6b73e0eca16b27
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9898
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
2023-11-03 09:24:30 +00:00
Florian Klink
1571d7195e fix(tvix/eval/benches): use black_box properly
The purpose of black_box is to actually prevent the compiler from being
able to optimize computation of the benchmarked function away.

To accomplish this, we need to actually *use* black_box to blackbox the
input data away, rather than the return type.

Change-Id: I5438982f57509fbf7b85034346a2739d76aef1fa
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9902
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
2023-11-02 20:39:52 +00:00
Florian Klink
269ab866f1 refactor(tvix/eval/tests): migrate to tempfile
tempdir pulls in remove_dir_all 0.5.3 with
https://rustsec.org/advisories/RUSTSEC-2023-0018.html, and we use
tempfile everywhere else too, so let's just migrate to that.

Change-Id: I735ade7b65e12fc26e3d43ca95fcfa07fcc64642
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9565
Reviewed-by: Connor Brewster <cbrewster@hey.com>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
2023-10-08 22:47:33 +00:00
Adam Joseph
fac32d0a5f feat(tvix/eval): test case for b/281
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-26 14:13:05 +00:00
Adam Joseph
05f42519b5 fix(tvix/eval): fix b/281 by adding Value::Catchable
This commit makes catchable errors a variant of Value.

The main downside of this approach is that we lose the ability to
use Rust's `?` syntax for propagating catchable errors.

Change-Id: Ibe89438d8a70dcec29e016df692b5bf88a5cad13
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9289
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: Adam Joseph <adam@westernsemico.com>
Tested-by: BuildkiteCI
2023-09-24 21:54:10 +00:00
Adam Joseph
926459ce69 refactor(tvix/eval): factor CatchableErrorKind out of ErrorKind
This commit creates a separate enum for "catchable" errors (the kind
that `builtins.tryEval` can detect).

Change-Id: Ie81d1112526d852255d9842f67045f88eab192af
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9287
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: Adam Joseph <adam@westernsemico.com>
2023-09-24 21:54:09 +00:00
Florian Klink
731996fbfe docs(tvix/eval): fix some broken docstr references
There's some more left, but they've been renamed/refactored out of
sight.

Change-Id: I41579dedc74342b4c5f8cb39d2995b5b0c90b0f4
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9372
Tested-by: BuildkiteCI
Reviewed-by: Connor Brewster <cbrewster@hey.com>
Autosubmit: flokli <flokli@flokli.de>
2023-09-22 09:14:33 +00:00
Adam Joseph
9256621009 fix(tvix/eval): update identifier quoting to match cppnix 2.17
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-09-15 00:00:59 +00:00
Adam Joseph
6fa80ecc3d fix(tvix/eval): make compare_strict_eval_tests use Strict
Previously, compare_strict_eval_tests() was using Strictness::Lazy.
This appears to be a minor oversight from
0ab6494286.

This commit corrects that, by changing Strictness::Lazy to
Strictness::Strict.

Change-Id: Ia2389a5d30481cd322ed55230731340b795e5d87
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9282
Tested-by: BuildkiteCI
Autosubmit: Adam Joseph <adam@westernsemico.com>
Reviewed-by: sterni <sternenseemann@systemli.org>
2023-09-09 16:11:57 +00:00
Vincent Ambo
d546b9a15d docs(tvix/eval): document remaining opcodes
Change-Id: Iad8a5f78930872719b6481ddf1bbad8532bfa888
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7981
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
2023-09-08 21:38:42 +00:00
Vincent Ambo
35e8dc97cf docs(tvix/eval): document attribute set related opcodes
Change-Id: Ib98a9fe8c9aa3f0e61c27d10285c0926cda7a969
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7979
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
2023-09-08 21:38:42 +00:00
Vincent Ambo
3cf37c7cb6 docs(tvix/eval): add documentation strings for some OpCode variants
Change-Id: I42e610740b3687e1fd897d36694cce425751a8bc
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7975
Tested-by: BuildkiteCI
Autosubmit: tazjin <tazjin@tvl.su>
Reviewed-by: flokli <flokli@flokli.de>
2023-09-08 21:38:42 +00:00
Linus Heckemann
a3dbb60213 fix(tvix/eval): off-by-one in replaceStrings
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-24 10:07:48 +00:00
Florian Klink
366e11b48a refactor(tvix/eval): don't use format! in write! args
Change-Id: I0b2fdb418c2e36280d5c551a81634e1742193903
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9105
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2023-08-20 21:52:21 +00:00
Florian Klink
c67b18bf45 refactor(tvix/eval/vm): don't put HashMap in a Box
HashMap already is on the heap.

Change-Id: I53763e17469359e85862f297b5c2e7c0d8c3a980
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9104
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2023-08-20 21:52:21 +00:00
Florian Klink
aead2001d7 refactor(tvix/eval): impl Default for SourceCode
… instead of new(). Suggested by clippy.

Change-Id: Iac7be733392afefc2b4ff2e38386eee95f3bce94
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9103
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2023-08-20 21:52:21 +00:00
Florian Klink
cf1a884ec5 refactor(tvix/eval/observer): cargo clippy
Change-Id: I11d7d55f31ddd27952f201a0abbed7445013f434
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9102
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2023-08-20 21:51:04 +00:00
Florian Klink
1a99b10b7e refactor(tvix/eval): cargo clippy
This passes a unit value to the function.

Change-Id: I4df3ad8fb0f35c0f110cee3349971ae28ce2878c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9101
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
2023-08-20 21:51:04 +00:00
Florian Klink
ded577a73b refactor(tvix/eval/io): cargo clippy &Path
Change-Id: Ifec2a4fc93c482e41ff5993183643b5126a7728b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9100
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
2023-08-20 21:51:04 +00:00
Florian Klink
def773d4d5 refactor(tvix/eval): cargo clippy (len() is usize)
Change-Id: I9d931ffcc03c6df7c0392dbc1c9a4ae0e3804213
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9099
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
2023-08-20 21:51:04 +00:00
Florian Klink
1f02dc7eba refactor(tvix/eval): cargo clippy &GenCo
Change-Id: I6b1b902ccbc12bf2acdb0fdf406d6ef336ff0b2f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9098
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
2023-08-20 21:51:04 +00:00