Handle hoisted Catch local in state machine rewriter

src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs

@@ -747,12 +747,12 @@ private static void RemoveLessDerivedMembers<TMember>(ArrayBuilder<MemberResolut
             // overriding methods. For the purposes of removing more stuff, we need to behave as
             // though that's what was there.
             //
-            // The presense of Giraffe.M(T2) does *not* justify the removal of Mammal.M(T3); it is
+            // The presence of Giraffe.M(T2) does *not* justify the removal of Mammal.M(T3); it is
             // not to be considered a method of Giraffe, but rather a method of Mammal for the
             // purposes of removing other methods. 
             //
-            // However, the presense of Mammal.M(T3) does justify the removal of Giraffe.M(T1). Why?
-            // Because the presense of Mammal.M(T3) justifies the removal of Animal.M(T1), and that
+            // However, the presence of Mammal.M(T3) does justify the removal of Giraffe.M(T1). Why?
+            // Because the presence of Mammal.M(T3) justifies the removal of Animal.M(T1), and that
             // is what is supposed to be in the set instead of Giraffe.M(T1).
             //
             // The resulting candidate set after the filtering according to the spec should be:

src/Compilers/VisualBasic/Portable/Lowering/IteratorRewriter/IteratorRewriter.IteratorMethodToClassRewriter.vb

@@ -1,15 +1,8 @@
 ' Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 
-Imports System.Collections.Generic
 Imports System.Collections.Immutable
-Imports System.Threading
-Imports Microsoft.Cci
-Imports Microsoft.CodeAnalysis
 Imports Microsoft.CodeAnalysis.CodeGen
-Imports Microsoft.CodeAnalysis.Collections
-Imports Microsoft.CodeAnalysis.Text
 Imports Microsoft.CodeAnalysis.VisualBasic.Symbols
-Imports Microsoft.CodeAnalysis.VisualBasic.Syntax
 
 Namespace Microsoft.CodeAnalysis.VisualBasic
 
@@ -33,14 +26,14 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
                            F As SyntheticBoundNodeFactory,
                            state As FieldSymbol,
                            current As FieldSymbol,
-                           HoistedVariables As IReadOnlySet(Of Symbol),
+                           hoistedVariables As IReadOnlySet(Of Symbol),
                            localProxies As Dictionary(Of Symbol, FieldSymbol),
                            SynthesizedLocalOrdinals As SynthesizedLocalOrdinalsDispenser,
                            slotAllocatorOpt As VariableSlotAllocator,
                            nextFreeHoistedLocalSlot As Integer,
                            diagnostics As DiagnosticBag)
 
-                MyBase.New(F, state, HoistedVariables, localProxies, SynthesizedLocalOrdinals, slotAllocatorOpt, nextFreeHoistedLocalSlot, diagnostics)
+                MyBase.New(F, state, hoistedVariables, localProxies, SynthesizedLocalOrdinals, slotAllocatorOpt, nextFreeHoistedLocalSlot, diagnostics)
 
                 Me._current = current
 

src/Compilers/VisualBasic/Portable/Lowering/LambdaRewriter/LambdaRewriter.vb

@@ -725,7 +725,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
 
             ' Catch node contains 3 important pieces
             ' 1) LocalOpt - like BoundBlock, catch may own variables, but it happens so that it never needs more than one.
-            ' 2) ExceptionVariable - presense of this variable indicates that caught exception needs to be stored. 
+            ' 2) ExceptionVariable - presence of this variable indicates that caught exception needs to be stored. 
             '                        in such case ExceptionVariable is used as a target of a one-time assignment
             '                        when Catch is entered.
             ' 3) Code (Filter and Body)

src/Compilers/VisualBasic/Portable/Lowering/StateMachineRewriter/StateMachineRewriter.StateMachineMethodToClassRewriter.vb

