Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions notes/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,15 @@ build time. See [type-vs-generic.md](type-vs-generic.md).

| doc | contents |
| --- | --- |
| [state-of-play.md](state-of-play.md) | **read first after a break**: which branch, which PR, what lands next and in what order |
| [plan.md](plan.md) | **the agreed plan**: complete the gap table → fix the generator → close the gaps |
| [parity.md](parity.md) | the feature parity table: Dapper's surface vs Dapper.AOT today |
| [tokens.md](tokens.md) | special string-token handling: `@ids` expansion, `{=literal}`, `?foo?`, etc |
| [type-vs-generic.md](type-vs-generic.md) | `Type`-based vs `<T>` APIs, and the "announce your types" design space |
| [test-suite-audit.md](test-suite-audit.md) | the Dapper test files as acceptance corpus, and what blocks each |
| [harness-baseline.md](harness-baseline.md) | real numbers from the suite with AOT enabled (Dapper repo, `aot-harness` branch) |
| [typehandler-registration.md](typehandler-registration.md) | type handlers: why the shipped attribute never worked, and the agreed replacement |
| [typehandlers-design.md](typehandlers-design.md) | type handlers: runtime dispatch (opt-in) and the declarative config direction |
| [dynamicparameters-design.md](dynamicparameters-design.md) | phase 3 item 1: delegate to the bag's own vanilla protocol; needs a small Dapper-side API |
| [provider-specialization.md](provider-specialization.md) | a performance direction: emit against the concrete provider the consumer already references, rather than the agnostic ADO.NET surface |
| [generator-audit.md](generator-audit.md) | **fix-first gate**: the capture model snapshots Roslyn symbols/nodes — retention + cache defeat |
Expand Down
42 changes: 37 additions & 5 deletions notes/harness-baseline.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,43 @@

First real numbers, 2026-08-18. Setup lives on the `aot-harness` branch of the **Dapper**
repo (sibling checkout; deliberately **local-only, not pushed** — it is a measurement rig,
not work-in-progress on the public repo): local package feed at `../DapperAOT/artifacts` (pack with
`NBGV_GitEngine=Disabled dotnet pack src/Dapper.AOT/Dapper.AOT.csproj -c Release -o artifacts`,
giving the stable harness version `1.0.0-g`; purge `~/.nuget/packages/dapper.aot/1.0.0-g`
between repacks), `[module: DapperAot]` in `DapperAotEnable.cs`, interceptors enabled, and a
`.globalconfig` raising DAP000 (Hidden by default) to warning.
not work-in-progress on the public repo): local package feed at `../DapperAOT/artifacts`,
`[module: DapperAot]` in `DapperAotEnable.cs` (plus `[module: UseRuntimeTypeHandlers]`, see
round 14), interceptors enabled, and a `.globalconfig` raising DAP000 (Hidden by default) to
warning.

**The repack recipe, with the three things that silently produce a stale measurement:**

```
dotnet build src/Dapper.AOT/Dapper.AOT.csproj -c Release # 1. pack does NOT build
rm -f artifacts/Dapper.AOT.1.0.0-g.nupkg # 2. pack skips if the nupkg exists
NBGV_GitEngine=Disabled dotnet pack src/Dapper.AOT/Dapper.AOT.csproj -c Release -o artifacts
rm -rf C:/Code/NugetPackageCache/dapper.aot/1.0.0-g # 3. NOT ~/.nuget/packages
```

1. `dotnet pack` reuses whatever is in `bin/Release`, so packing after only a Debug build ships
yesterday's DLLs — the symptom is the consumer failing to find a type you just added;
2. `GenerateNuspec` is skipped when the output looks up to date, so the `.nupkg` timestamp moves
while its contents do not. Delete it first;
3. this machine redirects the global packages folder to `C:\Code\NugetPackageCache`; purging
`~/.nuget/packages` does nothing.

Cheap assertion that the loop is honest, before trusting any number:

```
python -c "import zipfile;z=zipfile.ZipFile('artifacts/Dapper.AOT.1.0.0-g.nupkg');print(any(b'YourNewType' in z.read(n) for n in z.namelist() if n.endswith('.dll')))"
```

And the lesson from round 13, which cost a whole measurement: **check the build exit code, not
the presence of output** — `--no-build` over a failed build happily runs stale binaries.

**Regenerating interceptor goldens.** `InterceptorTests` writes `.output.cs` back to source
automatically via `[CallerFilePath]` — which does not work here, because the build is
deterministic and that path is `C:\_\test\...`. The test then reports "Could not find a part of
the path" and the golden is never written. Turning off `DeterministicSourcePaths` breaks the
checked-in `SqliteUsage.snapshot.cs` instead. What works: a throwaway test class in the test
project that calls `Execute<DapperInterceptorGenerator>` and writes the result to an absolute
path, run once per TFM (`-f net8.0`, `-f net48`) and then deleted.

