Eliminate generated ident collisions

[jneem]

This changes ident generation so that idents generated with Ident::fresh will never clash with idents generated with Ident::new. I think this was the originally intended behavior, but as discussed in the now-deleted docs for Ident::fresh, it didn't work.

There was a problem with pattern compilation, though, which used to generate idents with Ident::fresh, then turn those idents into strings to pass to %record/insert%. These strings would get turned back into idents via Ident::new and then they'd no longer match the original fresh idents.

I originally tried to solve this by having a special-case Ident::fake_fresh that was only used in pattern compilation to revert to the old behavior. It didn't work, because pattern compilation sometimes consumes fresh identifiers coming from other places.

So this PR takes another approach, by allowing record primops to take either an ident (represented as an enum tag) or a string. I'm not sure this is the right approach, and it probably has conflicts with #2302... As discussed somewhere (I can't find it ATM) the better approach might be to revisit how pattern compilation collects its bindings.

[github-actions]

Bencher Report

Branch	ident-generation
Testbed	ubuntu-latest

Click to view all benchmark results

Benchmark	Latency	microseconds (µs)
diagnostics-benches/inputs/goto-perf.ncl	📈 view plot 🚷 view threshold	11,658.00 µs
diagnostics-benches/inputs/large-record-tree.ncl	📈 view plot 🚷 view threshold	200,540.00 µs
diagnostics-benches/inputs/redis-replication-controller.ncl	📈 view plot 🚷 view threshold	305.33 µs
diagnostics-benches/inputs/small-record-tree.ncl	📈 view plot 🚷 view threshold	443.88 µs
fibonacci 10	📈 view plot 🚷 view threshold	259.68 µs
foldl arrays 50	📈 view plot 🚷 view threshold	497.72 µs
foldl arrays 500	📈 view plot 🚷 view threshold	3,106.00 µs
foldr strings 50	📈 view plot 🚷 view threshold	3,332.10 µs
foldr strings 500	📈 view plot 🚷 view threshold	31,424.00 µs
generate normal 250	📈 view plot 🚷 view threshold	18,981.00 µs
generate normal 50	📈 view plot 🚷 view threshold	1,120.90 µs
generate normal unchecked 1000	📈 view plot 🚷 view threshold	2,201.50 µs
generate normal unchecked 200	📈 view plot 🚷 view threshold	440.11 µs
init-diagnostics-benches/inputs/goto-perf.ncl	📈 view plot 🚷 view threshold	59,310.00 µs
init-diagnostics-benches/inputs/large-record-tree.ncl	📈 view plot 🚷 view threshold	228,670.00 µs
init-diagnostics-benches/inputs/redis-replication-controller.ncl	📈 view plot 🚷 view threshold	53,687.00 µs
init-diagnostics-benches/inputs/small-record-tree.ncl	📈 view plot 🚷 view threshold	54,848.00 µs
pidigits 100	📈 view plot 🚷 view threshold	2,141.20 µs
pipe normal 20	📈 view plot 🚷 view threshold	728.37 µs
pipe normal 200	📈 view plot 🚷 view threshold	5,082.30 µs
product 30	📈 view plot 🚷 view threshold	363.91 µs
requests-benches/inputs/goto-perf.ncl-000	📈 view plot 🚷 view threshold	3,122.10 µs
requests-benches/inputs/large-record-tree.ncl-000	📈 view plot 🚷 view threshold	692,440.00 µs
requests-benches/inputs/large-record-tree.ncl-001	📈 view plot 🚷 view threshold	81.92 µs
scalar 10	📈 view plot 🚷 view threshold	658.40 µs
sum 30	📈 view plot 🚷 view threshold	368.40 µs

🐰 View full continuous benchmarking report in Bencher

[jneem]

As discussed in the weekly, maybe instead of having a special path for pattern compilation/idents, pattern compilation should use arrays instead of records.

There's a more general design decision about whether we should be interning as much as we do right now. Maybe not everything that's currently Ident should be interned? (Like, maybe only intern at parsing time?)

[jneem]

With the latest pattern compilation changes, this still runs into issues with the fresh identifiers coming from generated contracts. I think the more thorough pattern compilation re-work is still a prerequisite for this.

[jneem]

With the new pattern compilation, I think this is finally ok. I tested it by adding a panic whenever Ident::new is called with a string starting with %, and it only failed on core/tests/integration/inputs/records/record_defs.ncl, which explicitly creates record fields starting with %. So I'm pretty sure we don't have any more accidental conversions from strings to generated idents.