@@ -47,18 +47,18 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
             ''' block that we are within has a finalizer state.  Initially true as we have the (trivial)
             ''' finalizer state of -1 at the top level.
             ''' </summary>
-            Protected HasFinalizerState As Boolean = True
+            Private _hasFinalizerState As Boolean = True
 
             ''' <summary>
             ''' If hasFinalizerState is true, this is the state for finalization from anywhere in this try block.
             ''' Initially set to -1, representing the no-op finalization required at the top level.
             ''' </summary>
-            Protected CurrentFinalizerState As Integer = -1
+            Private _currentFinalizerState As Integer = -1
 
             ''' <summary>
             ''' The set of local variables and parameters that were hoisted and need a proxy.
             ''' </summary>
-            Protected Friend ReadOnly HoistedVariables As IReadOnlySet(Of Symbol) = Nothing
+            Private ReadOnly _hoistedVariables As IReadOnlySet(Of Symbol) = Nothing
 
             Private ReadOnly _synthesizedLocalOrdinals As SynthesizedLocalOrdinalsDispenser
             Private _nextFreeHoistedLocalSlot As Integer
@@ -84,7 +84,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
                 Me.F = F
                 Me.StateField = stateField
                 Me.CachedState = F.SynthesizedLocal(F.SpecialType(SpecialType.System_Int32), SynthesizedLocalKind.StateMachineCachedState, F.Syntax)
-                Me.HoistedVariables = hoistedVariables
+                Me._hoistedVariables = hoistedVariables
                 Me._synthesizedLocalOrdinals = synthesizedLocalOrdinals
                 Me._nextFreeHoistedLocalSlot = nextFreeHoistedLocalSlot
 
@@ -148,15 +148,15 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
                     Me.Dispatches = New Dictionary(Of LabelSymbol, List(Of Integer))()
                 End If
 
-                If Not Me.HasFinalizerState Then
-                    Me.CurrentFinalizerState = Me.NextState
+                If Not Me._hasFinalizerState Then
+                    Me._currentFinalizerState = Me.NextState
                     Me.NextState += 1
-                    Me.HasFinalizerState = True
+                    Me._hasFinalizerState = True
                 End If
 
                 Dim resumeLabel As GeneratedLabelSymbol = Me.F.GenerateLabel(ResumeLabelName)
                 Me.Dispatches.Add(resumeLabel, New List(Of Integer)() From {stateNumber})
-                Me.FinalizerStateMap.Add(stateNumber, Me.CurrentFinalizerState)
+                Me.FinalizerStateMap.Add(stateNumber, Me._currentFinalizerState)
 
                 Return New StateInfo(stateNumber, resumeLabel)
             End Function
@@ -232,7 +232,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
 
             Private Function NeedsProxy(localOrParameter As Symbol) As Boolean
                 Debug.Assert(localOrParameter.Kind = SymbolKind.Local OrElse localOrParameter.Kind = SymbolKind.Parameter)
-                Return HoistedVariables.Contains(localOrParameter)
+                Return _hoistedVariables.Contains(localOrParameter)
             End Function
 
             Friend MustOverride Sub AddProxyFieldsForStateMachineScope(proxy As TProxy, proxyFields As ArrayBuilder(Of FieldSymbol))
@@ -254,22 +254,22 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
             Public Overrides Function VisitTryStatement(node As BoundTryStatement) As BoundNode
 
                 Dim oldDispatches As Dictionary(Of LabelSymbol, List(Of Integer)) = Me.Dispatches
-                Dim oldFinalizerState As Integer = Me.CurrentFinalizerState
-                Dim oldHasFinalizerState As Boolean = Me.HasFinalizerState
+                Dim oldFinalizerState As Integer = Me._currentFinalizerState
+                Dim oldHasFinalizerState As Boolean = Me._hasFinalizerState
 
                 Me.Dispatches = Nothing
-                Me.CurrentFinalizerState = -1
-                Me.HasFinalizerState = False
+                Me._currentFinalizerState = -1
+                Me._hasFinalizerState = False
 
                 Dim tryBlock As BoundBlock = Me.F.Block(DirectCast(Me.Visit(node.TryBlock), BoundStatement))
                 Dim dispatchLabel As GeneratedLabelSymbol = Nothing
                 If Me.Dispatches IsNot Nothing Then
                     dispatchLabel = Me.F.GenerateLabel("tryDispatch")
 
-                    If Me.HasFinalizerState Then
+                    If Me._hasFinalizerState Then
                         ' cause the current finalizer state to arrive here and then "return false"
                         Dim finalizer As GeneratedLabelSymbol = Me.F.GenerateLabel("finalizer")
