Merge remote-tracking branch 'upstream/master' into generateEqualsOptions

src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.Analysis.cs

@@ -73,7 +73,16 @@ internal sealed class Analysis : BoundTreeWalkerWithStackGuardWithoutRecursionOn
             public readonly HashSet<MethodSymbol> MethodsConvertedToDelegates = new HashSet<MethodSymbol>();
 
             /// <summary>
-            /// Any scope that a method in <see cref="MethodsConvertedToDelegates"/> closes over. If a scope is in this set, don't use a struct closure.
+            /// True if the method signature can't be rewritten to contain ref/out parameters.
+            /// </summary>
+            public bool CanTakeRefParameters(MethodSymbol closure) => !(closure.IsAsync
+                                                                        || closure.IsIterator
+                                                                        // We can't rewrite delegate signatures
+                                                                        || MethodsConvertedToDelegates.Contains(closure));
+
+            /// <summary>
+            /// Any scope that a method that <see cref="CanTakeRefParameters(MethodSymbol)"/> doesn't close over.
+            /// If a scope is in this set, don't use a struct closure.
             /// </summary>
             public readonly HashSet<BoundNode> ScopesThatCantBeStructs = new HashSet<BoundNode>();
 
@@ -325,7 +334,7 @@ internal void ComputeLambdaScopesAndFrameCaptures()
                         LambdaScopes.Add(lambda, innermostScope);
 
                         // Disable struct closures on methods converted to delegates, as well as on async and iterator methods.
