Add stable synthesized-name replay infrastructure for hot reload#20024
Add stable synthesized-name replay infrastructure for hot reload#20024NatElkins wants to merge 5 commits into
Conversation
21e474e to
26bbbcd
Compare
❗ Release notes requiredYou can open this PR in browser to add release notes: open in github.dev
Warning No PR link found in some release notes, please consider adding it.
|
de67aac to
42d793f
Compare
Add internal generated-name normalization and synthesized-name map replay support as a standalone slice. The new map state is side-channel based, all new compiler modules remain internal, and CompilerGlobalState preserves the existing no-map counter path while checking an accessor captured once per compiler state. Route existing IlxGen generated-name allocations through inert helper wrappers, add pure name-map and normalizer tests, add a normal compilation determinism guard over emitted generated names, and document the extracted seams in P5_REPORT.md. Verification: built FSharp.Compiler.Service, FSharp.Compiler.Service.Tests, FSharp.Compiler.ComponentTests, and FSharpSuite.Tests in Release; ran the migrated service test classes, the component determinism class, FSharpSuite DeterministicTests, and the FCS SurfaceArea class successfully.
42d793f to
7ea9a1f
Compare
Verified with the repository-wide Fantomas check.
github-actions
Bot
added
the
⚠️ Affects-Compiler-Output
|
🔍 Tooling Safety Check — Affects-Compiler-Output
|
T-Gro
left a comment
T-Gro
left a comment
There was a problem hiding this comment.
🤖 This review was generated by AI (@expert-reviewer agent). Findings may contain inaccuracies — please verify independently.
Overall this is careful, well-tested, well-documented infrastructure that is inert on the normal (no-map-installed) path — the added cost there is a single None check per generated name, and DeterministicTests plus the new guard cover regression. Two substantive notes below, both about the replay map's concurrency/determinism model rather than the untouched normal path.
| member _.Snapshot: seq<struct (string * string[])> = | ||
| lock syncLock (fun () -> | ||
| [| | ||
| for KeyValue(key, bucket) in buckets do |
There was a problem hiding this comment.
Snapshot yields buckets in ConcurrentDictionary enumeration order, which is not guaranteed to be stable across processes/runs. Within each bucket the order is deterministic (ResizeArray insertion order), but the top-level (key, names) grouping is not. For a feature whose whole premise is byte-for-byte stable output, any downstream consumer that persists or hashes/compares this sequence directly (baseline blob, snapshot diff) would see non-deterministic ordering unless it re-sorts by key. Recommend sorting by key with StringComparer.Ordinal here before returning so the snapshot is deterministic at the source rather than relying on every consumer to sort.
| /// </summary> | ||
| type FSharpSynthesizedTypeMaps() = | ||
| let syncLock = obj () | ||
| let buckets = ConcurrentDictionary<string, ResizeArray<string>>() |
There was a problem hiding this comment.
buckets and ordinals are ConcurrentDictionary, but every access to both goes through lock syncLock (GetOrAddName, BeginSession, Snapshot, loadSnapshotCore, UsesRecordedSnapshot). The concurrent collections' internal striped locking is therefore pure overhead and, more importantly, muddies the concurrency contract — a future reader may assume lock-free access is safe and add an unlocked read. Since correctness already depends entirely on syncLock, plain Dictionary would be clearer and cheaper. Not a correctness bug today; flagging to keep the intended single-lock model unambiguous.
2 participants
Adds the stable synthesized-name replay layer used by F# hot reload: a normalization grammar for compiler-generated names (
GeneratedNames.fs), a replay map (FSharpSynthesizedTypeMaps) that hands back recorded generation-0 names by allocation slot, and an internal side-channel (ICompilerGeneratedNameMap) threaded throughCompilerGlobalStateso a hot reload session can install the map for a delta compile.Why: Edit and Continue needs "small IL diff for small source diff". Generated names embed source lines, so adding one line renumbers every closure below it and a delta would rewrite unedited types. With the map installed, the compile replays the names the baseline allocated, so unchanged closures keep their names byte for byte. This mirrors how Roslyn preserves synthesized member names across generations.
Behavior without a map installed is unchanged: the accessor resolves once per
CompilerGlobalStateand each allocation takes the existing deterministic per-file path. A new emitted-metadata determinism guard plus the existingDeterministicTestssuite (#19732) verify that. Everything added is internal; the surface area baseline is untouched.Tests: 23 unit tests for the grammar and replay map (slot replay, snapshot canonicalization incl. mixed and gapped buckets, recorded-snapshot load, line-normalized pipe names), 1 no-map determinism guard,
SurfaceAreaTestandDeterministicTestsgreen.Sequencing
This PR is part of splitting the F# hot reload work (#19941) into small, independently reviewable PRs. The planned order:
--test:HotReloadDeltascapture hook (F# hot reload: Edit-and-Continue delta emission behind --test:HotReloadDeltas #19941 in its final, much smaller form).Role in the train: this is the naming foundation the baseline reader (wave 2) and delta emitter (wave 3) build on. Nothing on main calls the map yet; the session slice installs it.
Refresh status (2026-07-17)