Skip to content
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.

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

Fix regression in definite assignment analysis for out parameters in presence of local functions. #70563

Merged
merged 2 commits into from
Oct 26, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -1789,22 +1789,28 @@ protected override LocalState TopState()
{
if ((object)current != CurrentSymbol && current is MethodSymbol method)
{
// Enclosing method parameters are definitely assigned
// Enclosing method input parameters are definitely assigned
foreach (var parameter in method.Parameters)
{
int slot = GetOrCreateSlot(parameter);
if (slot > 0)
if (parameter.RefKind != RefKind.Out)
{
SetSlotAssigned(slot, ref topState);
int slot = GetOrCreateSlot(parameter);
if (slot > 0)
{
SetSlotAssigned(slot, ref topState);
}
}
}

if (method.TryGetThisParameter(out ParameterSymbol thisParameter) && thisParameter is not null)
{
int slot = GetOrCreateSlot(thisParameter);
if (slot > 0)
if (thisParameter.RefKind != RefKind.Out)
{
SetSlotAssigned(slot, ref topState);
int slot = GetOrCreateSlot(thisParameter);
if (slot > 0)
{
SetSlotAssigned(slot, ref topState);
}
}
}
}
Expand Down
88 changes: 88 additions & 0 deletions src/Compilers/CSharp/Test/Emit2/FlowAnalysis/FlowTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5938,5 +5938,93 @@ public class C
// public static string M() => nameof(c.c.c);
Diagnostic(ErrorCode.ERR_FeatureNotAvailableInVersion11, "c").WithArguments("instance member in 'nameof'", "12.0").WithLocation(5, 40));
}

[Fact]
[WorkItem("https://github.com/dotnet/roslyn/issues/69775")]
public void OutParameterIsNotAssigned_01()
{
var source = """
#pragma warning disable CS8321 // The local function is declared but never used

static void bug(out int a)
{
bool hasLevel() => true;
hasLevel();
}
""";

CreateCompilation(source).VerifyDiagnostics(
// (3,13): error CS0177: The out parameter 'a' must be assigned to before control leaves the current method
// static void bug(out int a)
Diagnostic(ErrorCode.ERR_ParamUnassigned, "bug").WithArguments("a").WithLocation(3, 13)
);
}

[Fact]
[WorkItem("https://github.com/dotnet/roslyn/issues/69775")]
public void OutParameterIsNotAssigned_02()
{
var source = """
#pragma warning disable CS8321 // The local function is declared but never used

static void bug(out int a)
{
bool hasLevel() => true;
}
""";

CreateCompilation(source).VerifyDiagnostics(
// (3,13): error CS0177: The out parameter 'a' must be assigned to before control leaves the current method
// static void bug(out int a)
Diagnostic(ErrorCode.ERR_ParamUnassigned, "bug").WithArguments("a").WithLocation(3, 13)
);
}

[Fact]
[WorkItem("https://github.com/dotnet/roslyn/issues/69775")]
public void OutParameterIsNotAssigned_03()
{
var source = """
#pragma warning disable CS8321 // The local function is declared but never used

class C
{
static void bug(out int a)
{
bool hasLevel() => true;
hasLevel();
}
}
""";

CreateCompilation(source).VerifyDiagnostics(
// (5,17): error CS0177: The out parameter 'a' must be assigned to before control leaves the current method
// static void bug(out int a)
Diagnostic(ErrorCode.ERR_ParamUnassigned, "bug").WithArguments("a").WithLocation(5, 17)
);
}

[Fact]
[WorkItem("https://github.com/dotnet/roslyn/issues/69775")]
public void OutParameterIsNotAssigned_04()
{
var source = """
#pragma warning disable CS8321 // The local function is declared but never used

class C
{
static void bug(out int a)
{
bool hasLevel() => true;
}
}
""";

CreateCompilation(source).VerifyDiagnostics(
// (5,17): error CS0177: The out parameter 'a' must be assigned to before control leaves the current method
// static void bug(out int a)
Diagnostic(ErrorCode.ERR_ParamUnassigned, "bug").WithArguments("a").WithLocation(5, 17)
);
}
}
}
13 changes: 11 additions & 2 deletions src/Compilers/CSharp/Test/Emit2/FlowAnalysis/LocalFunctions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -729,7 +729,13 @@ void Local()
Diagnostic(ErrorCode.ERR_ThisStructNotInAnonMeth, "_x").WithLocation(10, 20),
// (7,17): error CS0188: The 'this' object cannot be used before all of its fields have been assigned. Consider updating to language version '11.0' to auto-default the unassigned fields.
// var s = this;
Diagnostic(ErrorCode.ERR_UseDefViolationThisUnsupportedVersion, "this").WithArguments("11.0").WithLocation(7, 17)
Diagnostic(ErrorCode.ERR_UseDefViolationThisUnsupportedVersion, "this").WithArguments("11.0").WithLocation(7, 17),
// (12,9): error CS9015: Use of possibly unassigned field '_x'. Consider updating to language version '11.0' to auto-default the field.
// Local();
Diagnostic(ErrorCode.ERR_UseDefViolationFieldUnsupportedVersion, "Local()").WithArguments("_x", "11.0").WithLocation(12, 9),
// (5,12): error CS0171: Field 'S._x' must be fully assigned before control is returned to the caller. Consider updating to language version '11.0' to auto-default the field.
// public S(int x)
Diagnostic(ErrorCode.ERR_UnassignedThisUnsupportedVersion, "S").WithArguments("S._x", "11.0").WithLocation(5, 12)
);