Build: `dotnet build tests/Dapper.Tests/Dapper.Tests.csproj -f net10.0` (net481 and net8.0
legs not yet measured).
Expand Down
103 changes: 103 additions & 0 deletions notes/state-of-play.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
# State of play

**Read this first after a break.** The other notes describe *designs*; this one describes
*where the work currently is* - which branch, which PR, what lands next, and in what order.
Keep it current: it is the cheapest thing to update and the most expensive thing to lose.

Last updated 2026-08-25.

## Where things stand

Phases 1 and 2 of [plan.md](plan.md) are done and merged. Phase 3 (close the gaps, one feature
per round, each verified by a DB-backed run) is in progress; see
[harness-baseline.md](harness-baseline.md) for the round log and the current numbers.

Clean-main baseline: **677 passed / 793** on the Dapper suite, **533 of 725** call-sites
intercepted.

## In flight

| PR | branch | what | state |
| --- | --- | --- | --- |
| #206 | `typehandlers` | runtime `AddTypeHandler` registrations honored, behind `[module: UseRuntimeTypeHandlers]` (default off) | draft, rebased on main, tests green |
| #207 | `typehandler-registration-note` | the note explaining why the shipped `[TypeHandler<,>]` never worked, and the agreed route | draft/open, notes only |
| #208 | `typehandler-attributes` | the declarative replacement: `[TypeHandler(typeof(V), typeof(H))]`, `IDbValueHandler<T>`, the vanilla-handler shim, obsoletes | **ready for review**, complete |

**Landing order (revised 2026-08-25): #207 whenever, then #208, then #206.**

The original plan was "#206 first", on the grounds that it was finished and restores the corpus
number. That was wrong for a concrete reason: **#206's `docs/rules/DAP053.md` prescribes
`[module: TypeHandler(typeof(V), typeof(H))]`**, which only exists in #208. Merging #206 alone
ships a diagnostic whose documented fix does not compile. So either #208 goes first, or #206's
rule doc is softened to stop naming an API that is not there yet.

The rule still worth keeping from the original reasoning: *an attribute lands with the behavior
it gates* - `[UseRuntimeTypeHandlers]` belongs to #206, not #208, because shipping it where it
would do nothing is the exact sin these PRs exist to fix.

**#206 is also discardable**, and that is a live option rather than a formality. Given the
position that runtime config need not be mirrored, the only things it buys are a migration path
for existing JIT users and the corpus 705-vs-677. Closing it and declaring handlers in the Dapper
suite instead is coherent: it costs a suite edit and removes a whole opt-in surface from the
public API. #208 alone is a complete story; #206 alone is not.

**Diagnostic ids**, allocated so the two can land in either order: **DAP053** = #206
(`[UseRuntimeTypeHandlers]` + `PublishAot`); **DAP054** = #208 (runtime registration with no
declarative counterpart); **DAP055** = #208 (registration naming an unusable handler). Next
free: DAP056.

## The position these PRs encode

Marc's call, 2026-08-23: **keep vanilla's *call* API; we are not obliged to mirror its *config*
API.** Static, build-time registration is the supported path; runtime registration is a
migration mode, opt-in, and documented as not AOT-publishable.

The reasoning, so it does not have to be re-derived:

- runtime registration reaches `SqlMapper.TypeHandlerCache<T>` - a generic instantiated over a
runtime-chosen type - which ILC cannot resolve and nothing warns about at publish. Issue #165
is that crash on a deployed app;
- it keeps the world open, so nothing reachable from the registry can be trimmed;
- every runtime knob respected is a permanent per-operation cost (the enum gate in #206 is the
worked example);
- generated code baked its decision at compile time, so a later registration is either ignored
or forces a per-operation check. There is no third option;
- only the static form can be checked at build.

Attribute spelling is **non-generic, `typeof`-based**: on .NET Framework, `GetCustomAttributes()`
throws `NotSupportedException` for the whole call when an assembly or type carries a generic
attribute, poisoning unrelated reflection. Same wall protobuf-net hit, same resolution. Probed
directly; do not re-open this.

## What #208 still needs

Nothing blocking - it is ready for review. Closed since the first draft:

- DAP055 now reports a registration naming something generated code cannot use (was a silent
skip, which was the failure mode the PR exists to kill);
- `Tokenize` is wired: the handler's per-column token travels in the row factory's `state`
channel - one int array per query, filled by a second pass over the token span, indexed
positionally in `Read`. `TypeHandlerProtocolTests` pins the contract from outside the
generator;
- member-scoped `[TypeHandler(typeof(H))]` was **removed** rather than implemented: the
attribute was advertising a form nothing reads, which is the same no-op sin. Widening
`AttributeUsage` and adding a constructor are both non-breaking, so it stays a future option.

Still future work, not gaps in this PR: enum auto-handlers and `[TypeMap]`/settings equivalents,
per the declarative-config direction in [typehandlers-design.md](typehandlers-design.md).

## Adjacent things not to lose

