tweag / nickel

Better configuration for less
https://nickel-lang.org/
MIT License
2.43k stars 93 forks source link

Add a type parameter to TypeF for contracts #2078

Closed yannham closed 4 weeks ago

yannham commented 1 month ago

The TypeF recursion scheme is parametrized by a number of (Rust) types: the type of Nickel types, the type of enum rows and the type of record rows. There is one more Rust type that can be stored in a TypeF: a contract. It's currently hardcoded to be a RichTerm, which is fine because we never want to use another Rust type to store something else

With the development of the experimental bytecode interpreter, we are changing the representation of terms. Because we want to keep mailine Nickel operational during the transition, we have to copy paste all the typ module somewhere else, replacing RichTerm with the type of the new AST.

There is an alternative: as TypeF is already parametrized by a bunch of things, why not also parametrize the type of what's stored inside TypeF::Contract? This adds a bit more complexity, and requires an additional closure when mapping on or recursing into a Type. But the price doesn't seem too high, and we can reuse the same implementation for both mainline Nickel and bytecode Nickel.

There is an unexpected benefit: in fact, we do store something else than a RichTerm! During typechecking, we need to store an additional environment alongside a contract in order to properly compute contract equality. This led to having an additional constructor GenericUnifType::Contract, that must be handled separately from concrete types. When converting from a Type to a UnifType, the node TypeF::Flat is converted toGenericUnifType::Contract, and all typechecking functions then assume the invariant that they will never encounter a TypeF::Flat. But the Rust type system can't guarantee it. There's also a bespoke runtime error for that (although we're pretty sure it can't happen), a bunch of debug_assert, etc.

We can thus kill two birds with one stone: by parametrizing TypeF, we can now instantiate it with (RichTerm, Environment) for unification types. No need for runtime invariants or ad-hoc treatment anymore.

Doing so, we also rename TypeF::Flat to TypeF::Contract, the former being a historical but bad name.

github-actions[bot] commented 1 month ago

🐰 Bencher Report

Branch2078/merge
Testbedubuntu-latest

⚠️ WARNING: The following Measure does not have a Threshold. Without a Threshold, no Alerts will ever be generated!

Click here to create a new Threshold
For more information, see the Threshold documentation.
To only post results if a Threshold exists, set the --ci-only-thresholds CLI flag.

Click to view all benchmark results
BenchmarkLatencynanoseconds (ns)
fibonacci 10πŸ“ˆ view plot
⚠️ NO THRESHOLD
486,400.00
foldl arrays 50πŸ“ˆ view plot
⚠️ NO THRESHOLD
1,697,700.00
foldl arrays 500πŸ“ˆ view plot
⚠️ NO THRESHOLD
6,655,300.00
foldr strings 50πŸ“ˆ view plot
⚠️ NO THRESHOLD
7,044,900.00
foldr strings 500πŸ“ˆ view plot
⚠️ NO THRESHOLD
62,522,000.00
generate normal 250πŸ“ˆ view plot
⚠️ NO THRESHOLD
44,732,000.00
generate normal 50πŸ“ˆ view plot
⚠️ NO THRESHOLD
2,083,700.00
generate normal unchecked 1000πŸ“ˆ view plot
⚠️ NO THRESHOLD
3,367,300.00
generate normal unchecked 200πŸ“ˆ view plot
⚠️ NO THRESHOLD
756,840.00
pidigits 100πŸ“ˆ view plot
⚠️ NO THRESHOLD
3,217,500.00
pipe normal 20πŸ“ˆ view plot
⚠️ NO THRESHOLD
1,509,600.00
pipe normal 200πŸ“ˆ view plot
⚠️ NO THRESHOLD
10,409,000.00
product 30πŸ“ˆ view plot
⚠️ NO THRESHOLD
825,310.00
scalar 10πŸ“ˆ view plot
⚠️ NO THRESHOLD
1,519,200.00
sum 30πŸ“ˆ view plot
⚠️ NO THRESHOLD
823,440.00
🐰 View full continuous benchmarking report in Bencher