Fix #2372: report a field's address loads on their own - #4119
Open
siegfriedpammer wants to merge 1 commit into
Open
Fix #2372: report a field's address loads on their own#4119siegfriedpammer wants to merge 1 commit into
siegfriedpammer wants to merge 1 commit into
Conversation
Loading a field's address was counted as both a read and a write, so a use that only reads - calling a method on a value-type field emits ldflda and writes nothing - was listed under "Assigned By". An address load says only that something needed a reference to the field. What happens through that reference is up to the consumer, and the IL scan here does not look at it, so the load is neither a read nor a write and gets a category of its own. Special-casing the methods known not to mutate their receiver would answer this one screenshot and leave every ref argument still guessing. Assisted-by: Claude:claude-opus-5:Claude Code
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
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.
Fixes #2372.
FieldAccessAnalyzer.CanBeReferenceansweredtrueforLdflda/Ldsfldano matter whichanalyser was asking:
So an address load appeared under both "Read By" and "Assigned By", and the read-only use from
the report -
flag.ToString()on a value-type field, which emitsldsfldaand writes nothing -was listed as an assignment.
What changed
The
bool showWritesflag becameFieldAccessKind { Read, Write, AddressOf }, each opcode groupnow belongs to exactly one kind, and address loads get an analyser of their own:
Order 30 follows "Read By" (10) and "Assigned By" (20).
Shownow excludes constants foraddress-of as well as for writes: a
constis inlined at every use, so there is no storage topoint at.
This is the third category suggested on the issue rather than the
System.Boolean::ToStringallow-list also floated there. An address load says only that something needed a reference to the
field; what happens through that reference is up to the consumer, and this analyser is an IL scan
that never looks at the consumer. Naming the methods known not to mutate their receiver would
answer this one screenshot and leave every
refargument still guessing.Tests
FieldAccessAnalyzerTests, over aFieldAccessfixture with an instance and a static field: anaddress load is not an assignment, is not a read either, is reported by the new analyser, and
plain reads and writes are unaffected. Restoring the old
return trueturns 4 of the 7 red,including both halves of this issue.
ILSpy.Tests: 1280 tests, no failures.Prepared by an AI agent (Claude, claude-opus-5, via Claude Code) and reviewed by @siegfriedpammer.
🤖 Generated with Claude Code