Skip to content

Do not consider ref argument to in/ref readonly parameter as being written to #77682

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 21, 2025
Merged

Do not consider ref argument to in/ref readonly parameter as being written to #77682

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 24 additions & 17 deletions src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1129,7 +1129,7 @@ public override BoundNode VisitConvertedTupleLiteral(BoundConvertedTupleLiteral

private BoundNode VisitTupleExpression(BoundTupleExpression node)
{
VisitArguments(node.Arguments, default(ImmutableArray<RefKind>), null);
VisitArguments(node.Arguments, default(ImmutableArray<RefKind>), null, default, false);
return null;
}

Expand All @@ -1142,15 +1142,15 @@ public override BoundNode VisitTupleBinaryOperator(BoundTupleBinaryOperator node

public override BoundNode VisitDynamicObjectCreationExpression(BoundDynamicObjectCreationExpression node)
{
VisitArguments(node.Arguments, node.ArgumentRefKindsOpt, null);
VisitArguments(node.Arguments, node.ArgumentRefKindsOpt, null, node.ArgsToParamsOpt, node.Expanded);
VisitRvalue(node.InitializerExpressionOpt);
return null;
}

public override BoundNode VisitDynamicIndexerAccess(BoundDynamicIndexerAccess node)
{
VisitRvalue(node.Receiver);
VisitArguments(node.Arguments, node.ArgumentRefKindsOpt, null);
VisitArguments(node.Arguments, node.ArgumentRefKindsOpt, null, default, false);
return null;
}

Expand All @@ -1163,7 +1163,7 @@ public override BoundNode VisitDynamicMemberAccess(BoundDynamicMemberAccess node
public override BoundNode VisitDynamicInvocation(BoundDynamicInvocation node)
{
VisitRvalue(node.Expression);
VisitArguments(node.Arguments, node.ArgumentRefKindsOpt, null);
VisitArguments(node.Arguments, node.ArgumentRefKindsOpt, null, default, false);
return null;
}

Expand Down Expand Up @@ -1246,7 +1246,7 @@ public override BoundNode VisitArgList(BoundArgList node)
public override BoundNode VisitArgListOperator(BoundArgListOperator node)
{
// When we have M(__arglist(x, y, z)) we must visit x, y and z.
VisitArguments(node.Arguments, node.ArgumentRefKindsOpt, null);
VisitArguments(node.Arguments, node.ArgumentRefKindsOpt, null, default, false);
return null;
}

Expand Down Expand Up @@ -1387,7 +1387,7 @@ void visitArgumentsAndCompleteAnalysis(BoundCall node)
VisitLocalFunctionUse(localFunc, node.Syntax, isCall: true);
}

VisitArgumentsAfterCall(node.Arguments, node.ArgumentRefKindsOpt, node.Method);
VisitArgumentsAfterCall(node.Arguments, node.ArgumentRefKindsOpt, node.Method, node.ArgsToParamsOpt, node.Expanded);
VisitReceiverAfterCall(node.ReceiverOpt, node.Method);
}
}
Expand Down Expand Up @@ -1481,7 +1481,7 @@ public override BoundNode VisitIndexerAccess(BoundIndexerAccess node)
{
var method = GetReadMethod(node.Indexer);
VisitReceiverBeforeCall(node.ReceiverOpt, method);
VisitArguments(node.Arguments, node.ArgumentRefKindsOpt, method);
VisitArguments(node.Arguments, node.ArgumentRefKindsOpt, method, node.ArgsToParamsOpt, node.Expanded);
if ((object)method != null)
{
VisitReceiverAfterCall(node.ReceiverOpt, method);
Expand Down Expand Up @@ -1513,11 +1513,11 @@ public override BoundNode VisitEventAssignmentOperator(BoundEventAssignmentOpera
/// <summary>
/// Do not call for a local function.
/// </summary>
protected virtual void VisitArguments(ImmutableArray<BoundExpression> arguments, ImmutableArray<RefKind> refKindsOpt, MethodSymbol method)
protected virtual void VisitArguments(ImmutableArray<BoundExpression> arguments, ImmutableArray<RefKind> refKindsOpt, MethodSymbol method, ImmutableArray<int> argsToParamsOpt, bool expanded)
{
Debug.Assert(method?.OriginalDefinition.MethodKind != MethodKind.LocalFunction);
VisitArgumentsBeforeCall(arguments, refKindsOpt);
VisitArgumentsAfterCall(arguments, refKindsOpt, method);
VisitArgumentsAfterCall(arguments, refKindsOpt, method, argsToParamsOpt, expanded);
}

private void VisitArgumentsBeforeCall(ImmutableArray<BoundExpression> arguments, ImmutableArray<RefKind> refKindsOpt)
Expand All @@ -1537,10 +1537,11 @@ private void VisitArgumentsBeforeCall(ImmutableArray<BoundExpression> arguments,
}
}

#nullable enable
/// <summary>
/// Writes ref and out parameters
/// </summary>
private void VisitArgumentsAfterCall(ImmutableArray<BoundExpression> arguments, ImmutableArray<RefKind> refKindsOpt, MethodSymbol method)
private void VisitArgumentsAfterCall(ImmutableArray<BoundExpression> arguments, ImmutableArray<RefKind> refKindsOpt, MethodSymbol? method, ImmutableArray<int> argsToParamsOpt, bool expanded)
{
for (int i = 0; i < arguments.Length; i++)
{
Expand All @@ -1553,6 +1554,11 @@ private void VisitArgumentsAfterCall(ImmutableArray<BoundExpression> arguments,
case RefKindExtensions.StrictIn:
break;
case RefKind.Ref:
if (method is null || Binder.GetCorrespondingParameter(i, method.Parameters, argsToParamsOpt, expanded)?.RefKind.IsWritableReference() != false)
{
goto case RefKind.Out;
}
break;
case RefKind.Out:
// passing as a byref argument is also a potential write
WriteArgument(arguments[i], refKind, method);
Expand All @@ -1562,6 +1568,7 @@ private void VisitArgumentsAfterCall(ImmutableArray<BoundExpression> arguments,
}
}
}
#nullable disable

protected static RefKind GetRefKind(ImmutableArray<RefKind> refKindsOpt, int index)
{
Expand Down Expand Up @@ -2060,7 +2067,7 @@ protected virtual void VisitLvalueParameter(BoundParameter node)

public override BoundNode VisitObjectCreationExpression(BoundObjectCreationExpression node)
{
VisitArguments(node.Arguments, node.ArgumentRefKindsOpt, node.Constructor);
VisitArguments(node.Arguments, node.ArgumentRefKindsOpt, node.Constructor, node.ArgsToParamsOpt, node.Expanded);
VisitRvalue(node.InitializerExpressionOpt);
return null;
}
Expand Down Expand Up @@ -3535,7 +3542,7 @@ public override BoundNode VisitConvertedStackAllocExpression(BoundConvertedStack
public override BoundNode VisitAnonymousObjectCreationExpression(BoundAnonymousObjectCreationExpression node)
{
// visit arguments as r-values
VisitArguments(node.Arguments, default(ImmutableArray<RefKind>), node.Constructor);
VisitArguments(node.Arguments, default(ImmutableArray<RefKind>), node.Constructor, default, false);

return null;
}
Expand Down Expand Up @@ -3597,7 +3604,7 @@ public override BoundNode VisitObjectInitializerMember(BoundObjectInitializerMem
method = GetReadMethod(property);
}

VisitArguments(node.Arguments, node.ArgumentRefKindsOpt, method);
VisitArguments(node.Arguments, node.ArgumentRefKindsOpt, method, node.ArgsToParamsOpt, node.Expanded);
}

return null;
Expand All @@ -3619,21 +3626,21 @@ public override BoundNode VisitCollectionElementInitializer(BoundCollectionEleme
TLocalState savedState = savedState = this.State.Clone();
SetUnreachable();

VisitArguments(node.Arguments, default(ImmutableArray<RefKind>), node.AddMethod);
VisitArguments(node.Arguments, default(ImmutableArray<RefKind>), node.AddMethod, node.ArgsToParamsOpt, node.Expanded);

this.State = savedState;
}
else
{
VisitArguments(node.Arguments, default(ImmutableArray<RefKind>), node.AddMethod);
VisitArguments(node.Arguments, default(ImmutableArray<RefKind>), node.AddMethod, node.ArgsToParamsOpt, node.Expanded);
}

return null;
}

public override BoundNode VisitDynamicCollectionElementInitializer(BoundDynamicCollectionElementInitializer node)
{
VisitArguments(node.Arguments, default(ImmutableArray<RefKind>), method: null);
VisitArguments(node.Arguments, default(ImmutableArray<RefKind>), method: null, default, false);
return null;
}

Expand Down Expand Up @@ -3751,7 +3758,7 @@ public override BoundNode VisitReadOnlySpanFromArray(BoundReadOnlySpanFromArray
public override BoundNode VisitFunctionPointerInvocation(BoundFunctionPointerInvocation node)
{
VisitRvalue(node.InvokedExpression);
VisitArguments(node.Arguments, node.ArgumentRefKindsOpt, node.FunctionPointer.Signature);
VisitArguments(node.Arguments, node.ArgumentRefKindsOpt, node.FunctionPointer.Signature, default, false);
return null;
}

Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6824,7 +6824,7 @@ private static bool HasImplicitTypeArguments(SyntaxNode syntax)
return nameSyntax.Kind() != SyntaxKind.GenericName;
}

protected override void VisitArguments(ImmutableArray<BoundExpression> arguments, ImmutableArray<RefKind> refKindsOpt, MethodSymbol method)
protected override void VisitArguments(ImmutableArray<BoundExpression> arguments, ImmutableArray<RefKind> refKindsOpt, MethodSymbol method, ImmutableArray<int> argsToParamsOpt, bool expanded)
{
// Callers should be using VisitArguments overload below.
throw ExceptionUtilities.Unreachable();
Expand Down
47 changes: 47 additions & 0 deletions src/Compilers/CSharp/Test/Emit3/RefReadonlyParameterTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8664,4 +8664,51 @@ static void Main()
// d = (ref int x) => { x = 42; }; // should be an error
Diagnostic(ErrorCode.ERR_BadParamRef, "x").WithArguments("1", "in").WithLocation(7, 22));
}

[Theory, CombinatorialData, WorkItem("https://github.com/dotnet/roslyn/issues/77528")]
public void UnassignedField(
[CombinatorialValues("in", "ref")] string arg,
[CombinatorialValues("ref readonly", "in")] string param)
{
var source = $$"""
#pragma warning disable CS9191 // The 'ref' modifier for argument corresponding to 'in' parameter is equivalent to 'in'. Consider using 'in' instead.

M({{arg}} C.F);

static void M({{param}} int i) { }

static class C
{
public static int F;
}
""";
CreateCompilation(source).VerifyDiagnostics(
// (9,23): warning CS0649: Field 'C.F' is never assigned to, and will always have its default value 0
// public static int F;
Diagnostic(ErrorCode.WRN_UnassignedInternalField, "F").WithArguments("C.F", "0").WithLocation(9, 23));
}

[Theory, CombinatorialData, WorkItem("https://github.com/dotnet/roslyn/issues/77528")]
public void UnassignedField_NamedArguments(
[CombinatorialValues("in", "ref")] string arg,
[CombinatorialValues("ref readonly", "in")] string param)
{
var source = $$"""
#pragma warning disable CS9191 // The 'ref' modifier for argument corresponding to 'in' parameter is equivalent to 'in'. Consider using 'in' instead.

M(j: ref C.F1, i: {{arg}} C.F2);

static void M({{param}} int i, ref int j) { }

static class C
{
public static int F1;
public static int F2;
}
""";
CreateCompilation(source).VerifyDiagnostics(
// (10,23): warning CS0649: Field 'C.F2' is never assigned to, and will always have its default value 0
// public static int F2;
Diagnostic(ErrorCode.WRN_UnassignedInternalField, "F2").WithArguments("C.F2", "0").WithLocation(10, 23));
}
}