Commit graph

1110 commits

Author SHA1 Message Date
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