Add ResetCompilerGeneratedNameState to compiler-generated name generators#20017
Draft
NatElkins wants to merge 1 commit into
Draft
Add ResetCompilerGeneratedNameState to compiler-generated name generators#20017NatElkins wants to merge 1 commit into
NatElkins wants to merge 1 commit into
Conversation
NatElkins
force-pushed
the
fix-generated-name-counter-reset
branch
from
5395d42 to
748ff7c
Compare
Contributor
❗ Release notes requiredYou can open this PR in browser to add release notes: open in github.dev
|
NatElkins
added a commit
to NatElkins/fsharp
that referenced
this pull request
Jul 1, 2026
…ethod CDI emission Adds an internal AbstractIL module implementing, byte for byte, the three Portable PDB CustomDebugInformation blob formats Roslyn persists per method for Edit and Continue (EnC Local Slot Map, EnC Lambda and Closure Map, EnC State Machine State Map), with serializers, deserializers, a portable PDB read-back helper, and an occurrence-key packing helper for deterministic syntax-offset slots. Plumbs an optional methodCustomDebugInfoRows side channel through the IL binary writer options into the portable PDB generator so a compilation can attach CDI rows to named methods. Names that do not identify exactly one method row are dropped. All existing writer call sites pass an empty map, so emitted PDBs are byte-identical to before. No in-tree caller populates the map yet; the consumer is the F# hot reload work in dotnet#19941, following the same pattern as dotnet#20017 (land isolated, test-covered infrastructure first, wire the feature later). Tests: blob round-trips, Roslyn golden-byte encodings, cross-validation against CDI blobs emitted by a real Roslyn compilation, fail-closed occurrence-key packing (including an int32-overflow regression where a wrapped negative key previously escaped the bound check), and end-to-end synthetic PDB emission proving correct MethodDef parenting, zero rows for an empty map, and no rows for absent or ambiguous names.
NatElkins
added a commit
to NatElkins/fsharp
that referenced
this pull request
Jul 1, 2026
Adds an internal, standalone ECMA-335 Edit-and-Continue metadata delta writer to AbstractIL: delta #- table stream and heap construction (DeltaMetadataTables, DeltaMetadataSerializer, DeltaTableLayout, DeltaIndexSizing), ECMA-335 II.24.2.6 coded-index encoding (DeltaMetadataEncoding), EncLog/EncMap emission, generation GUID chaining, user-string and standalone-signature token calculators (IlxDeltaStreams), and the coordinating writer (FSharpDeltaMetadataWriter) over a plain row-description input model (DeltaMetadataTypes, ILDeltaHandles, ILMetadataHeaps). The writer's inputs are row records (names, tokens, signatures, RVAs) plus heap offsets; it has no dependency on any semantic diffing or session machinery. It compiles with no in-tree consumer by design: the consumer is the F# hot reload work in dotnet#19941, following the same upstreaming pattern as dotnet#20017 and dotnet#20018 (land isolated, test-covered infrastructure first, wire the feature in a later PR). One line of ilwrite.fsi is touched to expose the pre-existing markerForUnicodeBytes so the delta writer reuses the exact string-marker logic of the full writer. No behavior change for any existing code path. Tests (130): coded-index encodings asserted against the production definitions and ECMA-335 II.24.2.6 order, System.Reflection.Metadata reader parity over emitted deltas, EncLog/EncMap correctness, stream layout, heap and index sizing, multi-generation heap-offset chaining asserted against computed expected values, standalone-signature rows asserted at baseline+1 from a real seeded baseline, and serializer failure paths.
Contributor
Author
|
/azp run |
|
Commenter does not have sufficient privileges for PR 20017 in repo dotnet/fsharp |
NatElkins
added a commit
to NatElkins/fsharp
that referenced
this pull request
Jul 2, 2026
Per-edit hot reload currently requires an external 'dotnet build -t:Compile' to refresh the obj assembly the delta emitter reads, making every edit pay a full MSBuild+fsc invocation on top of the in-process check (measured ~7s/edit vs ~4.7s for a plain build). This adds an experimental, flag-gated path that produces the same on-disk obj DLL and PDB in-process from the checked project the session already has, collapsing the two compiles into one (spike-measured ~3.0s/edit with a correct one-method delta). - FSharpChecker.CompileFromCheckedProject (internal): emits the assembly from cached typecheck results (finalize CCU, dedupe QualifiedNameOfFile, optimize, IlxGen, write DLL + portable PDB to the requested path), adapted from the dotnet#19267 prototype. Before codegen it clears leaked hot-reload closure-name state and resets the compiler-generated-name counters (ResetCompilerGeneratedNameState) so the emit lays out generated names exactly like a fresh-process build; without this an unchanged closure drifts to a new occurrence suffix and the delta mapping degenerates. - FSharpCheckProjectResults.CompilationData and TcImports.NormalizeAssemblyRef (internal) supply the compilation inputs, from the same prototype. - ResetCompilerGeneratedNameState on the name generators, matching dotnet#20017 so the branch converges with main when that PR lands. - Session wiring: when FSHARP_HOTRELOAD_INPROCESS_COMPILE is truthy, EmitHotReloadDelta runs the in-process compile after session-active validation and before the output freshness pipeline, failing closed as DeltaEmissionFailed on any compile error. With the flag unset the code path is unchanged beyond a single environment read. The external build remains the default; dotnet-watch integration opts in separately. - The emitted portable PDB keeps the sibling-PDB freshness contract: sequence points for line-shifted, unedited methods come from the new compile, not a stale external-build PDB (regression-tested; the test fails without the PDB emission). Verified: service HotReload suite 414 passed 0 failed (including the new flag on/off contract test asserting one updated method with no external rebuild, a +1 LineUpdate for an unedited shifted method, and the unchanged stale-output refusal with the flag off); component HotReload suite 229 passed 0 failed (flag-off neutrality).
…tors Compiler-generated occurrence names (name@line-N) are allocated from process-wide counters on CompilerGlobalState that accumulate across compilations. When a warm checker re-emits the same project in-process, an unchanged closure therefore gets a different occurrence suffix than the previous emit, so consumers that align generated names across compilations (Edit-and-Continue delta emission, dotnet#19941) cannot match them. Add an internal ResetCompilerGeneratedNameState to NiceNameGenerator (clears the per-(name, file) occurrence counters), StableNiceNameGenerator (clears the cached stable names and the inner counters), and an aggregate on CompilerGlobalState that resets all three generators, restoring the fresh-process name layout. Callers must ensure no compilation is concurrently generating names. No in-tree caller yet; the consumer is the hot reload emit path in dotnet#19941. Covered by unit tests proving drift without reset, exact replay after reset, and that the stable-name cache itself is cleared.
NatElkins
force-pushed
the
fix-generated-name-counter-reset
branch
from
748ff7c to
40b666b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds an internal
ResetCompilerGeneratedNameStatemethod to the compiler-generated name generators (NiceNameGenerator,StableNiceNameGenerator, and an aggregate onCompilerGlobalState) that restores the fresh-process generated-name layout.Why
Compiler-generated occurrence names (
name@line-N) are allocated from counters onCompilerGlobalStatethat accumulate across compilations. When a warm checker re-emits the same project in-process, an unchanged closure gets a different occurrence suffix than the previous emit (for exampledata@10in one emit anddata@10-3in the next). Consumers that need to align generated names across successive compilations then cannot match them.The concrete consumer is F# hot reload (#19941): Edit-and-Continue delta emission from a warm checker needs each emit to lay out generated names exactly as a fresh process would, otherwise unchanged closures appear renamed and the delta mapping fails. Measured in the hot reload spike: a one-method edit produced an ambiguous synthesized-type mapping purely because of this drift.
There is intentionally no in-tree caller in this PR; the caller lands with the hot reload emit path in #19941. This PR carries the primitive plus tests so it can be reviewed on its own.
What
NiceNameGenerator.ResetCompilerGeneratedNameState()clears the per-(name, file)occurrence counters.StableNiceNameGenerator.ResetCompilerGeneratedNameState()clears the cached stable names and the inner generator's counters.CompilerGlobalState.ResetCompilerGeneratedNameState()resets all three generator fields.Tests
CompilerGlobalStateTests.fs(new): proves names drift across repeated generation without reset; that replaying the same call sequence after reset reproduces the exact original names; that the stable-name cache itself is cleared (same(name, uniq)re-queried with a different range is recomputed, not served stale); and that the aggregate reset reaches all three generators.Verified locally:
FSharp.Compiler.Service.fsprojbuilds clean, the new test class passes 3/3,fantomas --checkclean.