diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs index d02d0d62cc8d79443a686e1a0bf4d5febc514fa1..58239b7969e0a21a7163526a68703bfdcb9cdd0d 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs @@ -64,10 +64,13 @@ public override BoundNode VisitTryStatement(BoundTryStatement node) // to execute the finally block before occurring. Also, we do not handle branches // *into* the finally block. SetState(finallyState); - var tryAndCatchPending = SavePending(); var unsetInFinally = AllBitsSet(); VisitFinallyBlock(node.FinallyBlockOpt, ref unsetInFinally); - foreach (var pend in tryAndCatchPending.PendingBranches) + + // all the branches that are still pending must have been introduced in try/finally + // with exception of YieldReturn, the branches logically go through finally + // so we must Union/Intersect finally state as appropriate + foreach (var pend in PendingBranches) { if (pend.Branch == null) continue; // a tracked exception if (pend.Branch.Kind != BoundKind.YieldReturnStatement) @@ -77,7 +80,6 @@ public override BoundNode VisitTryStatement(BoundTryStatement node) } } - RestorePending(tryAndCatchPending); UnionWith(ref endState, ref this.State); if (trackUnassignments) IntersectWith(ref endState, ref unsetInFinally); } @@ -87,13 +89,16 @@ public override BoundNode VisitTryStatement(BoundTryStatement node) return null; } - protected virtual void VisitTryBlock(BoundStatement tryBlock, BoundTryStatement node, ref TLocalState tryState) + protected virtual void VisitTryBlock(BoundBlock tryBlock, BoundTryStatement node, ref TLocalState tryState) { + var oldPending = SavePending(); VisitStatement(tryBlock); + RestorePending(oldPending); } protected virtual void VisitCatchBlock(BoundCatchBlock catchBlock, ref TLocalState finallyState) { + var oldPending = SavePending(); if (catchBlock.ExceptionSourceOpt != null) { VisitLvalue(catchBlock.ExceptionSourceOpt); @@ -106,11 +111,14 @@ protected virtual void VisitCatchBlock(BoundCatchBlock catchBlock, ref TLocalSta } VisitStatement(catchBlock.Body); + RestorePending(oldPending); } - protected virtual void VisitFinallyBlock(BoundStatement finallyBlock, ref TLocalState unsetInFinally) + protected virtual void VisitFinallyBlock(BoundBlock finallyBlock, ref TLocalState unsetInFinally) { + var oldPending = SavePending(); VisitStatement(finallyBlock); // this should generate no pending branches + RestorePending(oldPending); } } } diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/ControlFlowPass.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/ControlFlowPass.cs index 5ec54947d87dcdd40ffeb597c9637c538b2415b2..3f944be4ee11e2f6f97dcc38e905d008bb6ab9d4 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/ControlFlowPass.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/ControlFlowPass.cs @@ -245,32 +245,10 @@ private void CheckReachable(BoundStatement statement) } } - protected override void VisitTryBlock(BoundStatement tryBlock, BoundTryStatement node, ref LocalState tryState) - { - if (node.CatchBlocks.IsEmpty) - { - base.VisitTryBlock(tryBlock, node, ref tryState); - return; - } - - var oldPending = SavePending(); // we do not support branches into a try block - base.VisitTryBlock(tryBlock, node, ref tryState); - RestorePending(oldPending); - } - - protected override void VisitCatchBlock(BoundCatchBlock catchBlock, ref LocalState finallyState) - { - var oldPending = SavePending(); // we do not support branches into a catch block - base.VisitCatchBlock(catchBlock, ref finallyState); - RestorePending(oldPending); - } - - protected override void VisitFinallyBlock(BoundStatement finallyBlock, ref LocalState endState) + protected override void VisitFinallyBlock(BoundBlock finallyBlock, ref LocalState endState) { var oldPending1 = SavePending(); // we do not support branches into a finally block - var oldPending2 = SavePending(); // track only the branches out of the finally block base.VisitFinallyBlock(finallyBlock, ref endState); - RestorePending(oldPending2); // resolve branches that remain within the finally block foreach (var branch in PendingBranches) { if (branch.Branch == null) continue; // a tracked exception diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/DataFlowPass.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/DataFlowPass.cs index 034ba0a081736838ce84372db34a0013ac051500..e9e5c7a4dc365a93c8975848bfad509a25067a51 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/DataFlowPass.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/DataFlowPass.cs @@ -2034,7 +2034,7 @@ public override BoundNode VisitBaseReference(BoundBaseReference node) #region TryStatements private OptionalState _tryState; - protected override void VisitTryBlock(BoundStatement tryBlock, BoundTryStatement node, ref LocalState tryState) + protected override void VisitTryBlock(BoundBlock tryBlock, BoundTryStatement node, ref LocalState tryState) { if (trackUnassignments) { @@ -2098,7 +2098,7 @@ private void VisitCatchBlockInternal(BoundCatchBlock catchBlock, ref LocalState } } - protected override void VisitFinallyBlock(BoundStatement finallyBlock, ref LocalState unsetInFinally) + protected override void VisitFinallyBlock(BoundBlock finallyBlock, ref LocalState unsetInFinally) { if (trackUnassignments) { diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/PreciseAbstractFlowPass.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/PreciseAbstractFlowPass.cs index 525037fd5ca0b7959001d5dbac74bb93bd35b6a6..229d825d5b9c0190a27d3faf4d47681f3a9e5aad 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/PreciseAbstractFlowPass.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/PreciseAbstractFlowPass.cs @@ -809,12 +809,10 @@ protected struct SavedPending public readonly ArrayBuilder PendingBranches; public readonly PooledHashSet LabelsSeen; - public SavedPending(ref ArrayBuilder pendingBranches, ref PooledHashSet labelsSeen) + public SavedPending(ArrayBuilder pendingBranches, PooledHashSet labelsSeen) { this.PendingBranches = pendingBranches; this.LabelsSeen = labelsSeen; - pendingBranches = ArrayBuilder.GetInstance(); - labelsSeen = PooledHashSet.GetInstance(); } } @@ -826,7 +824,11 @@ public SavedPending(ref ArrayBuilder pendingBranches, ref PooledH protected SavedPending SavePending() { Debug.Assert(!this.IsConditionalState); - var result = new SavedPending(ref _pendingBranches, ref _labelsSeen); + var result = new SavedPending(_pendingBranches, _labelsSeen); + + _pendingBranches = ArrayBuilder.GetInstance(); + _labelsSeen = PooledHashSet.GetInstance(); + if (_trackExceptions) { _pendingBranches.Add(new PendingBranch(null, this.State)); @@ -836,7 +838,10 @@ protected SavedPending SavePending() } /// - /// We use this to restore the old set of pending branches after visiting a construct that contains nested statements. + /// We use this when closing a block that may contain labels or branches + /// - branches to new labels are resolved + /// - new labels are removed (no longer can be reached) + /// - unresolved pending branches are carried forward /// /// The old pending branches, which are to be merged with the current ones protected void RestorePending(SavedPending oldPending) @@ -881,20 +886,22 @@ protected void RestorePending(SavedPending oldPending) } } - int i = 0; - int n = _pendingBranches.Count; if (_trackExceptions) { Debug.Assert(oldPending.PendingBranches[0].Branch == null); Debug.Assert(this.PendingBranches[0].Branch == null); this.IntersectWith(ref oldPending.PendingBranches[0].State, ref this.PendingBranches[0].State); - i++; - } - for (; i < n; i++) - { - oldPending.PendingBranches.Add(this.PendingBranches[i]); + for (int i = 1, c = this.PendingBranches.Count; i < c; i++) + { + oldPending.PendingBranches.Add(this.PendingBranches[i]); + } } + else + { + oldPending.PendingBranches.AddRange(this.PendingBranches); + } + _pendingBranches.Free(); _pendingBranches = oldPending.PendingBranches; diff --git a/src/Compilers/CSharp/Portable/Lowering/StateMachineRewriter/IteratorAndAsyncCaptureWalker.cs b/src/Compilers/CSharp/Portable/Lowering/StateMachineRewriter/IteratorAndAsyncCaptureWalker.cs index 51de89c3375bb7cf21a1d07d78e80df81f32d267..0777d660bed685e5c6042d6bd6c916c1c58eae73 100644 --- a/src/Compilers/CSharp/Portable/Lowering/StateMachineRewriter/IteratorAndAsyncCaptureWalker.cs +++ b/src/Compilers/CSharp/Portable/Lowering/StateMachineRewriter/IteratorAndAsyncCaptureWalker.cs @@ -298,7 +298,7 @@ public override BoundNode VisitTryStatement(BoundTryStatement node) return null; } - protected override void VisitFinallyBlock(BoundStatement finallyBlock, ref LocalState unsetInFinally) + protected override void VisitFinallyBlock(BoundBlock finallyBlock, ref LocalState unsetInFinally) { if (_seenYieldInCurrentTry) { diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenAsyncTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenAsyncTests.cs index 3a2cc4fed5545dd8977b0a044820fc8551335611..e6f3f2a1d43622585dde26e00fd06f58f98a748c 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenAsyncTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenAsyncTests.cs @@ -5021,8 +5021,8 @@ public void CaptureAssignedInOuterFinally() { var source = @" -using System; -using System.Threading.Tasks; + using System; + using System.Threading.Tasks; public class Program { diff --git a/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs b/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs index df5b97edfc5574a0f14cef517261266a51098c6d..52a160e7d6016429040b28c47425a2c6f31f24f9 100644 --- a/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/FlowTests.cs @@ -2351,5 +2351,52 @@ void TestFunc(int i) "; CreateCompilationWithMscorlib45(source).VerifyDiagnostics(); } + + [Fact, WorkItem(19831, "https://github.com/dotnet/roslyn/issues/19831")] + public void AssignedInFinallyUsedInTry() + { + var source = +@" + public class Program + { + static void Main(string[] args) + { + Test(); + } + + public static void Test() + { + object obj; + + try + { + goto l3; + + l1: + goto l2; + + l3: + goto l1; + + + l2: + + // Should be compile error + // 'obj' is uninitialized + obj.ToString(); + } + finally + { + obj = 1; + } + } + } +"; + CreateCompilationWithMscorlib45(source).VerifyDiagnostics( + // (28,17): error CS0165: Use of unassigned local variable 'obj' + // obj.ToString(); + Diagnostic(ErrorCode.ERR_UseDefViolation, "obj").WithArguments("obj").WithLocation(28, 17) + ); + } } } diff --git a/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/TryLockUsingStatementTests.cs b/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/TryLockUsingStatementTests.cs index 3c3dddbeed877936279b483d0abf7c55ee2a4c59..8157aa26eb322fffa7c9a359de34758eb63d219f 100644 --- a/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/TryLockUsingStatementTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/TryLockUsingStatementTests.cs @@ -256,7 +256,7 @@ public void TryMethod(out byte para) Assert.Equal(0, controlFlowAnalysisResults.ReturnStatements.Count()); Assert.False(controlFlowAnalysisResults.EndPointIsReachable); Assert.Equal(null, GetSymbolNamesJoined(dataFlowAnalysisResults.VariablesDeclared)); - Assert.Equal("by", GetSymbolNamesJoined(dataFlowAnalysisResults.AlwaysAssigned)); + Assert.Equal(null, GetSymbolNamesJoined(dataFlowAnalysisResults.AlwaysAssigned)); Assert.Equal("by", GetSymbolNamesJoined(dataFlowAnalysisResults.DataFlowsIn)); Assert.Equal("by", GetSymbolNamesJoined(dataFlowAnalysisResults.DataFlowsOut)); Assert.Equal("by", GetSymbolNamesJoined(dataFlowAnalysisResults.ReadInside)); diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/SemanticErrorTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/SemanticErrorTests.cs index b10a89b4745223da639e7f2e28f6b778a4444d7f..b72a0a5a63390620446c2476b801480c8e706919 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/SemanticErrorTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/SemanticErrorTests.cs @@ -4889,10 +4889,13 @@ static void Main(string[] args) VerifyDiagnostics( // (9,26): error CS0157: Control cannot leave the body of a finally clause // finally { i = 3; goto lab1; }// invalid - Diagnostic(ErrorCode.ERR_BadFinallyLeave, "goto"), + Diagnostic(ErrorCode.ERR_BadFinallyLeave, "goto").WithLocation(9, 26), + // (10,5): warning CS0162: Unreachable code detected + // lab1: + Diagnostic(ErrorCode.WRN_UnreachableCode, "lab1").WithLocation(10, 5), // (6,13): warning CS0219: The variable 'i' is assigned but its value is never used // int i = 0; - Diagnostic(ErrorCode.WRN_UnreferencedVarAssg, "i").WithArguments("i") + Diagnostic(ErrorCode.WRN_UnreferencedVarAssg, "i").WithArguments("i").WithLocation(6, 13) ); } diff --git a/src/Compilers/VisualBasic/Test/Emit/CodeGen/CodeGenAsyncTests.vb b/src/Compilers/VisualBasic/Test/Emit/CodeGen/CodeGenAsyncTests.vb index 09c2b41046a4bd45d8de4d7d07d5ced22388e785..13428e55a0f7d25defd8cd9dc3e3c0fccaaca43b 100644 --- a/src/Compilers/VisualBasic/Test/Emit/CodeGen/CodeGenAsyncTests.vb +++ b/src/Compilers/VisualBasic/Test/Emit/CodeGen/CodeGenAsyncTests.vb @@ -9145,6 +9145,46 @@ False CompileAndVerify(compilation, expectedOutput:=expectedOutput) CompileAndVerify(compilation.WithOptions(TestOptions.ReleaseExe), expectedOutput:=expectedOutput) End Sub + + + Public Sub CaptureAssignedInOuterFinally() + Dim source = + +imports System.Threading.Tasks +imports System + +Module Module1 + + Sub Main() + Test().Wait() + System.Console.WriteLine("success") + End Sub + + Async Function Test() As Task + Dim obj = New Object() + + Try + For i = 0 To 3 + ' NRE on second iteration + obj.ToString() + Await Task.Yield() + Next + + Finally + obj = Nothing + End Try + End Function +End Module + + + + + Dim expectedOutput = "success" + + Dim compilation = CompilationUtils.CreateCompilationWithReferences(source, references:=LatestVbReferences, options:=TestOptions.DebugExe) + CompileAndVerify(compilation, expectedOutput:=expectedOutput) + CompileAndVerify(compilation.WithOptions(TestOptions.ReleaseExe), expectedOutput:=expectedOutput) + End Sub End Class End Namespace