about summary refs log tree commit diff
path: root/tvix/eval/src/value
AgeCommit message (Collapse)AuthorFilesLines
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>
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-16 r/4881 refactor(tvix/eval): fix current clippy lintssterni1-8/+5
Change-Id: I88482453a62955515a0dcc0b243351b2bbac5236 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6618 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-09-15 r/4867 feat(tvix/eval): Support builtins.bitAndWilliam Carroll1-0/+1
Bitwise `and` on integers. Change-Id: I9f2a9182a057af26906683acd97a40dfabbdded8 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6548 Reviewed-by: wpcarro <wpcarro@gmail.com> Autosubmit: wpcarro <wpcarro@gmail.com> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-09-15 r/4865 feat(tvix/eval): implement Value::coerce_to_path()sterni2-1/+10
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-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-15 r/4862 feat(tvix/eval): Support builtins.headWilliam Carroll1-0/+4
TL;DR: - support `builtins.head` - define `ErrorKind::IndexOutOfBounds` and canonical error code - support basic unit tests Change-Id: I859107ffb4e220cba1be8c2ac41d1913dcca37ff Reviewed-on: https://cl.tvl.fyi/c/depot/+/6544 Reviewed-by: wpcarro <wpcarro@gmail.com> Autosubmit: wpcarro <wpcarro@gmail.com> Reviewed-by: sterni <sternenseemann@systemli.org> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-09-15 r/4857 feat(tvix/eval): implement correct toString behaviorsterni1-0/+142
Implement C++ Nix's `EvalState::coerceToString` minus some of the Path / store handling. This is currently only used for `toString` which does all possible coercions, but we've already prepared the weaker coercion variant which is e.g. used for builtins that expect string arguments. `EvalState::coerceToPath` is still missing for builtins that need a path, but it'll be easy to build on top of this. Change-Id: I78d15576b18921791d04b6b1e964b951fdef22c6 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6571 Autosubmit: sterni <sternenseemann@systemli.org> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-09-13 r/4845 fix(tvix/eval): add branch for directly comparing two thunksVincent Ambo1-0/+1
Pointed out by sterni in cl/6370 Change-Id: I324d8049a2702ced8f30ad43a64d63ae79dd0eab Reviewed-on: https://cl.tvl.fyi/c/depot/+/6569 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-13 r/4839 fix(tvix/eval): address current clippy lintsVincent Ambo1-4/+4
Change-Id: I5288849d0e93511b0b5664fa92f1c6882e4a1356 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6563 Reviewed-by: sterni <sternenseemann@systemli.org> 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-11 r/4800 refactor(tvix/eval): introduce Upvalues struct in closures & thunksVincent Ambo2-12/+20
This struct will be responsible for tracking upvalues (and is a convenient place to introduce optimisations for reducing value clones) instead of a plain value vector. The main motivation for this is that the upvalues will have to capture the `with`-stack fully and I want to avoid duplicating the logic for this between the two capturing types. Change-Id: I6654f8739fc2e04ca046e6667d4a015f51724e99 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6485 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
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 Ambo3-31/+34
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/4769 fix(tvix/eval): hold thunk borrow as shortly as possibleVincent Ambo1-7/+6
At the point where control flow exits Thunk::force (which may be due to recursing), it is vital that there is no longer a borrow to the inner thunk representation, otherwise this can cause accidental infinite recursion (which will be detected, but cause failures on valid code). Change-Id: I2846f3142830ae3110a4f5d2299e9d7928634504 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6436 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-08 r/4749 fix(tvix/eval): don't panic when printing a black holeVincent Ambo1-2/+6
This could occur when the disassembler is enabled and tracing the runtime while a thunk is being evaluated, as it would not be possible for the *tracer* to borrow the thunk at this exact moment. However, we know that if the borrowing fails here we are dealing with a not-fully evaluated thunk (blackhole), which should just print the internal representation. Change-Id: I4bdb4f17818d55795368e3d28842048f488f0a91 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6416 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-08 r/4748 refactor(tvix/eval): return call frame result from VM::callVincent Ambo1-2/+2
Previously, "calling" (setting up the VM run loop for executing a call frame) and "running" (running this loop to completion) were separate operations. This was basically an attempt to avoid nesting `VM::run` invocations. However, doing things this way introduced some tricky bugs for exiting out of the call frames of thunks vs. builtins & closures. For now, we unify the two operations and always return the value to the caller directly. For now this makes calls a little less effective, but it gives us a chance to nail down some other strange behaviours and then re-optimise this afterwards. To make sure we tackle this again further down I've added it to the list of known possible optimisations. Change-Id: I96828ab6a628136e0bac1bf03555faa4e6b74ece Reviewed-on: https://cl.tvl.fyi/c/depot/+/6415 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-08 r/4745 refactor(tvix/eval): add macros for generating Value castersVincent Ambo1-62/+42
The casting methods of `Value` are pretty verbose, and actually incorrect before this commit as they did not account for inner thunk values. To address this, we first attempt to make them correct by introducing a standard macro to generate them and traverse the inner thunk(s) if necessary. This is likely to be a performance hit as it will now involve more cloning of values. We can do multiple things to alleviate this, but should do some measurements first. Change-Id: If315d6e2afe7b69db727df535bc6cbfb89a691aa Reviewed-on: https://cl.tvl.fyi/c/depot/+/6412 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-08 r/4744 refactor(tvix/eval): pass a VM reference to builtinsVincent Ambo1-4/+14
This makes it possible for builtins to force values on their own, without the VM having to apply a strictness mask to the arguments first. Change-Id: Ib49a94e56ca2a8d515c39647381ab55a727766e3 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6411 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-08 r/4742 fix(tvix/eval): thread thunk forcing errors through correctlyVincent Ambo1-3/+3
With this, if an error occurs while forcing a thunk (which is very likely) it is threaded through to the top by wrapping it in the ErrorKind::ThunkForce variant. We could use this to generate "stacktrace-like" error output if we wanted, or simply jump through and discard everything except the innermost error. Change-Id: I3c1c8708c2f73ae062815adf490ce935b1979da8 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6409 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-08 r/4741 feat(tvix/eval): ensure all errors always carry a spanVincent Ambo4-64/+39
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/4740 feat(tvix/eval): Support builtins.lengthWilliam Carroll2-0/+15
Get the length of a list Change-Id: I41d91e96d833269541a1b3c23b7cc879f96d1e5a Reviewed-on: https://cl.tvl.fyi/c/depot/+/6407 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
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/4704 feat(tvix/eval): implement NixList::iterVincent Ambo1-0/+4
This does not require a custom iterator type (for now?) Change-Id: I5beb194bd8629571bd4040c69c977c27149807fa Reviewed-on: https://cl.tvl.fyi/c/depot/+/6371 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-07 r/4703 fix(tvix/eval): thread Display & PartialEq through to thunk valuesVincent Ambo2-1/+21
If a thunk is already evaluated, there are cases where due to the memoisation implementation something might observe a value wrapped in a thunk. In these cases, the implementation of `Display` and `PartialEq` must delegate to the underlying value. Note that there are a handful of other cases like these which we need to cover. It is a little tricky to write integration tests for these directly, especially as some of the open-upvalue optimisations coming down the pipe will reduce the number of observable thunks. One test that covers a part of this behaviour is currently disabled (needs some more machinery), but it's being brought back in the next commits. Change-Id: Iaa8cd338c12236af844bbc99d8cec2205f0d0095 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6370 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-07 r/4702 feat(tvix/eval): implement NixAttrs::iter()Vincent Ambo2-1/+121
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-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-07 r/4700 refactor(tvix/eval): encapsulate all thunk-forcing logic in moduleVincent Ambo1-20/+41
The VM previously took care of repeatedly forcing a thunk until it reached an evaluated state. This logic is now encapsulated inside of the `Thunk::force` implementation. In addition, force no longer returns a reference to the value by default, leaving it up to callers to decide whether they want to borrow the value or not (a helper is provided for this). Change-Id: I2aa7da922058ad1c57fbf8bfc7785aab7971c02b Reviewed-on: https://cl.tvl.fyi/c/depot/+/6365 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-07 r/4688 feat(tvix/eval): implement OpForce in VMVincent Ambo1-1/+31
This operation forces the evaluation of a thunk. There is some potential here for making an implementation that avoids some copies, but the thunk machinery is tricky to get right so the first priority is to make sure it is correct by keeping the implementation simple. Change-Id: Ib381455b02f42ded717faff63f55afed4c8fb7e3 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6352 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-06 r/4682 fix(tvix/eval): allocate Thunk::upvalues with known capacityVincent Ambo1-1/+1
The capacity (i.e. number of builtins) is known from the lambda, so we can size it correctly right away. Change-Id: Iab0b5a3f47d450fa9866c091ebbbed935b934907 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6351 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-06 r/4677 refactor(tvix/eval): introduce UpvalueCarrier traitVincent Ambo2-18/+40
This trait abstracts over the commonalities of upvalue handling between closures and thunks. It allows the VM to simplify the code used for setting up upvalues, without duplicating between the two different types. Note that this does not yet refactor the VM code to optimally make use of this. Change-Id: If8de5181f26ae1fa00d554f1ae6ea473ee4b6070 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6347 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-06 r/4676 refactor(tvix/eval): simplify thunk representationsVincent Ambo1-7/+7
For now, do not distinguish between closing and non-closing thunks, it will make the initial implementation easier. See Knuth etc. Change-Id: I0bd51e0f89f2c77e90bac63b507e5027b649e3d8 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6346 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-06 r/4673 feat(tvix/eval): introduce Value::Thunk variantVincent Ambo2-1/+61
Introduces the representation of runtime thunks, that is lazily evaluated values. Their representation is very similar to closures. Change-Id: I24d1ab7947c070ae72ca6260a7bbe6198bc8c7c5 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6343 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-06 r/4667 feat(tvix/eval): implement OpFinalise instructionVincent Ambo1-0/+11
This instruction finalises the initialisation of deferred upvalues in closures (and soon, thunks). The compiler does not yet emit this instruction, some more accounting is needed for that. Change-Id: Ic4181b26e19779e206f51e17388559400da5f93a Reviewed-on: https://cl.tvl.fyi/c/depot/+/6337 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-06 r/4666 feat(tvix/eval): set up deferred upvalues at runtimeVincent Ambo1-1/+5
This will leave a sentinel value in the upvalue slot in which the actual value is to be captured after resolution once a scope is fully set up. Change-Id: I12b37b0dc8d32603b03e675c3bd039468e70b354 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6336 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-06 r/4659 fix(tvix/eval): instantiate *new* closures from blueprints each timeVincent Ambo2-5/+8
The previous closure refactoring introduced a bug in which the same closure object would get mutated constantly for each instance of a closure, which is incorrect behaviour. This commit instead introduces an explicit new Value variant for the internal "blueprint" that the compiler generates (essentially just the lambda) and uses this variant to construct the closure at runtime. If the blueprint ever leaks out to a user somehow that is a critical bug and tvix-eval will panic. As a ~treat~ test for this, the fibonacci function is being used as it is a self-recursive closure (i.e. different instantiations of the same "blueprint") getting called with different values and it's good to have it around. Change-Id: I485de675e9bb0c599ed7d5dc0f001eb34ab4c15f Reviewed-on: https://cl.tvl.fyi/c/depot/+/6323 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-06 r/4658 fix(tvix/eval): correctly thread through dynamic upvaluesVincent Ambo1-1/+7
This puts together the puzzle pieces for threading dynamic upvalues (that is, upvalues resolved from the `with`-stack) all the way through. Reading the test case enclosed in this commit and walking through it is recommended to understand what problem is being tackled here. In short, because the compiler can not statically know *which* with-scope a dynamic argument is resolved from it needs to lay the groundwork for resolving from *all* possible scopes. There are multiple different approaches to doing this. The approach chosen in this commit is that if a dynamic upvalue is detected, the compiler will emit instructions to close over this dynamic value in *all* enclosing lambda contexts. It uses a new instruction for this that will leave around a sentinel value in case an identifier could not be resolved, and wire the location of this found value (or sentinel) up through the upvalues to the next level of nesting. In this tradeoff, tvix potentially closes over more upvalues than are needed (but in practice, how often do people create *really* deep `with`-stacks? and in *this* kind of code situation? maybe we should even warn for this!) but avoids keeping the entire attribute sets themselves around. Looking at the test case, each surrounding closure will close over *all* dynamic identifiers that are referenced later on visible to it, but only the last one for each identifier will actually end up being used. This also covers our bases for an additional edge-case this creates, in which an identifier potentially resolves to a dynamic upvalue *and* to a dynamic value within the function's own scope (again, would anyone really do this?) by introducing a resolution instruction for that particular case. There is likely some potential for cleaning up this code which is quite ugly in some parts, but as this implementation is now carefully calibrated to work I decided it is time to commit it and clean it up in subsequent commits. Change-Id: Ib701e3e6da39bd2c95938d1384036ff4f9fb3749 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6322 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-06 r/4651 refactor(tvix/eval): encapsulate internal mutability within ClosureVincent Ambo1-7/+30
This is required to efficiently construct the upvalue array at runtime, as there are situations where during Closure construction multiple things already have a reference to the closure (e.g. a self-reference). Change-Id: I35263b845fdc695dc873de489f5168d39b370f6a Reviewed-on: https://cl.tvl.fyi/c/depot/+/6312 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-04 r/4635 feat(tvix/eval): implement upvalue resolution in `with` scopesVincent Ambo2-0/+17
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/4626 feat(tvix/eval): add Value::to_closureVincent Ambo1-0/+11
... same as the others Change-Id: I9c8868388c10b0b6484c5bdd3799d801296c6979 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6292 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-09-03 r/4625 feat(tvix/eval): compile creation of closure objectsVincent Ambo1-1/+11
Fully implements the instructions for compiling closure objects (without runtime handling yet). Closure (and thunk) objects are created at runtime by capturing all known upvalues. To represent this, the instructions for creating them need to have a variable number of arguments. Due to that, this commit introduces new variants in OpCode that are not actually operations, but data. If the VM is implemented correctly, the instruction pointer should never point at these. Due to this, the VM will panic if it sees a data operand during an execution run. Change-Id: Ic56b49b3a42736dc437751e76df0e89c8d0619c6 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6291 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-09-03 r/4623 feat(tvix/eval): implement compilation of upvalue accessVincent Ambo1-0/+2
This adds a new upvalue tracking structure in the compiler to resolve upvalues and track their positions within a function when compiling a closure. The compiler will emit runtime upvalue access instructions after this commit, but the creation of the runtime closure object etc. is not yet wired up. Change-Id: Ib0c2c25f686bfd45f797c528753068858e3a770d Reviewed-on: https://cl.tvl.fyi/c/depot/+/6289 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
generated by cgit-pink 1.4.1 (git 2.44.2) at 2024-11-05T18·37+0000