Skip to content

Fix IDE0059 false positive for out arguments read in sibling arguments (#58564)#84309

Draft
jcouv wants to merge 2 commits into
dotnet:mainfrom
jcouv:fix-issue-58564-ide0059-out
Draft

Fix IDE0059 false positive for out arguments read in sibling arguments (#58564)#84309
jcouv wants to merge 2 commits into
dotnet:mainfrom
jcouv:fix-issue-58564-ide0059-out

Conversation

@jcouv

@jcouv jcouv commented Jun 26, 2026

Copy link
Copy Markdown
Member

Fixes #58564 (also resolves the closed duplicate #61347).

Problem

IDE0059 ("Unnecessary assignment of a value") was a false positive when a local is initialized and then passed both as an out argument and read in another argument of the same call:

int a = 2;
bar(out a, a);   // IDE0059 wrongly flags `a = 2`; applying the suggested fix yields CS0165

The initial assignment is genuinely live — its value flows into the second argument before the out write takes effect — so removing it does not compile.

Root cause

IDE0059 relies on the shared liveness pass in SymbolUsageAnalysis.Walker, used by both the operation-tree and control-flow-graph paths. The walker visits invocation arguments in evaluation order, so out a (arg 1) is visited before the sibling read a (arg 2). An out argument was recorded as a definite write immediately, which cleared the prior reaching write a = 2. The sibling read then "read" the just-recorded out-write instead of a = 2, leaving the real assignment unread → false IDE0059. (ref args were unaffected because they record a read first and use a maybe-written write that doesn't clear the prior write; in args are reads only.)

Fix

Extend the walker's existing pending-write deferral mechanism — previously used only for assignment / deconstruction targets — to also cover out arguments. The out write is now recorded only after all sibling arguments of the same call have been visited, so sibling reads correctly see the prior value. The write keeps its original definite-write semantics, so a genuinely dead out-assignment (x = 1; M2(out x); with no sibling read) is still reported.

  • MakePendingWrite defers writes whose parent is IArgumentOperation { Parameter.RefKind: RefKind.Out }, keyed on the containing call (map key widened from IAssignmentOperation to IOperation).
  • New ProcessPendingWritesForArgumentContainer flushes those deferred writes and returns the pooled set to the pool.
  • Flushed after base.Visit… in VisitInvocation, VisitObjectCreation (constructors: new C(out x, x)), VisitRaiseEvent (defensive for VB), and VisitFunctionPointerInvocation — the complete set of operations that own IArgumentOperations, so no deferred write is ever lost. One change fixes both analysis paths.

Tests

Added to RemoveUnusedValueAssignmentTests:

  • NonRedundantAssignment_BeforeUseAsOutAndReadArgument — the repro, M(out x, x) reports nothing.
  • NonRedundantAssignment_BeforeUseAsConstructorOutAndReadArgumentnew C(out x, x) (object-creation path).
  • NonRedundantAssignments_BeforeUseAsOutAndReadArgumentsM(out x, x, out y, y).
  • NonRedundantAssignment_BeforeReadAndUseAsOutArgumentM(x, out x).
  • NonRedundantAssignment_BeforeUseAsFunctionPointerOutAndReadArgumentdelegate*<out int, int, void> path.
  • Assignment_BeforeUseAsOutArgumentWithoutSiblingRead — control: a dead out-assignment is still flagged.

Validation: RemoveUnusedValueAssignmentTests + RemoveUnusedValueExpressionStatementTests pass 617/617; broader remove-unused filter 749/749. No regressions.

Microsoft Reviewers: Open in CodeFlow
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1 participant