about summary refs log tree commit diff
path: root/tvix/eval/docs (follow)
AgeCommit message (Collapse)AuthorFilesLines
2023-06-20 r/6336 fix(tvix/eval): only finalise formal arguments if defaultingsterni1-0/+10
When dealing with a formal argument in a function argument pattern that has a default expression, there are two different things that can happen at runtime: Either we select its value from the passed attribute successfully or we need to use the default expression. Both of these may be thunks and both of these may need finalisers. However, in the former case this is taken care of elsewhere, the value will always be finalised already if necessary. In the latter case we may need to finalise the thunk resulting from the default expression. However, the thunk corresponding to the expression may never end up in the local's stack slot. Since finalisation goes by stack slot (and not constants), we need to prevent a case where we don't fall back to the default expression, but finalise anyways. Previously, we worked around this by making `OpFinalise` ignore non-thunks. Since finalisation of already evaluated thunks still crashed, the faulty compilation of function pattern arguments could still cause a crash. As a new approach, we reinstate the old behavior of `OpFinalise` to crash whenever encountering something that is either not a thunk or doesn't need finalisation. This can also help catching (similar) miscompilations in the future. To then prevent the crash, we need to track whether we have fallen back or not at runtime. This is done using an additional phantom on the stack that holds a new `FinaliseRequest` value. When it comes to finalisation we check this value and conditionally execute `OpFinalise` based on its value. Resolves b/261 and b/265 (partially). Change-Id: Ic04fb80ec671a2ba11fa645090769c335fb7f58b Reviewed-on: https://cl.tvl.fyi/c/depot/+/8705 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Autosubmit: sterni <sternenseemann@systemli.org>
2023-06-15 r/6315 chore(tvix/eval): fix markdown labeled link syntaxsterni1-2/+2
Change-Id: I639dc0801090eaba56b61858e28204b5a0e631b6 Reviewed-on: https://cl.tvl.fyi/c/depot/+/8784 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Autosubmit: sterni <sternenseemann@systemli.org>
2023-03-14 r/5994 docs(tvix/eval): suggested layout adjustment to VM loop diagramAdam Joseph1-11/+11
Change-Id: I5467cd66801ad8fe6c4ec0ae337763f1762cea1c Reviewed-on: https://cl.tvl.fyi/c/depot/+/8252 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Autosubmit: Adam Joseph <adam@westernsemico.com>
2023-03-14 r/5993 docs(tvix/eval): document inner workings of the new VM loopVincent Ambo1-45/+284
Change-Id: I8224bf039f739c401900b5a2ddc839810c87cf6e Reviewed-on: https://cl.tvl.fyi/c/depot/+/8226 Tested-by: BuildkiteCI Reviewed-by: Adam Joseph <adam@westernsemico.com>
2023-02-16 r/5856 docs(tvix/eval): add proposal for VM loop restructuringVincent Ambo1-0/+76
Change-Id: Ib991d68724a73886a8343d7f785b5b3aedd637ed Reviewed-on: https://cl.tvl.fyi/c/depot/+/8103 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2023-02-03 r/5831 refactor(tvix/eval): statically resolve select from constant attrsVincent Ambo1-10/+4
When resolving a select expression (`attrs.name` or `attrs.name or default`), if the set compiles to a constant attribute set (as is most notably the case with `builtins`) we can backtrack and replace that attribute set directly with the compiled value. For something like `builtins.length`, this will directly emit an `OpConstant` that leaves the `length` builtin on the stack. Change-Id: I639654e065a06e8cfcbcacb528c6da7ec9e513ee Reviewed-on: https://cl.tvl.fyi/c/depot/+/7957 Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de>
2023-01-25 r/5758 docs(tvix/eval): builtins.add is not equivalent to +sterni1-3/+3
While it is in the given example, i.e. for integer addition, to claim that they are equivalent is a bit misleading: builtins.add is less overloaded than +, i.e. builtins.add "foo" "bar" will fail whereas "foo" + "bar" performs string concatenation. Change-Id: Ib52d530d1ab289b367565b286f06a76dd518d4fb Reviewed-on: https://cl.tvl.fyi/c/depot/+/7929 Autosubmit: sterni <sternenseemann@systemli.org> Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
2023-01-21 r/5721 refactor(tvix/eval): administer antidote for poisonAdam Joseph1-2/+2
The codebase contains a lot of complexity and odd roundabout handling for shadowing globals. I'm pretty sure none of this is necessary, and all of it disappears if you simply make the globals part of the ordinary identifier resolution chain, with their own scope up above the root scope. Then the ordinary shadowing routines do the right thing, and no special cases or new terminology are required. This commit does that. Note by tazjin: This commit was originally abandoned when Adam decided not to take away reviewer bandwidth for this at the time (eval was still in a much earlier stage). As we've recently done some significant refactoring of globals initialisation this came up again, and it seems we can easily cover the use-cases of the poison tracking in other ways now, so I've rebased, updated and resurrected the CL. Co-Authored-By: Vincent Ambo <tazjin@tvl.su> Signed-off-by: Adam Joseph <adam@westernsemico.com> Change-Id: Ib3309a47a7b31fa5bf10466bade0d876b76ae462 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7089 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de>
2023-01-12 r/5653 docs(tvix): add build-references / string-context documentVincent Ambo1-0/+175
This explains my current thinking on string contexts. Thanks to everyone who gave input so far. Change-Id: I773219402a79a9d4753b4e7cfbf3a4a751a993a3 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7807 Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de>
2022-12-29 r/5530 docs(tvix/eval): sketch in place list/attr set update ideasterni1-0/+24
Change-Id: Ic7debbd8cbd3acdf5f3947288f2aa2964bd163a0 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7660 Autosubmit: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-11-08 r/5261 docs(tvix/eval): document abandoned thread-local vmAdam Joseph1-0/+233
This commit adds a markdown document which explains how the thread-local VM infrastructure works, in case it is useful in the future. Change-Id: Id10e32a9e3c5fa38a15d4bec9800f7234c59234a Reviewed-on: https://cl.tvl.fyi/c/depot/+/7193 Tested-by: BuildkiteCI Autosubmit: Adam Joseph <adam@westernsemico.com> Reviewed-by: tazjin <tazjin@tvl.su>
2022-11-04 r/5244 feat(tvix/eval): implement builtins.splitAdam Joseph1-1/+1
This implements builtins.split, and passes eval-okay-regex-split.nix (which is moved out of notyetpassing). Signed-off-by: Adam Joseph <adam@westernsemico.com> Change-Id: Ieb0975da2058966c697ee0e2f5b3f26ccabfae57 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7143 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-10-31 r/5225 docs(tvix/eval): builtins.md: note implementation statusAdam Joseph1-116/+123
We're getting close to the finish line, folks. I went through the list of builtins and there are only 33 that remain unimplemented. I've marked them, and indicated which are ready to be implemented vs which are waiting for other things. We can delete this column from the table once everything is implemented. Signed-off-by: Adam Joseph <adam@westernsemico.com> Change-Id: Idfaef93283536288b12e59aef5c3e1cd139bd133 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7140 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-30 r/5224 docs(tvix/eval): builtins.md: mark impureAdam Joseph1-7/+7
I believe that the currentTime, findFile, hashFile, pathExists, readDir, path (unless ?sha256), and readFile builtins are impure. This commit marks them as such in docs/builtins.md. Change-Id: Ib1b59fe643dde73cb2b00050b4ef9d3401ad22eb Signed-off-by: Adam Joseph <adam@westernsemico.com> Reviewed-on: https://cl.tvl.fyi/c/depot/+/7139 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-28 r/5217 docs(tvix/eval): add "intern literals" to future optimisationsAdam Joseph1-0/+6
Signed-off-by: Adam Joseph <adam@westernsemico.com> Change-Id: I460230863de853ca5248733bc977d4780b216f36 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7096 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-10-15 r/5133 fix(tvix/eval): bring foldl' strictness in line with C++ Nixsterni1-1/+6
Working on https://github.com/NixOS/nix/pull/7158, I discovered that C++ Nix actually is strict in the accumulator, just not in the first value. This seems due to the fact that in the C++ evaluator, function calls don't seem to be thunked unconditionally and foldl' just elects not to wrap it in a thunk (don't quote me on this summary, even though it seems to line up with the code for primop_foldlStrict and testable behavior). It doesn't seem worth it to risk breaking the odd Nix expression just to be strict in one more value per invocation of foldl' (i.e. the initial accumulator value `nul`), so let's match the existing C++ Nix behavior here. Change-Id: If59e62271a90d97cb440f0ca72a58ec7840d1690 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7022 Autosubmit: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-11 r/5105 fix(tvix/eval/builtins): force acc not list element in foldl'sterni1-0/+2
When investigating discrepancies between foldl' in tvix and C++ Nix, I discovered that C++ Nix's foldl' doesn't seem to be strict at all. Since this seemed wrong, I looked into Haskell's foldl' implementation which doesn't force the list elements (`val` in our code), but the accumulation value (`res` in our code). You can look at the code here: https://hackage.haskell.org/package/base-4.17.0.0/docs/src/GHC.List.html#foldl%27 This actually makes a lot of sense: If `res` is not forced after each application of `op`, we'll end up thunks nested as deeply as the list is long, potentially taking up a lot of space. This can be limited by forcing the `res` thunk before applying `op` again (and creating a new thunk). I've also PR-ed an equivalent change for C++ Nix at https://github.com/NixOS/nix/pull/7158. Since this is not merged nor backported to our Nix 2.3 fork, I've not copied the eval fail test yet, since it wouldn't when checking our tests against C++ Nix in depot. Change-Id: I34edf6fc3031fc1485c3e714f2280b4fba8f004b Reviewed-on: https://cl.tvl.fyi/c/depot/+/6947 Autosubmit: sterni <sternenseemann@systemli.org> Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-10-01 r/5008 docs(tvix/eval): start doc about problematic/weird lang behaviorsterni1-0/+39
The idea is that we can keep track of the more unexpected behavior, behavior that maybe should not be a thing at all and behavior we are not sure about yet. Change-Id: I70933f00af1230a7ab9d30e917b61199fe571caf Reviewed-on: https://cl.tvl.fyi/c/depot/+/6803 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-09-18 r/4896 docs(tvix/eval): note C++ implementation details for C++ Nixsterni1-0/+8
Just want to note those down somewhere before I forget them again, we can delete them later if we have it all figured out. Change-Id: Icafa2d8fc7ca39e38e9637b7eca6f2bbf487c2b8 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6632 Autosubmit: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-09-11 r/4821 docs(tvix/eval): propose builtin "inlining" optimisationsterni1-0/+18
Change-Id: I96a187792a1fd48cffd6b56ec22347aee8cae3af Reviewed-on: https://cl.tvl.fyi/c/depot/+/6526 Autosubmit: sterni <sternenseemann@systemli.org> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-09-11 r/4820 docs(tvix/eval): mention `?` and `or` for builtins optimisationsterni1-1/+3
Change-Id: Ifaa6da345d408a69ce46d6a0e7483352715c75bd Reviewed-on: https://cl.tvl.fyi/c/depot/+/6525 Autosubmit: sterni <sternenseemann@systemli.org> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-09-11 r/4804 docs(tvix/eval): add some notes on recursive attribute setsVincent Ambo1-0/+60
Change-Id: I36b826f12854a22e60a27ed1982ab5528c58bdad Reviewed-on: https://cl.tvl.fyi/c/depot/+/6489 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-11 r/4803 docs(tvix/eval): add optimisation note on eliminating `with` thunksVincent Ambo1-6/+9
Change-Id: I18d50ac8e157929a027f8bf284e65f1eb8950d5a Reviewed-on: https://cl.tvl.fyi/c/depot/+/6488 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-08 r/4748 refactor(tvix/eval): return call frame result from VM::callVincent Ambo1-0/+16
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/4746 docs(tvix/eval): add notes for builtins access optimisationVincent Ambo1-0/+10
Change-Id: Iadbfbe2864ae42fe5492ef3ede0925baee4872b2 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6413 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-06 r/4652 docs(tvix/eval): start a document on known optimisation potentialVincent Ambo1-0/+57
Change-Id: I9bc41e57e1cfdf536d7b9048bac2e7aff1ee2ffa Reviewed-on: https://cl.tvl.fyi/c/depot/+/6313 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-04 r/4648 docs(tvix/eval): pad pure column so it can fit "false"sterni1-107/+107
Change-Id: Iaedfc281db82de1e8eb2400db1118c8431d2579f Reviewed-on: https://cl.tvl.fyi/c/depot/+/6333 Tested-by: BuildkiteCI Autosubmit: sterni <sternenseemann@systemli.org> Reviewed-by: tazjin <tazjin@tvl.su>
2022-09-04 r/4647 docs(tvix/eval): add a list of builtins added in Nix >= 2.4sterni1-0/+11
`builtins.getFlake` doesn't interest us, of course, but some others may be worth (or easy) to implement. They are pretty low priority, though, since nixpkgs has compatiblity wrappers for the ones it uses. The new debugging-related builtins (break and traceVerbose) are interesting to note, but may not make sense to implement at all. Change-Id: Icae547aa3bd9d6ee6b87897ba8210eb9b9b044c7 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6332 Tested-by: BuildkiteCI Autosubmit: sterni <sternenseemann@systemli.org> Reviewed-by: tazjin <tazjin@tvl.su>
2022-09-02 r/4586 docs(tvix/eval): add an overview of all builtins in NixVincent Ambo1-0/+120
Change-Id: Ie187f3317046c6c9e59852d4a128f25ceed99309 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6252 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-08-13 r/4428 docs(tvix/eval): add design documentation for attrset opcodesVincent Ambo1-0/+122
Change-Id: I303b57e035543f4597c6247983d1d533e4014638 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6092 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
generated by cgit-pink 1.4.1 (git 2.44.0) at 2024-05-08T14·55+0000