- **The in-repo `type-handler` branch** (last commit literally "incomplete", 2024-11, off a
Dec-2023 main) carries the richest handler protocol written so far - `Tokenize` /
`Parse(reader, ordinal, token)` matching `RowFactory`, plus `EnumTypeHandler<T>` and
`[EnumString]`. The generator half predates phase 2 and would be rewritten; the library half
is the asset. Harvest before #208 settles.
- **External PRs #117 and #162** are the prior art for the static tier and have been open for
months awaiting a decision that only Marc can give. #162 is the more complete; its
generic-attribute spelling is superseded, its instance registry and interceptor goldens are
not. Triage write-ups exist outside this repo.
- **Dapper #2225 and #2228 are merged but unreleased** (latest release is 2.1.79, from May).
DapperAOT pins 2.1.72. When a release ships: bump Dapper and Dapper.StrongName, add the DAP052
positive twin and the defer-emit golden, and take the `TestUnexpectedDataMessage` parity that
was deferred to that bump.
65 changes: 65 additions & 0 deletions notes/typehandler-registration.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
# Type-handler registration: where we actually are, and the route forward

## The feature never landed

Dapper.AOT ships `TypeHandlerAttribute<TValue, TTypeHandler>` and an abstract `TypeHandler<T>`
(`src/Dapper.AOT/TypeHandlerT.cs`), and **nothing in the analyzer or generator reads either** -
`grep TypeHandler src/Dapper.AOT.Analyzers/` is empty. Consumers who write
`[module: TypeHandler<Foo, FooHandler>]` get a silent no-op; that is the substance of issues
#159, #165 and #173, and of the "wasted days" comments on them.

Three attempts exist, none merged:

- **PR #117** (2024-03) - allocates a handler per operation;
- **PR #162** (2025-07) - the most complete: a generator-side registry emitting one shared static
per file, interceptor goldens included. Blocking defect on review: it *narrows* the shipped
attribute's `AttributeUsage`, which is source-breaking. Its proposed DAP050/DAP051 ids are now
taken by main (next free is **DAP053**);
- **PR #206** (2026-08, open) - a different tier entirely: honor *vanilla runtime* registrations by
deferring to `SqlMapper.LookupDbType` at execution time plus a generated read-side bridge. Worth
677 -> 705/793 on the Dapper suite, but it is a compatibility shim for runtime config, not the
declarative model.

There is also an **in-repo `type-handler` branch** (last commit literally "incomplete", 2024-11,
off a Dec-2023 main) carrying the fullest design so far: a per-member/per-type attribute alongside
the per-value-type one; a much richer `TypeHandler<T>` with `Configure` / `SetNullValue` /
`IsDBNull` and - the valuable part - **`Tokenize(reader, columnOffset)` + `Parse(reader, ordinal,
token)`, the same token protocol `RowFactory` uses**, so a handler joins the generated read path
with no per-row type sniffing; and auto-handlers for enums (`EnumTypeHandler<T>`, `[EnumString]`).
The generator half is abandoned mid-statement and predates the phase-2 plain-data model, so it
would be rewritten; the *library* half is the asset to harvest.

## Constraints on any replacement

- **Binary compatibility is required**: the attribute and base class are in shipped packages, so
they cannot be deleted - only obsoleted.
- **No generic attributes.** On .NET Framework, `GetCustomAttributes()` over an assembly or type
carrying a generic attribute throws `NotSupportedException: Generic types are not valid` -
poisoning unrelated third-party reflection, not just our own reads (`CustomAttributeData` is
fine). Same wall protobuf-net hit recently, and the same resolution: **a `typeof`-based
non-generic spelling**.
- **Not `[Conditional("DEBUG")]`**, unlike most Dapper.AOT attributes, if a package is to ship
handler registrations for the types it owns - cross-assembly discovery needs the metadata to
survive (the protobuf-net surrogate pattern).

## Route forward (agreed)

1. mark the existing `TypeHandlerAttribute<,>` (and its `TypeHandler<T>` constraint type)
`[Obsolete(..., error: true)]` - it does nothing today, so this turns a silent no-op into a
compile-time message naming the replacement, and keeps binary compatibility;
2. a **new non-generic, `typeof`-based registration attribute** plus a **new runtime handler API**,
mirroring the *aim* of vanilla's `SqlMapper.AddTypeHandler` while owing nothing to its shape.
The vanilla *call* API stays supported; it is the *config* API we are not obliged to mirror;
3. a diagnostic in DapperAOT mode when a runtime handler registration is detected with **no
attribute-based registration for the same type** - the migration is tooling, not docs (a code
fix, per protobuf-net's `AotMigrationAnalyzer`). Detection only sees registrations syntactically
present in the compilation, so it is a helper, not a guarantee - which is the argument for
keeping #206's runtime bridge available as the safety net, behind a switch that can close the
world.

## Status

All three steps are implemented and in draft PRs; see [state-of-play.md](state-of-play.md) for
the landing order and the remaining gaps. Step 3's diagnostic is DAP053 in the draft and
renumbers to DAP054, because #206 takes DAP053 for the `[UseRuntimeTypeHandlers]` +
`PublishAot` warning.
Loading