Generator capture model: plain data instead of cached Roslyn objects (phase 1 of 2) - #187
Merged
Merged
Conversation
The cached pipeline values held Location + ITypeSymbol + IMethodSymbol (and MemberData an ITypeSymbol per member), and the raw Compilation fed the output step - so the incremental cache never hit (symbol equality does not hold across compilations) while the cached values pinned whole compilations in memory. See notes on the parity-notes branch (generator-audit.md) for the survey. New Dapper.CodeAnalysis.Model namespace with hand-written equatable plain-data types: LocationSnapshot (span values; reconstitutes a Location only at report time), EquatableArray<T> (structural equality - deliberately not ImmutableArray<T>, whose reference equality silently defeats caching), TypeAccessorModel/AccessorMember/ForwarderMethod, and GenerationEnvironment so the output step combines three compilation facts instead of the Compilation itself. All symbol projection moved into Parse; emit is byte-identical (the Accessors golden tests pass unchanged). ModelShapeTests gives the rule teeth: everything in the Model namespace is checked by reflection for Roslyn-typed fields and for IEquatable, so a symbol sneaking back into the cached model is a test failure, not a silent leak. PreGeneratedCodeWriter gains a bool-based ctor (the Compilation one remains for the interceptor generator until its turn).
…ched states SourceState (and all three subclasses) now carry a LocationSnapshot - plain span/path values - instead of a Location, which pins its whole syntax tree. Two facts that emit used to pull from the Location's tree are projected at parse instead: the normalized interceptor file path (SourceReferenceResolver.NormalizePath) and the language version (for CheckPrerequisites); the IncludeLocation SQL comment now uses the snapshot's mapped path/line. CommonComparer compares snapshots with the same path/start/end semantics the old Location-based comparer had, and the group key's UniqueLocation becomes LocationSnapshot? (value equality rather than reference - each call-site still groups distinctly). Byte-identical verified two ways: all golden fixtures pass unchanged (net10/net48), and the Dapper test-suite's generated file hashes equal before and after (f8d3a61f).
SuccessSourceState.Method becomes InterceptedMethod - plain data with what signature emission and parameter forwarding actually consume: Append-form return/parameter type strings, name, extension-ness, arity, and the precomputed NRT-shim fact (the IsAsync/annotation dance moves to parse). Grouping equality moves from SymbolEqualityComparer to the model's structural equality; HasParam/Forward walk MethodParam names. CodeWriter.GetAppendTypeName is the one canonical 'what Append(ITypeSymbol) would emit' helper so projections cannot drift from emission. Byte-identical verified: goldens unchanged (net10/net48), harness hash equal (f8d3a61f). The model shape test picks up the new types automatically.
CommandProperty held an INamedTypeSymbol and a Location in the cached model; it now stores the emitted type name, the short name (for diagnostics), the precomputed is-DbCommand and member-exists facts, and a LocationSnapshot - the symbol probes run at construction (parse time) instead of at emit. QueryColumns and CommandProperties move from ImmutableArray to EquatableArray, which needed default-vs-empty to be distinguishable (QueryColumns semantics depend on it), so EquatableArray no longer collapses empty to default. AdditionalCommandState moves into the Model namespace, putting it under the shape test's enforcement; the type-name grouping in WriteCommandProperties replaces the symbol grouping. Byte-identical: goldens unchanged (net10/net48), harness hash equal (f8d3a61f).
mgravell
added a commit
that referenced
this pull request
Aug 18, 2026
mgravell
added a commit
that referenced
this pull request
Aug 19, 2026
* Start the Dapper/Dapper.AOT parity accounting under notes/ The goal on record: enable Dapper.AOT in the Dapper test suite, announce types via attributes, and have it swallow everything - AOT-clean. - parity.md: the feature table, with impact/complexity per gap (several 'gaps' score zero because the concept doesn't exist under AOT, e.g. the ref-emit plan cache) - tokens.md: @ids expansion, {=literal}, ?foo? pseudo-positional, param filtering - type-vs-generic.md: the announced-types design space for Type-based APIs - test-suite-audit.md: the Dapper tests as acceptance corpus, sequenced * GetTypeDeserializer is valid API, not cache plumbing With announced types it's the same dispatch map (boxed materializer), and its generic strengthening already exists as GetRowParser<T>. The real hole is the write side: CreateParamInfoGenerator has no generic counterpart - recorded the GetParameterBinder<T> proposal, and the question of blessing CommandFactory<T>/RowFactory<T> as the supported surface. ReadChar and friends are plain AOT-safe statics, nothing to do. * Scope the accounting to the public API and observable behavior PublicAPI.Shipped.txt is the checklist; the contract is what reaches the provider and what comes back, never Dapper's internals. Cuts both ways: the dynamic row needs behavioral fidelity only (the type is internal), while the public infrastructure statics ARE in scope because extenders call them. * Record the decision: internals-asserting tests get adjusted, not maintained * New work item: warn (new DAP id) on use of the has-no-meaning APIs Plan-cache surface, CommandFlags.NoCache, possibly ConnectionStringComparer: supported-and-meaningless under AOT, which is a different statement to DAP001's unsupported-but-meaningful. Warning, not error - the code runs. * Measurement caveat: build-time DAP counts are an upper bound Some failure modes are silent until executed - handled means intercepted, not correct. Only the DB-backed test run catches silent divergence. * First harness baseline: 'handled 396 of 396' alongside 96 compile errors Two root-cause generator bugs (array-of-anonymous parameter emits the display string and wrecks the parse; inaccessible row types are emitted rather than refused), plus two scorecard honesty problems (the denominator excludes unattempted APIs; handled does not mean compiles) and a zero- analyzer-diagnostics anomaly to re-check once the compile is clean. * Generator audit: the capture model snapshots Roslyn nodes; fix first Both generators' cached SourceState hold IMethodSymbol/ITypeSymbol/ Location (MemberMap even holds an IOperation), and the pipeline combines the raw CompilationProvider into the source output - so it behaves as a full-recompute generator with a memory leak. Recorded as a sequencing gate ahead of the gap-closing features, with the fix shape that worked for protobuf-net (plain equatable model, span-based locations, separate diagnostics branch, shape-enforcing test). * Record the agreed plan: gap table, then generator model, then features The line that resolves the phase-1/2 tension: nothing that adds parse-time state lands before the model rework completes; refusals and scorecard fixes are allowed ahead of it, which is what lets phase 1 see. * Round 2 numbers, and log the modern-interceptor-syntax work item * Round 3: the suite compiles with AOT enabled (4 fix PRs + 2 severity downgrades) * Scoreboard: all three TFM legs compile; local SQL Server available * Work item: [UnsafeAccessor] may lift the accessibility refusals (net8+) * Round 4: the honest scorecard says 53%, not 100% * Harvest the skip breakdown; flag the DAP016 corpus-shape decision * Round 5: first behavioral run - 84 failures, every one compiled clean * Note that aot-harness is deliberately local-only * Phase 2 log: approach and increments * Phase 2 log: increment 1 done * Phase 2 log: 3a done * Phase 2 log: 3b done * Phase 2 log: 3c-i done; two cached symbols remain * Phase 2 log: result-side plan done; one symbol left * Phase 2 log: cached model fully plain; only increment 4 remains * Phase 2 log: complete - PRs #187 + #188 * Phase 2 log: caching tests landed * Phase 2 log: readonly-field quirk fixed (#190) * Round 6: DAP051 + restructure takes interception to 68.1% * Round 6b: 612/760 behavioral; failures track interception growth honestly * DynamicParameters design: delegate to the bag; needs one small Dapper API * Round 7: DynamicParameters at 73.5%; First-pipeline drain divergence found * Round 7b: 612/762; every failure class maps to a planned feature * Interceptor-syntax migration: soft-target requirement recorded * Tokens: runtime-SQL design - per-factory memoized role scan * Record the feature-detection rule (DAP052) in the design note * Round 8: CommandBehavior parity fix (PR #196) * Round 8b: 616/762, suite loop 17s * Round 9: list expansion lands (PR #197), 638/762 * Round 10: custom parameters + the two bugs they uncovered, 658/793 * Round 11: dynamic-record fidelity, 672/793 * Sync with main; point parity rows at their open PRs The table lands on main via #186; from here each feature PR flips its own cells, so the table and the merge history cannot drift apart. Rows with an open PR say so, and the flip to a settled status is that PR's job.
This was referenced Aug 25, 2026
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.
The incremental pipeline's cached values held Roslyn objects —
Location,IMethodSymbol,ITypeSymbol, and (viaCommandProperty) anINamedTypeSymbol— which causes two silent failures at once: a cached symbol pins its entire compilation alive for as long as the driver holds the value (a real leak in a long-running IDE session), and symbol equality doesn't hold across compilations, so the incremental cache never hits anyway. TheTypeAccessorgenerator also fed the rawCompilationinto its output step, re-running generation on every keystroke.This converts everything except the two big type symbols (
ResultType/ParameterType— the row/command-factory projection, coming as a follow-up):Dapper.CodeAnalysis.Modelnamespace of hand-written equatable plain-data types:LocationSnapshot(span/path values; reconstitutes aLocationonly at report time),EquatableArray<T>(structural equality — deliberately notImmutableArray<T>, whose reference equality silently defeats caching; preserves default-vs-empty becauseQueryColumnssemantics need it),InterceptedMethod/MethodParam,AdditionalCommandState/CommandProperty, and the TypeAccessor models;TypeAccessorInterceptorGeneratorconverted end-to-end, including its compilation input: the output step now combines three projected facts (AllowUnsafe, assembly name, has-InterceptsLocationAttribute) instead of theCompilation;Location(interceptor file path and language version are projected at parse, where emit used to reach throughLocation.SourceTree) andIMethodSymbol(signature emission consumes projected strings; grouping becomes structural equality);ModelShapeTestswalks the Model namespace by reflection and fails on any Roslyn-typed field or missingIEquatable— the constraint has teeth, so a symbol sneaking back into the cached model is a test failure rather than a silent leak.Byte-identical output is verified two ways at every step: all golden fixtures pass unchanged (net10/net48), and the Dapper test suite's full generated file (725 call-sites) hashes identical before and after (
f8d3a61f). One behavior-preserving oddity is kept deliberately and flagged in review:HasPublicSettableInstanceMemberreturns true for readonly fields, which looks inverted — preserved as-is here, worth a separate look.The working log for this effort is
notes/phase2-log.mdon theparity-notesbranch.