github-actions[bot]
commented
4 days ago 
Bencher Report
| Branch | rfc007/records-as-field-defs |
| Testbed | ubuntu-latest |
Click to view all benchmark results
| Benchmark | Latency | nanoseconds (ns) |
|---|---|---|
| fibonacci 10 | 📈 view plot 🚷 view threshold | 483,720.00 |
| foldl arrays 50 | 📈 view plot 🚷 view threshold | 1,735,600.00 |
| foldl arrays 500 | 📈 view plot 🚷 view threshold | 6,912,700.00 |
| foldr strings 50 | 📈 view plot 🚷 view threshold | 7,289,000.00 |
| foldr strings 500 | 📈 view plot 🚷 view threshold | 64,364,000.00 |
| generate normal 250 | 📈 view plot 🚷 view threshold | 52,302,000.00 |
| generate normal 50 | 📈 view plot 🚷 view threshold | 2,243,400.00 |
| generate normal unchecked 1000 | 📈 view plot 🚷 view threshold | 3,668,000.00 |
| generate normal unchecked 200 | 📈 view plot 🚷 view threshold | 793,020.00 |
| pidigits 100 | 📈 view plot 🚷 view threshold | 3,234,800.00 |
| pipe normal 20 | 📈 view plot 🚷 view threshold | 1,520,000.00 |
| pipe normal 200 | 📈 view plot 🚷 view threshold | 10,646,000.00 |
| product 30 | 📈 view plot 🚷 view threshold | 833,290.00 |
| scalar 10 | 📈 view plot 🚷 view threshold | 1,504,400.00 |
| sum 30 | 📈 view plot 🚷 view threshold | 832,960.00 |
Instead of elaborating piecewise definitions (such as
{foo.bar = 1, foo.baz = 2}) directly at the parsing stage, this commit makes the new AST closer to the source language by making record a list of field definitions, where the field "name" (left hand side of=) can be a sequence of identifiers and dynamic strings. This representation is used internally by the parser; we now make it the default in the new AST, such that the migration of the parser in #2083 won't have to do this elaboration at all. The elaboration is offloaded to the conversion toRichTerm, which happens in theast::compatmodule.This makes the AST closer to the source language.
The first motivation is that it'll be better for the LSP, where some open issues on the tracker are caused by the inability to trace what the LSP get back to the original piecewise definitions.
The second reason is that we can't actually elaborate a piecewise definition while staying in the new AST correctly as of today: the new AST only has one record variant, which is recursive by default, but this doesn't match the way recursion and scoping work for piecewise definition. For example,
{foo.bar = 1, baz.foo = foo + 1}works fine in today's Nickel (evaluate to{foo = {bar = 1}, baz {foo = 2}}), but if we elaborate it in the new AST naively, we'll get an infinite recursion:{foo = {bar = 1}, baz = {foo = foo + 1}}.Mailine Nickel currently uses a non recursive
Recordfor that, but we don't want to introduce such "runtime dictionary" in the new AST as they can't be expressed in the source language. Instead, we rather keep records as piecewise defined without transformation and will do further elaboration when needed later in the pipeline, during typechecking, future compilation, or in the meantime when converting the new AST representation to mainline Nickel.