Move MakeFrames logic into Analysis

This is part of the effort to move as much logic from LambdaRewriter
into Analysis as possible. The more logic there is in the Rewriter, the
more difficult it is to find calculation problems until the last
possible moment, and it's also very difficult to calculate useful
information for later analysis passes, like the final debug and closure
IDs for each environment.

src/Compilers/CSharp/Portable/CSharpParseOptions.cs

@@ -61,7 +61,7 @@ public override IEnumerable<string> PreprocessorSymbolNames
             LanguageVersion languageVersion,
             DocumentationMode documentationMode,
             SourceCodeKind kind,
-            IEnumerable<string> preprocessorSymbols,
+            ImmutableArray<string> preprocessorSymbols,
             IReadOnlyDictionary<string, string> features)
             : base(kind, documentationMode)
         {

src/Compilers/CSharp/Portable/Compiler/TypeCompilationState.cs

@@ -66,7 +66,7 @@ internal MethodWithBody(MethodSymbol method, BoundStatement body, ImportChain im
 
         public readonly CSharpCompilation Compilation;
 
-        public LambdaFrame StaticLambdaFrame;
+        public ClosureEnvironment StaticLambdaFrame;
 
         /// <summary>
         /// A graph of method->method references for this(...) constructor initializers.

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

@@ -31,7 +31,7 @@ private LambdaCapturedVariable(SynthesizedContainer frame, TypeSymbol type, stri
             _isThis = isThisParameter;
         }
 
-        public static LambdaCapturedVariable Create(LambdaFrame frame, Symbol captured, ref int uniqueId)
+        public static LambdaCapturedVariable Create(ClosureEnvironment frame, Symbol captured, ref int uniqueId)
         {
             Debug.Assert(captured is LocalSymbol || captured is ParameterSymbol);
 
@@ -87,7 +87,7 @@ private static TypeSymbol GetCapturedVariableFieldType(SynthesizedContainer fram
             if ((object)local != null)
             {
                 // if we're capturing a generic frame pointer, construct it with the new frame's type parameters
-                var lambdaFrame = local.Type.OriginalDefinition as LambdaFrame;
+                var lambdaFrame = local.Type.OriginalDefinition as ClosureEnvironment;
                 if ((object)lambdaFrame != null)
                 {
                     // lambdaFrame may have less generic type parameters than frame, so trim them down (the first N will always match)

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

 // Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 
+using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Diagnostics;
 using Microsoft.CodeAnalysis.CodeGen;
 using Microsoft.CodeAnalysis.CSharp.Symbols;
-using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Roslyn.Utilities;
 
 namespace Microsoft.CodeAnalysis.CSharp
 {
     /// <summary>
     /// A class that represents the set of variables in a scope that have been
-    /// captured by lambdas within that scope.
+    /// captured by nested functions within that scope.
     /// </summary>
-    internal sealed class LambdaFrame : SynthesizedContainer, ISynthesizedMethodBodyImplementationSymbol
+    internal sealed class ClosureEnvironment : SynthesizedContainer, ISynthesizedMethodBodyImplementationSymbol
     {
-        private readonly TypeKind _typeKind;
         private readonly MethodSymbol _topLevelMethod;
-        private readonly MethodSymbol _containingMethod;
-        private readonly MethodSymbol _constructor;
-        private readonly MethodSymbol _staticConstructor;
-        private readonly FieldSymbol _singletonCache;
         internal readonly SyntaxNode ScopeSyntaxOpt;
         internal readonly int ClosureOrdinal;
-
-        internal LambdaFrame(MethodSymbol topLevelMethod, MethodSymbol containingMethod, bool isStruct, SyntaxNode scopeSyntaxOpt, DebugId methodId, DebugId closureId)
+        /// <summary>
+        /// The closest method/lambda that this frame is originally from. Null if nongeneric static closure.
+        /// Useful because this frame's type parameters are constructed from this method and all methods containing this method.
+        /// </summary>
+        internal readonly MethodSymbol OriginalContainingMethodOpt;
+        internal readonly FieldSymbol SingletonCache;
+        internal readonly MethodSymbol StaticConstructor;
+        public readonly IEnumerable<Symbol> CapturedVariables;
+
+        public override TypeKind TypeKind { get; }
+        internal override MethodSymbol Constructor { get; }
+
+        internal ClosureEnvironment(
+            IEnumerable<Symbol> capturedVariables,
+            MethodSymbol topLevelMethod,
+            MethodSymbol containingMethod,
+            bool isStruct,
+            SyntaxNode scopeSyntaxOpt,
+            DebugId methodId,
+            DebugId closureId)
             : base(MakeName(scopeSyntaxOpt, methodId, closureId), containingMethod)
         {
-            _typeKind = isStruct ? TypeKind.Struct : TypeKind.Class;
+            CapturedVariables = capturedVariables;
+            TypeKind = isStruct ? TypeKind.Struct : TypeKind.Class;
             _topLevelMethod = topLevelMethod;
-            _containingMethod = containingMethod;
-            _constructor = isStruct ? null : new LambdaFrameConstructor(this);
+            OriginalContainingMethodOpt = containingMethod;
+            Constructor = isStruct ? null : new LambdaFrameConstructor(this);
             this.ClosureOrdinal = closureId.Ordinal;
 
             // static lambdas technically have the class scope so the scope syntax is null 
             if (scopeSyntaxOpt == null)
             {
-                _staticConstructor = new SynthesizedStaticConstructor(this);
+                StaticConstructor = new SynthesizedStaticConstructor(this);
                 var cacheVariableName = GeneratedNames.MakeCachedFrameInstanceFieldName();
-                _singletonCache = new SynthesizedLambdaCacheFieldSymbol(this, this, cacheVariableName, topLevelMethod, isReadOnly: true, isStatic: true);
+                SingletonCache = new SynthesizedLambdaCacheFieldSymbol(this, this, cacheVariableName, topLevelMethod, isReadOnly: true, isStatic: true);
             }
 
             AssertIsClosureScopeSyntax(scopeSyntaxOpt);
@@ -77,69 +91,25 @@ private static void AssertIsClosureScopeSyntax(SyntaxNode syntaxOpt)
             throw ExceptionUtilities.UnexpectedValue(syntaxOpt.Kind());
         }
 
-        public override TypeKind TypeKind
-        {
-            get { return _typeKind; }
-        }
-
-        internal override MethodSymbol Constructor
-        {
-            get { return _constructor; }
-        }
-
-        internal MethodSymbol StaticConstructor
-        {
-            get { return _staticConstructor; }
-        }
-
-        /// <summary>
-        /// The closest method/lambda that this frame is originally from. Null if nongeneric static closure.
-        /// Useful because this frame's type parameters are constructed from this method and all methods containing this method.
-        /// </summary>
-        internal MethodSymbol ContainingMethod
-        {
-            get { return _containingMethod; }
-        }
-
         public override ImmutableArray<Symbol> GetMembers()
         {
             var members = base.GetMembers();
-            if ((object)_staticConstructor != null)
+            if ((object)StaticConstructor != null)
             {
-                members = ImmutableArray.Create<Symbol>(_staticConstructor, _singletonCache).AddRange(members);
+                members = ImmutableArray.Create<Symbol>(StaticConstructor, SingletonCache).AddRange(members);
             }
 
             return members;
         }
 
-        internal FieldSymbol SingletonCache
-        {
-            get { return _singletonCache; }
-        }
-
         // display classes for static lambdas do not have any data and can be serialized.
-        internal override bool IsSerializable
-        {
-            get { return (object)_singletonCache != null; }
-        }
+        internal override bool IsSerializable => (object)SingletonCache != null;
 
-        public override Symbol ContainingSymbol
-        {
-            get { return _topLevelMethod.ContainingSymbol; }
-        }
+        public override Symbol ContainingSymbol => _topLevelMethod.ContainingSymbol;
 
-        bool ISynthesizedMethodBodyImplementationSymbol.HasMethodBodyDependency
-        {
-            get
-            {
-                // the lambda method contains user code from the lambda:
-                return true;
-            }
-        }
+        // The lambda method contains user code from the lambda
+        bool ISynthesizedMethodBodyImplementationSymbol.HasMethodBodyDependency => true;
 
-        IMethodSymbol ISynthesizedMethodBodyImplementationSymbol.Method
-        {
-            get { return _topLevelMethod; }
-        }
+        IMethodSymbol ISynthesizedMethodBodyImplementationSymbol.Method => _topLevelMethod;
     }
 }

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

@@ -6,7 +6,7 @@ namespace Microsoft.CodeAnalysis.CSharp
 {
     internal sealed class LambdaFrameConstructor : SynthesizedInstanceConstructor, ISynthesizedMethodBodyImplementationSymbol
     {
-        internal LambdaFrameConstructor(LambdaFrame frame)
+        internal LambdaFrameConstructor(ClosureEnvironment frame)
             : base(frame)
         {
         }

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

@@ -4,6 +4,7 @@
 using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Diagnostics;
+using Microsoft.CodeAnalysis.CodeGen;
 using Microsoft.CodeAnalysis.CSharp.Symbols;
 using Microsoft.CodeAnalysis.PooledObjects;
 using Roslyn.Utilities;
@@ -25,14 +26,14 @@ internal sealed partial class Analysis : BoundTreeWalkerWithStackGuardWithoutRec
             [DebuggerDisplay("{ToString(), nq}")]
             public sealed class Scope
             {
-                public Scope Parent { get; }
+                public readonly Scope Parent;
 
-                public ArrayBuilder<Scope> NestedScopes { get; } = ArrayBuilder<Scope>.GetInstance();
+                public readonly ArrayBuilder<Scope> NestedScopes = ArrayBuilder<Scope>.GetInstance();
 
                 /// <summary>
                 /// A list of all closures (all lambdas and local functions) declared in this scope.
                 /// </summary>
-                public ArrayBuilder<Closure> Closures { get; } = ArrayBuilder<Closure>.GetInstance();
+                public readonly ArrayBuilder<Closure> Closures = ArrayBuilder<Closure>.GetInstance();
 
                 /// <summary>
                 /// A list of all locals or parameters that were declared in this scope and captured
@@ -45,7 +46,7 @@ public sealed class Scope
                 /// non-deterministic compilation, and if we generate duplicate proxies we'll generate
                 /// wasteful code in the best case and incorrect code in the worst.
                 /// </remarks>
-                public SetWithInsertionOrder<Symbol> DeclaredVariables { get; } = new SetWithInsertionOrder<Symbol>();
+                public readonly SetWithInsertionOrder<Symbol> DeclaredVariables = new SetWithInsertionOrder<Symbol>();
 
                 /// <summary>
                 /// The bound node representing this scope. This roughly corresponds to the bound
@@ -53,13 +54,19 @@ public sealed class Scope
                 /// methods/closures are introduced into their Body's scope and do not get their
                 /// own scope.
                 /// </summary>
-                public BoundNode BoundNode { get; }
+                public readonly BoundNode BoundNode;
 
                 /// <summary>
                 /// The closure that this scope is nested inside. Null if this scope is not nested
                 /// inside a closure.
                 /// </summary>
-                public Closure ContainingClosureOpt { get; }
+                public readonly Closure ContainingClosureOpt;
+
+                /// <summary>
+                /// Environments created in this scope to hold <see cref="DeclaredVariables"/>.
+                /// </summary>
+                public readonly ArrayBuilder<ClosureEnvironment> DeclaredEnvironments
+                    = ArrayBuilder<ClosureEnvironment>.GetInstance();
 
                 public Scope(Scope parent, BoundNode boundNode, Closure containingClosure)
                 {
@@ -83,6 +90,7 @@ public void Free()
                         closure.Free();
                     }
                     Closures.Free();
+                    DeclaredEnvironments.Free();
                 }
 
                 public override string ToString() => BoundNode.Syntax.GetText().ToString();
@@ -102,9 +110,14 @@ public sealed class Closure
                 /// <summary>
                 /// The method symbol for the original lambda or local function.
                 /// </summary>
-                public MethodSymbol OriginalMethodSymbol { get; }
+                public readonly MethodSymbol OriginalMethodSymbol;
+
+                public readonly PooledHashSet<Symbol> CapturedVariables = PooledHashSet<Symbol>.GetInstance();
 
-                public PooledHashSet<Symbol> CapturedVariables { get; } = PooledHashSet<Symbol>.GetInstance();
+                public readonly ArrayBuilder<ClosureEnvironment> CapturedEnvironments
+                    = ArrayBuilder<ClosureEnvironment>.GetInstance();
+
+                public ClosureEnvironment ContainingEnvironmentOpt;
 
                 public Closure(MethodSymbol symbol)
                 {
@@ -115,6 +128,7 @@ public Closure(MethodSymbol symbol)
                 public void Free()
                 {
                     CapturedVariables.Free();
+                    CapturedEnvironments.Free();
                 }
             }
 
@@ -164,17 +178,36 @@ private void RemoveUnneededReferences(ParameterSymbol thisParam)
                     }
                 }
 
+                // True if there are any closures in the tree which
+                // capture 'this' and another variable
+                bool captureMoreThanThis = false;
+
                 VisitClosures(ScopeTree, (scope, closure) =>
                 {
                     if (!capturesVariable.Contains(closure.OriginalMethodSymbol))
                     {
                         closure.CapturedVariables.Clear();
                     }
+
                     if (capturesThis.Contains(closure.OriginalMethodSymbol))
                     {
                         closure.CapturedVariables.Add(thisParam);
+                        if (closure.CapturedVariables.Count > 1)
+                        {
+                            captureMoreThanThis |= true;
+                        }
                     }
                 });
+
+                if (!captureMoreThanThis && capturesThis.Count > 0)
+                {
+                    // If we have closures which capture 'this', and nothing else, we can
+                    // remove 'this' from the declared variables list, since we don't need
+                    // to create an environment to hold 'this' (since we can emit the
+                    // lowered methods directly onto the containing class)
+                    bool removed = ScopeTree.DeclaredVariables.Remove(thisParam);
+                    Debug.Assert(removed);
+                }
             }
 
             /// <summary>
@@ -193,6 +226,19 @@ public static void VisitClosures(Scope scope, Action<Scope, Closure> action)
                 }
             }
 
+            /// <summary>
+            /// Visit the tree with the given root and run the <paramref name="action"/>
+            /// </summary>
+            public static void VisitScopeTree(Scope treeRoot, Action<Scope> action)
+            {
+                action(treeRoot);
+
+                foreach (var nested in treeRoot.NestedScopes)
+                {
+                    VisitScopeTree(nested, action);
+                }
+            }
+
             /// <summary>
             /// Builds a tree of <see cref="Scope"/> nodes corresponding to a given method.
             /// <see cref="Build(BoundNode, MethodSymbol, HashSet{MethodSymbol}, DiagnosticBag)"/>
@@ -267,6 +313,12 @@ private void Build()
                 {
                     // Set up the current method locals
                     DeclareLocals(_currentScope, _topLevelMethod.Parameters);
+                    // Treat 'this' as a formal parameter of the top-level method
+                    if (_topLevelMethod.TryGetThisParameter(out var thisParam))
+                    {
+                        DeclareLocals(_currentScope, ImmutableArray.Create<Symbol>(thisParam));
+                    }
+
                     Visit(_currentScope.BoundNode);
                 }
 
@@ -468,12 +520,6 @@ private void AddIfCaptured(Symbol symbol, SyntaxNode syntax)
                             return;
                         }
 
-                        // The 'this' parameter isn't declared in method scope
-                        if (symbol is ParameterSymbol param && param.IsThis)
-                        {
-                            return;
-                        }
-
                         if (_localToScope.TryGetValue(symbol, out var declScope))
                         {
                             declScope.DeclaredVariables.Add(symbol);

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

 // Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 
+using System.Collections.Generic;
+using System.Diagnostics;
 using System.Linq;
+using Microsoft.CodeAnalysis.CodeGen;
 using Microsoft.CodeAnalysis.CSharp.Symbols;
 using Microsoft.CodeAnalysis.PooledObjects;
 using Roslyn.Utilities;
@@ -29,12 +32,6 @@ internal sealed partial class Analysis
                                                                         // 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 PooledHashSet<BoundNode> ScopesThatCantBeStructs = PooledHashSet<BoundNode>.GetInstance();
-
             /// <summary>
             /// Blocks that are positioned between a block declaring some lifted variables
             /// and a block that contains the lambda that lifts said variables.
@@ -44,29 +41,44 @@ internal sealed partial class Analysis
             /// </summary>
             public readonly PooledHashSet<BoundNode> NeedsParentFrame = PooledHashSet<BoundNode>.GetInstance();
 
-            /// <summary>
-            /// Optimized locations of lambdas. 
-            /// 
-            /// Lambda does not need to be placed in a frame that corresponds to its lexical scope if lambda does not reference any local state in that scope.
-            /// It is advantageous to place lambdas higher in the scope tree, ideally in the innermost scope of all scopes that contain variables captured by a given lambda.
-            /// Doing so reduces indirections needed when captured locals are accessed. For example locals from the innermost scope can be accessed with no indirection at all.
-            /// <see cref="ComputeLambdaScopesAndFrameCaptures"/> needs to be called to compute this.
-            /// </summary>
-            public readonly SmallDictionary<MethodSymbol, BoundNode> LambdaScopes =
-                new SmallDictionary<MethodSymbol, BoundNode>(ReferenceEqualityComparer.Instance);
-
             /// <summary>
             /// The root of the scope tree for this method.
             /// </summary>
             public readonly Scope ScopeTree;
 
-            private Analysis(Scope scopeTree, PooledHashSet<MethodSymbol> methodsConvertedToDelegates)
+            private readonly MethodSymbol _topLevelMethod;
+            private readonly int _topLevelMethodOrdinal;
+            private readonly MethodSymbol _substitutedSourceMethod; 
+            private readonly VariableSlotAllocator _slotAllocatorOpt;
+            private readonly TypeCompilationState _compilationState;
+
+            private Analysis(
+                Scope scopeTree,
+                PooledHashSet<MethodSymbol> methodsConvertedToDelegates,
+                MethodSymbol topLevelMethod,
+                int topLevelMethodOrdinal,
+                MethodSymbol substitutedSourceMethod,
+                VariableSlotAllocator slotAllocatorOpt,
+                TypeCompilationState compilationState)
             {
                 ScopeTree = scopeTree;
                 MethodsConvertedToDelegates = methodsConvertedToDelegates;
+                _topLevelMethod = topLevelMethod;
+                _topLevelMethodOrdinal = topLevelMethodOrdinal;
+                _substitutedSourceMethod = substitutedSourceMethod;
+                _slotAllocatorOpt = slotAllocatorOpt;
+                _compilationState = compilationState;
             }
 
-            public static Analysis Analyze(BoundNode node, MethodSymbol method, DiagnosticBag diagnostics)
+            public static Analysis Analyze(
+                BoundNode node,
+                MethodSymbol method,
+                int topLevelMethodOrdinal,
+                MethodSymbol substitutedSourceMethod,
+                VariableSlotAllocator slotAllocatorOpt,
+                TypeCompilationState compilationState,
+                ArrayBuilder<ClosureDebugInfo> closureDebugInfo,
+                DiagnosticBag diagnostics)
             {
                 var methodsConvertedToDelegates = PooledHashSet<MethodSymbol>.GetInstance();
                 var scopeTree = ScopeTreeBuilder.Build(
@@ -74,7 +86,21 @@ public static Analysis Analyze(BoundNode node, MethodSymbol method, DiagnosticBa
                     method,
                     methodsConvertedToDelegates,
                     diagnostics);
-                return new Analysis(scopeTree, methodsConvertedToDelegates);
+                Debug.Assert(scopeTree != null);
+
+                var analysis = new Analysis(
+                    scopeTree,
+                    methodsConvertedToDelegates,
+                    method,
+                    topLevelMethodOrdinal,
+                    substitutedSourceMethod,
+                    slotAllocatorOpt,
+                    compilationState);
+
+                analysis.RemoveUnneededReferences(method.ThisParameter);
+                analysis.MakeAndAssignEnvironments(closureDebugInfo);
+                analysis.ComputeLambdaScopesAndFrameCaptures(method.ThisParameter);
+                return analysis;
             }
 
             private static BoundNode FindNodeToAnalyze(BoundNode node)
@@ -107,12 +133,17 @@ private static BoundNode FindNodeToAnalyze(BoundNode node)
             }
 
             /// <summary>
-            /// Create the optimized plan for the location of lambda methods and whether scopes need access to parent scopes
-            ///  </summary>
-            internal void ComputeLambdaScopesAndFrameCaptures(ParameterSymbol thisParam)
+            /// Must be called only after <see cref="Closure.CapturedEnvironments"/>
+            /// has been calculated.
+            ///
+            /// Finds the most optimal capture environment to place a closure in. 
+            /// This roughly corresponds to the 'highest' Scope in the tree where all
+            /// the captured variables for this closure are in scope. This minimizes
+            /// the number of indirections we may have to traverse to access captured
+            /// variables.
+            /// </summary>
+            private void ComputeLambdaScopesAndFrameCaptures(ParameterSymbol thisParam)
             {
-                RemoveUnneededReferences(thisParam);
-
                 VisitClosures(ScopeTree, (scope, closure) =>
                 {
                     if (closure.CapturedVariables.Count > 0)
@@ -124,6 +155,15 @@ internal void ComputeLambdaScopesAndFrameCaptures(ParameterSymbol thisParam)
 
                 (Scope innermost, Scope outermost) FindLambdaScopeRange(Closure closure, Scope closureScope)
                 {
+                    // If the closure only captures this, put the method directly in the
+                    // top-level method's containing type
+                    if (closure.CapturedVariables.Count == 1 &&
+                        closure.CapturedVariables.Single() is ParameterSymbol param &&
+                        param.IsThis)
+                    {
+                        return (null, null);
+                    }
+
                     Scope innermost = null;
                     Scope outermost = null;
 
@@ -149,8 +189,8 @@ internal void ComputeLambdaScopesAndFrameCaptures(ParameterSymbol thisParam)
                          curScope != null && capturedVars.Count > 0;
                          curScope = curScope.Parent)
                     {
-                        if (!(capturedVars.Overlaps(curScope.DeclaredVariables) ||
-                              capturedVars.Overlaps(curScope.Closures.Select(c => c.OriginalMethodSymbol))))
+                        if (!(capturedVars.RemoveAll(curScope.DeclaredVariables) ||
+                              capturedVars.RemoveAll(curScope.Closures.Select(c => c.OriginalMethodSymbol))))
                         {
                             continue;
                         }
@@ -160,9 +200,6 @@ internal void ComputeLambdaScopesAndFrameCaptures(ParameterSymbol thisParam)
                         {
                             innermost = curScope;
                         }
-
-                        capturedVars.RemoveAll(curScope.DeclaredVariables);
-                        capturedVars.RemoveAll(curScope.Closures.Select(c => c.OriginalMethodSymbol));
                     }
 
                     // If any captured variables are left, they're captured above method scope
@@ -183,33 +220,119 @@ void RecordClosureScope(Scope innermost, Scope outermost, Closure closure)
                     //
                     // Example: 
                     //   if a lambda captures a method's parameter and `this`, 
-                    //   its innermost scope depth is 0 (method locals and parameters) 
-                    //   and outermost scope is -1
+                    //   its innermost scope is the root Scope (method locals and parameters) 
+                    //   and outermost Scope is null
                     //   Such lambda will be placed in a closure frame that corresponds to the method's outer block
                     //   and this frame will also lift original `this` as a field when created by its parent.
                     //   Note that it is completely irrelevant how deeply the lexical scope of the lambda was originally nested.
                     if (innermost != null)
                     {
-                        LambdaScopes.Add(closure.OriginalMethodSymbol, innermost.BoundNode);
-
-                        // Disable struct closures on methods converted to delegates, as well as on async and iterator methods.
-                        var markAsNoStruct = !CanTakeRefParameters(closure.OriginalMethodSymbol);
-                        if (markAsNoStruct)
-                        {
-                            ScopesThatCantBeStructs.Add(innermost.BoundNode);
-                        }
+                        closure.ContainingEnvironmentOpt = innermost.DeclaredEnvironments[0];
 
                         while (innermost != outermost)
                         {
                             NeedsParentFrame.Add(innermost.BoundNode);
                             innermost = innermost.Parent;
-                            if (markAsNoStruct && innermost != null)
+                        }
+                    }
+                }
+            }
+
+            private void MakeAndAssignEnvironments(ArrayBuilder<ClosureDebugInfo> closureDebugInfo)
+            {
+                VisitScopeTree(ScopeTree, scope =>
+                {
+                    if (scope.DeclaredVariables.Count > 0)
+                    {
+                        // First walk the nested scopes to find all closures which
+                        // capture variables from this scope. They all need to capture
+                        // this environment. This includes closures which captured local
+                        // functions that capture those variables, so multiple passes may
+                        // be needed. This will also decide if the environment is a struct
+                        // or a class.
+                        bool isStruct = true;
+                        var closures = new SetWithInsertionOrder<Closure>();
+                        bool addedItem;
+                        do
+                        {
+                            addedItem = false;
+                            VisitClosures(scope, (closureScope, closure) =>
                             {
-                                ScopesThatCantBeStructs.Add(innermost.BoundNode);
-                            }
+                                if (!closures.Contains(closure) &&
+                                    (closure.CapturedVariables.Overlaps(scope.DeclaredVariables) ||
+                                     closure.CapturedVariables.Overlaps(closures.Select(c => c.OriginalMethodSymbol))))
+                                {
+                                    closures.Add(closure);
+                                    addedItem = true;
+                                    isStruct &= CanTakeRefParameters(closure.OriginalMethodSymbol);
+                                }
+                            });
+                        } while (addedItem == true);
+
+                        // Next create the environment and add it to the declaration scope
+                        // Currently all variables declared in the same scope are added
+                        // to the same closure environment
+                        var env = MakeEnvironment(scope, scope.DeclaredVariables, isStruct);
+                        scope.DeclaredEnvironments.Add(env);
+
+                        foreach (var closure in closures)
+                        {
+                            closure.CapturedEnvironments.Add(env);
                         }
                     }
+                });
+
+                ClosureEnvironment MakeEnvironment(Scope scope, IEnumerable<Symbol> capturedVariables, bool isStruct)
+                {
+                    var scopeBoundNode = scope.BoundNode;
+
+                    var syntax = scopeBoundNode.Syntax;
+                    Debug.Assert(syntax != null);
+
+                    DebugId methodId = GetTopLevelMethodId();
+                    DebugId closureId = GetClosureId(syntax, closureDebugInfo);
+
+                    var containingMethod = scope.ContainingClosureOpt?.OriginalMethodSymbol ?? _topLevelMethod;
+                    if ((object)_substitutedSourceMethod != null && containingMethod == _topLevelMethod)
+                    {
+                        containingMethod = _substitutedSourceMethod;
+                    }
+
+                    return new ClosureEnvironment(
+                        capturedVariables,
+                        _topLevelMethod,
+                        containingMethod,
+                        isStruct,
+                        syntax,
+                        methodId,
+                        closureId);
+                }
+            }
+
+            internal DebugId GetTopLevelMethodId()
+            {
+                return _slotAllocatorOpt?.MethodId ?? new DebugId(_topLevelMethodOrdinal, _compilationState.ModuleBuilderOpt.CurrentGenerationOrdinal);
+            }
+
+            private DebugId GetClosureId(SyntaxNode syntax, ArrayBuilder<ClosureDebugInfo> closureDebugInfo)
+            {
+                Debug.Assert(syntax != null);
+
+                DebugId closureId;
+                DebugId previousClosureId;
+                if (_slotAllocatorOpt != null && _slotAllocatorOpt.TryGetPreviousClosure(syntax, out previousClosureId))
+                {
+                    closureId = previousClosureId;
                 }
+                else
+                {
+                    closureId = new DebugId(closureDebugInfo.Count, _compilationState.ModuleBuilderOpt.CurrentGenerationOrdinal);
+                }
+
+                int syntaxOffset = _topLevelMethod.CalculateLocalSyntaxOffset(syntax.SpanStart, syntax.SyntaxTree);
+                closureDebugInfo.Add(new ClosureDebugInfo(syntaxOffset, closureId));
+
+                return closureId;
             }
 
             /// <summary>
@@ -347,7 +470,6 @@ Closure Helper(Scope scope)
             public void Free()
             {
                 MethodsConvertedToDelegates.Free();
-                ScopesThatCantBeStructs.Free();
                 NeedsParentFrame.Free();
                 ScopeTree.Free();
             }

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

@@ -140,7 +140,7 @@ public BoundStatement RewriteLocalFunctionReferences(BoundStatement loweredBody)
                         _framePointers.TryGetValue(synthesizedLambda.ContainingType, out _innermostFramePointer);
                     }
 
-                    var containerAsFrame = synthesizedLambda.ContainingType as LambdaFrame;
+                    var containerAsFrame = synthesizedLambda.ContainingType as ClosureEnvironment;
 
                     // Includes type parameters from the containing type iff
                     // the containing type is a frame. If it is a frame then
@@ -201,11 +201,11 @@ public BoundStatement RewriteLocalFunctionReferences(BoundStatement loweredBody)
                     // will always be a LambdaFrame, it's always a capture frame
                     var frameType = (NamedTypeSymbol)loweredSymbol.Parameters[i].Type.OriginalDefinition;
 
-                    Debug.Assert(frameType is LambdaFrame);
+                    Debug.Assert(frameType is ClosureEnvironment);
 
                     if (frameType.Arity > 0)
                     {
-                        var typeParameters = ((LambdaFrame)frameType).ConstructedFromTypeParameters;
+                        var typeParameters = ((ClosureEnvironment)frameType).ConstructedFromTypeParameters;
                         Debug.Assert(typeParameters.Length == frameType.Arity);
                         var subst = this.TypeMap.SubstituteTypeParameters(typeParameters);
                         frameType = frameType.Construct(subst);

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

@@ -15,11 +15,11 @@ namespace Microsoft.CodeAnalysis.CSharp
     internal sealed class SynthesizedLambdaMethod : SynthesizedMethodBaseSymbol, ISynthesizedMethodBodyImplementationSymbol
     {
         private readonly MethodSymbol _topLevelMethod;
-        private readonly ImmutableArray<TypeSymbol> _structClosures;
+        private readonly ImmutableArray<NamedTypeSymbol> _structEnvironments;
 
         internal SynthesizedLambdaMethod(
             NamedTypeSymbol containingType,
-            ImmutableArray<TypeSymbol> structClosures,
+            ImmutableArray<ClosureEnvironment> structEnvironments,
             ClosureKind closureKind,
             MethodSymbol topLevelMethod,
             DebugId topLevelMethodId,
@@ -43,15 +43,15 @@ internal sealed class SynthesizedLambdaMethod : SynthesizedMethodBaseSymbol, ISy
             TypeMap typeMap;
             ImmutableArray<TypeParameterSymbol> typeParameters;
             ImmutableArray<TypeParameterSymbol> constructedFromTypeParameters;
-            LambdaFrame lambdaFrame;
+            ClosureEnvironment lambdaFrame;
 
-            lambdaFrame = this.ContainingType as LambdaFrame;
+            lambdaFrame = this.ContainingType as ClosureEnvironment;
             switch (closureKind)
             {
                 case ClosureKind.Singleton: // all type parameters on method (except the top level method's)
                 case ClosureKind.General: // only lambda's type parameters on method (rest on class)
                     Debug.Assert(lambdaFrame != null);
-                    typeMap = lambdaFrame.TypeMap.WithConcatAlphaRename(lambdaNode.Symbol, this, out typeParameters, out constructedFromTypeParameters, lambdaFrame.ContainingMethod);
+                    typeMap = lambdaFrame.TypeMap.WithConcatAlphaRename(lambdaNode.Symbol, this, out typeParameters, out constructedFromTypeParameters, lambdaFrame.OriginalContainingMethodOpt);
                     break;
                 case ClosureKind.ThisOnly: // all type parameters on method
                 case ClosureKind.Static:
@@ -62,28 +62,30 @@ internal sealed class SynthesizedLambdaMethod : SynthesizedMethodBaseSymbol, ISy
                     throw ExceptionUtilities.UnexpectedValue(closureKind);
             }
 
-            if (!structClosures.IsDefaultOrEmpty && typeParameters.Length != 0)
+            if (!structEnvironments.IsDefaultOrEmpty && typeParameters.Length != 0)
             {
-                var constructedStructClosures = ArrayBuilder<TypeSymbol>.GetInstance();
-                foreach (var closure in structClosures)
+                var constructedStructClosures = ArrayBuilder<NamedTypeSymbol>.GetInstance();
+                foreach (var env in structEnvironments)
                 {
-                    var frame = (LambdaFrame)closure;
                     NamedTypeSymbol constructed;
-                    if (frame.Arity == 0)
+                    if (env.Arity == 0)
                     {
-                        constructed = frame;
+                        constructed = env;
                     }
                     else
                     {
-                        var originals = frame.ConstructedFromTypeParameters;
+                        var originals = env.ConstructedFromTypeParameters;
                         var newArgs = typeMap.SubstituteTypeParameters(originals);
-                        constructed = frame.Construct(newArgs);
+                        constructed = env.Construct(newArgs);
                     }
                     constructedStructClosures.Add(constructed);
                 }
-                structClosures = constructedStructClosures.ToImmutableAndFree();
+                _structEnvironments = constructedStructClosures.ToImmutableAndFree();
+            }
+            else
+            {
+                _structEnvironments = ImmutableArray<NamedTypeSymbol>.CastUp(structEnvironments);
             }
-            _structClosures = structClosures;
 
             AssignTypeMapAndTypeParameters(typeMap, typeParameters);
         }
@@ -125,8 +127,9 @@ private static string MakeName(string topLevelMethodName, DebugId topLevelMethod
         // UNDONE: names from the delegate. Does it really matter?
         protected override ImmutableArray<ParameterSymbol> BaseMethodParameters => this.BaseMethod.Parameters;
 
-        protected override ImmutableArray<TypeSymbol> ExtraSynthesizedRefParameters => _structClosures;
-        internal int ExtraSynthesizedParameterCount => this._structClosures.IsDefault ? 0 : this._structClosures.Length;
+        protected override ImmutableArray<TypeSymbol> ExtraSynthesizedRefParameters
+            => ImmutableArray<TypeSymbol>.CastUp(_structEnvironments);
+        internal int ExtraSynthesizedParameterCount => this._structEnvironments.IsDefault ? 0 : this._structEnvironments.Length;
 
         internal override bool GenerateDebugInfo => !this.IsAsync;
         internal override bool IsExpressionBodied => false;

src/Compilers/Core/Portable/InternalUtilities/ImmutableArrayExtensions.cs

@@ -9,24 +9,10 @@ namespace Roslyn.Utilities
     internal static class ImmutableArrayExtensions
     {
         internal static ImmutableArray<T> ToImmutableArrayOrEmpty<T>(this IEnumerable<T> items)
-        {
-            if (items == null)
-            {
-                return ImmutableArray.Create<T>();
-            }
-
-            return ImmutableArray.CreateRange<T>(items);
-        }
+            => items == null ? ImmutableArray<T>.Empty : ImmutableArray.CreateRange(items);
 
         internal static ImmutableArray<T> ToImmutableArrayOrEmpty<T>(this ImmutableArray<T> items)
-        {
-            if (items.IsDefault)
-            {
-                return ImmutableArray.Create<T>();
-            }
-
-            return items;
-        }
+            => items.IsDefault ? ImmutableArray<T>.Empty : items;
 
         // same as Array.BinarySearch but the ability to pass arbitrary value to the comparer without allocation
         internal static int BinarySearch<TElement, TValue>(this ImmutableArray<TElement> array, TValue value, Func<TElement, TValue, int> comparer)

src/Compilers/Core/Portable/InternalUtilities/SetWithInsertionOrder.cs

@@ -7,6 +7,7 @@
 using System.Linq;
 using System.Text;
 using System.Threading.Tasks;
+using Microsoft.CodeAnalysis.PooledObjects;
 
 namespace Roslyn.Utilities
 {
@@ -16,46 +17,45 @@ namespace Roslyn.Utilities
     /// </summary>
     internal sealed class SetWithInsertionOrder<T> : IEnumerable<T>, IReadOnlySet<T>
     {
-        private HashSet<T> _set = new HashSet<T>();
-        private uint _nextElementValue = 0;
-        private T[] _elements = null;
+        private HashSet<T> _set = null;
+        private ArrayBuilder<T> _elements = null;
 
         public bool Add(T value)
         {
-            if (!_set.Add(value)) return false;
-            var thisValue = _nextElementValue++;
-            if (_elements == null)
+            if (_set == null)
             {
-                _elements = new T[10];
+                _set = new HashSet<T>();
+                _elements = new ArrayBuilder<T>();
             }
-            else if (_elements.Length <= thisValue)
+
+            if (!_set.Add(value))
             {
-                Array.Resize(ref _elements, _elements.Length * 2);
+                return false;
             }
 
-            _elements[thisValue] = value;
+            _elements.Add(value);
+            return true;
+        }
+
+        public bool Remove(T value)
+        {
+            if (!_set.Remove(value))
+            {
+                return false;
+            }
+            _elements.RemoveAt(_elements.IndexOf(value));
             return true;
         }
 
-        public int Count => (int)_nextElementValue;
+        public int Count => _elements?.Count ?? 0;
 
-        public bool Contains(T value) => _set.Contains(value);
+        public bool Contains(T value) => _set?.Contains(value) ?? false;
 
         public IEnumerator<T> GetEnumerator()
-        {
-            for (int i = 0; i < _nextElementValue; i++) yield return _elements[i];
-        }
+            => _elements?.GetEnumerator() ?? SpecializedCollections.EmptyEnumerator<T>();
 
         IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
 
-        /// <summary>
-        /// An enumerable that yields the set's elements in insertion order.
-        /// </summary>
-        public SetWithInsertionOrder<T> InInsertionOrder => this;
-
-        public ImmutableArray<T> AsImmutable()
-        {
-            return (_elements == null) ? ImmutableArray<T>.Empty : ImmutableArray.Create(_elements, 0, (int)_nextElementValue);
-        }
+        public ImmutableArray<T> AsImmutable() => _elements.ToImmutableArrayOrEmpty();
     }
 }

src/Compilers/VisualBasic/Portable/Symbols/Source/SourceNamedTypeSymbol.vb

@@ -1312,7 +1312,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols
                 MakeDeclaredInterfacesInPart(syntaxRef.SyntaxTree, syntaxRef.GetVisualBasicSyntax(), interfaces, basesBeingResolved, diagnostics)
             Next
 
-            Return interfaces.InInsertionOrder.AsImmutable
+            Return interfaces.AsImmutable
         End Function
 
         Private Function GetInheritsLocation(base As NamedTypeSymbol) As Location

src/Compilers/VisualBasic/Portable/VisualBasicParseOptions.vb

@@ -47,7 +47,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
             languageVersion As LanguageVersion,
             documentationMode As DocumentationMode,
             kind As SourceCodeKind,
-            preprocessorSymbols As IEnumerable(Of KeyValuePair(Of String, Object)),
+            preprocessorSymbols As ImmutableArray(Of KeyValuePair(Of String, Object)),
             features As ImmutableDictionary(Of String, String))
 
             MyBase.New(kind, documentationMode)