Uh oh!
There was an error while loading. Please reload this page.
Add ECMA-335 EnC metadata delta writer - #20019
Conversation
bf3d940 to
73cc205Compare❗ Release notes requiredYou can open this PR in browser to add release notes: open in github.dev
|
73cc205 to
431357cCompareea20008 to
16962d4Compare16962d4 to
c4eb63aCompare|
Commenter does not have sufficient privileges for PR 20019 in repo dotnet/fsharp |
This comment has been minimized.
This comment has been minimized.
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.
Summary
Reviewed the ten new AbstractIL delta-writer sources and the two heap/exposure edits (skipped the ~5k lines of test files beyond spot-checking their intent). The ECMA-335 encoding is careful and internally consistent, and the design leans hard on System.Reflection.Metadata reader-parity tests, which is the right way to pin byte layout. I verified the pieces most prone to silent corruption and found them correct:
- Coded-index tag/table definitions (
DeltaMetadataEncoding,ILDeltaHandles.*.CodedTag) match ECMA-335 II.24.2.6, and the sizing math (codedBignesslimit =2^(16-tagBits), simple-index>= 0x10000) is right. - Fat method-body header (flags byte +
0x30size nibble), and both small (12-byte clause / 1-byte DataSize /<=0xFFcutoff) and fat (24-byte clause / 3-byte DataSize) exception sections are encoded per II.25.4. - Compressed-integer length prefixes (1/2/4-byte forms) and the metadata-root /
#~stream header layout match the spec, andValid/Sorted64-bit masks correctly cover the PDB tables at 0x30–0x37 (TableCount = 64). - EncLog parent/member
Add*pairing stays adjacent and EncMap is token-sorted.
No correctness, security, or error-handling blockers found. The two inline notes below are non-blocking maintainability/hygiene items.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
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.
Verified the repository-wide Fantomas check, the service test project build with zero warnings and errors, and all 134 DeltaMetadata tests.
e7d79c0 to
33fceefCompareNatElkins
commented
Jul 22, 2026
The only Azure failure in this run was |
T-Gro
left a comment
There was a problem hiding this comment.
Approving. Deep review clean: both prior findings fixed; shared #US encoder verified across empty/ascii/non-ascii/2- and 4-byte cases; determinism byte-identical; 134/134 delta-metadata tests pass. Adding NO_RELEASE_NOTES.
T-Gro
commented
Jul 23, 2026
CI is red: |
T-Gro
commented
Jul 23, 2026
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Head branch was pushed to by a user without write access
NatElkins
commented
Jul 24, 2026
@T-Gro this PR now has a current FCS release note for the metadata delta writer in docs/release-notes/.FSharp.Compiler.Service/11.0.100.md, with exactly one link to #20019. I tried to remove NO_RELEASE_NOTES, but GitHub requires maintainer permission to remove repository labels. Could you remove that label when you get a chance? It is not the cause of the current checkout failure, but it is no longer appropriate for this PR. |
NatElkins
commented
Jul 24, 2026
The only Azure failure in build 1525314 is @T-Gro could you rerun that failed job when you get a chance? The other exact-head jobs are still clean. |
T-Gro
left a comment
There was a problem hiding this comment.
Re-approving after rebase (approval dismissed on force-push). Content unchanged; the only real-CI red is the known IcedTasks file-lock flake.
T-Gro
commented
Jul 25, 2026
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
🔍 Tooling Safety Check — Affects-Compiler-Output
|
T-Gro
commented
Aug 3, 2026
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Uh oh!
There was an error while loading. Please reload this page.
* Add ECMA-335 EnC metadata delta writer 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.
Summary
Adds an internal, standalone ECMA-335 Edit-and-Continue metadata delta writer to AbstractIL, with focused System.Reflection.Metadata-readable metadata tests. It has no behavior change and no in-tree semantic consumer yet; the consumer is the F# hot reload work in #19941. This follows the same upstreaming pattern as #20017 and #20018: land isolated infrastructure first, with deterministic byte and layout evidence, then wire the feature in a later PR.
What
Ten new internal AbstractIL files (file names kept from the development branch for diffability):
ILDeltaHandles- offset/handle primitives, coded-index discriminated unions, delta tokensILMetadataHeaps- heap sizes and offset bookkeepingIlxDeltaStreams- user-string and standalone-signature token calculators (continue numbering from a baseline)DeltaMetadataEncoding- ECMA-335 II.24.2.6 coded-index encodingDeltaMetadataTypes- the writer's input model: plain row-description records (method/parameter/property/event rows, method-semantics updates, custom attribute rows) and the definition key typesDeltaMetadataTables/DeltaTableLayout/DeltaIndexSizing/DeltaMetadataSerializer- delta#-table stream, heap construction, layout and index sizing, EncLog/EncMap emission, generation GUID chainingFSharpDeltaMetadataWriter- the coordinating writerThe writer's inputs are row records (names, tokens, signature blobs, RVAs) plus baseline heap offsets and row counts. It imports nothing from any diffing, baseline, or session machinery; a grep gate for such imports is part of how the extraction was validated. One line of
ilwrite.fsiexposes the pre-existingmarkerForUnicodeBytesso the delta writer reuses the full writer's exact string-marker logic.Tests
130 tests in
tests/FSharp.Compiler.Service.Tests/DeltaMetadata/:Encoding.CodedIndicesdefinitions, with ECMA-335 II.24.2.6 table order cross-checked against System.Reflection.Metadata'sTableIndexas an independent reference;SrmReaderParityTests);baseline + 1from a real seeded baseline;All additions are internal; no public surface area change.
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).Each slice is rebased on current main, carries its own tests, and is flag-off zero-cost. Everything stays draft until reviewed in order.
Role in the train: the hot reload session emits its metadata deltas through this writer.
Refresh status (2026-07-17)
Refresh status (2026-07-22)
Refresh status (2026-07-24)
06e0ad8a7c, with dotnet/fsharp main1dc395ad34merged.NO_RELEASE_NOTESlabel.UseDotNetNode/libuv clock assertion before restore, compiler, build, or test execution; a privileged rerun is requested. The base-branch fork-checkout failure remains the documented Secure release-note checks for fork pull requests #20081 exception.