about summary refs log tree commit diff
path: root/tvix/eval/src/value/attrs.rs
AgeCommit message (Collapse)AuthorFilesLines
2022-09-15 r/4864 feat(tvix/eval): Support builtins.attrNamesWilliam Carroll1-2/+10
Define `.len()` method on `NixAttrs` to preallocate the capacity of the result vector. Also anchor an errant comment to its context (I think). Change-Id: I268f15025d453d7b3ae1146558c80e51433dd2a8 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6546 Reviewed-by: wpcarro <wpcarro@gmail.com> Reviewed-by: sterni <sternenseemann@systemli.org> Autosubmit: wpcarro <wpcarro@gmail.com> Tested-by: BuildkiteCI
2022-09-13 r/4836 feat(tvix/eval): implement initial fancy formatting for errorsVincent Ambo1-5/+1
This very closely follows the way it's done for warnings, but errors have a lot more information available in some cases which we do not surface yet. Note also that due to requiring the `CodeMap`, this is not yet called from eval.rs as the way that is threaded through needs to be refactored, so only the method for reporting these errors as strings is implemented so far. Next steps for this will be to add a generic diagnostics module that reduces some of the boilerplate for this between warnings & errors, and which will also give us a good point in the future to switch to a fancier diagnostics crate. Change-Id: If6bb209f8e7a568d866e516a90335b9b2afbf66d Reviewed-on: https://cl.tvl.fyi/c/depot/+/6534 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-09-10 r/4787 fix(tvix/eval): reintroduce 'InvalidAttribuetName' error variantVincent Ambo1-1/+5
As pointed out by sterni in cl/6205, this is actually possible in syntactically valid expressions like { ${12 + 13} = 12; } Change-Id: Id8a1e3aceb551f288f9050c4eea563eb6572f1a7 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6461 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-10 r/4786 fix(tvix/eval): fix doc comment syntax where applicableVincent Ambo1-20/+23
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-08 r/4741 feat(tvix/eval): ensure all errors always carry a spanVincent Ambo1-8/+6
Previously error spans were optional because the information about code spans was not available at runtime. Now that this information has been added, the error type will always carry a span. This change is very invasive all throughout the codebase. This is due to the fact that many functions that are called *by* the VM expected to return `EvalResult`, but this no longer works as the span information is not available to those functions - only to the VM itself. To work around this the majority of these functions have been changed to return `Result<T, ErrorKind>` instead and an accompanying macro in the VM constructs the "real" error. Note that this implementatino currently has a bug where errors occuring within thunks will yield the location at which the thunk was forced, not the location at which the error occured within the code. This will be fixed soon, but the commit is large enough as is. Change-Id: Ib1ecb81a4d09d464a95ea7ea9e589f3bd08d5202 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6408 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-07 r/4706 fix(tvix/eval): address current clippy lintsVincent Ambo1-0/+1
Note that I've allowed `needless_lifetimes` for the attribute set iterator, as I find the type easier to understand with these annotations present. Change-Id: I33abb17837ee4813076cdb9a87f54bac4a37044e Reviewed-on: https://cl.tvl.fyi/c/depot/+/6373 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-07 r/4702 feat(tvix/eval): implement NixAttrs::iter()Vincent Ambo1-1/+74
Implementing iteration over NixAttrs requires a custom iterator type in order to encapsulate the different representations. The BTreeMap for example has its own iterator type which needs to be encapsulated. This is mostly boilerplate code, but for a change some simple unit tests have been added in. Change-Id: Ie13b063241d461b810876f95f53878388e918ef2 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6367 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-03 r/4616 refactor(tvix/eval): avoid cloning in NixAttrs::update if possibleVincent Ambo1-20/+27
Refactors the update function to take the attribute sets by value instead. To facilitate this, we use an equivalent of the currently unstable `Rc::clone_or_unwrap` in the VM when encountering attribute sets, so that in cases where the only references to the attrs being updated are the ones on the stack those clones are avoided completely. This does make update() a little bit more tricky internally, as some optimised branches can directly return the moved value, and others need to destructure with ownership. For this reason there are now two different match statements handling the different ownership cases. Change-Id: Ia77d3ba5c86afb75b9f1f51758bda61729ba5aab Reviewed-on: https://cl.tvl.fyi/c/depot/+/6279 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-03 r/4614 refactor(tvix/eval): slightly more readable AttrsRep::selectVincent Ambo1-11/+5
Suggestion from grfn in cl/6158. Change-Id: I16dcf2296a5ec5d299d5a080ca099b8eda6c254e Reviewed-on: https://cl.tvl.fyi/c/depot/+/6278 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-09-03 r/4611 refactor(tvix/eval): get rid of Value::Blackhole variantVincent Ambo1-2/+2
This is no longer needed for anything and the extra clone here is not really more costly than constructing a blackhole value in a different place. Change-Id: I5c63085b1b4418b629ea58a42e3bfe9a9b586d76 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6275 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-09-03 r/4608 fix(tvix/eval): address all current clippy lintsVincent Ambo1-1/+1
Change-Id: I758fc4f3b9078de7ca6228a75a4351c3e085c4cf Reviewed-on: https://cl.tvl.fyi/c/depot/+/6272 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-09-02 r/4601 refactor(tvix/eval): avoid a use of Value::BlackholeVincent Ambo1-2/+2
The blackhole allocation is not going to be cheaper than cloning this. Change-Id: Id3ad44812decb4392830be06645e67bb0a982b96 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6267 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-09-02 r/4597 refactor(tvix/eval): add NixAttrs::contains functionVincent Ambo1-0/+12
This avoids copying around the value more than needed. Change-Id: I35949d16dad7fb8f76e0f641eaccf48322144777 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6263 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-02 r/4592 refactor(tvix/eval): implement clearer mechanism for globalsVincent Ambo1-0/+8
The set of things that can leak out of `builtins` into the global scope is statically known (it is what Nix 2.3 leaks there, essentially). This is a mild change over the previous mechanism, where instead at the point where the `builtins` set is constructed we "lift" the globals out of there (if they exist). This way users will still eventually be able to add additional builtins, HOWEVER they will not be able to leak them into the global scope. Note that upstream Nix technically leaks _all_ builtins into the global scope using the `__*` prefix, but we are trying to avoid this in Tvix if it is not required in nixpkgs. Change-Id: Ie9dec2ce33740134f3b2464eba3749f421dd5953 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6258 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-01 r/4571 feat(tvix/eval): carry optional SyntaxNode in error typeVincent Ambo1-5/+7
This starts paving the way for nicer, source-code based error reporting. Right now the code paths in the VM do not emit annotated errors, as we do not yet preserve that structure from the compiler. However, error emitting code paths in the compiler have been amended to include known nodes. Change-Id: I1b74410ffd891c40cd913361bd73c4336ec8aa5b Reviewed-on: https://cl.tvl.fyi/c/depot/+/6235 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-08-30 r/4541 fix(tvix/eval): emit correct count in OpAttrPathVincent Ambo1-1/+1
Not sure how exactly this snuck in, but it caused some subtle breakages in deeply nested attribute sets. Change-Id: I8049ce912405d3750031f79cc8d86ff1c3c02c2b Reviewed-on: https://cl.tvl.fyi/c/depot/+/6208 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-7/+7
Change-Id: I35741856f34b86a538f226a8eaf8806edede60ec Reviewed-on: https://cl.tvl.fyi/c/depot/+/6207 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-08-30 r/4539 chore(tvix/eval): minor readability improvement in attrsVincent Ambo1-12/+9
Change-Id: If9d9eaf60934e96ec4b41c57818afe0c2a99c862 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6206 Reviewed-by: grfn <grfn@gws.fyi> Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-08-30 r/4538 refactor(tvix/eval): remove Error::InvalidKeyTypeVincent Ambo1-5/+1
We're confident that we're handling all branches that can reasonably occur from valid AST, any other cases should be considered a critical evaluator bug and panic rather than surfacing something that looks like user error. Change-Id: If96966eb32b8ff12fcaeb9ea3b0c8fc51b6abd11 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6205 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-08-30 r/4537 fix(tvix/eval): address various clippy lintsVincent Ambo1-17/+9
Change-Id: I3ea0f51475e80948adfeb5d1620c1f2665cc39bc Reviewed-on: https://cl.tvl.fyi/c/depot/+/6201 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-08-30 r/4536 feat(tvix/eval): implement attribute set equalityVincent Ambo1-2/+48
Change-Id: Ia25f02610f2575e5e7fca81643e05b40f4a07820 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6200 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-08-30 r/4535 fix(tvix/eval): `null` in dynamic attribute keys skips the elementVincent Ambo1-0/+7
This is actually *tested* behaviour in C++ Nix, so we need to implement it here, too. Change-Id: Ic4a4659a2f04cdd928cbe78a85dae90401515371 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6199 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-08-27 r/4514 refactor(tvix/eval): use `write!` macro instead of `f.write_fmt`Vincent Ambo1-3/+3
grfn pointed out in cl/6082 that this is actually the desugaring of the write! macro, so it doesn't make sense to write it out. Change-Id: If7c055b042ad22b034722aec1eaadba92736d684 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6180 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org> Reviewed-by: grfn <grfn@gws.fyi>
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>
2022-08-26 r/4498 refactor(tvix/value): replace static representation with SmolStrVincent Ambo1-3/+3
The only uses of the static variant were for `"name"` and `"value"`, which are both small enough to fit into a SmolStr. The size of NixString accomodates `String` anyways, so we may as well inline them. Additionally smol_str is already in the dependency graph because rnix uses it, and using it for representations of identifiers is sensible. Change-Id: I9969312256d1657d69128e54c47dc7294a18ce58 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6165 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org> Reviewed-by: grfn <grfn@gws.fyi>
2022-08-26 r/4492 feat(tvix/eval): implement attribute set access operatorVincent Ambo1-0/+25
Fairly straightforward, handling the optimised representations manually and otherwise delegating to BTreeMap. Note that parsing of raw identifiers is not yet implemented. Encountering an identifier node usually means that there is locals access going on, so we need a special case for compiling a node in such a way that an identifier's literal value ends up on the stack. Change-Id: I13fbab7ac657b17ef3f4c5859fe737c321890c8a Reviewed-on: https://cl.tvl.fyi/c/depot/+/6158 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org> Reviewed-by: grfn <grfn@gws.fyi>
2022-08-25 r/4481 refactor(tvix/value): ensure internal attrs representation is hiddenVincent Ambo1-44/+49
Wraps the attrs representation in an additional newtype struct with a private field in order to hide the representation from other modules. This is done in order to avoid accidental leakage of the internals outside of value::attrs. Change-Id: I68d1d02514aa0443df4c39801001a3f1f6cc5d5c Reviewed-on: https://cl.tvl.fyi/c/depot/+/6146 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-08-25 r/4475 feat(tvix/eval): implement attrset update (`//`) operatorVincent Ambo1-1/+54
The underlying implementation does a few tricks based on which pair of attrset representations is encountered. Particularly the effect of short-circuiting the empty cases might be relevant in nixpkgs/NixOS, due to the use of lib.optionalAttrs. Change-Id: I22b978b1c69af12926489a71087c6a6219c012f3 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6140 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-08-24 r/4466 fix(tvix/value): align Display representation with NixVincent Ambo1-6/+2
Displaying the optimised representation is not useful anymore. Change-Id: Icb962ff8865ec4207c144fbcb1aae87483b0fb7c Reviewed-on: https://cl.tvl.fyi/c/depot/+/6131 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-08-24 r/4462 fix(tvix/value): add ident_str representation of stringsVincent Ambo1-1/+1
When printing strings as identifiers (in attribute sets), the string should only be quoted and escaped if it contains escape characters. Change-Id: If2bcfa1e93dc8f00be4d7a57ec1d82fc679103c3 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6127 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi> Autosubmit: tazjin <tazjin@tvl.su>
2022-08-24 r/4457 feat(tvix/value): introduce string representation with &'static strVincent Ambo1-14/+8
For cases where the strings are statically known (such as the oft-occuring name/value), this can be a useful optimisation. It's also much more convenient in tests. Change-Id: Ie462b684805bd4986ea5e85ca4bff663bc2d3c3c Reviewed-on: https://cl.tvl.fyi/c/depot/+/6111 Tested-by: BuildkiteCI Reviewed-by: eta <tvl@eta.st>
2022-08-24 r/4456 test(tvix/value): add simple attrset construction testsVincent Ambo1-0/+3
These do not yet test nested attribute sets; we need to add some more inspection primitives first. Change-Id: Icfc99bf17c73ebefc0d882a84f0ca73ec688a54d Reviewed-on: https://cl.tvl.fyi/c/depot/+/6110 Reviewed-by: eta <tvl@eta.st> Tested-by: BuildkiteCI
2022-08-24 r/4455 feat(tvix/value): implement nested attribute set literalsVincent Ambo1-14/+44
With this change, nested attribute sets can now be created from literals. This required some logic for dealing with cases where at a deeper nesting point a literal attribute set was constructed from an optimised representation. For example, this is valid Nix code: ```nix { a = {}; # creates optimised empty representation a.b = 1; # wants to add a `b = 1` to it b = { name = "foo"; value = "bar"; }; # creates optimised K/V repr b.foo = 42; # wants to add an additional `foo = 42` } ``` In these cases, the attribute set must be coerced to a map representation first which is achieved by the new internal NixAttr::map_mut helper. Change-Id: Ia61d3d9d14c4e0f5e207c00f6a2f4daa3265afb2 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6109 Reviewed-by: eta <tvl@eta.st> Tested-by: BuildkiteCI
2022-08-24 r/4454 refactor(tvix/value): encapsulate attrset logic within value::attrsVincent Ambo1-0/+188
The internal optimisations of the set representation were previously leaking into the VM, which is highly undesirable. Keeping it encapsulated allows us to do additional optimisations within value::attrs without being concerned about its use in the VM. Change-Id: I7e7020bb0983b9d355d3db747b049b2faa60131f Reviewed-on: https://cl.tvl.fyi/c/depot/+/6108 Reviewed-by: eta <tvl@eta.st> Tested-by: BuildkiteCI
2022-08-13 r/4437 fix(tvix/value): KV struct needs to carry name as Value, tooVincent Ambo1-1/+2
Users may construct a pair that falls into the name/value optimisation but where `name` is not actually a string, as from the language perspective there is nothing special about this attribute set. We also can not conditionally apply this by forcing the key at this point, as this would change the language semantics. Therefore, the name in the optimised representation is also carried as `Value`. Change-Id: I5be8a4c98ba19ebdfb7203a929f714a04492512e Reviewed-on: https://cl.tvl.fyi/c/depot/+/6101 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-08-13 r/4423 feat(tvix/eval): add Value variants for strings & attrsetsVincent Ambo1-0/+6
Change-Id: Idebf663ab7fde3955aae50f635320f7eb6c353e8 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6087 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-08-13 r/4422 feat(tvix/eval): add module for attribute set implementationsVincent Ambo1-0/+36
Change-Id: I6002bd5e5596b93325dea6c862370ba5235c0f08 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6086 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>