comp = CreateCompilation(source, parseOptions: TestOptions.Regular11);
Expand Down Expand Up @@ -1497,7 +1503,10 @@ void Local()
comp.VerifyDiagnostics(
// (12,20): error CS1673: Anonymous methods, lambda expressions, query expressions, and local functions inside structs cannot access instance members of 'this'. Consider copying 'this' to a local variable outside the anonymous method, lambda expression, query expression, or local function and using the local instead.
// S s2 = this;
Diagnostic(ErrorCode.ERR_ThisStructNotInAnonMeth, "this").WithLocation(12, 20)
Diagnostic(ErrorCode.ERR_ThisStructNotInAnonMeth, "this").WithLocation(12, 20),
// (14,9): error CS9015: Use of possibly unassigned field '_x'. Consider updating to language version '11.0' to auto-default the field.
// Local();
Diagnostic(ErrorCode.ERR_UseDefViolationFieldUnsupportedVersion, "Local()").WithArguments("_x", "11.0").WithLocation(14, 9)
);

comp = CreateCompilation(source, parseOptions: TestOptions.Regular11);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9691,9 +9691,9 @@ struct B
B(string x) {}
public int y;
}
"));
"), thisIsAssignedOnEntry: false);

static void verify(DataFlowAnalysis analysis)
static void verify(DataFlowAnalysis analysis, bool thisIsAssignedOnEntry = true)
{
Assert.Null(GetSymbolNamesJoined(analysis.AlwaysAssigned));
Assert.Equal("x", GetSymbolNamesJoined(analysis.Captured));
Expand All @@ -9704,8 +9704,8 @@ static void verify(DataFlowAnalysis analysis)
Assert.Null(GetSymbolNamesJoined(analysis.DataFlowsIn));
Assert.Null(GetSymbolNamesJoined(analysis.DataFlowsOut));

Assert.Equal("this, x", GetSymbolNamesJoined(analysis.DefinitelyAssignedOnEntry, sort: true));
Assert.Equal("this, x", GetSymbolNamesJoined(analysis.DefinitelyAssignedOnExit, sort: true));
Assert.Equal((thisIsAssignedOnEntry ? "this, " : "") + "x", GetSymbolNamesJoined(analysis.DefinitelyAssignedOnEntry, sort: true));
Assert.Equal((thisIsAssignedOnEntry ? "this, " : "") + "x", GetSymbolNamesJoined(analysis.DefinitelyAssignedOnExit, sort: true));

Assert.Equal("x", GetSymbolNamesJoined(analysis.ReadInside));
Assert.Equal("this", GetSymbolNamesJoined(analysis.ReadOutside));
Expand Down Expand Up @@ -10804,7 +10804,7 @@ void M(out int x)
int X() => /*<bind>*/x = 1/*</bind>*/;
}
}
"));
"), xIsAssignedOnEntry: false);

verify(CompileAndAnalyzeDataFlowExpression(@"
class B
Expand All @@ -10816,7 +10816,7 @@ void M(out int x)
static int X() => /*<bind>*/x = 1/*</bind>*/;
}
}
"));
"), xIsAssignedOnEntry: false);

verify(CompileAndAnalyzeDataFlowExpression(@"
class B
Expand Down Expand Up @@ -10865,7 +10865,7 @@ class B
int X() => /*<bind>*/x = 1/*</bind>*/;
}
}
"));
"), xIsAssignedOnEntry: false);

verify(CompileAndAnalyzeDataFlowExpression(@"
class B
Expand All @@ -10877,7 +10877,7 @@ class B
static int X() => /*<bind>*/x = 1/*</bind>*/;
}
}
"));
"), xIsAssignedOnEntry: false);

verify(CompileAndAnalyzeDataFlowExpression(@"
class B
Expand Down Expand Up @@ -10938,7 +10938,7 @@ class B
}
"));

static void verify(DataFlowAnalysis analysis)
static void verify(DataFlowAnalysis analysis, bool xIsAssignedOnEntry = true)
{
Assert.Equal("x", GetSymbolNamesJoined(analysis.AlwaysAssigned));
Assert.Equal("x", GetSymbolNamesJoined(analysis.Captured));
Expand All @@ -10949,7 +10949,7 @@ static void verify(DataFlowAnalysis analysis)
Assert.Null(GetSymbolNamesJoined(analysis.DataFlowsIn));
Assert.Equal("x", GetSymbolNamesJoined(analysis.DataFlowsOut));

Assert.Equal("this, x", GetSymbolNamesJoined(analysis.DefinitelyAssignedOnEntry, sort: true));
Assert.Equal("this" + (xIsAssignedOnEntry ? ", x" : ""), GetSymbolNamesJoined(analysis.DefinitelyAssignedOnEntry, sort: true));
Assert.Equal("this, x", GetSymbolNamesJoined(analysis.DefinitelyAssignedOnExit, sort: true));

Assert.Null(GetSymbolNamesJoined(analysis.ReadInside));
Expand Down