about summary refs log tree commit diff
path: root/tvix/eval/src/value/string.rs
AgeCommit message (Collapse)AuthorFilesLines
2024-08-18 r/8515 refactor(tvix/eval): Pull context out into its own moduleAspen Smith1-1031/+0
I'm gonna be doing some poking around in the internals of Context, so in preparation this pulls it out into its own module. Change-Id: I72ea7df80b5f36f838934ee07bdba66874c334c9 Reviewed-on: https://cl.tvl.fyi/c/depot/+/12189 Autosubmit: aspen <root@gws.fyi> Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
2024-08-11 r/8480 chore(tvix/eval): Update module comment for value::stringAspen Smith1-3/+2
This sentence is a little stale; let's just link to NixString directly for the authoritative source of truth. Change-Id: I64e065c4148d29702b09820a0e7724a65fae7c67 Reviewed-on: https://cl.tvl.fyi/c/depot/+/12181 Autosubmit: aspen <root@gws.fyi> Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de>
2024-08-10 r/8474 feat(tvix/eval): Store hash in key of internerAspen Smith1-7/+20
Rather than storing the leaked allocation for the string as the key in the interner, store the hash (using NoHashHashBuilder). I thought this would improve performance, but it doesn't: hello outpath time: [736.85 ms 748.42 ms 760.42 ms] change: [-2.0754% +0.4798% +2.7096%] (p = 0.72 > 0.05) No change in performance detected. but it at least doesn't *hurt* performance, and it *does* avoid an `unsafe`, so it's probably net good. Change-Id: Ie413955bdb6f04b1f468f511e5ebce56e329fa37 Reviewed-on: https://cl.tvl.fyi/c/depot/+/12049 Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de> Autosubmit: aspen <root@gws.fyi>
2024-08-08 r/8455 feat(tvix/eval): Put interner in a thread-local RefCellAspen Smith1-9/+8
Rather than making the interner be a global lazy_static mutex, put it in a thread-local RefCell. This doesn't change anything in terms of sharing (since we're currently actually just single threaded), but avoids the overhead of a mutex, for a nice performance boost (compared to the mutex version): hello outpath time: [726.71 ms 729.79 ms 735.69 ms] change: [-5.7277% -3.9733% -2.1144%] (p = 0.00 < 0.05) Performance has improved. Change-Id: I240b238dcbaf854ebafc3017b4425fb7d7b91b03 Reviewed-on: https://cl.tvl.fyi/c/depot/+/12048 Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de> Autosubmit: aspen <root@gws.fyi>
2024-08-08 r/8454 feat(tvix/eval): Intern (and leak) small strings, behind a mutexAspen Smith1-5/+82
This is the most naive version of string interning possible - we store a map from the string itself to the pointer behind a global mutex, and memoize the allocation of all strings below a threshold length (16 bytes, for now) into that map. This requires leaking /all/ strings, since it's not easy to know just from the pointer that a string has been interned - so interning is disabled if string leaking is also disabled. In the case where we're leaking strings (the default), even the naive version of this gets us a pretty nice perfomance boost: hello outpath time: [742.54 ms 745.89 ms 749.14 ms] change: [-2.8722% -2.0135% -1.0654%] (p = 0.00 < 0.05) Performance has improved. However, in the case where we're not leaking strings, we have to keep track of which strings have and haven't been interned, which makes this a little worse: hello outpath time: [779.30 ms 792.82 ms 808.74 ms] change: [+2.5258% +4.0884% +5.8931%] (p = 0.00 < 0.05) Performance has regressed. Hopefully we can close the gap here a bit with some clever tricks (coming next). Change-Id: If08cb48ede703c7fe3bdd8d617443f8a561ad09b Reviewed-on: https://cl.tvl.fyi/c/depot/+/12047 Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de> Autosubmit: aspen <root@gws.fyi>
2024-08-07 r/8453 feat(tvix/eval): Forbid Hash{Map,Set}, use Fx insteadAspen Smith1-6/+12
Per https://nnethercote.github.io/perf-book/hashing.html, we have basically no reason to use the default hasher over a faster, non-DoS-resistant hasher. This gives a nice perf boost basically for free: hello outpath time: [704.76 ms 714.91 ms 725.63 ms] change: [-7.2391% -6.1018% -4.9189%] (p = 0.00 < 0.05) Performance has improved. Change-Id: If5587f444ed3af69f8af4eead6af3ea303b4ae68 Reviewed-on: https://cl.tvl.fyi/c/depot/+/12046 Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de> Reviewed-by: Ilan Joselevich <personal@ilanjoselevich.com> Autosubmit: aspen <root@gws.fyi>
2024-08-05 r/8443 feat(tvix/eval): Leak strings (with flag to disable)Aspen Smith1-3/+15
Default to always leaking strings, and copying strings by copying pointers rather than cloning the underlying allocation. This (somewhat bafflingly) doesn't seem to affect any benchmarks, but paves the way for some tricks around string allocation that do. Unfortunately, we can't do this (yet?) for contextful strings, for reasons I don't currently understand but which I will address later, when I address contextful strings more holistically. I've left a flag in here to disable this, both to test the cloning logic for strings for when/if we decide to bring this back, and to allow people who care more about memory usage than perf to disable leaking. Change-Id: Iec44bcbfe9b3d20389d2450b9a551792a79b9b26 Reviewed-on: https://cl.tvl.fyi/c/depot/+/12045 Autosubmit: aspen <root@gws.fyi> Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de>
2024-06-26 r/8314 refactor(tvix/eval): prefix ctx iterators in NixStringFlorian Klink1-3/+15
NixString::iter_plain() didn't make much sense, especially without a docstring. Rename it to iter_ctx_plain(), and copy the docstring from NixContext. Do the same for the two other context element types too. Change-Id: I1bbfcb967d8d9b14487d069bfe3a1f762253ef4d Reviewed-on: https://cl.tvl.fyi/c/depot/+/11882 Tested-by: BuildkiteCI Reviewed-by: Ilan Joselevich <personal@ilanjoselevich.com> Reviewed-by: flokli <flokli@flokli.de>
2024-06-05 r/8217 feat(tvix/glue): Implement builtins.storePathAspen Smith1-0/+6
This one's relatively simple - we just check if the store path exists, and if it does we make a new contextful string containing the store path as its only context element. Automatic testing seems tricky for this (I think?) so I tested it manually: tvix-repl> builtins.storePath /nix/store/yn46i4xx5alh7gs6fpkxk430i34rp2q9-hello-2.12.1 => "/nix/store/yn46i4xx5alh7gs6fpkxk430i34rp2q9-hello-2.12.1" :: string Change-Id: I8a0d9726e4102ab872c53c2419679c2c855a5a18 Reviewed-on: https://cl.tvl.fyi/c/depot/+/11696 Tested-by: BuildkiteCI Autosubmit: aspen <root@gws.fyi> Reviewed-by: flokli <flokli@flokli.de>
2024-05-30 r/8183 fix(tvix/eval): proptests require arbitrary featureFlorian Klink1-1/+1
`cargo test --no-default-features` fails, if we don't conditionalize this on the `arbitrary` feature too. Change-Id: I81a277810119fed0cfc37c942c422f731aa14b2e Reviewed-on: https://cl.tvl.fyi/c/depot/+/11726 Autosubmit: flokli <flokli@flokli.de> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Reviewed-by: Connor Brewster <cbrewster@hey.com>
2024-05-25 r/8168 fix(tvix/eval): drop superfluous string context checkedef1-4/+1
cl/11712 simultaneously introduced this check and made it unnecessary, since NixString::context should never return `Some` for empty contexts now. Change-Id: I41a655ff33910e8326cbb7d7526eb91bd19e9585 Reviewed-on: https://cl.tvl.fyi/c/depot/+/11713 Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
2024-05-25 r/8167 fix(tvix/eval): disallow empty but allocated string contextsedef1-3/+25
Both `Some(NixContext::new())` and `None` represent empty contexts, but the former trips up `NixString::has_context`, and seems likely to trip up other things. We could hide the difference in the accessors, but we don't really *want* the distinction to exist, since heap-allocating a null value is pretty much always a mistake. Change-Id: Ie84d26fb0d4b59e68354891ba13bde3bae40ab6e Reviewed-on: https://cl.tvl.fyi/c/depot/+/11712 Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de>
2024-05-23 r/8164 feat(tvix/eval): rm NixContext::join, add take_context & IntoIteratorFlorian Klink1-16/+23
In places where we want to extend context with that from another NixString, use take_context() to split it off, then call .extend(), making use of IntoIterator to avoid a bunch of clones. Change-Id: I2460141a3ed776c64c36132b2203b6a1d710b922 Reviewed-on: https://cl.tvl.fyi/c/depot/+/11705 Tested-by: BuildkiteCI Autosubmit: flokli <flokli@flokli.de> Reviewed-by: edef <edef@edef.eu>
2024-05-23 r/8162 feat(tvix/eval): add NixContext::extendFlorian Klink1-1/+9
This is a slightly less annoying version of `join`, which does not consume self. It's more consistent with HashSet::extend(). Change-Id: Ifd0872da36fe8e7b2aa6948674cb8e4023abe9d7 Reviewed-on: https://cl.tvl.fyi/c/depot/+/11703 Autosubmit: flokli <flokli@flokli.de> Tested-by: BuildkiteCI Reviewed-by: edef <edef@edef.eu>
2024-04-22 r/7991 fix(tvix/eval): don't impl From<NixString> for StringFlorian Klink1-5/+1
This caught me by accident in an earlier revision of cl/11500 - I had a `NixString`, wanted to return it as a `String`, so I was naively calling `s.into()`. That unfortunately gave me the `Display` implementation of `NixString`, which quotes strings, causing an annoying error further up the stack. NixStrings are bytes, we can keep the impl From<NixString> for BString, but having a `.into()` suddenly do quoting is more than unexpected. Change-Id: I5434ba94bfe6c493d0a57e68225ecc22daa4b948 Reviewed-on: https://cl.tvl.fyi/c/depot/+/11505 Autosubmit: flokli <flokli@flokli.de> Reviewed-by: tazjin <tazjin@tvl.su> Reviewed-by: raitobezarius <tvl@lahfa.xyz> Tested-by: BuildkiteCI
2024-03-31 r/7806 feat(tvix/eval): implement `unsafeDiscardOutputDependency`Ryan Lahfa1-0/+6
This builtin only transforms any `NixContextElement::Derivation` into the trivial `NixContextElement::Plain`. This is a forgetful functor on derivation-deep context strings. The test coverage of this change is done in cl/11264. Change-Id: Icd00778c97766be6db8a6bdabaa59e9724353ec5 Signed-off-by: Ryan Lahfa <tvl@lahfa.xyz> Reviewed-on: https://cl.tvl.fyi/c/depot/+/11262 Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de>
2024-02-23 r/7597 feat(tvix/eval): implement `builtins.hashString`Padraic-O-Mhuiris1-0/+6
Implements md5, sha1, sha256 and sha512 using the related crates from the RustCrypto hashes project (https://github.com/RustCrypto/hashes) Change-Id: I00730dea44ec9ef85309edc27addab0ae88814b8 Reviewed-on: https://cl.tvl.fyi/c/depot/+/11005 Tested-by: BuildkiteCI Reviewed-by: aspen <root@gws.fyi>
2024-02-21 r/7591 feat(tvix/eval): Store string context alongside dataAspen Smith1-39/+350
Previously, Nix strings were represented as a Box (within Value) pointing to a tuple of an optional context, and another Box pointing to the actual string allocation itself. This is pretty inefficient, both in terms of memory usage (we use 48 whole bytes for a None context!) and in terms of the extra indirection required to get at the actual data. It was necessary, however, because with native Rust DSTs if we had something like `struct NixString(Option<NixContext>, BStr)` we could only pass around *fat* pointers to that value (with the length in the pointer) and that'd make Value need to be bigger (which is a waste of both memory and cache space, since that memory would be unused for all other Values). Instead, this commit implements *manual* allocation of a packed string representation, with the length *in the allocation* as a field past the context. This requires a big old pile of unsafe Rust, but the payoff is clear: hello outpath time: [882.18 ms 897.16 ms 911.23 ms] change: [-15.143% -13.819% -12.500%] (p = 0.00 < 0.05) Performance has improved. Fortunately this change can be localized entirely within value/string.rs, since we were abstracting things out nicely. Change-Id: Ibf56dd16c9c503884f64facbb7f0ac596463efb6 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10852 Tested-by: BuildkiteCI Reviewed-by: raitobezarius <tvl@lahfa.xyz> Autosubmit: aspen <root@gws.fyi>
2024-02-13 r/7506 fix(tvix/eval): Replace inner NixString repr with Box<Bstr>Aspen Smith1-23/+57
Storing a full BString here incurs the extra overhead of the capacity for the inner byte-vector, which we basically never use as Nix strings are immutable (and we don't do any mutation / sharing analysis). Switching to a Box<BStr> cuts us from 72 bytes to 64 bytes per string (and there are a lot of strings!) Change-Id: I11f34c14a08fa02759f260b1c78b2a2b981714e4 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10794 Autosubmit: aspen <root@gws.fyi> Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
2024-01-31 r/7460 fix(tvix): Represent strings as byte arraysAspen Smith1-54/+95
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-03 r/7331 feat(tvix/eval): implement `getContext` primopRyan Lahfa1-0/+4
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 r/7317 feat(tvix/eval): context-aware `coerce_to_string`Ryan Lahfa1-0/+4
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>
2023-12-29 r/7280 feat(tvix/eval): introduce `NixContext`Ryan Lahfa1-6/+191
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-05 r/7119 fix(tvix/eval): Return error rather than panicking on bad substringAspen Smith1-0/+9
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-11-25 r/7064 fix(tvix): ensure PartialOrd/Ord agree for StorePath & NixStringVincent Ambo1-1/+1
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-05 r/6955 chore(tvix): fix trivial clippy lintsVincent Ambo1-4/+4
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-09-15 r/6589 fix(tvix/eval): update identifier quoting to match cppnix 2.17Adam Joseph1-1/+12
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-07-11 r/6404 fix(tvix/eval): use byte, not codepoint index for slicing in escapesterni1-1/+5
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>
2023-03-13 r/5968 chore(tvix/eval): remove `From<SmolStr> for NixString` instanceVincent Ambo1-4/+3
No longer needed, and in some cases caused some extra work. Change-Id: I64e8e7292573bdc92a9c7a8e470e33f8c526f311 Reviewed-on: https://cl.tvl.fyi/c/depot/+/8152 Reviewed-by: raitobezarius <tvl@lahfa.xyz> Tested-by: BuildkiteCI
2023-03-13 r/5967 refactor(tvix/eval): simplify NixString representation(s)Vincent Ambo1-36/+13
Instead of the two different representations (which we don't really use much), use a `Box<str>` (which potentially shaves another 8 bytes off `Value`). NixString values themselves are immutable anyways (which was a guarantee we already had with `SmolStr`), so this doesn't change anything else. Change-Id: I1d8454c056c21ecb0aebc473cfb3ae06cd70dbb6 Reviewed-on: https://cl.tvl.fyi/c/depot/+/8151 Reviewed-by: raitobezarius <tvl@lahfa.xyz> Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
2023-01-12 r/5652 feat(tvix/eval): implement builtins.toJSONVincent Ambo1-3/+4
Implements `Serialize` for `tvix_eval::Value`. Special care is taken with serialisation of attribute sets, and forcing of thunks. The tests should cover both cases well. Change-Id: I9bb135bacf6f87bc6bd0bd88cef0a42308e6c335 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7803 Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI Autosubmit: tazjin <tazjin@tvl.su>
2023-01-10 r/5640 feat(tvix/eval): implement serde::Deserialize for ValueRyan Lahfa1-0/+36
Co-Authored-By: Vincent Ambo <tazjin@tvl.su> Change-Id: Ib6f7d1f4f4faac36b44f5f75cccc57bf912cf606 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7626 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2023-01-04 r/5582 fix(tvix/eval): ' is allowed in nonfirst position in Nix identifierssterni1-1/+1
With this is_valid_nix_identifier should line up with the upstream lexer definition: ID [a-zA-Z\_][a-zA-Z0-9\_\'\-]* While we're working on this, add a simple test checking the various formatting rules. Interestingly, it would not be suitable as an identity test, since you have to write { "assert" = null; } in order to avoid an evaluation error, but C++ Nix is happy to print this as { assert = null; } – maybe should be considered to be a bug. Change-Id: I0a4e1ccb5033a80f3767fb8d1c4bba08d303c5d8 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7744 Autosubmit: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-12-25 r/5486 fix(tvix/eval): fix current clippy warningsVincent Ambo1-1/+1
It's been a while since the last time, so quite a lot of stuff has accumulated here. Change-Id: I0762827c197b30a917ff470fd8ae8f220f6ba247 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7597 Reviewed-by: grfn <grfn@gws.fyi> Autosubmit: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-11-04 r/5241 fix(tvix/eval): quote keys which are not valid identifiersAdam Joseph1-1/+24
The impl Display for NixAttrs needs to wrap double quotes around any keys which are not valid Nix identifiers. This commit does that, and adds a test (which fails prior to this commit and passes after this commit). Change-Id: Ie31ce91e8637cb27073f23f115db81feefdc6424 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7084 Autosubmit: Adam Joseph <adam@westernsemico.com> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-17 r/5153 feat(tvix/eval): Record formals on lambdaGriffin Smith1-0/+7
In preparation for both implementing the `functionArgs` builtin and adding support for validating closed formals, record information about the formal arguments to a function *on the Lambda itself*. This may seem a little odd for the purposes of just closed formal checking, but is something we have to have anyway for builtins.functionArgs so I figured I'd do it this way to kill both birds with one stone. Change-Id: Ie3770a607bf352a1eb395c79ca29bb25d5978cd8 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7001 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-10 r/5095 feat(tvix/eval): Implement builtins.readFileGriffin Smith1-0/+7
Change-Id: If3fd0b087009a2bfbad8bb7aca0aa20de906eb12 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6921 Tested-by: BuildkiteCI Reviewed-by: Adam Joseph <adam@westernsemico.com> Reviewed-by: kanepyork <rikingcoding@gmail.com> Autosubmit: grfn <grfn@gws.fyi> Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-10 r/5087 feat(tvix/eval): Initial resolution of `<...>` pathsGriffin Smith1-0/+7
This commit implements (lazy) resolution of `<...>` paths via either the NIX_PATH environment variable, or the -I command-line flag - both handled via EvalOptions. As a result, EvalOptions can no longer derive Copy, meaning we have to clone it at each line of the repl - this is probably not a huge deal as repl performance is not exactly an inner loop and we're not cloning very much. Internally, this works by creating a thunk which pushes a constant containing the string inside the brackets to the stack, then a new opcode to resolve that path via the `NixPath`. To get that opcode to work, we now have to pass in the NixPath when constructing the VM. This (intentionally) leaves out proper implementation of path resolution via `findFile` (cppnix just calls whatever identifier called findFile is in scope!!!) as that's widely considered a bit of a misfeature, but if we do decide to implement that down the road it likely wouldn't be more than a few extra ops within the thunk introduced here. Change-Id: Ibc979b7e425b65cbe88599940520239a4a10cee2 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6918 Autosubmit: grfn <grfn@gws.fyi> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-10 r/5084 feat(tvix/eval): Implement builtins.concatStringsSepGriffin Smith1-0/+15
Change-Id: I6e46bcdbf3b5258b60edb017709fee577eb8ec74 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6907 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-09-17 r/4894 test(tvix/eval): Add proptests covering trait impls for StringGriffin Smith1-0/+11
Add a suite of proptests covering the laws of the handwritten stdlib trait impls (Eq, Ord, and Hash) for String, generated from a new set of macros for generating those tests which can be applied to other types. Change-Id: Ib3276c9e96fca497aece094e5612707d3dc77ccd Reviewed-on: https://cl.tvl.fyi/c/depot/+/6626 Autosubmit: grfn <grfn@gws.fyi> Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-17 r/4893 test(tvix/eval): Test StringRepr::Smol as wellGriffin Smith1-1/+8
The From<String> impl for NixString only generates StringRepr::Heap strings, but we want to make sure we're testing StringRepr::Smol too Change-Id: I6d04b9cf12ef8462fe2788e0c6414b165f40311d Reviewed-on: https://cl.tvl.fyi/c/depot/+/6629 Autosubmit: grfn <grfn@gws.fyi> Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-17 r/4892 test(tvix/eval): impl Arbitrary for NixStringGriffin Smith1-0/+17
Change-Id: I3fe2d410c789429493a1278d571ca8fe74c2a69d Reviewed-on: https://cl.tvl.fyi/c/depot/+/6625 Autosubmit: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-09-15 r/4865 feat(tvix/eval): implement Value::coerce_to_path()sterni1-1/+8
This function is necessary for all builtins that expect some form of path as an argument. It is merely a wrapper around coerce_to_string that can shortcut if we already have a path. The absolute path check is done in the same way as in C++ Nix for compatibility, although it should probably be revised in the long term (think about Windows, for example). Since coercing to a path is not an operation possible in the language directly, this function can live in the builtins module as the only place it is required. Change-Id: I69ed5455c00d193fea88b8fa83e28907a761cab5 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6574 Autosubmit: sterni <sternenseemann@systemli.org> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-09-10 r/4786 fix(tvix/eval): fix doc comment syntax where applicableVincent Ambo1-10/+10
As pointed out by grfn on cl/6091 Change-Id: I28308577b7cf99dffb4a4fd3cc8783eb9ab4d0d6 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6460 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-07 r/4701 chore(tvix/eval): provide 'static references to "name"/"value"Vincent Ambo1-0/+3
These static strings show up a bunch when dealing with the internals of attribute sets, and having them available as static references is required. Due to the way const expressions are evaluated, taking a reference to the existing NixString::NAME / NixString::VALUE items does not work and the references themselves need to be const-evaluated. Change-Id: If6e75847af978118a3b266fe6a3242321722434d Reviewed-on: https://cl.tvl.fyi/c/depot/+/6366 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-04 r/4635 feat(tvix/eval): implement upvalue resolution in `with` scopesVincent Ambo1-0/+6
These need to be handled specially by the runtime if the compiler determines that a given local must be resolved via `with`. Note that this implementation has a bug: It currently allows `with` inside of nested lambdas to shadow statically known identifiers. This will be cleaned up in the next commit. Change-Id: If196b99cbd1a0f2dbb4a40a0e88cdb09a009c6b9 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6299 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-03 r/4607 fix(tvix/eval): correctly escape `${` in stringsVincent Ambo1-11/+15
Without this escape, it is possible for Nix to produce escaped representations which are not literal Nix values again. This was fixed in upstream Nix in https://github.com/NixOS/nix/pull/4012 (though only for eval, not in the REPL) and the updated test is picked from upstream after that commit. Because we run the C++ Nix tests against our test suite as well, this also bumps our custom Nix 2.3 to a commit that includes the cherry-picked fix from the PR above. Change-Id: I478547ade65f655c606ec46f7143932064192283 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6271 Reviewed-by: grfn <grfn@gws.fyi> Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-01 r/4568 fix(tvix/eval): fix several string escapingsVincent Ambo1-2/+4
These were missing an additional level of escaping, silly oversight caught by an upstream test. Change-Id: I0312084475e4b88c83945614e9aa5b34c6bc3ec2 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6232 Reviewed-by: sterni <sternenseemann@systemli.org> Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-08-30 r/4540 docs(tvix/eval): Use correct syntax for module doc commentsVincent Ambo1-3/+2
Change-Id: I35741856f34b86a538f226a8eaf8806edede60ec Reviewed-on: https://cl.tvl.fyi/c/depot/+/6207 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-08-27 r/4512 chore(tvix/eval): explicitly set #[repr(transparent)] on wrappersVincent Ambo1-0/+1
For representation wrappers that are used to control the visibility of type internals, this ensures that the wrapper does not increase the size of the type. In practice, the optimiser likely does this anyways but it is good to guarantee it. Change-Id: Ic6df7d668fe6006dfbd5b6cfcfc2088afa95b810 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6178 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
generated by cgit-pink 1.4.1 (git 2.44.2) at 2024-12-01T07·31+0000