about summary refs log tree commit diff
path: root/tvix/eval/src/value
AgeCommit message (Collapse)AuthorFilesLines
2022-10-29 r/5221 feat(tvix/eval): Implement comparison for listsGriffin Smith2-1/+24
Lists are compared lexicographically in C++ nix as of [0], and our updated nix test suites depend on this. This implements comparison of list values in `Value::nix_cmp` using a very similar algorithm to what C++ does - similarly to there, this requires passing in the VM so we can force thunks in the list elements as we go. [0]: https://github.com/NixOS/nix/commit/09471d2680292af48b2788108de56a8da755d661# Change-Id: I5d8bb07f90647a1fec83f775243e21af856afbb1 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7070 Autosubmit: grfn <grfn@gws.fyi> Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-10-28 r/5218 docs(tvix/eval): warn that AttrsRep::KV is not for Key-Value pairsAdam Joseph1-1/+8
I assumed that AttrsRep::KV represented attrsets with a single attribute as a Key-Value pair. That is not the case. Let's warn other people about this. Signed-off-by: Adam Joseph <adam@westernsemico.com> Change-Id: Ie3d2765fcc1ab705c153ab94ffe77bbd6d4ab39e Reviewed-on: https://cl.tvl.fyi/c/depot/+/7093 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-26 r/5200 feat(tvix/eval): add NixList::force_elements()Adam Joseph1-0/+5
This adds a function NixList::force_elements() which forces each element of the list shallowly. This behavior is needed for `builtins.replaceStrings`, and probably a few other builtins as well. Signed-off-by: Adam Joseph <adam@westernsemico.com> Change-Id: I3f0681acbbfe50e781b5f07b6a441647f5e6f8da Reviewed-on: https://cl.tvl.fyi/c/depot/+/7094 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-25 r/5198 feat(tvix/eval): add builtins.{floor,ceil}James Landrein1-0/+1
Change-Id: I4e6c4f96f6f5097a5c637eb3dbbd7bb8b34b7d52 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7032 Autosubmit: j4m3s <james.landrein@gmail.com> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su> Reviewed-by: grfn <grfn@gws.fyi>
2022-10-24 r/5193 refactor(tvix/eval): Implement value comparison with a methodGriffin Smith1-0/+21
Rather than implementing all of the interesting semantics of value comparison with a macro bound to the VM, implement the bulk of the logic with a method on Value itself that returns an Ordering, and then use the macro to implement the comparison against that Ordering. This has no functional change, but paves the way to implementing lexicographic comparison of list values, which is supported in the latest version of upstream nix. Change-Id: I8af1a020b41577021af5939f5edc160c407d4a9e Reviewed-on: https://cl.tvl.fyi/c/depot/+/7069 Autosubmit: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-24 r/5192 feat(tvix/eval): Implement builtins.mapAttrsGriffin Smith1-0/+10
I played around a little bit with doing this in-place, but ended up going with this perhaps slightly clone-heavy approach for now because ideally most clones on Value are cheap - but later we should benchmark alternate approaches that get to reuse allocations better if necessary or possible. Change-Id: If998eb2056cedefdf2fb480b0568ac8329ccfc44 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7068 Autosubmit: grfn <grfn@gws.fyi> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-24 r/5189 feat(nix/eval): Implement builtins.groupByGriffin Smith2-0/+23
Change-Id: I3e0aa017a7100cbeb86d2e5747471b36affcc102 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7038 Autosubmit: grfn <grfn@gws.fyi> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-23 r/5186 feat(tvix/eval): initial attempt at setting lambda namesVincent Ambo1-9/+7
When compiling a lambda, take the name of the outer slot (if available) and store it as the name on the lambda. These names are then shown in the observer, and nowhere else (so far). It is of course common for these things to thread through many different context levels (e.g. `f = a: b: c: ...`), in this setup only the outermost closure or thunk gains the name, but it's better than nothing. Change-Id: I681ba74e624f2b9e7a147144a27acf364fe6ccc7 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7065 Reviewed-by: grfn <grfn@gws.fyi> Autosubmit: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-23 r/5178 fix(tvix/eval): detect cycles when printing infinite valuesVincent Ambo4-24/+45
Using the same method as in Thunk::deep_force, detect cycles when printing values by maintaining a set of already seen thunks. With this, display of infinite values matches that of Nix: > nix-instantiate --eval --strict -E 'let as = { x = 123; y = as; }; in as' { x = 123; y = { x = 123; y = <CYCLE>; }; } > tvix-eval -E 'let as = { x = 123; y = as; }; in as' => { x = 123; y = { x = 123; y = <CYCLE>; }; } :: set Change-Id: I007b918d5131d82c28884e46e46ff365ef691aa8 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7056 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-10-22 r/5175 feat(tvix/eval): Implement builtins.deepSeqGriffin Smith3-6/+78
This is done via a new `deepForce` function on Value. Since values can be cyclical (for example, see the test-case), we need to do some extra work to avoid RefCell borrow errors if we ever hit a graph cycle: While deep-forcing values, we keep a set of thunks that we have already seen and avoid doing any work on the same thunk twice. The set is encapsulated in a separate type to stop potentially invalid pointers from leaking out. Finally, since deep_force is conceptually similar to `VM::force_for_output` (but more suited to usage in eval since it doesn't clone the values) this removes the latter, replacing it with the former. Co-Authored-By: Vincent Ambo <tazjin@tvl.su> Change-Id: Iefddefcf09fae3b6a4d161a5873febcff54b9157 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7000 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi> Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-19 r/5159 feat(tvix/eval): deduplicate overlap between Closure and ThunkAdam Joseph2-74/+99
This commit deduplicates the Thunk-like functionality from Closure and unifies it with Thunk. Specifically, we now have one and only one way of breaking reference cycles in the Value-graph: Thunk. No other variant contains a RefCell. This should make it easier to reason about the behavior of the VM. InnerClosure and UpvaluesCarrier are no longer necessary. This refactoring allowed an improvement in code generation: `Rc<RefCell<>>`s are now created only for closures which do not have self-references or deferred upvalues, instead of for all closures. OpClosure has been split into two separate opcodes: - OpClosure creates non-recursive closures with no deferred upvalues. The VM will not create an `Rc<RefCell<>>` when executing this instruction. - OpThunkClosure is used for closures with self-references or deferred upvalues. The VM will create a Thunk when executing this opcode, but the Thunk will start out already in the `ThunkRepr::Evaluated` state, rather than in the `ThunkRepr::Suspeneded` state. To avoid confusion, OpThunk has been renamed OpThunkSuspended. Thanks to @sterni for suggesting that all this could be done without adding an additional variant to ThunkRepr. This does however mean that there will be mutating accesses to `ThunkRepr::Evaluated`, which was not previously the case. The field `is_finalised:bool` has been added to `Closure` to ensure that these mutating accesses are performed only on finalised Closures. Both the check and the field are present only if `#[cfg(debug_assertions)]`. Change-Id: I04131501029772f30e28da8281d864427685097f Signed-off-by: Adam Joseph <adam@westernsemico.com> Reviewed-on: https://cl.tvl.fyi/c/depot/+/7019 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-19 r/5158 feat(tvix/eval): NixList::concat(): avoid an unnecessary moveAdam Joseph1-4/+3
In `a++b`, the previous implementation would move `b` (i.e. memcpy its elements) twice. Let's do that only once. We sure do call NixList.clone() a whole lot. At some point in the future we probably want to do a SmolStr-type split for NixList into a two-variant enum where one side is an Rc<Vec<Value>> for lists longer than a certain length. Signed-off-by: Adam Joseph <adam@westernsemico.com> Change-Id: I32154d18785a1f663454a8b9d4afd3e78bffdf9c Reviewed-on: https://cl.tvl.fyi/c/depot/+/7040 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-17 r/5155 feat(nix/eval): Implement builtins.functionArgsGriffin Smith1-0/+4
Now that we're tracking formals on Lambda this ends up being quite easy; we just pull them off of the Lambda for the argument closure and use them to construct the result attribute set. Change-Id: I811cb61ec34c6bef123a4043000b18c0e4ea0125 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7003 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-17 r/5154 feat(tvix/eval): Validate closed formalsGriffin Smith2-5/+73
Validate "closed formals" (formal parameters without an ellipsis) via a new ValidateClosedFormals op, which checks the arguments (in an attr set at the top of the stack) against the formal parameters on the Lambda in the current frame, and returns a new UnexpectedArgument error (including the span of the formals themselves!!) if any arguments aren't allowed Change-Id: Idcc47a59167a83be1832a6229f137d84e426c56c Reviewed-on: https://cl.tvl.fyi/c/depot/+/7002 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-17 r/5153 feat(tvix/eval): Record formals on lambdaGriffin Smith3-2/+22
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-17 r/5151 docs(tvix/eval) comments for various fieldsAdam Joseph2-0/+15
Change-Id: I8dcddf2b419761e475e71215c199eef2f7dc61dc Reviewed-on: https://cl.tvl.fyi/c/depot/+/7028 Autosubmit: Adam Joseph <adam@westernsemico.com> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-16 r/5146 refactor(tvix/eval): make OpFindFile use internal UnresolvedPathsterni1-2/+6
To assert that OpFindFile is only emitted for specially compiled SPATH expressions, as well as make sure it doesn't accidentally operate on “ordinary values”, introduce an UnresolvedPath internal value. If OpFindFile sees a non-UnresolvedPath value, it'll crash. Note that this change is not done purely for OpFindFile: We may want to compile SPATH expressions as function calls to __findFile (like C++ Nix does) in the future, so the UnresolvedPath value would definitely need to be an ordinary string again then. Rather, this change is done in preparation for resolving home dir relative paths at runtime (since they depend on the environment) for which we'll need a similar mechanism to OpFindFile. Change-Id: I6acf287f35197cd9e13377079f972b9d36e5b22e Reviewed-on: https://cl.tvl.fyi/c/depot/+/7023 Autosubmit: sterni <sternenseemann@systemli.org> Reviewed-by: Adam Joseph <adam@westernsemico.com> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-16 r/5142 feat(tvix/eval): remove Clone instance from Chunk and LambdaAdam Joseph1-1/+1
Lambda has a quite large and variable-sized runtime representation, unlike Rc<Lambda>. It would be easy to accidentally call clone() on this and create input-dependent performance regressions. Nothing in the codebase is currently using Lambda.clone(). Let's remove the derived instance. If it's really needed it is very easy to add it back in, but whoever does that will have to look at the struct they're adding Clone to, which will hopefully prompt them to think about whether or not that's really what they want to do. Change-Id: I7806a741862ab4402229839ce3f4ea75aafd6d12 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7029 Autosubmit: Adam Joseph <adam@westernsemico.com> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-16 r/5140 refactor(tvix/eval) remove Value::DynamicUpvalueMissingAdam Joseph1-6/+0
I believe this variant is left over from a previous implementation. If not, please let me know. Signed-off-by: Adam Joseph <adam@westernsemico.com> Change-Id: I02a3bf2f63794d09e96a5a92a034c0ad3d1ff221 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7027 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-15 r/5135 feat(tvix/eval): Implement builtins.fromJSONGriffin Smith2-4/+58
Using `serde_json` for parsing JSON here, plus an `impl FromJSON for Value`. The latter is primarily to stay "dependency light" for now - likely going with an actual serde `Deserialize` impl in the future is going to be way better as it allows saving significantly on intermediary allocations. Change-Id: I152a0448ff7c87cf7ebaac927c38912b99de1c18 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6920 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-15 r/5132 feat(tvix/eval): implement builtins.dirOfAdam Joseph1-0/+1
This commit causes the test eval-okay-builtins.nix to pass. It also adds tests/tvix_tests/eval-okay-dirof.nix which has better coverage than the nix tests for this builtin. Signed-off-by: Adam Joseph <adam@westernsemico.com> Change-Id: I71d96b48680696fd6e4fea3a9861742b35cfaa66 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6987 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-13 r/5119 refactor(tvix/eval): factor out all calls to canon_pathAdam Joseph2-0/+16
Right now we're pretending that the Rust library path_clean does the same thing that cppnix's canonPath() does. This is not true. It's close enough for the test suite, but may come back to bite us. Let's create our own canon_path() function and call that in all the places where we intend to match the behavior of cppnix's canonPath(). That way when we fix this we can fix it once, in one place. Signed-off-by: Adam Joseph <adam@westernsemico.com> Change-Id: Ia6f9577f62f49ef352ff9cfa5efdf37c32d31b11 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6993 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-10-12 r/5111 fix(tvix/eval): remove overlapping instancesAdam Joseph1-12/+0
I broke the build. I didn't understand that before hitting "submit" you need to re-test your changes on latest HEAD (and that CI doesn't do that for you); I failed to re-test cl/6912 following the merge of cl/6907. This commit fixes the build by removing the overlapping instances. Change-Id: I2a720d2c60cc7103b350f78102a8998f93bac828 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6965 Tested-by: BuildkiteCI Autosubmit: Adam Joseph <adam@westernsemico.com> Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-12 r/5110 feat(tvix/eval): From<Into<NixString>> for ValueAdam Joseph1-0/+9
Signed-off-by: Adam Joseph <adam@westernsemico.com> Change-Id: I27615baf4104ab96600708b12a780599a3714bae Reviewed-on: https://cl.tvl.fyi/c/depot/+/6912 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-10 r/5097 fix(tvix/eval): Actually trace spans for thunksGriffin Smith1-18/+30
Currently, the span on *all* thunk force errors is the span at which the thunk is forced, which for recursive thunk forcing ends up just being the same span over and over again. This changes the span on thunk force errors to be the span at which point the thunk is *created*, which is a bit more helpful (though the printing atm is a little... crowded). To make this work, we have to thread through the span at which a thunk is created into a field on the thunk itself. Change-Id: I81474810a763046e2eb3a8f07acf7d8ec708824a Reviewed-on: https://cl.tvl.fyi/c/depot/+/6932 Autosubmit: grfn <grfn@gws.fyi> Reviewed-by: Adam Joseph <adam@westernsemico.com> 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/5094 refactor(tvix/eval): after calling, the caller has to popVincent Ambo2-5/+5
Previously the various call functions either returned `EvalResult<()>` or `EvalResult<Value>`, which was confusing. Now only vm::call_with returns a Value directly, and other parts of the API just leave the stack top in the post-call state. This makes it easier to reason about what's going on in non-tail-call cases (which are making a comeback). Change-Id: I264ffc683a11aca72dd06e2220a5ff6e7c5fc2b0 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6936 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-10-10 r/5087 feat(tvix/eval): Initial resolution of `<...>` pathsGriffin Smith3-7/+14
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/5085 feat(tvix/eval): Allow adding strings to pathsGriffin Smith1-0/+6
Implement adding paths and strings via OpAdd. Since the nix rules are quite obscure, I'm electing to test this one with an oracle test to avoid the danger of getting the actual asserted result wrong. Change-Id: Icdcca3690ca2e8459e386c1f29cc48eaaa39e9a3 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6914 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 Smith2-0/+21
Change-Id: I6e46bcdbf3b5258b60edb017709fee577eb8ec74 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6907 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-10 r/5083 feat(tvix/eval): Support builtins.readDirWilliam Carroll1-0/+6
Co-authored-by: Griffin Smith <root@gws.fyi> Change-Id: I5ff19efbe87d8f571f22ab0480500505afa624c5 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6552 Autosubmit: wpcarro <wpcarro@gmail.com> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-10 r/5080 feat(tvix/eval): Implement builtins.elemGriffin Smith1-0/+6
Change-Id: Id99c1d33f87ad9866990d3483d3531e9e48f861f Reviewed-on: https://cl.tvl.fyi/c/depot/+/6916 Autosubmit: grfn <grfn@gws.fyi> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-10 r/5075 feat(tvix/eval): Implement builtins.genListGriffin Smith1-0/+6
Change-Id: Iabe28656229f508226b244d81382e517961eb3cf Reviewed-on: https://cl.tvl.fyi/c/depot/+/6901 Autosubmit: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-08 r/5067 refactor(tvix/eval): Encapsulate Value::Attrs constructionGriffin Smith2-2/+10
Factor out the construction of Value::Attrs (including the Rc) into a new `attrs` constructor function, to abstract away the presence of the Rc itself. Change-Id: I42fd4c3841e1db368db999ddd651277ff995f025 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6892 Autosubmit: grfn <grfn@gws.fyi> Reviewed-by: sterni <sternenseemann@systemli.org> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-08 r/5066 fix(tvix/eval): Force thunks when comparing against ground valsGriffin Smith1-2/+8
Thunks correctly force when comparing for equality against other thunks, but weren't being forced correctly when comparing against non-thunk values, in either direction. Change-Id: Ia03702895ec4d70aed3445c1b0a9a7a641d1a300 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6897 Autosubmit: grfn <grfn@gws.fyi> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-06 r/5040 refactor(tvix/eval): builtins now contain closuresVincent Ambo1-7/+15
For some upcoming builtins (notably, import) we need to capture arguments in the builtin's implementation. To allow this, we can no longer use function pointers for builtins, but must use a reference-counted closure object instead. Unfortunately this adds an extra pointer operation to every builtin call. We should benchmark this later against having a split builtin representation. Change-Id: I109d98d0e25998870542f47573eb1ec2e546f2a2 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6856 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-10-03 r/5021 refactor(tvix/eval): implement IntoIterator for NixListVincent Ambo1-4/+9
This is the same code as before, just moved into a trait impl to gain access to stuff that needs IntoIterator Change-Id: Iff9375cd05593dd2681fa85ccc7f4554bf944a02 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6842 Autosubmit: tazjin <tazjin@tvl.su> Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-29 r/4990 chore(tvix/eval): remove existing nested key implementationVincent Ambo2-79/+2
This implementation, which only ever worked for non-recursive attribute sets, is no longer needed and thus removed here. We have a new implementation of these nested keys coming up instead. Change-Id: I0c2875154026a4f5f6e0aa038e465f54444bf721 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6783 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-20 r/4943 refactor(tvix/eval): add VM::call_value helper methodVincent Ambo1-17/+3
This makes it possible to call a callable value (builtin or closure/lambda) directly, without unwrapping it first. This is needed for pretty much all higher-order functions to work correctly. This is mostly equivalent to the previous code in coerce_to_string for calling `__toString`, except it expects the argument(s) to already be placed on the stack. Note that the span for the `NotCallable` error is not currently guaranteed to make any sense, will experiment with this. Change-Id: I821224368d438a28900858b343defc1817e46a0a Reviewed-on: https://cl.tvl.fyi/c/depot/+/6717 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-20 r/4942 test(tvix/eval): Make proptests a bit smallerGriffin Smith2-4/+4
Generate smaller recursive values for generated Values, and run fewer cases for the attrs proptests which are particularly egregious. Change-Id: Ia35c7c120270feaf045be1deb440c87ebb185c27 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6716 Autosubmit: grfn <grfn@gws.fyi> Reviewed-by: tazjin <tazjin@tvl.su> Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-20 r/4940 chore(tvix/eval): clean up this one weird trick ...Vincent Ambo1-3/+0
... Rust developers don't want you to know about! Change-Id: Ic43b670ac0982c726bfb8cd27cb57b17934e4b70 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6715 Reviewed-by: grfn <grfn@gws.fyi> Reviewed-by: sterni <sternenseemann@systemli.org> Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
2022-09-19 r/4918 refactor(tvix/eval): handle forcing in Builtin::applysterni1-4/+12
Instead of arity, we pass a array reference to Builtin::new that describes how many arguments there are and which of them need to be forced, eliminating the need to force manually. Note that this change doesn't fix some of the instances where the the Builtin doesn't consider that the value could be a Thunk. Change-Id: Iadb58bb79886c30dc6b09dcf0ffad8abf28036a1 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6662 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-09-18 r/4914 refactor(tvix/eval): Simplify forcing in builtinsGriffin Smith1-0/+33
Refactor the `force!` macro to a method on `Value` which returns a smart-pointer-esque type, which simplifies the callsite and eliminates rightward drift, especially for high-arity builtins. Change-Id: I97a7837580accfb4bbd03b24f2acdbd38645efa5 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6656 Autosubmit: grfn <grfn@gws.fyi> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-09-18 r/4910 fix(tvix/eval): Force thunks during equality comparisonGriffin Smith1-5/+8
Thunks might be encountered deep in equality comparison (eg nested inside a list or attr-set), at which point we need to force them in order to compare them for equality (or else we panic when trying to get at their value). Fixes: b/192 Change-Id: I912151085f8298f30d5214c7965251c9266443f2 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6652 Autosubmit: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-09-18 r/4909 chore(tvix/eval): Pass in VM to nix_eqGriffin Smith4-19/+54
Pass in, but ignore, a mutable reference to the VM to the `nix_eq` functions, in preparation for using that VM to force thunks during comparison. Change-Id: I565435d8dfb33768f930fdb5a6b0fb1365d7e161 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6651 Autosubmit: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-09-18 r/4908 refactor(tvix/eval): Don't (ab)use PartialEq for Nix equalityGriffin Smith7-113/+191
Using rust's PartialEq trait to implement Nix equality semantics is reasonably fraught with peril, both because the actual laws are different than what nix expects, and (more importantly) because certain things actually require extra context to compare for equality (for example, thunks need to be forced). This converts the manual PartialEq impl for Value (and all its descendants) to a *derived* PartialEq impl (which requires a lot of extra PartialEq derives on miscellanious other types within the codebase), and converts the previous nix-semantics equality comparison into a new `nix_eq` method. This returns an EvalResult, even though it can't currently return an error, to allow it to fail when eg forcing thunks (which it will do soon). Since the PartialEq impls for Value and NixAttrs are now quite boring, this converts the generated proptests for those into handwritten ones that cover `nix_eq` instead Change-Id: If3da7171f88c22eda5b7a60030d8b00c3b76f672 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6650 Autosubmit: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-09-18 r/4906 test(tvix/eval): Add Eq-laws tests for NixAttrsGriffin Smith1-0/+11
As before, this limits the cases to a relatively small number because otherwise things get quite large. Change-Id: I5371dc56418fca52e1dd1d905b20868f647091ba Reviewed-on: https://cl.tvl.fyi/c/depot/+/6649 Autosubmit: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-09-18 r/4905 test(tvix/eval): Add tests for the Eq laws of ValueGriffin Smith1-0/+16
Only running 20 cases for now, since Value can get quite big if you let it run for a while. Change-Id: I09ef19da22c789c4869793836c98937c44595340 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6648 Autosubmit: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-09-18 r/4904 fix(tvix/value): Properly match on Path for PartialEqGriffin Smith1-0/+1
this was found by proptests! Change-Id: I16d6a6ece3b20cdddd6f78c94cc87befb1b651e6 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6647 Autosubmit: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-09-18 r/4902 test(tvix/eval): impl Arbitrary for ValueGriffin Smith4-0/+128
Impl Arbitrary for Value (and NixAttrs and NixList) in the same way we did for NixString. Value currently only generates non-"internal" values (no thunks, AttrNotFound, etc.) and can't generate functions (builtins or closures), because those'd require full generation of tvix bytecode, which is a bit more work than I'd like to do now - there's a `todo!` left in the code for a place where we could allow opting-in to internal values and functions later. Change-Id: I07a59e2b1d89cfaa912d4ecebd642caf4ddb040a Reviewed-on: https://cl.tvl.fyi/c/depot/+/6627 Autosubmit: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
generated by cgit-pink 1.4.1 (git 2.44.2) at 2024-12-19T13·13+0000