-                        var markAsNoStruct = MethodsConvertedToDelegates.Contains(lambda) || lambda.IsAsync || lambda.IsIterator;
+                        var markAsNoStruct = !CanTakeRefParameters(lambda);
                         if (markAsNoStruct)
                         {
                             ScopesThatCantBeStructs.Add(innermostScope);

src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.cs

@@ -1311,35 +1311,15 @@ private DebugId GetLambdaId(SyntaxNode syntax, ClosureKind closureKind, int clos
             if (_analysis.LambdaScopes.TryGetValue(node.Symbol, out lambdaScope))
             {
                 containerAsFrame = _frames[lambdaScope];
-                var structClosureParamBuilder = ArrayBuilder<TypeSymbol>.GetInstance();
-                while (containerAsFrame != null && containerAsFrame.IsValueType)
+                structClosures = _analysis.CanTakeRefParameters(node.Symbol)
+                    ? GetStructClosures(containerAsFrame, lambdaScope)
+                    : default(ImmutableArray<TypeSymbol>);
+
+                if (containerAsFrame?.IsValueType == true)
                 {
-                    structClosureParamBuilder.Add(containerAsFrame);
-                    if (this._analysis.NeedsParentFrame.Contains(lambdaScope))
-                    {
-                        var found = false;
-                        while (this._analysis.ScopeParent.TryGetValue(lambdaScope, out lambdaScope))
-                        {
-                            if (_frames.TryGetValue(lambdaScope, out containerAsFrame))
-                            {
-                                found = true;
-                                break;
-                            }
-                        }
-                        if (found)
-                        {
-                            continue;
-                        }
-                    }
-                    // can happen when scope no longer needs parent frame, or we're at the outermost level and the "parent frame" is top level "this".
-                    lambdaScope = null;
+                    // Lower directly onto the containing type
                     containerAsFrame = null;
-                }
-                // Reverse it because we're going from inner to outer, and parameters are in order of outer to inner
-                structClosureParamBuilder.ReverseContents();
-                structClosures = structClosureParamBuilder.ToImmutableAndFree();
-                if (containerAsFrame == null)
-                {
+                    lambdaScope = null;
                     closureKind = ClosureKind.Static; // not exactly... but we've rewritten the receiver to be a by-ref parameter
                     translatedLambdaContainer = _topLevelMethod.ContainingType;
                     closureOrdinal = LambdaDebugInfo.StaticClosureOrdinal;
@@ -1440,6 +1420,47 @@ private DebugId GetLambdaId(SyntaxNode syntax, ClosureKind closureKind, int clos
             return synthesizedMethod;
         }
 
+        /// <summary>
+        /// Builds a list of all the struct-based closure environments that will
+        /// need to be passed as arguments to the method.
+        /// </summary>
+        private ImmutableArray<TypeSymbol> GetStructClosures(LambdaFrame containerAsFrame, BoundNode lambdaScope)
+        {
+            var structClosureParamBuilder = ArrayBuilder<TypeSymbol>.GetInstance();
+            while (containerAsFrame?.IsValueType == true ||
+                   _analysis.NeedsParentFrame.Contains(lambdaScope))
+            {
+                if (containerAsFrame?.IsValueType == true)
+                {
+                    structClosureParamBuilder.Add(containerAsFrame);
+                }
+                if (_analysis.NeedsParentFrame.Contains(lambdaScope)
+                    && FindParentFrame(ref containerAsFrame, ref lambdaScope))
+                {
+                    continue;
+                }
+
+                // can happen when scope no longer needs parent frame, or we're at the outermost level and the "parent frame" is top level "this".
+                break;
+            }
+
+            // Reverse it because we're going from inner to outer, and parameters are in order of outer to inner
+            structClosureParamBuilder.ReverseContents();
+            return structClosureParamBuilder.ToImmutableAndFree();
+
+            bool FindParentFrame(ref LambdaFrame container, ref BoundNode scope)
+            {
+                while (_analysis.ScopeParent.TryGetValue(scope, out scope))
+                {
+                    if (_frames.TryGetValue(scope, out container))
+                    {
+                        return true;
+                    }
+                }
+                return false;
+            }
+        }
+
         private void AddSynthesizedMethod(MethodSymbol method, BoundStatement body)
         {
             if (_synthesizedMethods == null)

src/Compilers/CSharp/Portable/Lowering/StateMachineRewriter/IteratorAndAsyncCaptureWalker.cs

@@ -212,6 +212,9 @@ private void CaptureVariable(Symbol variable, SyntaxNode syntax)
 
         protected override void EnterParameter(ParameterSymbol parameter)
         {
+            // Async and iterators should never have ref parameters aside from `this`
+            Debug.Assert(parameter.IsThis || parameter.RefKind == RefKind.None);
+
             // parameters are NOT initially assigned here - if that is a problem, then
             // the parameters must be captured.
             GetOrCreateSlot(parameter);

src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenLocalFunctionTests.cs

@@ -30,6 +30,54 @@ public static IMethodSymbol FindLocalFunction(this CommonTestBase.CompilationVer
     [CompilerTrait(CompilerFeature.LocalFunctions)]
     public class CodeGenLocalFunctionTests : CSharpTestBase
     {
+        [Fact]
+        [WorkItem(17890, "https://github.com/dotnet/roslyn/issues/17890")]
+        public void Repro17890()
+        {
+            var comp = CreateCompilationWithMscorlib46(@"
+using System;
+using System.Collections.Generic;
+using System.Linq;
+
+public class Class
+{
+   public class Item
+   {
+      public int Id { get; set; }
+   }
+
+   public class ItemsContainer : IDisposable
+   {
+      public List<Item> Items { get; set; }
+
+      public void Dispose()
+      {
+      }
+   }
+
+   public static void CompilerError()
+   {
+      using (var itemsContainer = new ItemsContainer())
+      {
+         Item item = null;
+
+         itemsContainer.Items.Where(x => x.Id == item.Id);
+
+         void Local1()
+         {
+            itemsContainer.Items = null;
+         }
+
+         void Local2()
+         {
+            Local1();
+         }
+      }
+   }
+}", references: new[] { LinqAssemblyRef });
+            CompileAndVerify(comp);
+        }
+
         [Fact(Skip = "https://github.com/dotnet/roslyn/issues/16783")]
         [WorkItem(16783, "https://github.com/dotnet/roslyn/issues/16783")]
         public void GenericDefaultParams()
@@ -104,6 +152,312 @@ void Local(string s = nameof(Local))
             CompileAndVerify(comp, expectedOutput: "Local");
         }
 
+        [Fact]
+        [WorkItem(16895, "https://github.com/dotnet/roslyn/issues/16895")]
+        public void CaptureVarNestedLambdaSkipScope()
+        {
+            var src = @"
+using System;
+class C
+{
+
+    public static void Main()
+    {
+        var d = """";
+        {
+            int x = 0;
+            void M()
+            {
+                if (d != null)
+                {
+                    Action a = () => x++;
+                    a();
+                }
+            }
+            M();
+            Console.WriteLine(x);
+        }
+    }
+}";
+            CompileAndVerify(src, expectedOutput: "1");
+        }
+
+        [Fact]
+        [WorkItem(16895, "https://github.com/dotnet/roslyn/issues/16895")]
+        public void CaptureVarNestedLambdaSkipScope2()
+        {
+            var src = @"
+using System;
+class C
+{
+    class D : IDisposable { public void Dispose() {} }
+
+    public static void Main()
+    {
+        using (var d = new D())
+        {
+            int x = 0;
+            void M()
+            {
+                if (d != null)
+                {
+                    Action a = () => x++;
+                    a();
+                }
+            }
+            M();
+            Console.WriteLine(x);
+        }
+    }
+}";
+            CompileAndVerify(src, expectedOutput: "1");
+        }
+
+        [Fact]
+        [WorkItem(16895, "https://github.com/dotnet/roslyn/issues/16895")]
+        public void CaptureVarNestedLambdaSkipScope3()
+        {
+            var src = @"
+using System;
+class C
+{
+
+    public static void Main()
+    {
+        var d = """";
+        {
+            int x = 0;
+            void M()
+            {
+                if (d != null)
+                {
+                    void Local() => x++;
+                    Action a = Local;
+                    a();
+                }
+            }
+            M();
+            Console.WriteLine(x);
+        }
+    }
+}";
+            CompileAndVerify(src, expectedOutput: "1");
+        }
+
+        [Fact]
+        [WorkItem(16895, "https://github.com/dotnet/roslyn/issues/16895")]
+        public void CaptureVarNestedLambdaSkipScope4()
+        {
+            var src = @"
+using System;
+class C
+{
+
+    public static void Main()
+    {
+        var d = """";
+        {
+            int y = 0;
+            {
+                int x = 0;
+                void M()
+                {
+                    if (d != null)
+                    {
+                        Action a = () => x++;;
+                        a();
+                    }
+                }
+                M();
+                Console.WriteLine(x);
+            }
+            y++;
+        }
+    }
+}";
+            CompileAndVerify(src, expectedOutput: "1");
+        }
+
+        [Fact]
+        [WorkItem(16895, "https://github.com/dotnet/roslyn/issues/16895")]
+        public void CaptureVarNestedLambdaSkipScope5()
+        {
+            var src = @"
+using System;
+class C
+{
+
+    public static void Main()
+    {
+        int x = 0;
+        {
+            int y = 0;
+            void L()
+            {
+                int z = 0;
+                void L2()
+                {
+                    if (x == 0 && z == 0)
+                    {
+                        Action a = () => y++;
+                        a();
+                    }
+                }
+                L2();
+            }
+            L();
+            Console.WriteLine(y);
+        }
+    }
+}";
+            CompileAndVerify(src, expectedOutput: "1");
+        }
+
+        [Fact]
+        [WorkItem(16895, "https://github.com/dotnet/roslyn/issues/16895")]
+        public void CaptureVarNestedLambdaSkipScope6()
+        {
+            var src = @"
+using System;
+class C
+{
+
+    public static void Main()
+    {
+        int x = 0;
+        {
+            int y = 0;
+            void L()
+            {
+                int z = 0;
+                void L2()
+                {
+                    if (x == 0 && y == 0)
+                    {
+                        Action a = () => z++;
+                        a();
+                    }
+                    y++;
+                }
+                L2();
+                Console.WriteLine(z);
+            }
+            L();
+            Console.WriteLine(y);
+        }
+        ((Action)(() => x++))();
+        Console.WriteLine(x);
+    }
+}";
+            CompileAndVerify(src, expectedOutput: @"1
+1
+1");
+        }
+
+        [Fact]
+        [WorkItem(16895, "https://github.com/dotnet/roslyn/issues/16895")]
+        public void CaptureVarNestedLambdaSkipScope7()
+        {
+            var src = @"
+using System;
+using System.Threading.Tasks;
+class C
+{
+
+    public static void Main()
+    {
+        int x = 0;
+        {
+            int y = 0;
+            void L()
+            {
+                if (x == 0)
+                {
+                    async Task L2()
+                    {
+                        await Task.Delay(1);
+                        y++;
+                    }
+                    L2().Wait();
+                }
+            }
+            L();
+            Console.WriteLine(y);
+        }
+        Console.WriteLine(x);
+    }
+}";
+            CompileAndVerify(src,
+                additionalRefs: new[] { MscorlibRef_v46 },
+                expectedOutput: @"1
+0");
+        }
+
+        [Fact]
+        [WorkItem(16895, "https://github.com/dotnet/roslyn/issues/16895")]
+        public void CaptureVarNestedLambdaSkipScope8()
+        {
+            var src = @"
+using System;
+using System.Collections.Generic;
+class C
+{
+
+    public static void Main()
+    {
+        int x = 0;
+        {
+            int y = 0;
+            void L()
+            {
+                if (x == 0)
+                {
+                    IEnumerable<int> L2()
+                    {
+                        yield return 0;
+                        y++;
+                    }
+                    foreach (var i in L2()) { }
+                }
+            }
+            L();
+            Console.WriteLine(y);
+        }
+        Console.WriteLine(x);
+    }
+}";
+            CompileAndVerify(src,
+                additionalRefs: new[] { MscorlibRef_v46 },
+                expectedOutput: @"1
+0");
+        }
+
+        [Fact]
+        [WorkItem(16895, "https://github.com/dotnet/roslyn/issues/16895")]
+        public void LocalFunctionCaptureSkipScope()
+        {
+            var src = @"
+using System;
+class C
+{
+    public static void Main(string[] args)
+    {
+        {
+            int uncaptured = 0;
+            uncaptured++;
+
+            {
+                int x = 0;
+                bool Local(int y) => x == 0 && args == null && y == 0;
+                Local(0);
+            }
+        }
+    }
+}";
+            CompileAndVerify(src);
+        }
+
+
         [Fact]
         [WorkItem(16399, "https://github.com/dotnet/roslyn/issues/16399")]
         public void RecursiveGenericLocalFunctionIterator()