Skip to content

Commit 0b8a8c0

Browse files
agockeagocke
authored
Fix control flow analysis for local functions (#40191)
Local function control flow analysis should now use the analysis walker from the base class instead of defining its own. Fixes #39946
1 parent cc3c5bc commit 0b8a8c0

File tree

9 files changed

+277
-46
lines changed

9 files changed

+277
-46
lines changed

‎src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -575,7 +575,10 @@ BoundBlock runAnalysis(BoundBlock block, DiagnosticBag blockDiagnostics)
575575
// rather we go directly to LowerBodyOrInitializer, which skips over flow analysis (which is in CompileMethod)
576576
// (the same thing - calling ControlFlowPass.Analyze in the lowering - is done for lambdas)
577577
// It's a bit of code duplication, but refactoring would make things worse.
578-
var endIsReachable = ControlFlowPass.Analyze(localSymbol.DeclaringCompilation, localSymbol, block, blockDiagnostics);
578+
// However, we don't need to report diagnostics here. They will be reported when analyzing the parent method.
579+
var ignored = DiagnosticBag.GetInstance();
580+
var endIsReachable = ControlFlowPass.Analyze(localSymbol.DeclaringCompilation, localSymbol, block, ignored);
581+
ignored.Free();
579582
if (endIsReachable)
580583
{
581584
if (ImplicitReturnIsOkay(localSymbol))

‎src/Compilers/CSharp/Portable/FlowAnalysis/AbstractRegionControlFlowPass.cs‎

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@
55

66
namespace Microsoft.CodeAnalysis.CSharp
77
{
8-
// Note: this code has a copy-and-paste sibling in AbstractRegionDataFlowPass. Any fix to
9-
// one should be applied to the other.
108
internal abstract class AbstractRegionControlFlowPass : ControlFlowPass
119
{
1210
internal AbstractRegionControlFlowPass(
@@ -27,22 +25,12 @@ public override BoundNode Visit(BoundNode node)
2725

2826
// Control flow analysis does not normally scan the body of a lambda, but region analysis does.
2927
public override BoundNode VisitLambda(BoundLambda node)
30-
{
31-
return VisitLocalFunctionOrLambda(node.Body);
32-
}
33-
34-
public override BoundNode VisitLocalFunctionStatement(BoundLocalFunctionStatement node)
35-
{
36-
return VisitLocalFunctionOrLambda(node.Body);
37-
}
38-
39-
private BoundNode VisitLocalFunctionOrLambda(BoundBlock body)
4028
{
4129
var oldPending = SavePending(); // We do not support branches *into* a lambda.
4230
LocalState finalState = this.State;
4331
this.State = TopState();
4432
var oldPending2 = SavePending();
45-
VisitAlways(body);
33+
VisitAlways(node.Body);
4634
RestorePending(oldPending2); // process any forward branches within the lambda body
4735
ImmutableArray<PendingBranch> pendingReturns = RemoveReturns();
4836
RestorePending(oldPending);

‎src/Compilers/CSharp/Portable/FlowAnalysis/AbstractRegionDataFlowPass.cs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@
77

88
namespace Microsoft.CodeAnalysis.CSharp
99
{
10-
// Note: this code has a copy-and-paste sibling in AbstractRegionControlFlowPass.
11-
// Any fix to one should be applied to the other.
1210
internal abstract class AbstractRegionDataFlowPass : DefiniteAssignmentPass
1311
{
1412
internal AbstractRegionDataFlowPass(

‎src/Compilers/CSharp/Portable/FlowAnalysis/ControlFlowPass.cs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,11 @@ protected override ImmutableArray<PendingBranch> RemoveReturns()
199199
var result = base.RemoveReturns();
200200
foreach (var pending in result)
201201
{
202+
if (pending.Branch is null)
203+
{
204+
continue;
205+
}
206+
202207
switch (pending.Branch.Kind)
203208
{
204209
case BoundKind.GotoStatement:
@@ -388,10 +393,5 @@ public override BoundNode VisitBlock(BoundBlock node)
388393
_currentBlock = parentBlock;
389394
return result;
390395
}
391-
392-
public override BoundNode VisitLocalFunctionStatement(BoundLocalFunctionStatement localFunc)
393-
{
394-
return null;
395-
}
396396
}
397397
}

‎src/Compilers/CSharp/Test/IOperation/IOperation/IOperationTests_IBranchOperation.cs‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1404,9 +1404,6 @@ void local()
14041404
// file.cs(9,13): error CS0159: No such label 'label' within the scope of the goto statement
14051405
// goto label;
14061406
Diagnostic(ErrorCode.ERR_LabelNotFound, "goto").WithArguments("label").WithLocation(9, 13),
1407-
// file.cs(12,1): warning CS0164: This label has not been referenced
1408-
// label: ;
1409-
Diagnostic(ErrorCode.WRN_UnreferencedLabel, "label").WithLocation(12, 1)
14101407
};
14111408

14121409
VerifyFlowGraphAndDiagnosticsForTest<BlockSyntax>(source, expectedGraph, expectedDiagnostics);
@@ -1474,9 +1471,6 @@ void local(bool input)
14741471
// file.cs(11,13): error CS0159: No such label 'label' within the scope of the goto statement
14751472
// goto label;
14761473
Diagnostic(ErrorCode.ERR_LabelNotFound, "goto").WithArguments("label").WithLocation(11, 13),
1477-
// file.cs(14,1): warning CS0164: This label has not been referenced
1478-
// label: ;
1479-
Diagnostic(ErrorCode.WRN_UnreferencedLabel, "label").WithLocation(14, 1)
14801474
};
14811475

14821476
VerifyFlowGraphAndDiagnosticsForTest<BlockSyntax>(source, expectedGraph, expectedDiagnostics);

‎src/Compilers/CSharp/Test/Semantic/FlowAnalysis/LocalFunctions.cs‎

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -383,18 +383,15 @@ void L3()
383383
}
384384
}");
385385
comp.VerifyDiagnostics(
386-
// (14,13): error CS0159: No such label 'label1' within the scope of the goto statement
387-
// goto label1;
388-
Diagnostic(ErrorCode.ERR_LabelNotFound, "goto").WithArguments("label1").WithLocation(14, 13),
389-
// (19,13): error CS0139: No enclosing loop out of which to break or continue
390-
// break;
391-
Diagnostic(ErrorCode.ERR_NoBreakOrCont, "break;").WithLocation(19, 13),
392-
// (24,13): error CS0139: No enclosing loop out of which to break or continue
393-
// continue;
394-
Diagnostic(ErrorCode.ERR_NoBreakOrCont, "continue;").WithLocation(24, 13),
395-
// (6,9): warning CS0164: This label has not been referenced
396-
// label1:
397-
Diagnostic(ErrorCode.WRN_UnreferencedLabel, "label1").WithLocation(6, 9));
386+
// (19,13): error CS0139: No enclosing loop out of which to break or continue
387+
// break;
388+
Diagnostic(ErrorCode.ERR_NoBreakOrCont, "break;").WithLocation(19, 13),
389+
// (24,13): error CS0139: No enclosing loop out of which to break or continue
390+
// continue;
391+
Diagnostic(ErrorCode.ERR_NoBreakOrCont, "continue;").WithLocation(24, 13),
392+
// (14,13): error CS0159: No such label 'label1' within the scope of the goto statement
393+
// goto label1;
394+
Diagnostic(ErrorCode.ERR_LabelNotFound, "goto").WithArguments("label1").WithLocation(14, 13));
398395
}
399396

400397
[Fact]

‎src/Compilers/CSharp/Test/Semantic/FlowAnalysis/RegionAnalysisTests.cs‎

Lines changed: 203 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4129,6 +4129,209 @@ static void Main(string[] args)
41294129

41304130
#region "lambda"
41314131

4132+
[Fact]
4133+
[WorkItem(39946, "https://github.com/dotnet/roslyn/issues/39946")]
4134+
public void DataFlowAnalysisLocalFunctions9()
4135+
{
4136+
var results = CompileAndAnalyzeControlAndDataFlowStatements(@"
4137+
class C
4138+
{
4139+
int Testing;
4140+
4141+
void M()
4142+
{
4143+
local();
4144+
4145+
/*<bind>*/
4146+
NewMethod();
4147+
/*</bind>*/
4148+
4149+
Testing = 5;
4150+
4151+
void local()
4152+
{ }
4153+
}
4154+
4155+
void NewMethod()
4156+
{
4157+
}
4158+
}");
4159+
var dataFlow = results.dataFlowAnalysis;
4160+
Assert.True(dataFlow.Succeeded);
4161+
Assert.Null(GetSymbolNamesJoined(dataFlow.Captured));
4162+
Assert.Null(GetSymbolNamesJoined(dataFlow.CapturedInside));
4163+
Assert.Null(GetSymbolNamesJoined(dataFlow.CapturedOutside));
4164+
Assert.Null(GetSymbolNamesJoined(dataFlow.VariablesDeclared));
4165+
Assert.Equal("this", GetSymbolNamesJoined(dataFlow.DataFlowsIn));
4166+
Assert.Null(GetSymbolNamesJoined(dataFlow.DataFlowsOut));
4167+
Assert.Equal("this", GetSymbolNamesJoined(dataFlow.DefinitelyAssignedOnEntry));
4168+
Assert.Equal("this", GetSymbolNamesJoined(dataFlow.DefinitelyAssignedOnExit));
4169+
Assert.Equal("this", GetSymbolNamesJoined(dataFlow.ReadInside));
4170+
Assert.Null(GetSymbolNamesJoined(dataFlow.WrittenInside));
4171+
Assert.Equal("this", GetSymbolNamesJoined(dataFlow.ReadOutside));
4172+
Assert.Equal("this", GetSymbolNamesJoined(dataFlow.WrittenOutside));
4173+
4174+
var controlFlow = results.controlFlowAnalysis;
4175+
Assert.True(controlFlow.Succeeded);
4176+
Assert.True(controlFlow.EndPointIsReachable);
4177+
}
4178+
4179+
[Fact]
4180+
[WorkItem(39946, "https://github.com/dotnet/roslyn/issues/39946")]
4181+
public void ControlFlowAnalysisLocalFunctions01()
4182+
{
4183+
var controlFlow = CompileAndAnalyzeControlFlowStatements(@"
4184+
class C
4185+
{
4186+
void M()
4187+
{
4188+
local();
4189+
4190+
/*<bind>*/
4191+
System.Console.WriteLine(0);
4192+
/*</bind>*/
4193+
4194+
void local()
4195+
{
4196+
throw null;
4197+
}
4198+
}
4199+
}");
4200+
4201+
Assert.True(controlFlow.Succeeded);
4202+
Assert.False(controlFlow.StartPointIsReachable);
4203+
Assert.False(controlFlow.EndPointIsReachable);
4204+
}
4205+
4206+
[Fact]
4207+
[WorkItem(39946, "https://github.com/dotnet/roslyn/issues/39946")]
4208+
public void ControlFlowAnalysisLocalFunctions02()
4209+
{
4210+
var controlFlow = CompileAndAnalyzeControlFlowStatements(@"
4211+
class C
4212+
{
4213+
void M()
4214+
{
4215+
/*<bind>*/
4216+
local();
4217+
System.Console.WriteLine(0);
4218+
/*</bind>*/
4219+
4220+
void local()
4221+
{
4222+
throw null;
4223+
}
4224+
}
4225+
}");
4226+
4227+
Assert.True(controlFlow.Succeeded);
4228+
Assert.True(controlFlow.StartPointIsReachable);
4229+
Assert.False(controlFlow.EndPointIsReachable);
4230+
}
4231+
4232+
[Fact]
4233+
[WorkItem(39946, "https://github.com/dotnet/roslyn/issues/39946")]
4234+
public void ControlFlowAnalysisLocalFunctions03()
4235+
{
4236+
var controlFlow = CompileAndAnalyzeControlFlowStatements(@"
4237+
class C
4238+
{
4239+
void M()
4240+
{
4241+
/*<bind>*/
4242+
System.Console.WriteLine(0);
4243+
/*</bind>*/
4244+
local();
4245+
4246+
void local()
4247+
{
4248+
throw null;
4249+
}
4250+
}
4251+
}");
4252+
4253+
Assert.True(controlFlow.Succeeded);
4254+
Assert.True(controlFlow.StartPointIsReachable);
4255+
Assert.True(controlFlow.EndPointIsReachable);
4256+
}
4257+
4258+
[Fact]
4259+
[WorkItem(39946, "https://github.com/dotnet/roslyn/issues/39946")]
4260+
public void ControlFlowAnalysisLocalFunctions04()
4261+
{
4262+
var controlFlow = CompileAndAnalyzeControlFlowStatements(@"
4263+
class C
4264+
{
4265+
void M()
4266+
{
4267+
System.Console.WriteLine(0);
4268+
local();
4269+
4270+
void local()
4271+
{
4272+
/*<bind>*/
4273+
throw null;
4274+
/*</bind>*/
4275+
}
4276+
}
4277+
}");
4278+
4279+
Assert.True(controlFlow.Succeeded);
4280+
Assert.True(controlFlow.StartPointIsReachable);
4281+
Assert.False(controlFlow.EndPointIsReachable);
4282+
}
4283+
4284+
[Fact]
4285+
[WorkItem(39946, "https://github.com/dotnet/roslyn/issues/39946")]
4286+
public void ControlFlowAnalysisLocalFunctions05()
4287+
{
4288+
var controlFlow = CompileAndAnalyzeControlFlowStatements(@"
4289+
class C
4290+
{
4291+
void M()
4292+
{
4293+
local();
4294+
4295+
void local()
4296+
{
4297+
/*<bind>*/
4298+
System.Console.WriteLine(0);
4299+
/*</bind>*/
4300+
throw null;
4301+
}
4302+
}
4303+
}");
4304+
4305+
Assert.True(controlFlow.Succeeded);
4306+
Assert.True(controlFlow.StartPointIsReachable);
4307+
Assert.True(controlFlow.EndPointIsReachable);
4308+
}
4309+
4310+
[Fact]
4311+
[WorkItem(39946, "https://github.com/dotnet/roslyn/issues/39946")]
4312+
public void ControlFlowAnalysisLocalFunctions06()
4313+
{
4314+
var controlFlow = CompileAndAnalyzeControlFlowStatements(@"
4315+
class C
4316+
{
4317+
void M()
4318+
{
4319+
void local()
4320+
{
4321+
throw null;
4322+
}
4323+
4324+
/*<bind>*/
4325+
System.Console.WriteLine(0);
4326+
/*</bind>*/
4327+
}
4328+
}");
4329+
4330+
Assert.True(controlFlow.Succeeded);
4331+
Assert.True(controlFlow.StartPointIsReachable);
4332+
Assert.True(controlFlow.EndPointIsReachable);
4333+
}
4334+
41324335
[Fact]
41334336
public void TestReturnStatements03()
41344337
{

‎src/Compilers/CSharp/Test/Semantic/Semantics/LocalFunctionTests.cs‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2040,13 +2040,10 @@ void Local()
20402040
}
20412041
}";
20422042
VerifyDiagnostics(source,
2043-
// (8,13): error CS0159: No such label 'A' within the scope of the goto statement
2044-
// goto A;
2045-
Diagnostic(ErrorCode.ERR_LabelNotFound, "goto").WithArguments("A").WithLocation(8, 13),
2046-
// (10,5): warning CS0164: This label has not been referenced
2047-
// A: Local();
2048-
Diagnostic(ErrorCode.WRN_UnreferencedLabel, "A").WithLocation(10, 5)
2049-
);
2043+
// (8,13): error CS0159: No such label 'A' within the scope of the goto statement
2044+
// goto A;
2045+
Diagnostic(ErrorCode.ERR_LabelNotFound, "goto").WithArguments("A").WithLocation(8, 13)
2046+
);
20502047
}
20512048

20522049
[Fact]

0 commit comments

Comments
 (0)