Skip to content

Fix net481 build break and private-constructor assert regressions from PR #130503#130618

Open
pavelsavara with Copilot wants to merge 1 commit into
mainfrom
copilot/fix-comment-issue-4957325583
Open

Fix net481 build break and private-constructor assert regressions from PR #130503#130618
pavelsavara with Copilot wants to merge 1 commit into
mainfrom
copilot/fix-comment-issue-4957325583

Conversation

Copilot AI commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

PR #130503 gated STJ member accessor selection on RuntimeFeature.IsDynamicCodeCompiled and routed interpreter-only runtimes (WASM/iOS/Mono interpreter) through ReflectionMemberAccessor. This broke main in two ways, reported in comment #4957325583 and comment #4957354177.

Description

  • net481 compile break (CS0117)ExceptionTests.cs referenced RuntimeFeature.IsDynamicCodeCompiled, which does not exist on .NET Framework. [SkipOnTargetFramework(NetFramework)] only skips at runtime, so the file still failed to compile. Guarded the reflection-accessor test with #if NET (the scenario is meaningless on .NET Framework, where dynamic code is always compiled).

  • Private-constructor assert crash on the reflection/emit accessors — routing to ReflectionMemberAccessor surfaced a latent Debug.Assert(... constructor.IsPublic ...) that aborts on supported private [JsonConstructor] constructors (e.g. PrivateParameterizedCtor_WithAttribute). Removed the invalid IsPublic clause from the asserts in ReflectionMemberAccessor (3 sites) and ReflectionEmitMemberAccessor (2 sites — it already invokes private ctors via skipVisibility: true), keeping the DeclaringType/!IsStatic checks.

// A private [JsonConstructor] is a supported scenario the assert wrongly rejected:
class PrivateParameterizedCtor_WithAttribute
{
    public int X { get; }
    [JsonConstructor]
    private PrivateParameterizedCtor_WithAttribute(int x) => X = x;
}

Co-authored-by: pavelsavara <271576+pavelsavara@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes regressions in System.Text.Json introduced by accessor selection changes: restores net481 test compilation and removes invalid debug assertions that rejected supported private [JsonConstructor] scenarios when using reflection-based accessors.

Changes:

  • Guard the reflection-accessor RemoteExecutor regression test behind #if NET to avoid RuntimeFeature.IsDynamicCodeCompiled references on .NET Framework.
  • Relax Debug.Assert constructor visibility checks in ReflectionMemberAccessor and ReflectionEmitMemberAccessor so private (but supported) constructors don’t trigger debug-time aborts.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/ExceptionTests.cs Wraps the reflection-accessor test in #if NET so net481 builds compile successfully.
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionMemberAccessor.cs Removes constructor.IsPublic from constructor assertions to allow private [JsonConstructor] delegates without debug asserts firing.
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionEmitMemberAccessor.cs Removes constructor.IsPublic from constructor assertions; skipVisibility: true already supports invoking non-public ctors.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/area-system-text-json
See info in area-owners.md if you want to be subscribed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants