Commit graph

17469 commits

Author SHA1 Message Date
Adam Joseph
499a443032 feat(tvix/eval): add NixList::force_elements()
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-26 12:32:54 +00:00
Adam Joseph
dc3543e0ca feat(tvix/eval): include filename of failing test when failing
Unfortunately we have to mangle test case filenames into rust-valid
symbols, since test-generator doesn't use `r#"..."` (deliberately?).
This means that when a test fails, there's nothing on the console
you can copy-and-paste in order to view/edit the code of the failing
test case.

This commit (partially) fixes it by including the unmangled name in
the panic!() string.  However failures due to panic!()s inside the
vm (including deliberate panics due to panic!()-debugging) still
won't display an unmangled filename.

Maybe we should reconsider the use of test-generator?

Change-Id: I2208a859ffab1264f17f48fd303ff5e19675967e
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7092
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-26 10:20:01 +00:00
James Landrein
8425c2016c feat(tvix/eval): add builtins.{floor,ceil}
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-25 08:46:37 +00:00
William Carroll
bd0bf6ea7d feat(wpcarro/compiler): Support Vector type
Support an array that dynamically resizes itself, and replace usages of `List`,
`Array`, and `Queue` with `Vec`.

Change-Id: I910b140b7c1bdddae40e08f8191986dccbc6fddf
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7080
Tested-by: BuildkiteCI
Reviewed-by: wpcarro <wpcarro@gmail.com>
2022-10-25 04:32:59 +00:00
William Carroll
ee235235b9 feat(wpcarro/compiler): Support string literal type
```
repl> "Hello, world"
String
repl> (fn x "testing")
a0 -> String
```

Change-Id: Ia76299a56aa4d2032c9a21277e2fddfb2e055831
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7079
Tested-by: BuildkiteCI
Reviewed-by: wpcarro <wpcarro@gmail.com>
2022-10-25 03:59:07 +00:00
William Carroll
1e9c3955bf refactor(wpcarro/compiler): Modularize debug fns
Define `debug.ml` and `prettify.ml` to clean-up some code.

Change-Id: Iee2e1ed666f2ccb5e56cc50054ca85b8ba513f3b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7078
Tested-by: BuildkiteCI
Reviewed-by: wpcarro <wpcarro@gmail.com>
2022-10-25 03:59:02 +00:00
William Carroll
a8876a4cda feat(wpcarro/scratch): Implement "Algorithm W"
I've been wanting to grok Haskell-style type inference for awhile, so instead of
just watching conference talks and reading papers about it, I've decided to
attempt to implement it to more readily test my understanding of it.

Change-Id: I69261202a3d74d55c6e38763d7ddfec73c392465
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6988
Tested-by: BuildkiteCI
Reviewed-by: wpcarro <wpcarro@gmail.com>
Autosubmit: wpcarro <wpcarro@gmail.com>
2022-10-24 17:42:43 +00:00
Griffin Smith
5bd0e723c1 refactor(tvix/eval): Implement value comparison with a method
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 13:44:02 +00:00
Griffin Smith
7b3bda9e08 feat(tvix/eval): Implement builtins.mapAttrs
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 13:44:02 +00:00
Vincent Ambo
8497b83569 feat(tvix/eval): add builtins.langVersion
The last bump in langVersion (5->6) in C++ Nix was due to making lists
comparable (commit `09471d2680292af48b2788108de56a8da755d661`), which
we support in Tvix with cl/7070.

Change-Id: Id3beed5150b8fb6e0a46a4d1b7e3942022a65346
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7074
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
2022-10-24 13:27:57 +00:00
Adam Joseph
f7ba502005 feat(tvix/eval): implement builtins.currentSystem
This commit implements builtins.currentSystem, by capturing the
cargo environment variable `TARGET` and exposing it to rustc as
`TVIX_CURRENT_SYSTEM` so it can be inserted into the source code
using `env!()`.

The resulting value needs to be massaged a bit, since it is an "LLVM
triple".  The current code should work for all the platforms for
which cppnix works (thanks qyliss for generating the list!).  It
does *not* reject all of the triples that cppnix's configure.ac
rejects -- it is much more forgiving.  We can tighten this up in a
future commit.

Signed-off-by: Adam Joseph <adam@westernsemico.com>
Change-Id: I947f504b2af5a7fee8cf0cb301421d2fc9174ce1
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6986
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-24 12:20:01 +00:00
Griffin Smith
e2f0967d3f feat(nix/eval): Implement builtins.groupBy
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-24 08:13:59 +00:00
sterni
dfa4c4847c refactor(sterni/nix/fun): key can also be an int
Change-Id: Ia27be70ef07d9aa7d13821a2808ded9cf16506c8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7073
Reviewed-by: sterni <sternenseemann@systemli.org>
Autosubmit: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
2022-10-23 21:08:52 +00:00
Griffin Smith
89aee6443c fix(tvix/eval): Use natural arg order for call_with
Since we push arguments onto a stack when calling multi-argument
functions, we actually were ending up calling `call_with` with the
arguments in the *reverse order* - we patched around this by passing the
arguments in the reverse order for `foldl'`, but it makes more sense to
have them just be the order that the function would be called with in
user surface code instead.

Change-Id: Ifddb98f46970ac89872383709c3ce758dc965c65
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7067
Reviewed-by: sterni <sternenseemann@systemli.org>
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
2022-10-23 17:39:55 +00:00
Vincent Ambo
d4569cb504 feat(tvix/eval): initial attempt at setting lambda names
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 15:58:53 +00:00
Vincent Ambo
1050a1d650 test(tvix/eval): add a test case for nested sibling access
Change-Id: I3898284bc4871e5483ead0c27e0f2832d66f2b29
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7061
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
2022-10-23 15:50:35 +00:00
Vincent Ambo
5612ec0f41 fix(tvix/eval): thunk let-expression
There are some rare scope cases with deferred access where this
doesn't behave correctly otherwise.

Change-Id: I6c774f5e62c1cb50b598026c54727017a52cd22d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7064
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
2022-10-23 15:50:35 +00:00
Vincent Ambo
5b305fab40 fix(tvix/eval): fix condition for useless inherit warning
The warning needs to consider whether it is occuring inside of a
thunk, i.e. the dynamic ancestry chain of lambda contexts must be
inspected and not just the current scope.

Change-Id: I5cf5482d67a8bbb9f03b0ecee7a62f58754f8e59
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7063
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: grfn <grfn@gws.fyi>
2022-10-23 15:50:35 +00:00
Vincent Ambo
9b52e5b9c2 refactor(tvix/eval): simplify check for deferring upvalue resolution
This check is now actually simply equivalent to checking whether the
target has been initialised or not.

Change-Id: I30660d11073ba313358f3a64234a90ed81abf74c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7062
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
2022-10-23 15:50:35 +00:00
Vincent Ambo
1f4420cb4a feat(tvix/eval): add mechanism for placeholder builtins
These are builtins which can be basically implemented as the identity
function from the perspective of pure evaluation, and which help us
get closer to evaluating nixpkgs.

For now, builtins added here will be "usable" and just emit a warning
about not being implemented yet.

Change-Id: I0fce94677f01c98c0392aeefb7ab353c7dc7ec82
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7060
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
2022-10-23 15:50:35 +00:00
Vincent Ambo
546fcf51cd refactor(tvix/eval): simplify self-reference check
Checking the computed depth and stack slot against the computed depth
and stack slot is equivalent to just checking the indices into the
locals vector against each other (i.e. "is the slot we're compiling
into the slot we're accessing?")

Change-Id: Ie85a68df073e3b2e3d9aba7fe8634c48eada81fc
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7059
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
2022-10-23 15:50:35 +00:00
Vincent Ambo
c9bf7c4cf1 chore(tvix/eval): return detailed TvixBug if an upvalue is missing
When capturing an upvalue, return a detailed TvixBug error that
contains metadata about what exactly was missing.

This particular thing helps with debugging some scope issues we still
seem to have.

Change-Id: I1089a1df4b3bbc63411a4907c3641a5dc3fad984
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7058
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
2022-10-23 15:50:35 +00:00
Vincent Ambo
60f24c3c53 fix(tvix/eval): detect cycles when printing infinite values
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-23 15:50:35 +00:00
Vincent Ambo
4ff06ba67d feat(tvix/eval): add TvixBug error kind
This can be raised in cases where some panics lurk (e.g. indexed
accesses).

Change-Id: I93f4d60568277e9c5635aa52f378645626d68c5e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7057
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
2022-10-22 18:30:12 +00:00
Vincent Ambo
359444360b test(tvix/eval): add a test for observing an infinite attribute set
Note that this test (ironically) fails if the observer is used (e.g.
with --trace-runtime), but that's a separate issue.

Change-Id: I952eaeac8b5a7acce9c66cd4744ec570280748e7
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7055
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
2022-10-22 18:20:48 +00:00
Griffin Smith
d4fa3152e9 feat(tvix/eval): Implement builtins.deepSeq
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-22 18:11:55 +00:00
Vincent Ambo
8724d2fff8 fix(tvix/eval): use top-level span for force_with_output
When forcing thunks in `force_with_output`, the call stack of the VM
is actually empty (as the calls are synthetic and no longer part of
the evaluation of the top-level expression).

This means that Tvix crashed when constructing error spans for the
`fallible` macro, as the assumption of there being an enclosing span
was violated.

To work around this, we instead pass the span for the whole top-level
expression to force_for_output and set this as the span for the
enclosing error chain. Existing output logic will already avoid
printing the entire expression as an error span.

This fixes b/213.

Change-Id: I93978e0deaf5bcb0f47a6fa95b3f5bebef5bad4c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7052
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
2022-10-22 17:01:41 +00:00
Florian Klink
6025242fc7 chore(buf): Use nixpkgs-provided buf
The version of buf used is quite old.

nixpkgs provides a more recent version, but it requires us to migrate
config to the latest version.

depot_scanner.proto doesn't honor some of the conventions, so we need
allow_comment_ignores and drop a bunch of comments in there.

Change-Id: Ic978fe92fb7c8471f58c137497528f18aad8f3ab
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7053
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: tazjin <tazjin@tvl.su>
2022-10-21 18:39:03 +00:00
sterni
64bb501de1 fix(tvix): distinguish search- and relative path resolution errors
Failures to resolve a nix search path lookup in angle brackets can be
caught using tryEval (if it reaches the runtime). Resolving relative
paths (either to the current directory or the current user's home) can
never be caught, even if they happen inside a thunk at runtime (which is
currently the case for home-relative paths).

Change-Id: I7f73221df66d82a381dd4063358906257826995a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7025
Autosubmit: sterni <sternenseemann@systemli.org>
Reviewed-by: Adam Joseph <adam@westernsemico.com>
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
2022-10-21 00:11:29 +00:00
sterni
5753fc14f5 refactor(nix/dependency-analyzer): use unsafeDiscardOutputDependency
Turns out that pathContextDrvPath already exists as a builtin which
is very convenient. Actually somewhat embarassing that I missed this
for so long.

Change-Id: Ieb5e113d70dec548b3053911ff9dbe9ed48402be
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7050
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-20 18:21:25 +00:00
sterni
b410655a5e test(tvix/verify-lang-tests): also check notyetpassing tests
Adding these to the C++ Nix CI ensures that it is possible for the
tests in that directory to pass at all. Mainly this would catch
situations like fixed in a previous CL when moving the tests around
would break them so that they wouldn't even pass in C++ Nix.

For this to work, we need to track skips by basename to be
directory-independent (assuming that every skipped test name is unique
is hopefully okay).

Change-Id: If6cb63ebdef0fc19b082b6a04e79ada2e47c658e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7048
Autosubmit: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-20 18:21:25 +00:00
Evgeny Zemtsov
b5b968e103 fix(nix/buildkite) don't fail if gated step has no branches
Change-Id: Id3365c18bc3904b42e8b129a513cff4480a2cb07
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7051
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-20 15:27:31 +00:00
sterni
bbf1e32737 fix(tvix/eval): restore .exp.xml files and skip in test suite
Change-Id: Iebda5e0d99925a0a8c1d6ae1d7a35397d127bf31
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7047
Autosubmit: sterni <sternenseemann@systemli.org>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
2022-10-20 08:40:01 +00:00
sterni
d9dfe65910 fix(tvix/eval): fix path in notyetpassing tests
cl/7036 moved these paths around, but neglected to update the relative
paths they contain. Without these updated, they will never start
passing!

Change-Id: Ib16468611af59729883e501be8486f43d850fd58
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7046
Autosubmit: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-20 02:35:42 +00:00
sterni
60363039ef test(tvix/eval): eval-okay-eq.nix can actually be tested
eval-okay-eq.nix is actually an ancient test case that used the ATerm
syntax for the test result. It was disabled for a while, since the
behavior got reverted for a bit, but works on any reasonably modern
Nix implementation. This change matches my [C++ Nix PR].

[C++ Nix PR]: https://github.com/NixOS/nix/pull/7196

Change-Id: I602fd7c83a0bc104ab502c8b6a74e4591272be1a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7045
Autosubmit: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-20 02:35:42 +00:00
sterni
0d9eabcaac fix(tvix/eval): eval-okay-pathexists test can be executed
Change-Id: Ibdcaa165024584370ce9578e67985a3526e44f77
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7044
Autosubmit: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-20 02:33:40 +00:00
sterni
8e188d6235 test(tvix/eval): update C++ Nix test suite to current master
The language test suite actually doesn't require flakes and the
new features are mostly sensible (added builtins) as well as some
tests for regressions the C++ implementation experienced.

The path interpolation test is not included in this update because there
is no way to construct an location-independent .exp file for it (the C++
repo also doesn't have one). We may still want to implement that feature
eventually (in case rnix adds support for it).

The C++ Nix revision used is ac0fb38e8a5a25a84fa17704bd31b453211263eb.

Change-Id: I75f1e780ddeeee6f6b1f28cf3c66c288dca2c20c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7043
Autosubmit: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-20 02:33:40 +00:00
sterni
e3637136d1 test(tvix/eval): enable nix_tests by default
Since cl/7036 we have a mechanism for dealing with the nix_tests we do
not pass yet.

Change-Id: I246c52963ae7f2500253f4035a77d7006dd35307
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7049
Autosubmit: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-20 02:28:38 +00:00
sterni
292a751388 test(tvix): nix-planned test verification using C++ Nix 2.3 and 2.11
Reimplement the test discovery of the lang tests script in Nix which
allows for a more flexible skipping logic that can e.g. react to the C++
Nix version used. This allows us to run the test suite against both
C++ Nix 2.3 and the latest C++ Nix version 2.11. The latter is mainly
useful, so we can implement newer Nix features and still verify them
against the C++ implementation.

Change-Id: I30c802844133b86b5e49f5e4f4fefacdb6215e0e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7042
Autosubmit: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-20 02:28:38 +00:00
Adam Joseph
f991ff9294 feat(tvix/eval): expect not-yet-passing tests to fail
It is helpful to be able to use the test suite as a regression test:
make a change to the compiler/vm, re-run the tests, and if there are
any failures you know it's your fault.

Right now we can't do that, because the expected-to-fail tests are
mixed in with the expected-to-pass tests.  So we can't use them as a
regression test.

Change-Id: Ied606882b9835a7effd7e75bfcf3e5f827e0a2c8
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7036
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
2022-10-19 15:01:05 +00:00
Adam Joseph
2bc38ce7dc docs(REVIEWS.md): gerrit-for-github guide, warn about attention sets
Change-Id: Ia20feacbb7394daaa1f1ed0d1e05c7441a187744
Signed-off-by: Adam Joseph <adam@westernsemico.com>
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7035
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
2022-10-19 10:42:32 +00:00
Adam Joseph
d978b556e6 feat(tvix/eval): deduplicate overlap between Closure and Thunk
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 10:38:54 +00:00
Adam Joseph
c91d86ee5c feat(tvix/eval): NixList::concat(): avoid an unnecessary move
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-19 10:37:30 +00:00
Adam Joseph
4b01e594d5 docs(tvix/eval): upvalues.rs: define "upvalue", comment with_stack
This commit adds a comment for the benefit of non-Lua-users
explaining what an upvalue is.  It also adds a comment explaining
what `with_stack` does, including a brief reminder of nix's
slightly-unusual-but-well-justified handling of this construct.

Signed-off-by: Adam Joseph <adam@westernsemico.com>
Change-Id: If6d0e133292451af906e1c728e34010f99be8c7c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7007
Reviewed-by: j4m3s <james.landrein@gmail.com>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
2022-10-18 09:38:16 +00:00
Vincent Ambo
13a5e7dd5b fix(tvix/eval): wrap dynamic resolution in an extra thunk
Without this change it was possible to cause situations (see the new
test) in which a `with`-namespace was forced prematurely.

Change-Id: I879ea7763b43edc693feace2c73c890d426fafd3
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7031
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Reviewed-by: Adam Joseph <adam@westernsemico.com>
2022-10-18 09:15:13 +00:00
Griffin Smith
0063e7e913 feat(nix/eval): Implement builtins.functionArgs
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 11:29:49 +00:00
Griffin Smith
2a3d498104 feat(tvix/eval): Validate closed formals
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 11:29:49 +00:00
Griffin Smith
e63d14419f feat(tvix/eval): Record formals on lambda
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 11:29:49 +00:00
Griffin Smith
89dbcbbb3d feat(tvix/eval): Implement builtins.seq
Since we already have infra for forcing arguments to builtins, this ends
up being almost *too* simple - we just return the second argument!

Change-Id: I070d3d0b551c4dcdac095f67b31e22e0de90cbd7
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6999
Reviewed-by: kanepyork <rikingcoding@gmail.com>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
2022-10-17 11:04:17 +00:00
Adam Joseph
6e30fbbf7b docs(tvix/eval) comments for various fields
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-17 09:51:18 +00:00