Notes: type-handler registration, and a state-of-play page - #207
Merged
Merged
Conversation
…route out The attribute and base class ship, but nothing in the analyzer or generator reads them, so every [module: TypeHandler<,>] written to date is a silent no-op. Records that, the three unmerged attempts and what each is worth, the richer protocol sitting on the incomplete type-handler branch, the constraints on a replacement (binary compat; no generic attributes, per the netfx GetCustomAttributes throw; not Conditional if cross-assembly), and the agreed route: obsolete the old pair, new typeof-based registration plus a new runtime API, and a diagnostic when a runtime registration has no attribute counterpart. Claude-Session: https://claude.ai/code/session_01GMLcMi7PXmALVsydfmkcmY
Losing a session cost most of a morning re-deriving which branch held what, so: state-of-play.md records where each piece of work is, the agreed landing order and why, the diagnostic-id allocation, and the adjacent things that are easy to forget - the incomplete in-repo type-handler branch, the two external PRs awaiting a decision, and the unreleased Dapper APIs we are pinned behind. The harness section gets the three ways a repack silently measures stale code (pack does not build; pack skips when the nupkg exists; the package cache is redirected on this machine), plus the golden-regeneration workaround for deterministic source paths - all of which bit today. Claude-Session: https://claude.ai/code/session_01GMLcMi7PXmALVsydfmkcmY
mgravell
changed the title
#206's rule doc prescribes the attribute that only exists in #208, so merging #206 first would ship a diagnostic whose documented fix does not compile. #208 goes first, or #206's doc gets softened. Also records the id allocation (DAP053/#206, DAP054+DAP055/#208) so the two can land in either order, and that closing #206 entirely remains a live option. Claude-Session: https://claude.ai/code/session_01GMLcMi7PXmALVsydfmkcmY
mgravell
added a commit
that referenced
this pull request
Sep 11, 2026
`state-of-play.md` is the "read first after a break" page and was the most wrong thing in the repo: it still listed #206/#207/#208 as in flight with a landing order to follow, and knew nothing of #209-#216. It now says what is true - nothing of ours in flight, the four open PRs are all external and all awaiting a decision. It also absorbs the account of **why #206 was closed**, which was sitting unmerged on the `typehandler-registration-note` branch and existed nowhere in main: gating stopped the bridge emitting for consumers who did not opt in, but the read-side check still sat in `RowFactory.GetValue<T>` - the type-flexible arm of every mapped member - so everyone paid for a feature almost nobody would enable. The emission-time fix that was considered and rejected is recorded with it, since that is the part most likely to be re-proposed. Two things promoted, because they gate real work and were buried: - the behavioural harness is local-only and does **not** exist on this machine (no `aot-harness` branch, no SQL Server, Windows-shaped repack recipe), so no phase-3 round can be closed and 677/793 cannot be re-measured until it is rebuilt; - net48 is unverified since #216 and #214, both of which changed interceptor goldens. Corrections: #117 and #162 are closed, not awaiting a decision; the `type-handler` branch harvest was filed as "before #208 settles", and #208 has settled; diagnostic ids as actually shipped (DAP053-056, next free DAP057). `parity.md`: the `CommandDefinition` row still said "27 overloads, every one skipped silently" - #214 moved the `Type`+`CommandDefinition` combinations out, so it is 21 of the 22 in that bucket. Cites the report rather than restating a number, which is what #213 was for. Also: `typehandlers-design.md` never existed in main - it was a file on the closed #206 branch - leaving dangling links in three notes. Pointed at the note that did land, `typehandler-registration.md`.
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.
Two things, both aimed at the same problem: work in this area was spread across branches, PR bodies and scratch directories, and nothing wrote down where it actually was.
notes/typehandler-registration.mdTypeHandlerAttribute<TValue, TTypeHandler>andTypeHandler<T>ship in the package, but nothing in the analyzer or generator reads either — so every[module: TypeHandler<,>]written to date is a silent no-op. That is the substance of issues #159, #165 and #173, and it means the feature is unlanded: free to redesign, subject only to binary compatibility.The note records the state (the unmerged attempts and what each is worth, including the richer handler protocol on the incomplete in-repo
type-handlerbranch), the constraints on a replacement, and the route: obsolete the shipped pair as an error, introduce a non-generictypeof-based registration attribute with a new runtime API, and diagnose a runtime registration that has no declarative counterpart. All three are now implemented in #206 and #208.Two constraints worth calling out as settled rather than open: no generic attributes (netfx
GetCustomAttributesthrowsNotSupportedExceptionover any assembly or type carrying one, poisoning unrelated reflection — the same wall protobuf-net hit, with the sametypeofresolution), and the new attribute must not be[Conditional("DEBUG")]if a package is to ship registrations for the types it owns.notes/state-of-play.mdThe page I wanted and did not have: which branch holds what, which PRs are open, the agreed landing order and the rule that produced it, the diagnostic-id allocation, and the adjacent things that are easy to lose — the incomplete in-repo branch, the two external type-handler PRs awaiting a decision, and the merged-but-unreleased Dapper APIs we are pinned behind.
notes/harness-baseline.mdalso gains the repack recipe that actually works. Three separate ways a measurement goes stale without erroring:dotnet packdoes not build, so packing after a Debug build ships the previous Release output;GenerateNuspecis skipped when the output looks current, so the.nupkgtimestamp moves while its contents do not; and the global packages folder is redirected, so purging~/.nuget/packagesdoes nothing. Plus the golden-regeneration workaround for deterministic source paths.Notes only — no code changes.