Skip to content

Select reflection member accessor based on IsDynamicCodeCompiled#473

Merged
eiriktsarpalis merged 6 commits into
mainfrom
eiriktsarpalis-reflection-dynamic-code-accessor
Jul 10, 2026
Merged

Select reflection member accessor based on IsDynamicCodeCompiled#473
eiriktsarpalis merged 6 commits into
mainfrom
eiriktsarpalis-reflection-dynamic-code-accessor

Conversation

@eiriktsarpalis

Copy link
Copy Markdown
Owner

Ports the fix from dotnet/runtime#130503 to PolyType's reflection shape provider.

Motivation

The reflection provider chose its Reflection.Emit-based member accessors whenever RuntimeFeature.IsDynamicCodeSupported was true. On runtimes where dynamic code is supported but only interpreted — the Mono interpreter, WebAssembly, and iOS — the emitted IL is itself interpreted, so it offers no throughput benefit over plain reflection while still pulling in the un-trimmable System.Reflection.Emit stack. RuntimeFeature.IsDynamicCodeCompiled is the correct signal for "emitting IL is actually worthwhile."

Changes

Primary fix — accessor selection

  • ReflectionTypeShapeProviderOptions.UseReflectionEmit now defaults to ReflectionHelpers.IsDynamicCodeCompiled instead of IsDynamicCodeSupported, so interpreted-only runtimes fall back to plain reflection accessors.
  • Added ReflectionHelpers.IsDynamicCodeCompiled (uses RuntimeFeature.IsDynamicCodeCompiled on NET, with a framework/reflection probe downlevel).
  • The provider constructor guard remains on IsDynamicCodeSupported (unchanged) — only the default selection moved.

Coupled fix — exception propagation

  • To keep behavior identical regardless of which accessor is selected, the reflection accessor now propagates user-code exceptions unwrapped instead of surfacing them as TargetInvocationException. Added InvokeNoWrapExceptions helpers (BindingFlags.DoNotWrapExceptions on NET; ExceptionDispatchInfo-rethrow downlevel) and routed the user-code invoke sites in ReflectionMemberAccessor through them. This matches the Reflection.Emit accessor, which emits direct calls.

Tests

  • Added MemberAccessorExceptionTests asserting that getters, setters, default constructors, and parameterized constructors surface the original user exception (not TargetInvocationException) across the reflection, reflection-emit, and source-gen providers. Verified to fail without the coupled fix.
  • make test-clr: 277919 total, 0 failed (Release; net8.0/net9.0/net10.0/net472; with coverage + crash/hang dumps).
  • make test-aot: 29 total, 0 failed (NativeAOT publish + source-generated smoke test).

eiriktsarpalis and others added 6 commits July 10, 2026 21:11
Ports the fix from dotnet/runtime#130503 to PolyType's reflection shape
provider.

- The default for ReflectionTypeShapeProviderOptions.UseReflectionEmit now
  gates on RuntimeFeature.IsDynamicCodeCompiled instead of
  IsDynamicCodeSupported. On runtimes where dynamic code is supported but only
  interpreted (Mono interpreter, WebAssembly, iOS), emitted IL is itself
  interpreted and offers no throughput benefit while still pulling in the
  un-trimmable System.Reflection.Emit stack, so plain reflection accessors are
  now preferred there.
- The reflection member accessor now propagates user-code exceptions unwrapped
  (via new InvokeNoWrapExceptions helpers) instead of wrapping them in a
  TargetInvocationException, matching the Reflection.Emit accessor's behavior.
- Adds MemberAccessorExceptionTests guarding the unwrapped-exception behavior
  across the reflection, reflection-emit, and source-gen providers.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Extract the identical downlevel catch blocks in the two InvokeNoWrapExceptions overloads into a single RethrowInnerException helper (compiled only for non-.NET TFMs, since .NET uses BindingFlags.DoNotWrapExceptions natively).

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
All of its call sites were converted to InvokeNoWrapExceptions, so the exception-wrapping Invoke extension is dead code; the full solution still compiles across all TFMs without it.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Remove the shared RethrowInnerException helper and inline the ExceptionDispatchInfo rethrow directly into each downlevel catch block, so the re-thrown user exception's stack trace doesn't gain an extra helper frame.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@eiriktsarpalis eiriktsarpalis merged commit fdeab5a into main Jul 10, 2026
7 checks passed
@eiriktsarpalis eiriktsarpalis deleted the eiriktsarpalis-reflection-dynamic-code-accessor branch July 10, 2026 23:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant