Skip to content

Display-class prediction robustness: stale UnsafeAccessorType names on cross-partial ordinal shifts; generic containing types/methods unguarded (runtime TypeLoadException) #310

Description

@DJGosnell

Combined finding from the 2026-07-07 multi-agent deep review (generator-architecture perspective). Both defects were adversarially verified; the generics defect was reproduced empirically (Roslyn 4.14 + .NET 10: predicted names fail Assembly.GetType, and the emitted [UnsafeAccessor] pattern throws TypeLoadException at first extractor invocation).

The display-class prediction subsystem (DisplayClassEnricher / DisplayClassNameResolver) already rests on documented-as-undocumented compiler behavior (GetMembers() ordering, <>c__DisplayClass{M}_{C} naming, pre-order closure ordinals). These two defects compound that fragility, and both manifest as silent wrong codegen surfacing at runtime (TypeLoadException / MissingFieldException), never as a compile error.

Defect 1 (high): Stale [UnsafeAccessorType] names after cross-partial ordinal shifts (in-place mutation + equality exclusion)

  • DisplayClassEnricher mutates cached RawCallSite instances in place (site.DisplayClassName = ..., DisplayClassEnricher.cs:145-162) — violating incremental-generator immutability — and DisplayClassName/CaptureKind/CapturedVariableTypes are deliberately excluded from RawCallSite.Equals (RawCallSite.cs:225-231, 452-505).
  • ComputeMethodOrdinal is the method's index in containingType.GetMembers() across all partial declarations (DisplayClassNameResolver.cs:24-36). Adding a member to another partial declaration of the containing type shifts the ordinal without touching the call-site file — discovery outputs stay cached (Line/Column unchanged), enrichment re-runs and computes the correct new name, but returns the same mutated instances; the driver marks them Cached (ReferenceEquals aliasing means even fixing Equals alone wouldn't help), so bind/translate/orchestrator are skipped and the cached CarrierPlan retains the OLD name baked into CapturedVariableExtractor.DisplayClassName.
  • CarrierEmitter.cs:404 emits [UnsafeAccessorType("{DisplayClassName}")] from that stale string while the compiler generates the NEW name → runtime TypeLoadException, healed only by a clean rebuild. Precondition is a persistent generator driver (compiler server / IDE-hosted builds — the normal dev inner loop).
  • Verifier refinement: the emission output action itself re-executes every run (it Combines CompilationProvider) but re-emits from the cached FileInterceptorGroup, so the outcome is exactly as described. Same-file edits self-heal because Line/Column ARE in Equals.
  • Fix: make enrichment produce copies instead of mutating cached instances, and include DisplayClassName/CaptureKind/CapturedVariableTypes in RawCallSite equality so ordinal shifts invalidate downstream caches. Related model-hygiene item: AssembledPlan.ProjectionInfo/JoinedTableInfos/TraceLines are mutated inside the output action (QuarryGenerator.cs:627-653) — same defect class; make these part of the immutable equatable model. (Coordinates with the generator-caching issue.)

Defect 2 (high): No guard for generic containing types/methods — invalid names compile silently

  • The display-class prefix is built with ToDisplayString(NameAndContainingTypesAndNamespaces) (DisplayClassEnricher.cs:130-132). For class Repo<T> this yields Ns.Repo+<>c__DisplayClass0_0 (genericsOptions=None drops type parameters entirely) while the actual compiled type is Ns.Repo`1+<>c__DisplayClass0_0 — no arity backtick, unresolvable. Generic methods fail identically (actual name carries a trailing `1 on the display class).
  • No arity handling, metadata-name conversion, or generic guard exists anywhere in the enricher, resolver, AnalyzabilityChecker, or CarrierAnalyzer eligibility gates (verified by grep; the sole IsGenericMethod check is in the unrelated RawSql path).
  • A Quarry chain with a captured local inside a generic class or generic method compiles cleanly and throws TypeLoadException at first execution (reproduced on .NET 10).
  • Fix: construct metadata-style names with arity (and verify [UnsafeAccessorType] supports the generic case), or detect generic containing types/methods during enrichment and disqualify the chain with an actionable QRY diagnostic. Given captured variables are only needed for value extraction, a diagnostic ("captured variables in generic types not supported — hoist to a local method argument") may be the pragmatic first step.

Hardening (both defects)

  • Canary test: compile + execute a captured-variable chain inside (a) a generic class, (b) a generic method — currently guaranteed runtime failure.
  • Incremental test: add a member to a second partial declaration of the containing type; assert the regenerated interceptor carries the new display class name (needs the tracked-steps infrastructure from the test-suite issue).
  • Consider a runtime self-check on first extractor use that converts TypeLoadException into an actionable Quarry error naming the chain and suggesting a rebuild/issue report.
  • Document the compiler assumptions and their failure modes in src/Quarry.Generator/llm.md (partially done) including the generics limitation until fixed.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions