Skip to content

Preserve readonly instance fields passed to ref parameters - #594

Merged
dadhi merged 1 commit into
dadhi:masterfrom
jogibear9988:fix/readonly-field-ref-arguments
Nov 8, 2026
Merged

dadhi merged 1 commit into
dadhi:masterfrom
jogibear9988:fix/readonly-field-ref-arguments

Conversation

@jogibear9988

@jogibear9988 jogibear9988 commented Nov 8, 2026 •

Copy link
Copy Markdown
Contributor

Passing a readonly instance field to a ref parameter currently exposes its storage, allowing the callee to change the field. For example, incrementing a readonly int through ref changes its stored value from 3 to 4, while System.Linq.Expressions passes a temporary and preserves 3.

Extend the existing readonly-field defensive-copy path to ref arguments by removing its byRefIndex == -1 restriction. Reuse the same local-address helper for both receivers and ref arguments. Writable field addresses and explicit assignment handling retain their existing branches.

The strict Standard/LightExpression regression compares the return value and retained owner storage against System.Linq.Expressions for readonly int and string fields passed to mutating methods.

Validation

Baseline: current master at 76f2b28f9363d3364be75dc9830c7e303aea8b3a. The new fixture fails on that baseline for both input APIs and passes with this change. Release solution build: zero warnings/errors.

Runtime TestX passed Legacy passed
.NET 10 334 1756
.NET 8 334 1755
.NET Framework 4.7.2 315 1639

Performance

Fresh comparison of the baseline above with this change: Windows ARM64, .NET 8.0.28 and 10.0.9, Release, tiering disabled. Prebuilt trees; Light conversion occurs outside timing. Compilation and delegate invocation are measured separately, using isolated load contexts, one CPU, nine alternating samples per phase, and two process rounds in opposite load order with approximately 100 ms pair calibration. Timings combine the 18 samples; allocations use thread allocation counters.

All measured expressions already work on the baseline. The changed path uses a ref-reading method on a readonly int field, avoiding an incorrect mutation as a performance benchmark. The readonly struct receiver, writable receiver and arithmetic cases are controls.

Measured compilation allocations are unchanged in every case; invocation allocates zero. The readonly ref argument changes IL to use a temporary. The readonly receiver and other controls retain their emitted IL. Timing varies between rounds; slower observations are included, and these measurements do not prove CPU-time neutrality.

All measured cases: current master → change
Runtime / API / workload Compile ns Compile B Invoke ns Invoke B Same IL
net10.0 / Standard / add_control 3320.5 → 3373.5 872 → 872 2.0 → 2.0 0 → 0 yes
net10.0 / Standard / readonly_read 4113.0 → 3923.2 952 → 952 2.0 → 2.1 0 → 0 yes
net10.0 / Standard / writable_read 3547.3 → 3761.8 944 → 944 2.0 → 2.0 0 → 0 yes
net10.0 / Standard / readonly_ref 3654.9 → 3815.2 976 → 976 2.0 → 2.1 0 → 0 no
net10.0 / Light / add_control 3192.1 → 3425.8 872 → 872 2.3 → 2.4 0 → 0 yes
net10.0 / Light / readonly_read 3345.8 → 3467.6 952 → 952 2.0 → 2.0 0 → 0 yes
net10.0 / Light / writable_read 2974.3 → 2985.4 944 → 944 2.1 → 2.0 0 → 0 yes
net10.0 / Light / readonly_ref 3236.5 → 3764.4 976 → 976 2.3 → 2.2 0 → 0 no
net8.0 / Standard / add_control 3269.5 → 3167.8 872 → 872 2.3 → 2.2 0 → 0 yes
net8.0 / Standard / readonly_read 3649.7 → 3656.2 952 → 952 2.4 → 2.1 0 → 0 yes
net8.0 / Standard / writable_read 3642.8 → 3651.5 944 → 944 2.0 → 2.0 0 → 0 yes
net8.0 / Standard / readonly_ref 3459.3 → 3629.1 976 → 976 2.0 → 2.0 0 → 0 no
net8.0 / Light / add_control 2281.1 → 2385.3 872 → 872 2.0 → 2.0 0 → 0 yes
net8.0 / Light / readonly_read 3270.5 → 3388.8 952 → 952 2.3 → 2.4 0 → 0 yes
net8.0 / Light / writable_read 2607.2 → 2698.7 944 → 944 2.1 → 2.1 0 → 0 yes
net8.0 / Light / readonly_ref 3727.9 → 3889.9 976 → 976 2.8 → 2.5 0 → 0 no

@dadhi

dadhi commented Nov 8, 2026 •

Copy link
Copy Markdown
Owner

is it duplicate of #592?

@jogibear9988
jogibear9988 force-pushed the fix/readonly-field-ref-arguments branch from 4e75f46 to 4ea2f50 Compare November 8, 2026 21:50
@jogibear9988

Copy link
Copy Markdown
Contributor Author

@dadhi They cover complementary uses of the same copy path:

I verified this on current master (76f2b28f): the readonly int changes from 3 to 4 and the readonly string is replaced, under both Standard and LightExpression. System's compiled implementation preserves both fields.

I have rebased this PR and extended the existing copy path by removing the byRefIndex == -1 restriction, rather than adding another copy branch. The assignment guard is preserved. The readonly ref regression checks both return values and retained field storage.

The full .NET 8, .NET 10 and .NET Framework runners pass. Fresh compilation/invocation measurements against current master are in the description: allocations are unchanged for all measured working cases, including readonly receiver controls.

@dadhi
dadhi merged commit 92f2b85 into dadhi:master Nov 8, 2026
4 checks passed
@jogibear9988
jogibear9988 deleted the fix/readonly-field-ref-arguments branch November 9, 2026 05:12
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.

2 participants