Goals:
- Distribute reviews to people who can do the reviews
- Not prevent anything from getting done
- Allow giving away more commit access
Anti-goals:
- Silo people into particular areas
- Discourage contributing to any area
This was drafted by glancing at git logs. It is not likely to be very
accurate; the goal here is that we figure out a way to distribute
reviews to the right people.
Change-Id: I8be44bf7fdeca23da8099124eec7bc3a30e34627
All changes are uniform and done with the same script, so checking only
some should suffice. For that reason, any tests involving multiple files
or custom CLI flags are not included in this commit.
Change-Id: Ib2d0e08937b56e241d99771a58aad34ed3ad308a
The coerce integer feature was not rebased before merge and we do not
have a merge queue, hence, after merge, the HEAD was in a broken state.
We take a commitment to invest into a merge queue now and do a fixup
here.
Change-Id: Ied9410690b542359859ab5f597f22ebceb857305
Signed-off-by: Raito Bezarius <raito@lix.systems>
This introduces a new (demanded?) feature for coercing integers in
interpolation arguments under the experimental feature
`coerce-integers`.
This feature is being introduced behind an *experimental feature flag*
due to the cautious approach we're taking. The codebase has a track
record of revealing unexpected behaviors, often in subtle ways, so we
want to give this sufficient time and exposure before making it stable.
To remove the experimental flag, we want to see **at least two releases
or six months of real-world usage -- whichever is longer** -- that
demonstrate strong confidence the feature doesn't introduce regressions
or unintended side effects. If that level of confidence is reached,
we'll proceed to stabilize it.
Change-Id: I825904719eeba8f0e2a93cd6b93cfe6cebd7d827
Signed-off-by: Raito Bezarius <raito@lix.systems>
That file was written once in 2008 and never updated since, and let's
just say that a lot of things have changed since
Change-Id: I66b0c87ecbba6ca653470966c9514edb21882ca3
It was introduced back in 2013, was disabled in 2014 again for dubious
reasons and according to horrors is unsound anyways and can never really
work.
It was the only disabled test, so I removed the "infrastructure" for
that in the test runner as well. functional2/lang will have much better
ways for skipping tests anyways
Change-Id: Icb8697fb85221e3206fb64cb917c03607ef278a7
Back in the days, this used to be the modus operandi, but then, still
many but less years ago, Eelco came along and changed it to passing in
the actual file. Of course, no motivation was provided, and it was only
done on half of the test runners for some reason, leaving us to wonder
what the true intentions of this code are …
Anyways, with this commit now everything standardises on passing in the
file by path instead of via stdin. Motivation:
- We need to `sed` out the path anyways for various other reasons,
including import tests and path value tests
- Given that, the presumed primary motivation for using stdin in the
first place becomes moot
- Bonus points for giving better error messages, especially in tests
that involve multiple input files
Change-Id: Ic6de1ec24f4c4d3c05e33d1ee053614784677513
Don't ask me the fuck why, but *somehow* Nix prints error locations
differently if the input file is passed as a path vs through stdin, and
I have a hunch that this might have to do with tabs
Change-Id: I186b0edb90edd48856da3621815463e372c37512
Currently, all tests are relative to `./tests/functional` instead of
`./tests/functional/lang`. Whether this is a historical artefact or as
intended, the current move is to align the tests with the new design of
functional2, preparing them for an easier migration.
Change-Id: Ie394691b071488a8000a005080b9167786d5bd9a
`ExprConcatStrings` tracks whether the expression is an interpolation or
not via an obscure boolean called `forceString`.
Instead, we rename it to `isInterpolation`.
This is a breaking change for the JSON AST representation.
Change-Id: I9f89337449b56f6e99a961e21169761f554c9896
Signed-off-by: Raito Bezarius <raito@lix.systems>
it's not useful. nix-instantiate will always point to the wrong location
because "at the toplevel" is not a position we can point to. all nesting
conversions will point to strange places: the *original* definition of a
lambda value (or partial applications thereof), or nowhere at all (since
only lambda values can have their positions determined in the formatter)
Change-Id: I56bd9e1cf3cae11b4198f148a141ca758e4fe8ac
this is much more useful than a frame pointing to the set but claiming
it as a function being called. if the function is actually at fault we
will now point to its attribute, although the position may be slightly
wrong if __toString was defined from set updates or builtin functions.
Change-Id: Ib3eb237a276d94426d9c6fc0e26eea72382d34a2
Now that we can correctly point to all expressions, we can remove
redundant intermediate traces to reduce clutter.
Co-authored-by: eldritch horrors <pennae@lix.systems>
Co-authored-by: Raito Bezarius <raito@lix.systems>
Change-Id: I3e9d7c1c7a6599a8e68302448bbb961d051002b7
I absolutely want to go back to allowing them, I am strongly of the
opinion that nothing should be special about NUL in a string, but this
will have to wait until at least the GC rewrite, so for now the least I
can do is to disarm this gun.
Change-Id: Id14b6037bc8b274c6c60ad970b1c74d436fb62a7
They are broken beyond repair, and barring a language version revision
this is our only option besides doing nothing about it.
Change-Id: I25fa4f032ca9b5ca67356946bcd889289583592e
The code for serialization Expr nodes back into (pseudo-)Nix has been
removed for being subtly error-prone and tedious to maintain. Instead,
`nix-instantiate --parse` now prints a JSON representation of the AST.
Usage patterns of the --parse flag I've found in the wild:
1. Check if a file is well-formed, i.e. discard output and test exit code
2. Get parser errors from a file, i.e. discard stdout and use stderr
3. Nixfmt uses --parse to test equivalence pre/post format, and that property is (should be?) preserved
None of these should break with the current change
Closes#487
Change-Id: Icdbaad17790f2ad8765fa08e02e6597ee4c7a909
Calls to `show` have been removed. To counter the loss of information,
the error positions have been improved and now correctly point to the
current selector instead of the entire select expression.
Change-Id: I4771fe874af1ac15828a9863550cd4369a8f0e94
The `show` functionality needs to be removed because it is deeply
flawed, and given that we already print position information in the
error message (which probably wasn't always the case in the past) the
assertion printing is redundant anyways.
Change-Id: I1f5e05ab73aaa0ec92994c2211463260fd374898
ExprLet was previously inheriting from ExprAttrs for the data, while
ignoring all
set-specific operations on it. The set specific code has now been split
off so that
let doesn't inherit it anymore:
- ExprAttrs (not an Expr), containing the attributes and the related
logic
- ExprLet : Expr, ExprAttrs
- ExprSet : Expr, ExprAttrs
Co-authored-by: eldritch horrors <pennae@lix.systems>
Change-Id: I63f2fbcd1e790b3cffb56eec1e7565ee3cdbf964
It was never intended to be a feature to be used, and moreover it is
inconsistent: One cannot override `+`, and overriding `__lessThan` won't
affect the builtins which do comparisons.
Change-Id: Iaba54a05aa4c2eb37cdb3dc0d731fcee5a86deba
The old behavior results in lots of concatenations happening for no good
reason and is an artifact of the technical limitations of the old parser
(combined with some lack of care for such details).
Change-Id: I0d78d6220ca6aeaa10bc437e48e08bf7922e0bb3
This is only a minor semantical distinction, but we should be able to
properly test it, and the parser tests rely on show for that.
Change-Id: I25e868cf9544e30cdff17deb5fd50a434e0f367e
This also bans various sneaking of negative numbers from the language
into unsuspecting builtins as was exposed while auditing the
consequences of changing the Nix language integer type to a newtype.
It's unlikely that this change comprehensively ensures correctness when
passing integers out of the Nix language and we should probably add a
checked-narrowing function or something similar, but that's out of scope
for the immediate change.
During the development of this I found a few fun facts about the
language:
- You could overflow integers by converting from unsigned JSON values.
- You could overflow unsigned integers by converting negative numbers
into them when going into Nix config, into fetchTree, and into flake
inputs.
The flake inputs and Nix config cannot actually be tested properly
since they both ban thunks, however, we put in checks anyway because
it's possible these could somehow be used to do such shenanigans some
other way.
Note that Lix has banned Nix language integer overflows since the very
first public beta, but threw a SIGILL about them because we run with
-fsanitize=signed-overflow -fsanitize-undefined-trap-on-error in
production builds. Since the Nix language uses signed integers, overflow
was simply undefined behaviour, and since we defined that to trap, it
did.
Trapping on it was a bad UX, but we didn't even entirely notice
that we had done this at all until it was reported as a bug a couple of
months later (which is, to be fair, that flag working as intended), and
it's got enough production time that, aside from code that is IMHO buggy
(and which is, in any case, not in nixpkgs) such as
https://git.lix.systems/lix-project/lix/issues/445, we don't think
anyone doing anything reasonable actually depends on wrapping overflow.
Even for weird use cases such as doing funny bit crimes, it doesn't make
sense IMO to have wrapping behaviour, since two's complement arithmetic
overflow behaviour is so *aggressively* not what you want for *any* kind
of mathematics/algorithms. The Nix language exists for package
management, a domain where bit crimes are already only dubiously in
scope to begin with, and it makes a lot more sense for that domain for
the integers to never lose precision, either by throwing errors if they
would, or by being arbitrary-precision.
This change will be ported to CppNix as well, to maintain language
consistency.
Fixes: https://git.lix.systems/lix-project/lix/issues/423
Change-Id: I51f253840c4af2ea5422b8a420aa5fafbf8fae75
Turns errors like this:
let
throwMsg = a: throw (a + " invalid bar");
in throwMsg "bullshit"
error:
… from call site
at «string»:3:4:
2| throwMsg = a: throw (a + " invalid bar");
3| in throwMsg "bullshit"
| ^
… while calling 'throwMsg'
at «string»:2:14:
1| let
2| throwMsg = a: throw (a + " invalid bar");
| ^
3| in throwMsg "bullshit"
… while calling the 'throw' builtin
at «string»:2:17:
1| let
2| throwMsg = a: throw (a + " invalid bar");
| ^
3| in throwMsg "bullshit"
error: bullshit invalid bar
into errors like this:
let
throwMsg = a: throw (a + " invalid bar");
in throwMsg "bullshit"
error:
… from call site
at «string»:3:4:
2| throwMsg = a: throw (a + " invalid bar");
3| in throwMsg "bullshit"
| ^
… while calling 'throwMsg'
at «string»:2:14:
1| let
2| throwMsg = a: throw (a + " invalid bar");
| ^
3| in throwMsg "bullshit"
… caused by explicit throw
at «string»:2:17:
1| let
2| throwMsg = a: throw (a + " invalid bar");
| ^
3| in throwMsg "bullshit"
error: bullshit invalid bar
Change-Id: I593688928ece20f97999d1bf03b2b46d9ac338cb
Turns errors like:
let
errpkg = throw "invalid foobar";
in errpkg.meta
error:
… while calling the 'throw' builtin
at «string»:2:12:
1| let
2| errpkg = throw "invalid foobar";
| ^
3| in errpkg.meta
error: invalid foobar
into errors like:
let
errpkg = throw "invalid foobar";
in errpkg.meta
error:
… while evaluating 'errpkg' to select 'meta' on it
at «string»:3:4:
2| errpkg = throw "invalid foobar";
3| in errpkg.meta
| ^
… while calling the 'throw' builtin
at «string»:2:12:
1| let
2| errpkg = throw "invalid foobar";
| ^
3| in errpkg.meta
error: invalid foobar
For the low price of one try/catch, you too can have the incorrect line
of code actually show up in the trace!
Change-Id: If8d6200ec1567706669d405c34adcd7e2d2cd29d
this gives about 20% performance improvements on pure parsing. obviously
it will be less on full eval, but depending on how much parsing is to be
done (e.g. including hackage-packages.nix or not) it's more like 4%-10%.
this has been tested (with thousands of core hours of fuzzing) to ensure
that the ASTs produced by the new parser are exactly the same as the old
one would have produced. error messages will change (sometimes by a lot)
and are not yet perfect, but we would rather leave this as is for later.
test results for running only the parser (excluding the variable binding
code) in a tight loop with inputs and parameters as given are promising:
- 40% faster on lix's package.nix at 10000 iterations
- 1.3% faster on nixpkgs all-packages.nix at 1000 iterations
- equivalent on all of nixpkgs concatenated at 100 iterations
(excluding invalid files, each file surrounded with parens)
more realistic benchmarks are somewhere in between the extremes, parsing
once again getting the largest uplift. other realistic workloads improve
by a few percentage points as well, notably system builds are 4% faster.
Benchmarks summary (from ./bench/summarize.jq bench/bench-*.json)
old/bin/nix --extra-experimental-features 'nix-command flakes' eval -f bench/nixpkgs/pkgs/development/haskell-modules/hackage-packages.nix
mean: 0.408s ± 0.025s
user: 0.355s | system: 0.033s
median: 0.389s
range: 0.388s ... 0.442s
relative: 1
new/bin/nix --extra-experimental-features 'nix-command flakes' eval -f bench/nixpkgs/pkgs/development/haskell-modules/hackage-packages.nix
mean: 0.332s ± 0.024s
user: 0.279s | system: 0.033s
median: 0.314s
range: 0.313s ... 0.361s
relative: 0.814
---
old/bin/nix --extra-experimental-features 'nix-command flakes' eval --raw --impure --expr 'with import <nixpkgs/nixos> {}; system'
mean: 6.133s ± 0.022s
user: 5.395s | system: 0.437s
median: 6.128s
range: 6.099s ... 6.183s
relative: 1
new/bin/nix --extra-experimental-features 'nix-command flakes' eval --raw --impure --expr 'with import <nixpkgs/nixos> {}; system'
mean: 5.925s ± 0.025s
user: 5.176s | system: 0.456s
median: 5.934s
range: 5.861s ... 5.943s
relative: 0.966
---
GC_INITIAL_HEAP_SIZE=10g old/bin/nix eval --extra-experimental-features 'nix-command flakes' --raw --impure --expr 'with import <nixpkgs/nixos> {}; system'
mean: 4.503s ± 0.027s
user: 3.731s | system: 0.547s
median: 4.499s
range: 4.478s ... 4.541s
relative: 1
GC_INITIAL_HEAP_SIZE=10g new/bin/nix eval --extra-experimental-features 'nix-command flakes' --raw --impure --expr 'with import <nixpkgs/nixos> {}; system'
mean: 4.285s ± 0.031s
user: 3.504s | system: 0.571s
median: 4.281s
range: 4.221s ... 4.328s
relative: 0.951
---
old/bin/nix --extra-experimental-features 'nix-command flakes' search --no-eval-cache github:nixos/nixpkgs/e1fa12d4f6c6fe19ccb59cac54b5b3f25e160870 hello
mean: 16.475s ± 0.07s
user: 14.088s | system: 1.572s
median: 16.495s
range: 16.351s ... 16.536s
relative: 1
new/bin/nix --extra-experimental-features 'nix-command flakes' search --no-eval-cache github:nixos/nixpkgs/e1fa12d4f6c6fe19ccb59cac54b5b3f25e160870 hello
mean: 15.973s ± 0.013s
user: 13.558s | system: 1.615s
median: 15.973s
range: 15.946s ... 15.99s
relative: 0.97
---
Change-Id: Ie66ec2d045dec964632c6541e25f8f0797319ee2
These were mistakenly labeled `eval-fail-*`.
Note that the `lang.sh` runner passes `parse-fail-*` tests on stdin, so
filenames are removed from error messages.
Change-Id: I7f3a0d78b6cfa87af29aaa1b7af19d5a57fd4ade
we now keep not a table of all positions, but a table of all origins and
their sizes. position indices are now direct pointers into the virtual
concatenation of all parsed contents. this slightly reduces memory usage
and time spent in the parser, at the cost of not being able to report
positions if the total input size exceeds 4GiB. this limit is not unique
to nix though, rustc and clang also limit their input to 4GiB (although
at least clang refuses to process inputs that are larger, we will not).
this new 4GiB limit probably will not cause any problems for quite a
while, all of nixpkgs together is less than 100MiB in size and already
needs over 700MiB of memory and multiple seconds just to parse. 4GiB
worth of input will easily take multiple minutes and over 30GiB of
memory without even evaluating anything. if problems *do* arise we can
probably recover the old table-based system by adding some tracking to
Pos::Origin (or increasing the size of PosIdx outright), but for time
being this looks like more complexity than it's worth.
since we now need to read the entire input again to determine the
line/column of a position we'll make unsafeGetAttrPos slightly lazy:
mostly the set it returns is only used to determine the file of origin
of an attribute, not its exact location. the thunks do not add
measurable runtime overhead.
notably this change is necessary to allow changing the parser since
apparently nothing supports nix's very idiosyncratic line ending choice
of "anything goes", making it very hard to calculate line/column
positions in the parser (while byte offsets are very easy).
(cherry picked from commit 5d9fdab3de0ee17c71369ad05806b9ea06dfceda)
Change-Id: Ie0b2430cb120c09097afa8c0101884d94f4bbf34
this needs a string comparison because there seems to be no other way to
get that information out of bison. usually the location info is going to
be correct (pointing at a bad token), but since EOF isn't a token as
such it'll be wrong in that this case.
this hasn't shown up much so far because a single line ending *is* a
token, so any file formatted in the usual manner (ie, ending in a line
ending) would have its EOF position reported correctly.
(cherry picked from commit 855fd5a1bb781e4f722c1d757ba43e866d370132)
Change-Id: I120c56a962f4286b1ae3b71da7b71ce8ec3e0535
the parser treats a plain \r as a newline, error reports do not. this
can lead to interesting divergences if anything makes use of this
feature, with error reports pointing to wrong locations in the input (or
even outside the input altogether).
(cherry picked from commit 2be6b143289e5479cc4a2667bb84e879116c2447)
Change-Id: Ieb7f7655bac8cb0cf5734c60bd41723388f2973c
