From caa783000516207aaa12caf255a672ce0a03bc1d Mon Sep 17 00:00:00 2001 From: Andy Gocke Date: Thu, 10 Aug 2017 20:02:07 -0700 Subject: [PATCH] Remove RemoveUnneededReferences from LamdaRewriter (#21367) Currently, the lambda rewriter has an early optimization pass in analysis that tries to find all local functions that only capture 'this' and remove references to local functions that do the same. There are two problems with this approach: 1) Generally, removing information from the tree is a bad idea because it hurts further analysis passes that may have needed that information. 2) The optimization strategy itself is very tricky and has a number of complex corner cases. This has lead to bugs, for example #19033. This PR deletes the current method and adds a new optimization routine at the end of the analysis, operating on assigned scopes and environments rather than removing captured variable analysis. The new optimization is as follows: if we end up with an environment containing only 'this', the environment can be removed, all containing methods can be moved to the top-level type, and all environments which capture the 'this' environment can instead directly capture the 'this' parameter. This produces almost the same results as the previous optimization, but is easier to validate as an algebraic equivalence. The baseline changes come from the new optimization being less aggressive about moving functions which only capture 'this' to the top level. This appears to be a wash -- some codegen gets slightly better, some gets slightly worse. Fixes #19033 Fixes #20577 --- .../Portable/Compiler/TypeCompilationState.cs | 2 +- .../LambdaRewriter/LambdaCapturedVariable.cs | 4 +- .../LambdaRewriter.Analysis.Tree.cs | 135 ++++--- .../LambdaRewriter/LambdaRewriter.Analysis.cs | 329 ++++++++++-------- ...Rewriter.LocalFunctionReferenceRewriter.cs | 6 +- .../Lowering/LambdaRewriter/LambdaRewriter.cs | 155 +++++---- ...me.cs => SynthesizedClosureEnvironment.cs} | 12 +- ...nthesizedClosureEnvironmentConstructor.cs} | 4 +- .../LambdaRewriter/SynthesizedLambdaMethod.cs | 6 +- .../Emit/CodeGen/CodeGenClosureLambdaTests.cs | 51 ++- .../Emit/CodeGen/CodeGenLocalFunctionTests.cs | 155 +++++++-- .../EditAndContinueClosureTests.cs | 50 +-- .../CSharp/Test/Emit/PDB/PDBLambdaTests.cs | 36 +- .../SetWithInsertionOrder.cs | 32 ++ .../ExpressionCompiler/LocalFunctionTests.cs | 6 +- 15 files changed, 608 insertions(+), 375 deletions(-) rename src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/{LambdaFrame.cs => SynthesizedClosureEnvironment.cs} (89%) rename src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/{LambdaFrameConstructor.cs => SynthesizedClosureEnvironmentConstructor.cs} (72%) diff --git a/src/Compilers/CSharp/Portable/Compiler/TypeCompilationState.cs b/src/Compilers/CSharp/Portable/Compiler/TypeCompilationState.cs index 2898d62ca6289..785b20d1f24d7 100644 --- a/src/Compilers/CSharp/Portable/Compiler/TypeCompilationState.cs +++ b/src/Compilers/CSharp/Portable/Compiler/TypeCompilationState.cs @@ -66,7 +66,7 @@ internal MethodWithBody(MethodSymbol method, BoundStatement body, ImportChain im public readonly CSharpCompilation Compilation; - public ClosureEnvironment StaticLambdaFrame; + public SynthesizedClosureEnvironment StaticLambdaFrame; /// /// A graph of method->method references for this(...) constructor initializers. diff --git a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaCapturedVariable.cs b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaCapturedVariable.cs index a67f98a89bffe..6ac017446b934 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaCapturedVariable.cs +++ b/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(ClosureEnvironment frame, Symbol captured, ref int uniqueId) + public static LambdaCapturedVariable Create(SynthesizedClosureEnvironment 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 ClosureEnvironment; + var lambdaFrame = local.Type.OriginalDefinition as SynthesizedClosureEnvironment; if ((object)lambdaFrame != null) { // lambdaFrame may have less generic type parameters than frame, so trim them down (the first N will always match) diff --git a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.Analysis.Tree.cs b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.Analysis.Tree.cs index f776ab258519f..3995278c7913a 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.Analysis.Tree.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.Analysis.Tree.cs @@ -119,6 +119,23 @@ public sealed class Closure public ClosureEnvironment ContainingEnvironmentOpt; + private bool _capturesThis; + + /// + /// True if this closure directly or transitively captures 'this' (captures + /// a local function which directly or indirectly captures 'this'). + /// Calculated in . + /// + public bool CapturesThis + { + get => _capturesThis; + set + { + Debug.Assert(value); + _capturesThis = value; + } + } + public Closure(MethodSymbol symbol) { Debug.Assert(symbol != null); @@ -132,81 +149,29 @@ public void Free() } } - /// - /// Optimizes local functions such that if a local function only references other local functions - /// that capture no variables, we don't need to create capture environments for any of them. - /// - private void RemoveUnneededReferences(ParameterSymbol thisParam) + public sealed class ClosureEnvironment { - var methodGraph = new MultiDictionary(); - var capturesThis = new HashSet(); - var capturesVariable = new HashSet(); - var visitStack = new Stack(); - VisitClosures(ScopeTree, (scope, closure) => - { - foreach (var capture in closure.CapturedVariables) - { - if (capture is MethodSymbol localFunc) - { - methodGraph.Add(localFunc, closure.OriginalMethodSymbol); - } - else if (capture == thisParam) - { - if (capturesThis.Add(closure.OriginalMethodSymbol)) - { - visitStack.Push(closure.OriginalMethodSymbol); - } - } - else if (capturesVariable.Add(closure.OriginalMethodSymbol) && - !capturesThis.Contains(closure.OriginalMethodSymbol)) - { - visitStack.Push(closure.OriginalMethodSymbol); - } - } - }); - - while (visitStack.Count > 0) - { - var current = visitStack.Pop(); - var setToAddTo = capturesVariable.Contains(current) ? capturesVariable : capturesThis; - foreach (var capturesCurrent in methodGraph[current]) - { - if (setToAddTo.Add(capturesCurrent)) - { - visitStack.Push(capturesCurrent); - } - } - } + public readonly SetWithInsertionOrder CapturedVariables; + + /// + /// Represents a that had its environment + /// pointer (a local pointing to the environment) captured like a captured + /// variable. Assigned in + /// + /// + public bool CapturesParent; - // True if there are any closures in the tree which - // capture 'this' and another variable - bool captureMoreThanThis = false; + public readonly bool IsStruct; + internal SynthesizedClosureEnvironment SynthesizedEnvironment; - VisitClosures(ScopeTree, (scope, closure) => + public ClosureEnvironment(IEnumerable capturedVariables, bool isStruct) { - if (!capturesVariable.Contains(closure.OriginalMethodSymbol)) - { - closure.CapturedVariables.Clear(); - } - - if (capturesThis.Contains(closure.OriginalMethodSymbol)) + CapturedVariables = new SetWithInsertionOrder(); + foreach (var item in capturedVariables) { - closure.CapturedVariables.Add(thisParam); - if (closure.CapturedVariables.Count > 1) - { - captureMoreThanThis |= true; - } + CapturedVariables.Add(item); } - }); - - 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); + IsStruct = isStruct; } } @@ -226,6 +191,31 @@ public static void VisitClosures(Scope scope, Action action) } } + /// + /// Visit all the closures and return true when the returns + /// true. Otherwise, returns false. + /// + public static bool CheckClosures(Scope scope, Func func) + { + foreach (var closure in scope.Closures) + { + if (func(scope, closure)) + { + return true; + } + } + + foreach (var nested in scope.NestedScopes) + { + if (CheckClosures(nested, func)) + { + return true; + } + } + + return false; + } + /// /// Visit the tree with the given root and run the /// @@ -491,6 +481,13 @@ private void AddIfCaptured(Symbol symbol, SyntaxNode syntax) return; } + if (symbol is MethodSymbol method && + _currentClosure.OriginalMethodSymbol == method) + { + // Is this recursion? If so there's no capturing + return; + } + if (symbol.ContainingSymbol != _currentClosure.OriginalMethodSymbol) { // Restricted types can't be hoisted, so they are not permitted to be captured diff --git a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.Analysis.cs b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.Analysis.cs index 4c77933cd3c64..98a3179932f70 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.Analysis.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.Analysis.cs @@ -1,6 +1,7 @@ // 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 System.Linq; using Microsoft.CodeAnalysis.CodeGen; @@ -32,15 +33,6 @@ internal sealed partial class Analysis // We can't rewrite delegate signatures || MethodsConvertedToDelegates.Contains(closure)); - /// - /// Blocks that are positioned between a block declaring some lifted variables - /// and a block that contains the lambda that lifts said variables. - /// If such block itself requires a closure, then it must lift parent frame pointer into the closure - /// in addition to whatever else needs to be lifted. - /// needs to be called to compute this. - /// - public readonly PooledHashSet NeedsParentFrame = PooledHashSet.GetInstance(); - /// /// The root of the scope tree for this method. /// @@ -97,9 +89,9 @@ internal sealed partial class Analysis slotAllocatorOpt, compilationState); - analysis.RemoveUnneededReferences(method.ThisParameter); - analysis.MakeAndAssignEnvironments(closureDebugInfo); + analysis.MakeAndAssignEnvironments(); analysis.ComputeLambdaScopesAndFrameCaptures(method.ThisParameter); + analysis.InlineThisOnlyEnvironments(); return analysis; } @@ -146,167 +138,233 @@ private void ComputeLambdaScopesAndFrameCaptures(ParameterSymbol thisParam) { VisitClosures(ScopeTree, (scope, closure) => { - if (closure.CapturedVariables.Count > 0) + if (closure.CapturedEnvironments.Count > 0) { - (Scope innermost, Scope outermost) = FindLambdaScopeRange(closure, scope); - RecordClosureScope(innermost, outermost, closure); + var capturedEnvs = PooledHashSet.GetInstance(); + capturedEnvs.AddAll(closure.CapturedEnvironments); + + // Find the nearest captured class environment, if one exists + var curScope = scope; + while (curScope != null) + { + if (capturedEnvs.RemoveAll(curScope.DeclaredEnvironments)) + { + // Right now we only create one environment per scope + Debug.Assert(curScope.DeclaredEnvironments.Count == 1); + var env = curScope.DeclaredEnvironments[0]; + if (!env.IsStruct) + { + closure.ContainingEnvironmentOpt = env; + break; + } + } + curScope = curScope.Parent; + } + + // Now we need to walk up the scopes to find environment captures + var oldEnv = curScope?.DeclaredEnvironments[0]; + while (curScope != null) + { + if (capturedEnvs.Count == 0) + { + break; + } + + var envs = curScope.DeclaredEnvironments.Where(e => !e.IsStruct); + if (!envs.IsEmpty()) + { + // Right now we only create one environment per scope + Debug.Assert(envs.IsSingle()); + var env = envs.First(); + Debug.Assert(!oldEnv.IsStruct); + oldEnv.CapturesParent = true; + oldEnv = env; + } + capturedEnvs.RemoveAll(curScope.DeclaredEnvironments); + curScope = curScope.Parent; + } + + if (capturedEnvs.Count > 0) + { + throw ExceptionUtilities.Unreachable; + } + + capturedEnvs.Free(); + } }); + } - (Scope innermost, Scope outermost) FindLambdaScopeRange(Closure closure, Scope closureScope) + /// + /// We may have ended up with a closure environment containing only + /// 'this'. This is basically equivalent to the containing type itself, + /// so we can inline the 'this' parameter into environments that + /// reference this one or lower closures directly onto the containing + /// type. + /// + private void InlineThisOnlyEnvironments() + { + // First make sure 'this' even exists + if (!_topLevelMethod.TryGetThisParameter(out var thisParam) || + thisParam == null) { - // 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); - } + return; + } - Scope innermost = null; - Scope outermost = null; + var topLevelEnvs = ScopeTree.DeclaredEnvironments; - var capturedVars = PooledHashSet.GetInstance(); - capturedVars.AddAll(closure.CapturedVariables); + // If it does exist, 'this' is always in the top-level environment + if (topLevelEnvs.Count == 0) + { + return; + } + + Debug.Assert(topLevelEnvs.Count == 1); + var env = topLevelEnvs[0]; - // If any of the captured variables are local functions we'll need - // to add the captured variables of that local function to the current - // set. This has the effect of ensuring that if the local function - // captures anything "above" the current scope then parent frame - // is itself captured (so that the current lambda can call that - // local function). - foreach (var captured in closure.CapturedVariables) + // The environment must contain only 'this' to be inlined + if (env.CapturedVariables.Count > 1 || + !env.CapturedVariables.Contains(thisParam)) + { + return; + } + + if (env.IsStruct) + { + // If everything that captures the 'this' environment + // lives in the containing type, we can remove the env + bool cantRemove = CheckClosures(ScopeTree, (scope, closure) => + { + return closure.CapturedEnvironments.Contains(env) && + closure.ContainingEnvironmentOpt != null; + }); + + if (!cantRemove) + { + RemoveEnv(); + } + } + else + { + // Class-based 'this' closures can move member functions + // to the top-level type and environments which capture + // the 'this' environment can capture 'this' directly + RemoveEnv(); + VisitClosures(ScopeTree, (scope, closure) => { - if (captured is LocalFunctionSymbol localFunc) + if (closure.ContainingEnvironmentOpt == env) { - var (found, _) = GetVisibleClosure(closureScope, localFunc); - capturedVars.AddAll(found.CapturedVariables); + closure.ContainingEnvironmentOpt = null; } - } - - for (var curScope = closureScope; - curScope != null && capturedVars.Count > 0; - curScope = curScope.Parent) + }); + + // Find all environments in the scope below that could + // capture the parent. If there are any, add 'this' to + // the list of captured variables and remove the parent + // link + VisitFirstLevelScopes(ScopeTree); + void VisitFirstLevelScopes(Scope scope) { - if (!(capturedVars.RemoveAll(curScope.DeclaredVariables) || - capturedVars.RemoveAll(curScope.Closures.Select(c => c.OriginalMethodSymbol)))) + var classEnvs = scope.DeclaredEnvironments.Where(e => !e.IsStruct); + if (classEnvs.IsEmpty()) { - continue; + // Keep looking for nested environments + foreach (var nested in scope.NestedScopes) + { + VisitFirstLevelScopes(nested); + } } - - outermost = curScope; - if (innermost == null) + else { - innermost = curScope; + foreach (var declEnv in classEnvs) + { + if (declEnv.CapturesParent) + { + declEnv.CapturedVariables.Insert(0, thisParam); + declEnv.CapturesParent = false; + } + } } } - - // If any captured variables are left, they're captured above method scope - if (capturedVars.Count > 0) - { - outermost = null; - } - - capturedVars.Free(); - - return (innermost, outermost); } - void RecordClosureScope(Scope innermost, Scope outermost, Closure closure) + void RemoveEnv() { - // 1) if there is innermost scope, lambda goes there as we cannot go any higher. - // 2) scopes in [innermostScope, outermostScope) chain need to have access to the parent scope. - // - // Example: - // if a lambda captures a method's parameter and `this`, - // 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) + topLevelEnvs.RemoveAt(topLevelEnvs.IndexOf(env)); + VisitClosures(ScopeTree, (scope, closure) => { - closure.ContainingEnvironmentOpt = innermost.DeclaredEnvironments[0]; - - while (innermost != outermost) + var index = closure.CapturedEnvironments.IndexOf(env); + if (index >= 0) { - NeedsParentFrame.Add(innermost.BoundNode); - innermost = innermost.Parent; + closure.CapturedEnvironments.RemoveAt(index); } - } + }); } } - private void MakeAndAssignEnvironments(ArrayBuilder closureDebugInfo) + private void MakeAndAssignEnvironments() { VisitScopeTree(ScopeTree, scope => { - if (scope.DeclaredVariables.Count > 0) + // Currently all variables declared in the same scope are added + // to the same closure environment + var variablesInEnvironment = scope.DeclaredVariables; + + // Don't create empty environments + if (variablesInEnvironment.Count == 0) + { + return; + } + + // 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(); + bool addedItem; + + // This loop is O(n), where n is the length of the chain + // L_1 <- L_2 <- L_3 ... + // where L_1 represents a local function that directly captures the current + // environment, L_2 represents a local function that directly captures L_1, + // L_3 represents a local function that captures L_2, and so on. + // + // Each iteration of the loop runs a visitor that is proportional to the + // number of closures in nested scopes, so we hope that the total number + // of nested functions and function chains is small in any real-world code. + do { - // 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(); - bool addedItem; - do + addedItem = false; + VisitClosures(scope, (closureScope, closure) => { - addedItem = false; - VisitClosures(scope, (closureScope, closure) => + if (!closures.Contains(closure) && + (closure.CapturedVariables.Overlaps(scope.DeclaredVariables) || + closure.CapturedVariables.Overlaps(closures.Select(c => c.OriginalMethodSymbol)))) { - 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); + 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); + // Next create the environment and add it to the declaration scope + var env = new ClosureEnvironment(variablesInEnvironment, isStruct); + scope.DeclaredEnvironments.Add(env); - foreach (var closure in closures) + _topLevelMethod.TryGetThisParameter(out var thisParam); + foreach (var closure in closures) + { + closure.CapturedEnvironments.Add(env); + if (thisParam != null && env.CapturedVariables.Contains(thisParam)) { - closure.CapturedEnvironments.Add(env); + closure.CapturesThis = true; } } }); - - ClosureEnvironment MakeEnvironment(Scope scope, IEnumerable 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() @@ -314,7 +372,7 @@ internal DebugId GetTopLevelMethodId() return _slotAllocatorOpt?.MethodId ?? new DebugId(_topLevelMethodOrdinal, _compilationState.ModuleBuilderOpt.CurrentGenerationOrdinal); } - private DebugId GetClosureId(SyntaxNode syntax, ArrayBuilder closureDebugInfo) + internal DebugId GetClosureId(SyntaxNode syntax, ArrayBuilder closureDebugInfo) { Debug.Assert(syntax != null); @@ -470,7 +528,6 @@ Closure Helper(Scope scope) public void Free() { MethodsConvertedToDelegates.Free(); - NeedsParentFrame.Free(); ScopeTree.Free(); } } diff --git a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.LocalFunctionReferenceRewriter.cs b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.LocalFunctionReferenceRewriter.cs index ffc7842e98836..95bb6cd7400f1 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.LocalFunctionReferenceRewriter.cs +++ b/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 ClosureEnvironment; + var containerAsFrame = synthesizedLambda.ContainingType as SynthesizedClosureEnvironment; // 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 ClosureEnvironment); + Debug.Assert(frameType is SynthesizedClosureEnvironment); if (frameType.Arity > 0) { - var typeParameters = ((ClosureEnvironment)frameType).ConstructedFromTypeParameters; + var typeParameters = ((SynthesizedClosureEnvironment)frameType).ConstructedFromTypeParameters; Debug.Assert(typeParameters.Length == frameType.Arity); var subst = this.TypeMap.SubstituteTypeParameters(typeParameters); frameType = frameType.Construct(subst); diff --git a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.cs b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.cs index 0b00ee2c34454..1e4c9b4c98d11 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.cs @@ -30,7 +30,7 @@ namespace Microsoft.CodeAnalysis.CSharp /// have captured variables. The result of this analysis is left in . /// /// Then we make a frame, or compiler-generated class, represented by an instance of - /// for each scope with captured variables. The generated frames are kept + /// for each scope with captured variables. The generated frames are kept /// in . Each frame is given a single field for each captured /// variable in the corresponding scope. These are maintained in . /// @@ -73,7 +73,7 @@ internal sealed partial class LambdaRewriter : MethodToClassRewriter // lambda frame for static lambdas. // initialized lazily and could be null if there are no static lambdas - private ClosureEnvironment _lazyStaticLambdaFrame; + private SynthesizedClosureEnvironment _lazyStaticLambdaFrame; // A mapping from every lambda parameter to its corresponding method's parameter. private readonly Dictionary _parameterMap = new Dictionary(); @@ -93,7 +93,7 @@ public MappedLocalFunction(SynthesizedLambdaMethod symbol, ClosureKind closureKi private readonly Dictionary _localFunctionMap = new Dictionary(); // for each block with lifted (captured) variables, the corresponding frame type - private readonly Dictionary _frames = new Dictionary(); + private readonly Dictionary _frames = new Dictionary(); // the current set of frame pointers in scope. Each is either a local variable (where introduced), // or the "this" parameter when at the top level. Keys in this map are never constructed types. @@ -268,7 +268,7 @@ protected override bool NeedsProxy(Symbol localOrParameter) diagnostics, assignLocals); - rewriter.SynthesizeClosureEnvironments(); + rewriter.SynthesizeClosureEnvironments(closureDebugInfoBuilder); // First, lower everything but references (calls, delegate conversions) // to local functions @@ -341,10 +341,10 @@ protected override NamedTypeSymbol ContainingType static partial void CheckLocalsDefined(BoundNode node); /// - /// Adds synthesized types to the compilation state + /// Adds synthesized types to the compilation state /// and creates hoisted fields for all locals captured by the environments. /// - private void SynthesizeClosureEnvironments() + private void SynthesizeClosureEnvironments(ArrayBuilder closureDebugInfo) { Analysis.VisitScopeTree(_analysis.ScopeTree, scope => { @@ -356,32 +356,59 @@ private void SynthesizeClosureEnvironments() Debug.Assert(scope.DeclaredEnvironments.Count == 1); var env = scope.DeclaredEnvironments[0]; + var frame = MakeFrame(scope, env.IsStruct); + env.SynthesizedEnvironment = frame; - CompilationState.ModuleBuilderOpt.AddSynthesizedDefinition(ContainingType, env); - if (env.Constructor != null) + CompilationState.ModuleBuilderOpt.AddSynthesizedDefinition(ContainingType, frame); + if (frame.Constructor != null) { AddSynthesizedMethod( - env.Constructor, + frame.Constructor, FlowAnalysisPass.AppendImplicitReturn( - MethodCompiler.BindMethodBody(env.Constructor, CompilationState, null), - env.Constructor)); + MethodCompiler.BindMethodBody(frame.Constructor, CompilationState, null), + frame.Constructor)); } foreach (var captured in env.CapturedVariables) { Debug.Assert(!proxies.ContainsKey(captured)); - var hoistedField = LambdaCapturedVariable.Create(env, captured, ref _synthesizedFieldNameIdDispenser); + var hoistedField = LambdaCapturedVariable.Create(frame, captured, ref _synthesizedFieldNameIdDispenser); proxies.Add(captured, new CapturedToFrameSymbolReplacement(hoistedField, isReusable: false)); - CompilationState.ModuleBuilderOpt.AddSynthesizedDefinition(env, hoistedField); + CompilationState.ModuleBuilderOpt.AddSynthesizedDefinition(frame, hoistedField); } _frames.Add(scope.BoundNode, env); } }); + + SynthesizedClosureEnvironment MakeFrame(Analysis.Scope scope, bool isStruct) + { + var scopeBoundNode = scope.BoundNode; + + var syntax = scopeBoundNode.Syntax; + Debug.Assert(syntax != null); + + DebugId methodId = _analysis.GetTopLevelMethodId(); + DebugId closureId = _analysis.GetClosureId(syntax, closureDebugInfo); + + var containingMethod = scope.ContainingClosureOpt?.OriginalMethodSymbol ?? _topLevelMethod; + if ((object)_substitutedSourceMethod != null && containingMethod == _topLevelMethod) + { + containingMethod = _substitutedSourceMethod; + } + + return new SynthesizedClosureEnvironment( + _topLevelMethod, + containingMethod, + isStruct, + syntax, + methodId, + closureId); } + } - private ClosureEnvironment GetStaticFrame(DiagnosticBag diagnostics, IBoundLambdaOrFunction lambda) + private SynthesizedClosureEnvironment GetStaticFrame(DiagnosticBag diagnostics, IBoundLambdaOrFunction lambda) { if (_lazyStaticLambdaFrame == null) { @@ -406,8 +433,7 @@ private ClosureEnvironment GetStaticFrame(DiagnosticBag diagnostics, IBoundLambd DebugId closureId = default(DebugId); // using _topLevelMethod as containing member because the static frame does not have generic parameters, except for the top level method's var containingMethod = isNonGeneric ? null : (_substitutedSourceMethod ?? _topLevelMethod); - _lazyStaticLambdaFrame = new ClosureEnvironment( - SpecializedCollections.EmptyEnumerable(), + _lazyStaticLambdaFrame = new SynthesizedClosureEnvironment( _topLevelMethod, containingMethod, isStruct: false, @@ -530,11 +556,12 @@ private static void InsertAndFreePrologue(ArrayBuilder result, A /// Introduce a frame around the translation of the given node. /// /// The node whose translation should be translated to contain a frame - /// The frame for the translated node + /// The environment for the translated node /// A function that computes the translation of the node. It receives lists of added statements and added symbols /// The translated statement, as returned from F - private BoundNode IntroduceFrame(BoundNode node, ClosureEnvironment frame, Func, ArrayBuilder, BoundNode> F) + private BoundNode IntroduceFrame(BoundNode node, Analysis.ClosureEnvironment env, Func, ArrayBuilder, BoundNode> F) { + var frame = env.SynthesizedEnvironment; var frameTypeParameters = ImmutableArray.Create(StaticCast.From(_currentTypeParameters).SelectAsArray(TypeMap.TypeSymbolAsTypeWithModifiers), 0, frame.Arity); NamedTypeSymbol frameType = frame.ConstructIfGeneric(frameTypeParameters); @@ -562,14 +589,7 @@ private BoundNode IntroduceFrame(BoundNode node, ClosureEnvironment frame, Func< if ((object)_innermostFramePointer != null) { proxies.TryGetValue(_innermostFramePointer, out oldInnermostFrameProxy); - if (_analysis.NeedsParentFrame.Contains(node) && - // If the frame pointer is a struct type that means the frame is a struct - // passed by-ref to a local function. Capturing parent frames for local - // functions is performed in RemapLambdaOrLocalFunction, rather than here - // (since struct frame pointers should never be captured, but instead be - // passed in a list to the needed local functions). - !(_innermostFramePointer.Kind == SymbolKind.Local && - ((LocalSymbol)_innermostFramePointer).Type.IsValueType)) + if (env.CapturesParent) { var capturedFrame = LambdaCapturedVariable.Create(frame, _innermostFramePointer, ref _synthesizedFieldNameIdDispenser); FieldSymbol frameParent = capturedFrame.AsMember(frameType); @@ -590,7 +610,7 @@ private BoundNode IntroduceFrame(BoundNode node, ClosureEnvironment frame, Func< // Capture any parameters of this block. This would typically occur // at the top level of a method or lambda with captured parameters. - foreach (var variable in frame.CapturedVariables) + foreach (var variable in env.CapturedVariables) { InitVariableProxy(syntax, variable, framePointer, prologue); } @@ -728,7 +748,7 @@ public override BoundNode VisitBaseReference(BoundBaseReference node) out NamedTypeSymbol constructedFrame) { var translatedLambdaContainer = synthesizedMethod.ContainingType; - var containerAsFrame = translatedLambdaContainer as ClosureEnvironment; + var containerAsFrame = translatedLambdaContainer as SynthesizedClosureEnvironment; // All of _currentTypeParameters might not be preserved here due to recursively calling upwards in the chain of local functions/lambdas Debug.Assert((typeArgumentsOpt.IsDefault && !originalMethod.IsGenericMethod) || (typeArgumentsOpt.Length == originalMethod.Arity)); @@ -866,9 +886,8 @@ private BoundSequence RewriteSequence(BoundSequence node, ArrayBuilder prologue, ArrayBuilder newLocals) => RewriteBlock(node, prologue, newLocals)); @@ -924,8 +943,7 @@ public override BoundNode VisitScope(BoundScope node) public override BoundNode VisitCatchBlock(BoundCatchBlock node) { // Test if this frame has captured variables and requires the introduction of a closure class. - ClosureEnvironment frame; - if (_frames.TryGetValue(node, out frame)) + if (_frames.TryGetValue(node, out var frame)) { return IntroduceFrame(node, frame, (ArrayBuilder prologue, ArrayBuilder newLocals) => { @@ -991,9 +1009,8 @@ private BoundNode RewriteCatch(BoundCatchBlock node, ArrayBuilder prologue, ArrayBuilder newLocals) => { @@ -1008,10 +1025,9 @@ public override BoundNode VisitSequence(BoundSequence node) public override BoundNode VisitStatementList(BoundStatementList node) { - ClosureEnvironment frame; // Test if this frame has captured variables and requires the introduction of a closure class. // That can occur for a BoundStatementList if it is the body of a method with captured parameters. - if (_frames.TryGetValue(node, out frame)) + if (_frames.TryGetValue(node, out var frame)) { return IntroduceFrame(node, frame, (ArrayBuilder prologue, ArrayBuilder newLocals) => { @@ -1034,9 +1050,8 @@ public override BoundNode VisitStatementList(BoundStatementList node) public override BoundNode VisitSwitchStatement(BoundSwitchStatement node) { - ClosureEnvironment frame; // Test if this frame has captured variables and requires the introduction of a closure class. - if (_frames.TryGetValue(node, out frame)) + if (_frames.TryGetValue(node, out var frame)) { return IntroduceFrame(node, frame, (ArrayBuilder prologue, ArrayBuilder newLocals) => { @@ -1105,7 +1120,7 @@ public override BoundNode VisitLocalFunctionStatement(BoundLocalFunctionStatemen { ClosureKind closureKind; NamedTypeSymbol translatedLambdaContainer; - ClosureEnvironment containerAsFrame; + SynthesizedClosureEnvironment containerAsFrame; BoundNode lambdaScope; DebugId topLevelMethodId; DebugId lambdaId; @@ -1178,7 +1193,7 @@ private DebugId GetLambdaId(SyntaxNode syntax, ClosureKind closureKind, int clos IBoundLambdaOrFunction node, out ClosureKind closureKind, out NamedTypeSymbol translatedLambdaContainer, - out ClosureEnvironment containerAsFrame, + out SynthesizedClosureEnvironment containerAsFrame, out BoundNode lambdaScope, out DebugId topLevelMethodId, out DebugId lambdaId) @@ -1186,40 +1201,37 @@ private DebugId GetLambdaId(SyntaxNode syntax, ClosureKind closureKind, int clos Analysis.Closure closure = Analysis.GetClosureInTree(_analysis.ScopeTree, node.Symbol); var structClosures = closure.CapturedEnvironments - .Where(env => env.IsStructType()).AsImmutable(); + .Where(env => env.IsStruct).Select(env => env.SynthesizedEnvironment).AsImmutable(); + int closureOrdinal; if (closure.ContainingEnvironmentOpt != null) { - containerAsFrame = closure.ContainingEnvironmentOpt; + containerAsFrame = closure.ContainingEnvironmentOpt?.SynthesizedEnvironment; - if (containerAsFrame?.IsValueType == true) - { - // Lower directly onto the containing type - 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; - } - else + closureKind = ClosureKind.General; + translatedLambdaContainer = containerAsFrame; + closureOrdinal = containerAsFrame.ClosureOrdinal; + // Find the scope of the containing environment + BoundNode tmpScope = null; + Analysis.VisitScopeTree(_analysis.ScopeTree, scope => { - closureKind = ClosureKind.General; - translatedLambdaContainer = containerAsFrame; - closureOrdinal = containerAsFrame.ClosureOrdinal; - // Find the scope of the containing environment - BoundNode tmpScope = null; - Analysis.VisitScopeTree(_analysis.ScopeTree, scope => + if (scope.DeclaredEnvironments.Contains(closure.ContainingEnvironmentOpt)) { - if (scope.DeclaredEnvironments.Contains(closure.ContainingEnvironmentOpt)) - { - tmpScope = scope.BoundNode; - } - }); - Debug.Assert(tmpScope != null); - lambdaScope = tmpScope; - } + tmpScope = scope.BoundNode; + } + }); + Debug.Assert(tmpScope != null); + lambdaScope = tmpScope; + } + else if (closure.CapturesThis) + { + lambdaScope = null; + containerAsFrame = null; + translatedLambdaContainer = _topLevelMethod.ContainingType; + closureKind = ClosureKind.ThisOnly; + closureOrdinal = LambdaDebugInfo.ThisOnlyClosureOrdinal; } - else if (closure.CapturedVariables.Count == 0) + else if (closure.CapturedEnvironments.Count == 0) { if (_analysis.MethodsConvertedToDelegates.Contains(node.Symbol)) { @@ -1238,11 +1250,12 @@ private DebugId GetLambdaId(SyntaxNode syntax, ClosureKind closureKind, int clos } else { - lambdaScope = null; + // Lower directly onto the containing type containerAsFrame = null; + lambdaScope = null; + closureKind = ClosureKind.Static; // not exactly... but we've rewritten the receiver to be a by-ref parameter translatedLambdaContainer = _topLevelMethod.ContainingType; - closureKind = ClosureKind.ThisOnly; - closureOrdinal = LambdaDebugInfo.ThisOnlyClosureOrdinal; + closureOrdinal = LambdaDebugInfo.StaticClosureOrdinal; } // Move the body of the lambda to a freshly generated synthetic method on its frame. @@ -1338,7 +1351,7 @@ private BoundNode RewriteLambdaConversion(BoundLambda node) ClosureKind closureKind; NamedTypeSymbol translatedLambdaContainer; - ClosureEnvironment containerAsFrame; + SynthesizedClosureEnvironment containerAsFrame; BoundNode lambdaScope; DebugId topLevelMethodId; DebugId lambdaId; diff --git a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaFrame.cs b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/SynthesizedClosureEnvironment.cs similarity index 89% rename from src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaFrame.cs rename to src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/SynthesizedClosureEnvironment.cs index 5ef934ebdc892..67b4377b6d42d 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaFrame.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/SynthesizedClosureEnvironment.cs @@ -10,10 +10,9 @@ namespace Microsoft.CodeAnalysis.CSharp { /// - /// A class that represents the set of variables in a scope that have been - /// captured by nested functions within that scope. + /// The synthesized type added to a compilation to hold captured variables for closures. /// - internal sealed class ClosureEnvironment : SynthesizedContainer, ISynthesizedMethodBodyImplementationSymbol + internal sealed class SynthesizedClosureEnvironment : SynthesizedContainer, ISynthesizedMethodBodyImplementationSymbol { private readonly MethodSymbol _topLevelMethod; internal readonly SyntaxNode ScopeSyntaxOpt; @@ -25,13 +24,11 @@ internal sealed class ClosureEnvironment : SynthesizedContainer, ISynthesizedMet internal readonly MethodSymbol OriginalContainingMethodOpt; internal readonly FieldSymbol SingletonCache; internal readonly MethodSymbol StaticConstructor; - public readonly IEnumerable CapturedVariables; public override TypeKind TypeKind { get; } internal override MethodSymbol Constructor { get; } - internal ClosureEnvironment( - IEnumerable capturedVariables, + internal SynthesizedClosureEnvironment( MethodSymbol topLevelMethod, MethodSymbol containingMethod, bool isStruct, @@ -40,11 +37,10 @@ internal sealed class ClosureEnvironment : SynthesizedContainer, ISynthesizedMet DebugId closureId) : base(MakeName(scopeSyntaxOpt, methodId, closureId), containingMethod) { - CapturedVariables = capturedVariables; TypeKind = isStruct ? TypeKind.Struct : TypeKind.Class; _topLevelMethod = topLevelMethod; OriginalContainingMethodOpt = containingMethod; - Constructor = isStruct ? null : new LambdaFrameConstructor(this); + Constructor = isStruct ? null : new SynthesizedClosureEnvironmentConstructor(this); this.ClosureOrdinal = closureId.Ordinal; // static lambdas technically have the class scope so the scope syntax is null diff --git a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaFrameConstructor.cs b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/SynthesizedClosureEnvironmentConstructor.cs similarity index 72% rename from src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaFrameConstructor.cs rename to src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/SynthesizedClosureEnvironmentConstructor.cs index b64c048f75971..76dd6c2642099 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaFrameConstructor.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/SynthesizedClosureEnvironmentConstructor.cs @@ -4,9 +4,9 @@ namespace Microsoft.CodeAnalysis.CSharp { - internal sealed class LambdaFrameConstructor : SynthesizedInstanceConstructor, ISynthesizedMethodBodyImplementationSymbol + internal sealed class SynthesizedClosureEnvironmentConstructor : SynthesizedInstanceConstructor, ISynthesizedMethodBodyImplementationSymbol { - internal LambdaFrameConstructor(ClosureEnvironment frame) + internal SynthesizedClosureEnvironmentConstructor(SynthesizedClosureEnvironment frame) : base(frame) { } diff --git a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/SynthesizedLambdaMethod.cs b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/SynthesizedLambdaMethod.cs index 1eee93f9df38b..9bb6e7418db6d 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/SynthesizedLambdaMethod.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/SynthesizedLambdaMethod.cs @@ -19,7 +19,7 @@ internal sealed class SynthesizedLambdaMethod : SynthesizedMethodBaseSymbol, ISy internal SynthesizedLambdaMethod( NamedTypeSymbol containingType, - ImmutableArray structEnvironments, + ImmutableArray structEnvironments, ClosureKind closureKind, MethodSymbol topLevelMethod, DebugId topLevelMethodId, @@ -43,9 +43,9 @@ internal sealed class SynthesizedLambdaMethod : SynthesizedMethodBaseSymbol, ISy TypeMap typeMap; ImmutableArray typeParameters; ImmutableArray constructedFromTypeParameters; - ClosureEnvironment lambdaFrame; + SynthesizedClosureEnvironment lambdaFrame; - lambdaFrame = this.ContainingType as ClosureEnvironment; + lambdaFrame = this.ContainingType as SynthesizedClosureEnvironment; switch (closureKind) { case ClosureKind.Singleton: // all type parameters on method (except the top level method's) diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenClosureLambdaTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenClosureLambdaTests.cs index 434f863cea41a..3a544712af69d 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenClosureLambdaTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenClosureLambdaTests.cs @@ -11,6 +11,53 @@ namespace Microsoft.CodeAnalysis.CSharp.UnitTests.CodeGen { public class CodeGenClosureLambdaTests : CSharpTestBase { + [Fact] + public void EnvironmentChainContainsUnusedEnvironment() + { + CompileAndVerify(@" +using System; +class C +{ + void M(int x) + { + { + int y = 10; + Action f1 = () => Console.WriteLine(y); + + { + int z = 5; + Action f2 = () => Console.WriteLine(z + x); + f2(); + } + f1(); + } + } + public static void Main() => new C().M(3); +}", expectedOutput: @"8 +10"); + } + + [Fact] + public void CaptureThisAsFramePointer() + { + var comp = @" +using System; +using System.Collections.Generic; + +class C +{ + int _z = 0; + void M(IEnumerable xs) + { + foreach (var x in xs) + { + Func captureFunc = k => x + _z; + } + } +}"; + CompileAndVerify(comp); + } + [Fact] public void StaticClosure01() { @@ -3655,8 +3702,8 @@ .maxstack 2 IL_000d: ldarg.0 IL_000e: call ""bool Program.c1.T()"" IL_0013: brfalse.s IL_002e - IL_0015: ldarg.0 - IL_0016: ldftn ""bool Program.c1.b__1_0(int)"" + IL_0015: ldloc.0 + IL_0016: ldftn ""bool Program.c1.<>c__DisplayClass1_0.b__0(int)"" IL_001c: newobj ""System.Func..ctor(object, System.IntPtr)"" IL_0021: ldc.i4.s 42 IL_0023: callvirt ""bool System.Func.Invoke(int)"" diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenLocalFunctionTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenLocalFunctionTests.cs index 39818e6069820..95631d9d75a73 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenLocalFunctionTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenLocalFunctionTests.cs @@ -30,6 +30,98 @@ public static IMethodSymbol FindLocalFunction(this CompilationVerifier verifier, [CompilerTrait(CompilerFeature.LocalFunctions)] public class CodeGenLocalFunctionTests : CSharpTestBase { + [Fact] + public void EnvironmentChainContainsStructEnvironment() + { + CompileAndVerify(@" +using System; +class C +{ + void M(int x) + { + { + int y = 10; + void L() => Console.WriteLine(y); + + { + int z = 5; + Action f2 = () => Console.WriteLine(z + x); + f2(); + } + L(); + } + } + public static void Main() => new C().M(3); +}", expectedOutput: @"8 +10"); + } + + [Fact] + public void Repro20577() + { + var comp = CreateStandardCompilation(@" +using System.Linq; + +public class Program { + public static void Main(string[] args) { + object v; + + void AAA() { + object BBB(object v2) { + var a = v; + ((object[])v2).Select(i => BBB(i)); + return null; + } + } + } +}", references: new[] { LinqAssemblyRef }); + CompileAndVerify(comp); + } + + [Fact] + public void Repro19033() + { + CompileAndVerify(@" +using System; + +class Program +{ + void Q(int n = 0) + { + { + object mc; + + string B(object map) + { + Action a = _ => B(new object()); + return n.ToString(); + } + } + } +}"); + } + + [Fact] + public void Repro19033_2() + { + CompileAndVerify(@" +using System; +class C +{ + static void F(Action a) + { + object x = null; + { + object y = null; + void G(object z) + { + F(() => G(x)); + } + } + } +}"); + } + [Fact] [WorkItem(18814, "https://github.com/dotnet/roslyn/issues/18814")] [WorkItem(18918, "https://github.com/dotnet/roslyn/issues/18918")] @@ -78,7 +170,7 @@ void L5() 1"); verifier.VerifyIL("C.M()", @" { - // Code size 46 (0x2e) + // Code size 47 (0x2f) .maxstack 2 .locals init (C.<>c__DisplayClass2_0 V_0) //CS$<>8__locals0 IL_0000: ldloca.s V_0 @@ -90,24 +182,24 @@ .maxstack 2 IL_0010: ldarg.0 IL_0011: ldfld ""int C._x"" IL_0016: call ""void System.Console.WriteLine(int)"" - IL_001b: ldloca.s V_0 - IL_001d: call ""void C.g__L12_0(ref C.<>c__DisplayClass2_0)"" - IL_0022: ldarg.0 - IL_0023: ldfld ""int C._x"" - IL_0028: call ""void System.Console.WriteLine(int)"" - IL_002d: ret + IL_001b: ldarg.0 + IL_001c: ldloca.s V_0 + IL_001e: call ""void C.g__L12_0(ref C.<>c__DisplayClass2_0)"" + IL_0023: ldarg.0 + IL_0024: ldfld ""int C._x"" + IL_0029: call ""void System.Console.WriteLine(int)"" + IL_002e: ret }"); // L1 verifier.VerifyIL("C.g__L12_0(ref C.<>c__DisplayClass2_0)", @" { - // Code size 13 (0xd) + // Code size 8 (0x8) .maxstack 2 IL_0000: ldarg.0 - IL_0001: ldfld ""C C.<>c__DisplayClass2_0.<>4__this"" - IL_0006: ldarg.0 - IL_0007: call ""void C.g__L22_1(ref C.<>c__DisplayClass2_0)"" - IL_000c: ret + IL_0001: ldarg.1 + IL_0002: call ""void C.g__L22_1(ref C.<>c__DisplayClass2_0)"" + IL_0007: ret }"); // L2 verifier.VerifyIL("C.g__L22_1(ref C.<>c__DisplayClass2_0)", @" @@ -122,35 +214,34 @@ .maxstack 2 // Skip some... L5 verifier.VerifyIL("C.g__L52_4(ref C.<>c__DisplayClass2_0, ref C.<>c__DisplayClass2_1)", @" { - // Code size 9 (0x9) - .maxstack 2 + // Code size 10 (0xa) + .maxstack 3 IL_0000: ldarg.0 IL_0001: ldarg.1 - IL_0002: call ""int C.g__L62_5(ref C.<>c__DisplayClass2_0, ref C.<>c__DisplayClass2_1)"" - IL_0007: pop - IL_0008: ret + IL_0002: ldarg.2 + IL_0003: call ""int C.g__L62_5(ref C.<>c__DisplayClass2_0, ref C.<>c__DisplayClass2_1)"" + IL_0008: pop + IL_0009: ret }"); // L6 verifier.VerifyIL("C.g__L62_5(ref C.<>c__DisplayClass2_0, ref C.<>c__DisplayClass2_1)", @" { - // Code size 35 (0x23) + // Code size 25 (0x19) .maxstack 4 .locals init (int V_0) - IL_0000: ldarg.1 + IL_0000: ldarg.2 IL_0001: ldfld ""int C.<>c__DisplayClass2_1.var2"" - IL_0006: ldarg.1 - IL_0007: ldfld ""C C.<>c__DisplayClass2_1.<>4__this"" - IL_000c: ldarg.1 - IL_000d: ldfld ""C C.<>c__DisplayClass2_1.<>4__this"" - IL_0012: ldfld ""int C._x"" - IL_0017: stloc.0 - IL_0018: ldloc.0 - IL_0019: ldc.i4.1 - IL_001a: add - IL_001b: stfld ""int C._x"" - IL_0020: ldloc.0 - IL_0021: add - IL_0022: ret + IL_0006: ldarg.0 + IL_0007: ldarg.0 + IL_0008: ldfld ""int C._x"" + IL_000d: stloc.0 + IL_000e: ldloc.0 + IL_000f: ldc.i4.1 + IL_0010: add + IL_0011: stfld ""int C._x"" + IL_0016: ldloc.0 + IL_0017: add + IL_0018: ret }"); } diff --git a/src/Compilers/CSharp/Test/Emit/Emit/EditAndContinue/EditAndContinueClosureTests.cs b/src/Compilers/CSharp/Test/Emit/Emit/EditAndContinue/EditAndContinueClosureTests.cs index bea69b33f55b3..748e6c12d1ca5 100644 --- a/src/Compilers/CSharp/Test/Emit/Emit/EditAndContinue/EditAndContinueClosureTests.cs +++ b/src/Compilers/CSharp/Test/Emit/Emit/EditAndContinue/EditAndContinueClosureTests.cs @@ -1408,8 +1408,8 @@ public int F() var reader0 = md0.MetadataReader; CheckNames(reader0, reader0.GetTypeDefNames(), "", "C", "<>c__DisplayClass0_0", "<>c"); - CheckNames(reader0, reader0.GetMethodDefNames(), "F", ".ctor", "b__0_1", ".ctor", "b__2", ".cctor", ".ctor", "b__0_0"); - CheckNames(reader0, reader0.GetFieldDefNames(), "a", "<>9", "<>9__0_0"); + CheckNames(reader0, reader0.GetMethodDefNames(), "F", ".ctor", ".ctor", "b__1", "b__2", ".cctor", ".ctor", "b__0_0"); + CheckNames(reader0, reader0.GetFieldDefNames(), "<>4__this", "a", "<>9", "<>9__0_0"); var generation0 = EmitBaseline.CreateInitialBaseline(md0, v0.CreateSymReader().GetEncMethodDebugInfo); var diff1 = compilation1.EmitDifference( @@ -1420,13 +1420,13 @@ public int F() var reader1 = diff1.GetMetadata().Reader; CheckNames(new[] { reader0, reader1 }, reader1.GetTypeDefNames(), "<>c__DisplayClass0#1_0#1"); - CheckNames(new[] { reader0, reader1 }, reader1.GetMethodDefNames(), ".ctor", "F", "b__0#1_1#1", ".ctor", "b__2#1", "b__0#1_0#1"); - CheckNames(new[] { reader0, reader1 }, reader1.GetFieldDefNames(), "a", "<>9__0#1_0#1"); + CheckNames(new[] { reader0, reader1 }, reader1.GetMethodDefNames(), ".ctor", "F", ".ctor", "b__1#1", "b__2#1", "b__0#1_0#1"); + CheckNames(new[] { reader0, reader1 }, reader1.GetFieldDefNames(), "<>4__this", "a", "<>9__0#1_0#1"); diff1.VerifySynthesizedMembers( - "C: {b__0#1_1#1, <>c__DisplayClass0#1_0#1, <>c}", - "C.<>c: {<>9__0#1_0#1, b__0#1_0#1}", - "C.<>c__DisplayClass0#1_0#1: {a, b__2#1}"); + "C: {<>c__DisplayClass0#1_0#1, <>c}", + "C.<>c__DisplayClass0#1_0#1: {<>4__this, a, b__1#1, b__2#1}", + "C.<>c: {<>9__0#1_0#1, b__0#1_0#1}"); var diff2 = compilation2.EmitDifference( diff1.NextGeneration, @@ -1436,8 +1436,8 @@ public int F() var reader2 = diff2.GetMetadata().Reader; CheckNames(new[] { reader0, reader1, reader2 }, reader2.GetTypeDefNames(), "<>c__DisplayClass1#2_0#2"); - CheckNames(new[] { reader0, reader1, reader2 }, reader2.GetMethodDefNames(), ".ctor", "F", "b__1#2_1#2", ".ctor", "b__2#2", "b__1#2_0#2"); - CheckNames(new[] { reader0, reader1, reader2 }, reader2.GetFieldDefNames(), "a", "<>9__1#2_0#2"); + CheckNames(new[] { reader0, reader1, reader2 }, reader2.GetMethodDefNames(), ".ctor", "F", ".ctor", "b__1#2", "b__2#2", "b__1#2_0#2"); + CheckNames(new[] { reader0, reader1, reader2 }, reader2.GetFieldDefNames(), "<>4__this", "a", "<>9__1#2_0#2"); } [Fact] @@ -1525,8 +1525,8 @@ public int F() var reader0 = md0.MetadataReader; CheckNames(reader0, reader0.GetTypeDefNames(), "", "C", "<>c__DisplayClass0_0`1", "<>c__0`1"); - CheckNames(reader0, reader0.GetMethodDefNames(), "F", ".ctor", "b__0_1", ".ctor", "b__2", ".cctor", ".ctor", "b__0_0"); - CheckNames(reader0, reader0.GetFieldDefNames(), "a", "<>9", "<>9__0_0"); + CheckNames(reader0, reader0.GetMethodDefNames(), "F", ".ctor", ".ctor", "b__1", "b__2", ".cctor", ".ctor", "b__0_0"); + CheckNames(reader0, reader0.GetFieldDefNames(), "<>4__this", "a", "<>9", "<>9__0_0"); var generation0 = EmitBaseline.CreateInitialBaseline(md0, v0.CreateSymReader().GetEncMethodDebugInfo); var diff1 = compilation1.EmitDifference( @@ -1537,13 +1537,13 @@ public int F() var reader1 = diff1.GetMetadata().Reader; CheckNames(new[] { reader0, reader1 }, reader1.GetTypeDefNames(), "<>c__DisplayClass0#1_0#1`1", "<>c__0#1`1"); - CheckNames(new[] { reader0, reader1 }, reader1.GetMethodDefNames(), "F", "b__0#1_1#1", ".ctor", "b__2#1", ".cctor", ".ctor", "b__0#1_0#1"); - CheckNames(new[] { reader0, reader1 }, reader1.GetFieldDefNames(), "a", "<>9", "<>9__0#1_0#1"); + CheckNames(new[] { reader0, reader1 }, reader1.GetMethodDefNames(), "F", ".ctor", "b__1#1", "b__2#1", ".cctor", ".ctor", "b__0#1_0#1"); + CheckNames(new[] { reader0, reader1 }, reader1.GetFieldDefNames(), "<>4__this", "a", "<>9", "<>9__0#1_0#1"); diff1.VerifySynthesizedMembers( - "C: {b__0#1_1#1, <>c__DisplayClass0#1_0#1, <>c__0#1}", "C.<>c__0#1: {<>9__0#1_0#1, b__0#1_0#1}", - "C.<>c__DisplayClass0#1_0#1: {a, b__2#1}"); + "C: {<>c__DisplayClass0#1_0#1, <>c__0#1}", + "C.<>c__DisplayClass0#1_0#1: {<>4__this, a, b__1#1, b__2#1}"); var diff2 = compilation2.EmitDifference( diff1.NextGeneration, @@ -1553,8 +1553,8 @@ public int F() var reader2 = diff2.GetMetadata().Reader; CheckNames(new[] { reader0, reader1, reader2 }, reader2.GetTypeDefNames(), "<>c__DisplayClass1#2_0#2`1", "<>c__1#2`1"); - CheckNames(new[] { reader0, reader1, reader2 }, reader2.GetMethodDefNames(), "F", "b__1#2_1#2", ".ctor", "b__2#2", ".cctor", ".ctor", "b__1#2_0#2"); - CheckNames(new[] { reader0, reader1, reader2 }, reader2.GetFieldDefNames(), "a", "<>9", "<>9__1#2_0#2"); + CheckNames(new[] { reader0, reader1, reader2 }, reader2.GetMethodDefNames(), "F", ".ctor", "b__1#2", "b__2#2", ".cctor", ".ctor", "b__1#2_0#2"); + CheckNames(new[] { reader0, reader1, reader2 }, reader2.GetFieldDefNames(), "<>4__this", "a", "<>9", "<>9__1#2_0#2"); } [Fact] @@ -1669,9 +1669,9 @@ public int F() new SemanticEdit(SemanticEditKind.Update, main0, main1, preserveLocalVariables: true))); diff1.VerifySynthesizedMembers( - "C: {b__1#1_1#1, <>c__DisplayClass1#1_0#1, <>c}", "C.<>c: {<>9__1#1_0#1, b__1#1_0#1}", - "C.<>c__DisplayClass1#1_0#1: {a, b__2#1}"); + "C.<>c__DisplayClass1#1_0#1: {<>4__this, a, b__1#1, b__2#1}", + "C: {<>c__DisplayClass1#1_0#1, <>c}"); var diff2 = compilation2.EmitDifference( diff1.NextGeneration, @@ -1680,10 +1680,10 @@ public int F() new SemanticEdit(SemanticEditKind.Update, main1, main2, preserveLocalVariables: true))); diff2.VerifySynthesizedMembers( - "C: {b__1#2_1#2, <>c__DisplayClass1#2_0#2, <>c, b__1#1_1#1, <>c__DisplayClass1#1_0#1}", + "C.<>c__DisplayClass1#2_0#2: {<>4__this, a, b__1#2, b__2#2}", + "C: {<>c__DisplayClass1#2_0#2, <>c, <>c__DisplayClass1#1_0#1}", "C.<>c: {<>9__1#2_0#2, b__1#2_0#2, <>9__1#1_0#1, b__1#1_0#1}", - "C.<>c__DisplayClass1#1_0#1: {a, b__2#1}", - "C.<>c__DisplayClass1#2_0#2: {a, b__2#2}"); + "C.<>c__DisplayClass1#1_0#1: {<>4__this, a, b__1#1, b__2#1}"); var diff3 = compilation3.EmitDifference( diff2.NextGeneration, @@ -1691,10 +1691,10 @@ public int F() new SemanticEdit(SemanticEditKind.Update, main2, main3, preserveLocalVariables: true))); diff3.VerifySynthesizedMembers( - "C: {b__1#2_1#2, <>c__DisplayClass1#2_0#2, <>c, b__1#1_1#1, <>c__DisplayClass1#1_0#1}", + "C.<>c__DisplayClass1#1_0#1: {<>4__this, a, b__1#1, b__2#1}", "C.<>c: {<>9__1#2_0#2, b__1#2_0#2, <>9__1#1_0#1, b__1#1_0#1}", - "C.<>c__DisplayClass1#1_0#1: {a, b__2#1}", - "C.<>c__DisplayClass1#2_0#2: {a, b__2#2}"); + "C.<>c__DisplayClass1#2_0#2: {<>4__this, a, b__1#2, b__2#2}", + "C: {<>c__DisplayClass1#2_0#2, <>c, <>c__DisplayClass1#1_0#1}"); } [Fact] diff --git a/src/Compilers/CSharp/Test/Emit/PDB/PDBLambdaTests.cs b/src/Compilers/CSharp/Test/Emit/PDB/PDBLambdaTests.cs index a63ab728ce7e1..f233175099fe0 100644 --- a/src/Compilers/CSharp/Test/Emit/PDB/PDBLambdaTests.cs +++ b/src/Compilers/CSharp/Test/Emit/PDB/PDBLambdaTests.cs @@ -537,50 +537,50 @@ public C(int a, int b) : base(() => a) - + - - - - + + + + - + - + - + - + - + - + diff --git a/src/Compilers/Core/Portable/InternalUtilities/SetWithInsertionOrder.cs b/src/Compilers/Core/Portable/InternalUtilities/SetWithInsertionOrder.cs index 97145d508a68e..e76d4519db1db 100644 --- a/src/Compilers/Core/Portable/InternalUtilities/SetWithInsertionOrder.cs +++ b/src/Compilers/Core/Portable/InternalUtilities/SetWithInsertionOrder.cs @@ -37,6 +37,36 @@ public bool Add(T value) return true; } + public bool Insert(int index, T value) + { + if (_set == null) + { + if (index > 0) + { + throw new IndexOutOfRangeException(); + } + Add(value); + } + else + { + if (!_set.Add(value)) + { + return false; + } + + try + { + _elements.Insert(index, value); + } + catch + { + _set.Remove(value); + throw; + } + } + return true; + } + public bool Remove(T value) { if (!_set.Remove(value)) @@ -57,5 +87,7 @@ public IEnumerator GetEnumerator() IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); public ImmutableArray AsImmutable() => _elements.ToImmutableArrayOrEmpty(); + + public T this[int i] => _elements[i]; } } diff --git a/src/ExpressionEvaluator/CSharp/Test/ExpressionCompiler/LocalFunctionTests.cs b/src/ExpressionEvaluator/CSharp/Test/ExpressionCompiler/LocalFunctionTests.cs index c5789b3f98909..d87cbad50fb8d 100644 --- a/src/ExpressionEvaluator/CSharp/Test/ExpressionCompiler/LocalFunctionTests.cs +++ b/src/ExpressionEvaluator/CSharp/Test/ExpressionCompiler/LocalFunctionTests.cs @@ -123,7 +123,7 @@ int G() // Code size 7 (0x7) .maxstack 1 .locals init (int V_0) - IL_0000: ldarg.0 + IL_0000: ldarg.1 IL_0001: ldfld ""C C.<>c__DisplayClass1_0.<>4__this"" IL_0006: ret }"); @@ -132,7 +132,7 @@ .locals init (int V_0) // Code size 7 (0x7) .maxstack 1 .locals init (int V_0) - IL_0000: ldarg.0 + IL_0000: ldarg.1 IL_0001: ldfld ""int C.<>c__DisplayClass1_0.y"" IL_0006: ret }"); @@ -146,7 +146,7 @@ .locals init (int V_0) // Code size 13 (0xd) .maxstack 2 .locals init (int V_0) - IL_0000: ldarg.0 + IL_0000: ldarg.1 IL_0001: ldfld ""C C.<>c__DisplayClass1_0.<>4__this"" IL_0006: ldc.i4.1 IL_0007: callvirt ""void C.F(int)""