-                        Me.Dispatches.Add(finalizer, New List(Of Integer)() From {Me.CurrentFinalizerState})
+                        Me.Dispatches.Add(finalizer, New List(Of Integer)() From {Me._currentFinalizerState})
 
                         Dim skipFinalizer As GeneratedLabelSymbol = Me.F.GenerateLabel("skipFinalizer")
                         tryBlock = Me.F.Block(Me.F.HiddenSequencePoint(),
@@ -293,8 +293,8 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
                     oldDispatches.Add(dispatchLabel, New List(Of Integer)(From kv In Dispatches.Values From n In kv Order By n Select n))
                 End If
 
-                Me.HasFinalizerState = oldHasFinalizerState
-                Me.CurrentFinalizerState = oldFinalizerState
+                Me._hasFinalizerState = oldHasFinalizerState
+                Me._currentFinalizerState = oldFinalizerState
 
                 Me.Dispatches = oldDispatches
 
@@ -329,28 +329,39 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
                 Return Me.MaterializeProxy(node, Me.Proxies(Me.TopLevelMethod.MeParameter))
             End Function
 
-            Public Overrides Function VisitCatchBlock(node As BoundCatchBlock) As BoundNode
-                ' Neither Async nor Iterator function allows await/yield in Catch block,
-                ' thus, it should not capture/hoist catch block local
-                Dim rewrittenCatchLocal As LocalSymbol = Nothing
-
-                Dim origLocal As LocalSymbol = node.LocalOpt
-                If origLocal IsNot Nothing Then
-                    ' local may be either the original local of a frame reference
-                    Debug.Assert(Not Me.Proxies.ContainsKey(origLocal), "captured local should not need rewriting")
+            Private Function TryRewriteLocal(local As LocalSymbol) As LocalSymbol
+                If NeedsProxy(local) Then
+                    ' no longer a local symbol
+                    Return Nothing
+                End If
 
-                    Dim newType = VisitType(origLocal.Type)
-                    If newType = origLocal.Type Then
+                Dim newLocal As LocalSymbol = Nothing
+                If Not LocalMap.TryGetValue(local, newLocal) Then
+                    Dim newType = VisitType(local.Type)
+                    If newType = local.Type Then
                         ' keeping same local
-                        rewrittenCatchLocal = origLocal
-
+                        newLocal = local
                     Else
                         ' need a local of a different type
-                        rewrittenCatchLocal = LocalSymbol.Create(origLocal, newType)
-                        Me.LocalMap.Add(origLocal, rewrittenCatchLocal)
+                        newLocal = LocalSymbol.Create(local, newType)
+                        LocalMap.Add(local, newLocal)
                     End If
                 End If
 
+                Return newLocal
+            End Function
+
+            Public Overrides Function VisitCatchBlock(node As BoundCatchBlock) As BoundNode
+                ' Yield/Await aren't supported in Catch block, but we need to
+                ' rewrite the type of the variable owned by the catch block.
+                ' Note that this variable might be a closure frame reference.
+                Dim rewrittenCatchLocal As LocalSymbol = Nothing
+
+                Dim origLocal As LocalSymbol = node.LocalOpt
+                If origLocal IsNot Nothing Then
+                    rewrittenCatchLocal = TryRewriteLocal(origLocal)
+                End If
+
                 Dim rewrittenExceptionVariable As BoundExpression = DirectCast(Me.Visit(node.ExceptionSourceOpt), BoundExpression)
 
                 ' rewrite filter and body

src/Compilers/VisualBasic/Test/Emit/CodeGen/CodeGenAsyncTests.vb

@@ -8300,6 +8300,80 @@ BC42356: This async method lacks 'Await' operators and so will run synchronously
 ]]></errors>)
             End Using
         End Sub
