Age | Commit message (Collapse) | Author | Files | Lines |
|
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
|
|
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
|
|
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>
|
|
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
|
|
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
|
|
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
|
|
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
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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
|
|
Change-Id: Ibddaa111a5b7a86c42dbe153ae8e53f9a5601a54
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10112
Tested-by: BuildkiteCI
Reviewed-by: Adam Joseph <adam@westernsemico.com>
|
|
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>
|
|
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>
|
|
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
|
|
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>
|
|
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>
|
|
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
|
|
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
|
|
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>
|
|
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
|
|
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
|
|
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>
|
|
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
|
|
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>
|
|
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>
|
|
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
|
|
Previously, compare_strict_eval_tests() was using Strictness::Lazy.
This appears to be a minor oversight from
0ab6494286e77abcdc0bba1c92386fc7e654ca12.
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>
|
|
Change-Id: Iad8a5f78930872719b6481ddf1bbad8532bfa888
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7981
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
|
|
Change-Id: Ib98a9fe8c9aa3f0e61c27d10285c0926cda7a969
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7979
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
… 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>
|
|
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>
|
|
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
|
|
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
|
|
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
|
|
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>
|
|
Change-Id: Iaeb9ed7024c2ce85373f8aec0d223f52e1a3a539
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9097
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
|
|
Change-Id: Ic58653254b36694f1991a18db9ceea0d532c2e31
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9068
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
|
|
builtins.div ought to truncate towards zero so that
-(builtins.div a b) == builtins.div (-a) b
-(builtins.div a b) == builtins.div a (-b)
Change-Id: I8b7c08cd7f4fa8a1363c786d42c8d484f6cd133d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9006
Autosubmit: sterni <sternenseemann@systemli.org>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
|
|
Makes use of https://github.com/tokio-rs/prost/pull/341, which makes our
bytes field cheaper to clone.
It's a bit annoying to configure due to
https://github.com/hyperium/tonic/issues/908, but the workaround does
get the job done.
Change-Id: I25714600b041bb5432d3adf5859b151e72b12778
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8975
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: flokli <flokli@flokli.de>
|
|
Some paths might use names that are not valid UTF-8. We should be able
to represent them.
We don't actually need to touch the PathInfo structures, as they need to
represent StorePaths, which come with their own harder restrictions,
which can't encode non-UTF8 data.
While this doesn't change any of the wire format of the gRPC messages,
it does however change the interface of tvix_eval::EvalIO - its
read_dir() method does now return a list of Vec<u8>, rather than
SmolStr. Maybe this should be OsString instead?
Change-Id: I821016d9a58ec441ee081b0b9f01c9240723af0b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8974
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
|
|
This explicitly documents behavior of C++ Nix that goes against the
intuition you'd gather from this document: that e.g. a simple select
from an attribute set causes a value to no longer be pointer equal to
its former self.
The point of documenting this is that we can show in a to be written
section on the use of pointer equality in nixpkgs that pointer equality
is only needed in a limited sense for evaluating it (C++ Nix's exterior
pointer equality). Tvix's pointer equality is far more powerful since
value identity preserving operations also preserve pointer equality,
generally speaking (this is because we implement interior pointer
equality in my made up terminology). This should eventually also be
documented.
Change-Id: I6ce7ef2d67b012f5ebc92f9e81bba33fb9dce7d0
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8856
Tested-by: BuildkiteCI
Autosubmit: sterni <sternenseemann@systemli.org>
Reviewed-by: tazjin <tazjin@tvl.su>
|
|
This fixes a subtle issue which would occasionally lead to a crash (e.g.
when evaluating (pkgs.systemd.outPath with --trace-runtime): With each
character in the string that has a multi byte representation in UTF-8,
the actual byte position and what tvix thought it was would get out of
sync. This could either lead to
* Tvix swallowing characters or jumbling characters if multi byte
characters would cause the tracked index to become out of sync with
the byte position before the first character to be escaped, or
* Tvix crashing if (in the same situation) the out of sync index would
be within a UTF-8 byte sequence.
Luckily, std's `char_indices()` iterator implements exactly what
`nix_escape_char()`'s original author had in mind with
`.chars().enumerate()`. Using `i + 1` for continuing is safe, since all
characters that need (in fact, can) to be escaped in Nix are represented
as a single byte in UTF-8.
Change-Id: I1c836f70cde3d72db1c644e9112852f0d824715e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8952
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Autosubmit: sterni <sternenseemann@systemli.org>
|
|
Change-Id: Ia04778391c198fde21da217bf697aa40157898b0
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8846
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
|
|
The change allows applications that use tvix_serde for parsing
nix-based configuration to extend the language with domain-specific
set of features.
Change-Id: Ia86612308a167c456ecf03e93fe0fbae55b876a6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8848
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
|