Skip to content

Commit

Permalink
Fix regression in definite assignment analysis for out parameters in …
Browse files Browse the repository at this point in the history 
…presence of local functions.

Fixes dotnet#69775.
  • Loading branch information
@AlekseyTs
AlekseyTs committed Oct 26, 2023
1 parent bb259b0 commit 474e087
Show file tree
Hide file tree
Showing 4 changed files with 122 additions and 19 deletions.
20 changes: 13 additions & 7 deletions src/Compilers/CSharp/Portable/FlowAnalysis/DefiniteAssignment.cs
View file
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
View file
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
View file
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
20 changes: 10 additions & 10 deletions src/Compilers/CSharp/Test/Emit2/FlowAnalysis/RegionAnalysisTests.cs
View file
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

0 comments on commit 474e087

Please sign in to comment.