+
+        <WorkItem(863, "https://github.com/dotnet/roslyn")>
+        <Fact()>
+        Public Sub CatchInIteratorStateMachine()
+            CompileAndVerify(
+<compilation>
+    <file name="a.vb">
+Imports System
+Imports System.Collections
+Class C
+    Shared Function F() As Object
+        Throw New ArgumentException()
+    End Function
+    Shared Iterator Function M() As IEnumerable
+        Dim o As Object
+        Try
+            o = F()
+        Catch e As Exception
+            o = e
+        End Try
+        Yield o
+    End Function
+    Shared Sub Main()
+        For Each o in M()
+            Console.WriteLine(o)
+        Next
+    End Sub
+End Class
+    </file>
+</compilation>,
+                options:=TestOptions.DebugExe,
+                useLatestFramework:=True,
+                expectedOutput:=
+"System.ArgumentException: Value does not fall within the expected range.
+   at C.F()
+   at C.VB$StateMachine_2_M.MoveNext()")
+        End Sub
+
+        <WorkItem(863, "https://github.com/dotnet/roslyn")>
+        <Fact()>
+        Public Sub CatchInAsyncStateMachine()
+            CompileAndVerify(
+<compilation>
+    <file name="a.vb">
+Imports System
+Imports System.Threading.Tasks
+Class C
+    Shared Function F() As Object
+        Throw New ArgumentException()
+    End Function
+    Shared Async Function M() As Task(Of Object)
+        Dim o As Object
+        Try
+            o = F()
+        Catch e As Exception
+            o = e
+        End Try
+        Return o
+    End Function
+    Shared Sub Main()
+        Dim o = M().Result
+        Console.WriteLine(o)
+    End Sub
+End Class
+    </file>
+</compilation>,
+                options:=TestOptions.DebugExe,
+                useLatestFramework:=True,
+                expectedOutput:=
+"System.ArgumentException: Value does not fall within the expected range.
+   at C.F()
+   at C.VB$StateMachine_2_M.MoveNext()")
+        End Sub
+
     End Class
 End Namespace
 

src/ExpressionEvaluator/CSharp/Test/ExpressionCompiler/LocalsTests.cs

@@ -2332,6 +2332,66 @@ .locals init (int V_0,
             locals.Free();
         }
 
+        [WorkItem(1115030)]
+        [Fact(Skip = "1115030")]
+        public void CatchInAsyncStateMachine()
+        {
+            var source =
+@"using System;
+using System.Threading.Tasks;
+class C
+{
+    static object F()
+    {
+        throw new ArgumentException();
+    }
+    static async Task M()
+    {
+        object o;
+        try
+        {
+            o = F();
+        }
+        catch (Exception e)
+        {
+#line 999
+            o = e;
+        }
+    }
+}";
+            var compilation0 = CreateCompilationWithMscorlib45(source, options: TestOptions.DebugDll);
+            var runtime = CreateRuntimeInstance(compilation0);
+            var context = CreateMethodContext(
+                runtime,
+                methodName: "C.<M>d__1.MoveNext",
+                atLineNumber: 999);
+            var testData = new CompilationTestData();
+            var locals = ArrayBuilder<LocalAndMethod>.GetInstance();
+            string typeName;
+            var assembly = context.CompileGetLocals(locals, argumentsOnly: false, typeName: out typeName, testData: testData);
+            VerifyLocal(testData, typeName, locals[0], "<>m0", "o", expectedILOpt:
+@"{
+  // Code size        7 (0x7)
+  .maxstack  1
+  .locals init (int V_0,
+                System.Exception V_1)
+  IL_0000:  ldarg.0
+  IL_0001:  ldfld      ""object C.<M>d__1.<o>5__1""
+  IL_0006:  ret
+}");
+            VerifyLocal(testData, typeName, locals[1], "<>m1", "e", expectedILOpt:
+@"{
+  // Code size        7 (0x7)
+  .maxstack  1
+  .locals init (int V_0,
+                System.Exception V_1)
+  IL_0000:  ldarg.0
+  IL_0001:  ldfld      ""System.Exception C.<M>d__1.<e>5__2""
+  IL_0006:  ret
+}");
+            locals.Free();
+        }
+
         private static void GetLocals(RuntimeInstance runtime, string methodName, bool argumentsOnly, ArrayBuilder<LocalAndMethod> locals, int count, out string typeName, out CompilationTestData testData)
         {
             var context = CreateMethodContext(runtime, methodName);