diff --git a/Src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AsyncMethodToStateMachineRewriter.cs b/Src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AsyncMethodToStateMachineRewriter.cs index 8b3ee9880ce93ea6329d6c7d6843878f8f728721..4cd0aaeb4470ab707ec5623dadd0c2319d4aacee 100644 --- a/Src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AsyncMethodToStateMachineRewriter.cs +++ b/Src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AsyncMethodToStateMachineRewriter.cs @@ -58,7 +58,7 @@ internal sealed class AsyncMethodToStateMachineRewriter : MethodToStateMachineRe SyntheticBoundNodeFactory F, FieldSymbol state, FieldSymbol builder, - HashSet variablesCaptured, + IReadOnlySet variablesCaptured, IReadOnlyDictionary nonReusableLocalProxies, DiagnosticBag diagnostics) : base(F, method, state, variablesCaptured, nonReusableLocalProxies, diagnostics, useFinalizerBookkeeping: false) diff --git a/Src/Compilers/CSharp/Portable/Lowering/IteratorRewriter/IteratorMethodToStateMachineRewriter.cs b/Src/Compilers/CSharp/Portable/Lowering/IteratorRewriter/IteratorMethodToStateMachineRewriter.cs index 521fa6357ec926c2b43220aaab7b412cddfc560b..dcf876fae4279b8d99ec0ddaa4e7c9fff0fcfe7a 100644 --- a/Src/Compilers/CSharp/Portable/Lowering/IteratorRewriter/IteratorMethodToStateMachineRewriter.cs +++ b/Src/Compilers/CSharp/Portable/Lowering/IteratorRewriter/IteratorMethodToStateMachineRewriter.cs @@ -6,6 +6,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.Linq; +using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.CSharp { @@ -52,7 +53,7 @@ internal sealed partial class IteratorMethodToStateMachineRewriter : MethodToSta MethodSymbol originalMethod, FieldSymbol state, FieldSymbol current, - HashSet variablesCaptured, + IReadOnlySet variablesCaptured, IReadOnlyDictionary initialProxies, DiagnosticBag diagnostics) : base(F, originalMethod, state, variablesCaptured, initialProxies, diagnostics, useFinalizerBookkeeping: false) diff --git a/Src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.Analysis.cs b/Src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.Analysis.cs index 5a31a55d19142cc06557c634b06455d32991905e..ad6e6f6824eac928a4c798da2abe4ed4b38be304 100644 --- a/Src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.Analysis.cs +++ b/Src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.Analysis.cs @@ -21,7 +21,7 @@ internal sealed class Analysis : BoundTreeWalker private readonly MethodSymbol topLevelMethod; private MethodSymbol currentParent; - private BoundNode currentBlock; + private BoundNode currentScope; // Some syntactic forms have an "implicit" receiver. When we encounter them, we set this to the // syntax. That way, in case we need to report an error about the receiver, we can use this @@ -39,36 +39,26 @@ internal sealed class Analysis : BoundTreeWalker public bool SeenLambda { get; private set; } /// - /// For each statement that defines variables, identifies the nearest enclosing statement that defines variables. + /// For each scope that defines variables, identifies the nearest enclosing scope that defines variables. /// - public readonly Dictionary blockParent = new Dictionary(); + public readonly Dictionary scopeParent = new Dictionary(); /// - /// For each captured variable, identifies the statement in which it will be moved to a frame class. This is - /// normally the block where the variable is introduced, but method parameters are moved + /// For each captured variable, identifies the scope in which it will be moved to a frame class. This is + /// normally the node where the variable is introduced, but method parameters are moved /// to a frame class within the body of the method. /// - public readonly Dictionary variableBlock = new Dictionary(); - - /// - /// The set of captured variables seen in the method body. - /// - public readonly HashSet variablesCaptured = new HashSet(); - + public readonly Dictionary variableScope = new Dictionary(); + /// /// The syntax nodes associated with each captured variable. /// - public readonly MultiDictionary capturedSyntax = new MultiDictionary(); - - /// - /// The set of variables that were declared anywhere inside an expression lambda. - /// - public readonly HashSet declaredInsideExpressionLambda = new HashSet(); - + public readonly MultiDictionary capturedVariables = new MultiDictionary(); + /// /// For each lambda in the code, the set of variables that it captures. /// - public readonly MultiDictionary captures = new MultiDictionary(); + public readonly MultiDictionary capturedVariablesByLambda = new MultiDictionary(); /// /// Blocks that are positioned between a block declaring some lifted variables @@ -97,6 +87,8 @@ internal sealed class Analysis : BoundTreeWalker private Analysis(MethodSymbol method) { + Debug.Assert((object)method != null); + this.currentParent = this.topLevelMethod = method; } @@ -109,16 +101,15 @@ public static Analysis Analyze(BoundNode node, MethodSymbol method) private void Analyze(BoundNode node) { - currentBlock = FindNodeToAnalyze(node); + currentScope = FindNodeToAnalyze(node); + + Debug.Assert(!inExpressionLambda); + Debug.Assert((object)topLevelMethod != null); - if ((object)topLevelMethod != null) + foreach (ParameterSymbol parameter in topLevelMethod.Parameters) { - foreach (ParameterSymbol parameter in topLevelMethod.Parameters) - { - // parameters are counted as if they are inside the block - variableBlock[parameter] = currentBlock; - if (inExpressionLambda) declaredInsideExpressionLambda.Add(parameter); - } + // parameters are counted as if they are inside the block + variableScope[parameter] = currentScope; } Visit(node); @@ -161,7 +152,7 @@ internal void ComputeLambdaScopesAndFrameCaptures() lambdaScopes = new Dictionary(ReferenceEqualityComparer.Instance); needsParentFrame = new HashSet(); - foreach (var lambda in captures.Keys) + foreach (var lambda in capturedVariablesByLambda.Keys) { // get innermost and outermost scopes from which a lambda captures @@ -171,12 +162,12 @@ internal void ComputeLambdaScopesAndFrameCaptures() int outermostScopeDepth = int.MaxValue; BoundNode outermostScope = null; - foreach (var v in captures[lambda]) + foreach (var variables in capturedVariablesByLambda[lambda]) { BoundNode curBlock = null; int curBlockDepth; - if (!variableBlock.TryGetValue(v, out curBlock)) + if (!variableScope.TryGetValue(variables, out curBlock)) { // this is something that is not defined in a block, like "Me" // Since it is defined outside of the method, the depth is -1 @@ -217,7 +208,7 @@ internal void ComputeLambdaScopesAndFrameCaptures() while (innermostScope != outermostScope) { needsParentFrame.Add(innermostScope); - blockParent.TryGetValue(innermostScope, out innermostScope); + scopeParent.TryGetValue(innermostScope, out innermostScope); } } } @@ -234,7 +225,7 @@ private int BlockDepth(BoundNode node) while (node != null) { result = result + 1; - if (!blockParent.TryGetValue(node, out node)) + if (!scopeParent.TryGetValue(node, out node)) { break; } @@ -260,18 +251,20 @@ public override BoundNode VisitCatchBlock(BoundCatchBlock node) private BoundNode PushBlock(BoundNode node, ImmutableArray locals) { - var previousBlock = currentBlock; - currentBlock = node; - if (currentBlock != previousBlock) // not top-level node of the method + // blocks are not allowed in expression lambda + Debug.Assert(!inExpressionLambda); + + var previousBlock = currentScope; + currentScope = node; + if (currentScope != previousBlock) // not top-level node of the method { // (Except for the top-level block) record the parent-child block structure - blockParent[currentBlock] = previousBlock; + scopeParent[currentScope] = previousBlock; } foreach (var local in locals) { - variableBlock[local] = currentBlock; - if (inExpressionLambda) declaredInsideExpressionLambda.Add(local); + variableScope[local] = currentScope; } return previousBlock; @@ -279,7 +272,7 @@ private BoundNode PushBlock(BoundNode node, ImmutableArray locals) private void PopBlock(BoundNode previousBlock) { - currentBlock = previousBlock; + currentScope = previousBlock; } public override BoundNode VisitSwitchStatement(BoundSwitchStatement node) @@ -336,30 +329,31 @@ public override BoundNode VisitLambda(BoundLambda node) Debug.Assert((object)node.Symbol != null); SeenLambda = true; var oldParent = currentParent; - var oldBlock = currentBlock; + var oldBlock = currentScope; currentParent = node.Symbol; - currentBlock = node.Body; - blockParent[currentBlock] = oldBlock; + currentScope = node.Body; + scopeParent[currentScope] = oldBlock; var wasInExpressionLambda = inExpressionLambda; inExpressionLambda = inExpressionLambda || node.Type.IsExpressionTree(); - // for the purpose of constructing frames parameters are scoped as if they are inside the lambda block - foreach (var parameter in node.Symbol.Parameters) + if (!inExpressionLambda) { - variableBlock[parameter] = currentBlock; - if (inExpressionLambda) declaredInsideExpressionLambda.Add(parameter); - } + // for the purpose of constructing frames parameters are scoped as if they are inside the lambda block + foreach (var parameter in node.Symbol.Parameters) + { + variableScope[parameter] = currentScope; + } - foreach (var local in node.Body.Locals) - { - variableBlock[local] = currentBlock; - if (inExpressionLambda) declaredInsideExpressionLambda.Add(local); + foreach (var local in node.Body.Locals) + { + variableScope[local] = currentScope; + } } var result = base.VisitBlock(node.Body); inExpressionLambda = wasInExpressionLambda; currentParent = oldParent; - currentBlock = oldBlock; + currentScope = oldBlock; return result; } @@ -375,13 +369,12 @@ private void ReferenceVariable(CSharpSyntaxNode syntax, Symbol symbol) LambdaSymbol lambda = currentParent as LambdaSymbol; if ((object)lambda != null && symbol.ContainingSymbol != lambda) { - variablesCaptured.Add(symbol); - capturedSyntax.Add(symbol, syntax); + capturedVariables.Add(symbol, syntax); // mark the variable as captured in each enclosing lambda up to the variable's point of declaration. for (; (object)lambda != null && symbol.ContainingSymbol != lambda; lambda = lambda.ContainingSymbol as LambdaSymbol) { - captures.Add(lambda, symbol); + capturedVariablesByLambda.Add(lambda, symbol); } } } diff --git a/Src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.cs b/Src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.cs index 302b7c4d1af93dca2dbf614f7ef812750dabc41f..fe3332834a96766c9e04eab41f1b0092f6da1877 100644 --- a/Src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.cs +++ b/Src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.cs @@ -52,11 +52,14 @@ namespace Microsoft.CodeAnalysis.CSharp /// the returned bound node. For example, the caller will typically perform iterator method and /// asynchronous method transformations, and emit IL instructions into an assembly. /// - partial class LambdaRewriter : MethodToClassRewriter + sealed partial class LambdaRewriter : MethodToClassRewriter { private readonly Analysis analysis; private readonly MethodSymbol topLevelMethod; + // A mapping from every lambda parameter to its corresponding method's parameter. + private readonly Dictionary parameterMap = new Dictionary(); + // for each block with lifted (captured) variables, the corresponding frame type private readonly Dictionary frames = new Dictionary(); @@ -114,7 +117,7 @@ partial class LambdaRewriter : MethodToClassRewriter TypeCompilationState compilationState, DiagnosticBag diagnostics, bool assignLocals) - : base(compilationState, analysis.variablesCaptured, diagnostics) + : base(compilationState, diagnostics) { Debug.Assert(analysis != null); Debug.Assert(thisType != null); @@ -134,6 +137,12 @@ partial class LambdaRewriter : MethodToClassRewriter this.synthesizedFieldNameIdDispenser = 1; } + protected override bool NeedsProxy(Symbol localOrParameter) + { + Debug.Assert(localOrParameter is LocalSymbol || localOrParameter is ParameterSymbol); + return analysis.capturedVariables.ContainsKey(localOrParameter); + } + /// /// Rewrite the given node to eliminate lambda expressions. Also returned are the method symbols and their /// bound bodies for the extracted lambda bodies. These would typically be emitted by the caller such as @@ -214,7 +223,6 @@ protected override NamedTypeSymbol ContainingType /// Check that the top-level node is well-defined, in the sense that all /// locals that are used are defined in some enclosing scope. /// - /// static partial void CheckLocalsDefined(BoundNode node); /// @@ -224,11 +232,10 @@ private void MakeFrames() { NamedTypeSymbol containingType = this.ContainingType; - foreach (Symbol captured in analysis.variablesCaptured) + foreach (Symbol captured in analysis.capturedVariables.Keys) { BoundNode node; - if (!analysis.variableBlock.TryGetValue(captured, out node) || - analysis.declaredInsideExpressionLambda.Contains(captured)) + if (!analysis.variableScope.TryGetValue(captured, out node)) { continue; } @@ -254,7 +261,7 @@ private void MakeFrames() if (hoistedField.Type.IsRestrictedType()) { - foreach (CSharpSyntaxNode syntax in analysis.capturedSyntax[captured]) + foreach (CSharpSyntaxNode syntax in analysis.capturedVariables[captured]) { // CS4013: Instance of type '{0}' cannot be used inside an anonymous function, query expression, iterator block or async method this.Diagnostics.Add(ErrorCode.ERR_SpecialByRefInLambda, syntax.Location, hoistedField.Type); @@ -387,12 +394,10 @@ private T IntroduceFrame(BoundNode node, LambdaFrame frame, Func prologue, ArrayBuilder newLocals) { - AddLocals(node.Locals, newLocals); + RewriteLocals(node.Locals, newLocals); foreach (var expr in node.SideEffects) { @@ -559,9 +575,9 @@ public override BoundNode VisitBlock(BoundBlock node) } } - protected BoundBlock RewriteBlock(BoundBlock node, ArrayBuilder prologue, ArrayBuilder newLocals) + private BoundBlock RewriteBlock(BoundBlock node, ArrayBuilder prologue, ArrayBuilder newLocals) { - AddLocals(node.Locals, newLocals); + RewriteLocals(node.Locals, newLocals); var newStatements = ArrayBuilder.GetInstance(); @@ -604,7 +620,7 @@ public override BoundNode VisitCatchBlock(BoundCatchBlock node) private BoundNode RewriteCatch(BoundCatchBlock node, ArrayBuilder prologue, ArrayBuilder newLocals) { - AddLocals(node.Locals, newLocals); + RewriteLocals(node.Locals, newLocals); var rewrittenCatchLocals = newLocals.ToImmutableAndFree(); // If exception variable got lifted, IntroduceFrame will give us frame init prologue. @@ -784,21 +800,22 @@ private BoundNode RewriteLambdaConversion(BoundLambda node) } // Move the body of the lambda to a freshly generated synthetic method on its frame. - bool lambdaIsStatic = analysis.captures[node.Symbol].IsEmpty(); + bool lambdaIsStatic = analysis.capturedVariablesByLambda[node.Symbol].IsEmpty(); var synthesizedMethod = new SynthesizedLambdaMethod(translatedLambdaContainer, topLevelMethod, node, lambdaIsStatic, CompilationState); if (CompilationState.Emitting) { CompilationState.ModuleBuilderOpt.AddSynthesizedDefinition(translatedLambdaContainer, synthesizedMethod); } - foreach(var parameter in node.Symbol.Parameters) + foreach (var parameter in node.Symbol.Parameters) { var ordinal = parameter.Ordinal; if (lambdaIsStatic) { // adjust for a dummy "this" parameter at the beginning of synthesized method signature - ordinal += 1; + ordinal++; } + parameterMap.Add(parameter, synthesizedMethod.Parameters[ordinal]); } @@ -887,7 +904,7 @@ private BoundNode RewriteLambdaConversion(BoundLambda node) // NOTE: We require "lambdaScope != null". // We do not want to introduce a field into an actual user's class (not a synthetic frame). var shouldCacheInLoop = lambdaScope != null && - lambdaScope != analysis.blockParent[node.Body] && + lambdaScope != analysis.scopeParent[node.Body] && InLoopOrLambda(node.Syntax, lambdaScope.Syntax); if (shouldCacheForStaticMethod || shouldCacheInLoop) diff --git a/Src/Compilers/CSharp/Portable/Lowering/MethodToClassRewriter.cs b/Src/Compilers/CSharp/Portable/Lowering/MethodToClassRewriter.cs index 9d19a081801b82cca297cc248377763a0b1b211f..d50e0a4ad0c810f5f8e5a36579065f90dac7f9a0 100644 --- a/Src/Compilers/CSharp/Portable/Lowering/MethodToClassRewriter.cs +++ b/Src/Compilers/CSharp/Portable/Lowering/MethodToClassRewriter.cs @@ -13,10 +13,6 @@ namespace Microsoft.CodeAnalysis.CSharp.Symbols /// internal abstract partial class MethodToClassRewriter : BoundTreeRewriter { - // A mapping from every lambda parameter to its corresponding method's parameter. This map remains - // empty if we are performing something other than a lambda transformation (e.g. async or iterator). - protected readonly Dictionary parameterMap = new Dictionary(); - // For each captured variable, information about its replacement. May be populated lazily (that is, not all // upfront) by subclasses. Specifically, the async rewriter produces captured symbols for temps, including // ref locals, lazily. @@ -27,11 +23,6 @@ internal abstract partial class MethodToClassRewriter : BoundTreeRewriter // though its containing method is not correct because the code is moved into another method) protected readonly Dictionary localMap = new Dictionary(); - /// - /// The set of captured variables seen in the method body. - /// - private readonly HashSet variablesCaptured = new HashSet(); - // A mapping for types in the original method to types in its replacement. This is mainly necessary // when the original method was generic, as type parameters in the original method are mapping into // type parameters of the resulting class. @@ -50,23 +41,18 @@ internal abstract partial class MethodToClassRewriter : BoundTreeRewriter protected readonly DiagnosticBag Diagnostics; - protected MethodToClassRewriter(TypeCompilationState compilationState, HashSet variablesCaptured, DiagnosticBag diagnostics) + protected MethodToClassRewriter(TypeCompilationState compilationState, DiagnosticBag diagnostics) { Debug.Assert(compilationState != null); Debug.Assert(diagnostics != null); this.CompilationState = compilationState; this.Diagnostics = diagnostics; - this.variablesCaptured = variablesCaptured; } - protected bool IsCaptured(Symbol localOrParameter) - { - Debug.Assert(localOrParameter is LocalSymbol || localOrParameter is ParameterSymbol); - return variablesCaptured.Contains(localOrParameter); - } + protected abstract bool NeedsProxy(Symbol localOrParameter); - protected void AddLocals(ImmutableArray locals, ArrayBuilder newLocals) + protected void RewriteLocals(ImmutableArray locals, ArrayBuilder newLocals) { foreach (var local in locals) { @@ -80,7 +66,7 @@ protected void AddLocals(ImmutableArray locals, ArrayBuilder RewriteLocalList(ImmutableArray locals) + private ImmutableArray RewriteLocals(ImmutableArray locals) { if (locals.IsEmpty) return locals; var newLocals = ArrayBuilder.GetInstance(); - AddLocals(locals, newLocals); + RewriteLocals(locals, newLocals); return newLocals.ToImmutableAndFree(); } @@ -120,7 +106,7 @@ public override BoundNode VisitCatchBlock(BoundCatchBlock node) { // Yield/await aren't supported in catch block atm, but we need to rewrite the type // of the variables owned by the catch block. Note that one of these variables might be a closure frame reference. - var newLocals = RewriteLocalList(node.Locals); + var newLocals = RewriteLocals(node.Locals); return node.Update( newLocals, @@ -135,14 +121,14 @@ public override BoundNode VisitCatchBlock(BoundCatchBlock node) public override BoundNode VisitBlock(BoundBlock node) { - var newLocals = RewriteLocalList(node.Locals); + var newLocals = RewriteLocals(node.Locals); var newStatements = VisitList(node.Statements); return node.Update(newLocals, newStatements); } public override BoundNode VisitSequence(BoundSequence node) { - var newLocals = RewriteLocalList(node.Locals); + var newLocals = RewriteLocals(node.Locals); var newSideEffects = VisitList(node.SideEffects); var newValue = (BoundExpression)this.Visit(node.Value); var newType = this.VisitType(node.Type); @@ -151,8 +137,8 @@ public override BoundNode VisitSequence(BoundSequence node) public override BoundNode VisitSwitchStatement(BoundSwitchStatement node) { - var newOuterLocals = RewriteLocalList(node.OuterLocals); - var newInnerLocals = RewriteLocalList(node.InnerLocals); + var newOuterLocals = RewriteLocals(node.OuterLocals); + var newInnerLocals = RewriteLocals(node.InnerLocals); BoundExpression boundExpression = (BoundExpression)this.Visit(node.BoundExpression); ImmutableArray switchSections = (ImmutableArray)this.VisitList(node.SwitchSections); return node.Update(newOuterLocals, boundExpression, node.ConstantTargetOpt, newInnerLocals, switchSections, node.BreakLabel, node.StringEquality); @@ -160,8 +146,8 @@ public override BoundNode VisitSwitchStatement(BoundSwitchStatement node) public override BoundNode VisitForStatement(BoundForStatement node) { - var newOuterLocals = RewriteLocalList(node.OuterLocals); - var newInnerLocals = RewriteLocalList(node.InnerLocals); + var newOuterLocals = RewriteLocals(node.OuterLocals); + var newInnerLocals = RewriteLocals(node.InnerLocals); BoundStatement initializer = (BoundStatement)this.Visit(node.Initializer); BoundExpression condition = (BoundExpression)this.Visit(node.Condition); BoundStatement increment = (BoundStatement)this.Visit(node.Increment); @@ -171,7 +157,7 @@ public override BoundNode VisitForStatement(BoundForStatement node) public override BoundNode VisitUsingStatement(BoundUsingStatement node) { - var newLocals = RewriteLocalList(node.Locals); + var newLocals = RewriteLocals(node.Locals); BoundMultipleLocalDeclarations declarationsOpt = (BoundMultipleLocalDeclarations)this.Visit(node.DeclarationsOpt); BoundExpression expressionOpt = (BoundExpression)this.Visit(node.ExpressionOpt); BoundStatement body = (BoundStatement)this.Visit(node.Body); @@ -289,32 +275,52 @@ private MethodSymbol GetOrCreateBaseFunctionWrapper(MethodSymbol methodBeingWrap return wrapper; } - public override BoundNode VisitParameter(BoundParameter node) + private bool TryReplaceWithProxy(Symbol parameterOrLocal, CSharpSyntaxNode syntax, out BoundNode replacement) { CapturedSymbolReplacement proxy; - if (proxies.TryGetValue(node.ParameterSymbol, out proxy)) + if (proxies.TryGetValue(parameterOrLocal, out proxy)) { - return proxy.Replacement(node.Syntax, frameType => FramePointer(node.Syntax, frameType)); + replacement = proxy.Replacement(syntax, frameType => FramePointer(syntax, frameType)); + return true; } - ParameterSymbol replacementParameter; - if (this.parameterMap.TryGetValue(node.ParameterSymbol, out replacementParameter)) + replacement = null; + return false; + } + + public sealed override BoundNode VisitParameter(BoundParameter node) + { + BoundNode replacement; + if (TryReplaceWithProxy(node.ParameterSymbol, node.Syntax, out replacement)) { - return new BoundParameter(node.Syntax, replacementParameter, replacementParameter.Type, node.HasErrors); + return replacement; } + // Non-captured and expression tree lambda parameters don't have a proxy. + return VisitUnhoistedParameter(node); + } + + protected virtual BoundNode VisitUnhoistedParameter(BoundParameter node) + { return base.VisitParameter(node); } - public override BoundNode VisitLocal(BoundLocal node) + public sealed override BoundNode VisitLocal(BoundLocal node) { - CapturedSymbolReplacement proxy; - if (proxies.TryGetValue(node.LocalSymbol, out proxy)) + BoundNode replacement; + if (TryReplaceWithProxy(node.LocalSymbol, node.Syntax, out replacement)) { - return proxy.Replacement(node.Syntax, frameType => FramePointer(node.Syntax, frameType)); + return replacement; } - Debug.Assert(!IsCaptured(node.LocalSymbol)); + // if a local needs a proxy it should have been allocated by its declaration node. + Debug.Assert(!NeedsProxy(node.LocalSymbol)); + + return VisitUnhoistedLocal(node); + } + + private BoundNode VisitUnhoistedLocal(BoundLocal node) + { LocalSymbol replacementLocal; if (this.localMap.TryGetValue(node.LocalSymbol, out replacementLocal)) { @@ -346,15 +352,15 @@ public override BoundNode VisitAssignmentOperator(BoundAssignmentOperator node) if (leftLocal.LocalSymbol.RefKind != RefKind.None && node.RefKind != RefKind.None && - IsCaptured(leftLocal.LocalSymbol)) + NeedsProxy(leftLocal.LocalSymbol)) { - Debug.Assert(!proxies.ContainsKey(leftLocal.LocalSymbol)); // we need to create the proxy at this time + Debug.Assert(!proxies.ContainsKey(leftLocal.LocalSymbol)); Debug.Assert(!IsStackAlloc(originalRight)); //spilling ref local variables throw ExceptionUtilities.Unreachable; } - if (IsCaptured(leftLocal.LocalSymbol) && !proxies.ContainsKey(leftLocal.LocalSymbol)) + if (NeedsProxy(leftLocal.LocalSymbol) && !proxies.ContainsKey(leftLocal.LocalSymbol)) { Debug.Assert(leftLocal.LocalSymbol.DeclarationKind == LocalDeclarationKind.None); // spilling temp variables diff --git a/Src/Compilers/CSharp/Portable/Lowering/StateMachineRewriter/MethodToStateMachineRewriter.cs b/Src/Compilers/CSharp/Portable/Lowering/StateMachineRewriter/MethodToStateMachineRewriter.cs index ab33e1c783f8edb2852a67c24222e25856f2efcd..177b47635f4404a94c902b1abc7c873d074c9426 100644 --- a/Src/Compilers/CSharp/Portable/Lowering/StateMachineRewriter/MethodToStateMachineRewriter.cs +++ b/Src/Compilers/CSharp/Portable/Lowering/StateMachineRewriter/MethodToStateMachineRewriter.cs @@ -16,18 +16,18 @@ internal abstract class MethodToStateMachineRewriter : MethodToClassRewriter SyntheticBoundNodeFactory F, MethodSymbol originalMethod, FieldSymbol state, - HashSet variablesCaptured, + IReadOnlySet variablesCaptured, IReadOnlyDictionary nonReusableLocalProxies, DiagnosticBag diagnostics, bool useFinalizerBookkeeping) - : base(F.CompilationState, variablesCaptured, diagnostics) + : base(F.CompilationState, diagnostics) { Debug.Assert(F != null); Debug.Assert(originalMethod != null); Debug.Assert(state != null); - Debug.Assert(variablesCaptured != null); Debug.Assert(nonReusableLocalProxies != null); Debug.Assert(diagnostics != null); + Debug.Assert(variablesCaptured != null); this.F = F; this.stateField = state; @@ -35,6 +35,7 @@ internal abstract class MethodToStateMachineRewriter : MethodToClassRewriter this.useFinalizerBookkeeping = useFinalizerBookkeeping; this.hasFinalizerState = useFinalizerBookkeeping; this.originalMethod = originalMethod; + this.variablesCaptured = variablesCaptured; foreach (var proxy in nonReusableLocalProxies) { @@ -42,6 +43,7 @@ internal abstract class MethodToStateMachineRewriter : MethodToClassRewriter } } + /// /// True if we need to generate the code to do the bookkeeping so we can "finalize" the state machine /// by executing code from its current state through the enclosing finally blocks. This is true for @@ -117,6 +119,19 @@ internal abstract class MethodToStateMachineRewriter : MethodToClassRewriter /// private EmptyStructTypeCache emptyStructTypeCache = new EmptyStructTypeCache(null); + /// + /// The set of captured variables seen in the method body. + /// It's the minimal set of variables that have to be hoisted since their def-use arc crosses await/yield. + /// Other variables may be hoisted to improve debugging experience. + /// + private readonly IReadOnlySet variablesCaptured; + + protected override bool NeedsProxy(Symbol localOrParameter) + { + Debug.Assert(localOrParameter.Kind == SymbolKind.Local || localOrParameter.Kind == SymbolKind.Parameter); + return variablesCaptured.Contains(localOrParameter); + } + protected override TypeMap TypeMap { get { return ((SynthesizedContainer)F.CurrentClass).TypeMap; } @@ -185,7 +200,7 @@ public override BoundNode VisitSequence(BoundSequence node) // statement level by the AwaitLiftingRewriter. foreach (var local in node.Locals) { - Debug.Assert(!IsCaptured(local) || proxies.ContainsKey(local)); + Debug.Assert(!NeedsProxy(local) || proxies.ContainsKey(local)); } return base.VisitSequence(node); @@ -208,7 +223,7 @@ private BoundStatement PossibleIteratorScope(ImmutableArray locals, var hoistedUserDefinedLocals = ArrayBuilder.GetInstance(); foreach (var local in locals) { - if (!IsCaptured(local)) + if (!NeedsProxy(local)) { continue; } @@ -503,14 +518,13 @@ public override BoundNode VisitAssignmentOperator(BoundAssignmentOperator node) return base.VisitAssignmentOperator(node); } - var left = (BoundLocal)node.Left; - var local = left.LocalSymbol; - if (!IsCaptured(local)) + var leftLocal = ((BoundLocal)node.Left).LocalSymbol; + if (!NeedsProxy(leftLocal)) { return base.VisitAssignmentOperator(node); } - if (proxies.ContainsKey(local)) + if (proxies.ContainsKey(leftLocal)) { Debug.Assert(node.RefKind == RefKind.None); return base.VisitAssignmentOperator(node); @@ -521,12 +535,12 @@ public override BoundNode VisitAssignmentOperator(BoundAssignmentOperator node) // Here we handle ref temps. By-ref synthesized variables are the target of a ref assignment operator before // being used in any other way. - Debug.Assert(local.SynthesizedLocalKind == SynthesizedLocalKind.AwaitSpill); + Debug.Assert(leftLocal.SynthesizedLocalKind == SynthesizedLocalKind.AwaitSpill); Debug.Assert(node.RefKind != RefKind.None); // We have an assignment to a variable that has not yet been assigned a proxy. // So we assign the proxy before translating the assignment. - return HoistRefInitialization(local, node); + return HoistRefInitialization(leftLocal, node); } /// diff --git a/Src/Compilers/CSharp/Portable/Lowering/StateMachineRewriter/StateMachineRewriter.cs b/Src/Compilers/CSharp/Portable/Lowering/StateMachineRewriter/StateMachineRewriter.cs index be0af5e431a1e33bb700667a04b4fec474853d0a..6e7406833b39fee1bd97dc3376f27a2bce24a8f6 100644 --- a/Src/Compilers/CSharp/Portable/Lowering/StateMachineRewriter/StateMachineRewriter.cs +++ b/Src/Compilers/CSharp/Portable/Lowering/StateMachineRewriter/StateMachineRewriter.cs @@ -1,5 +1,6 @@ // Copyright (c) Microsoft Open Technologies, Inc. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System; using System.Collections.Generic; using System.Collections.Immutable; using System.Diagnostics; @@ -19,7 +20,7 @@ internal abstract class StateMachineRewriter protected readonly SynthesizedContainer stateMachineClass; protected FieldSymbol stateField; protected IReadOnlyDictionary nonReusableLocalProxies; - protected HashSet variablesCaptured; + protected IReadOnlySet variablesCaptured; protected Dictionary initialParameters; protected StateMachineRewriter( @@ -89,11 +90,9 @@ protected BoundStatement Rewrite() } // fields for the captured variables of the method - var captured = IteratorAndAsyncCaptureWalker.Analyze(compilationState.ModuleBuilderOpt.Compilation, method, body); - - this.variablesCaptured = new HashSet(captured.Keys); - - this.nonReusableLocalProxies = CreateNonReusableLocalProxies(captured); + var variablesCaptured = IteratorAndAsyncCaptureWalker.Analyze(compilationState.ModuleBuilderOpt.Compilation, method, body); + this.nonReusableLocalProxies = CreateNonReusableLocalProxies(variablesCaptured); + this.variablesCaptured = variablesCaptured; GenerateMethodImplementations(); @@ -101,14 +100,14 @@ protected BoundStatement Rewrite() return ReplaceOriginalMethod(); } - private IReadOnlyDictionary CreateNonReusableLocalProxies(MultiDictionary captured) + private IReadOnlyDictionary CreateNonReusableLocalProxies(MultiDictionary variablesCaptured) { var proxies = new Dictionary(); var typeMap = stateMachineClass.TypeMap; var orderedCaptured = - from local in captured.Keys + from local in variablesCaptured.Keys orderby local.Name, (local.Locations.Length == 0) ? 0 : local.Locations[0].SourceSpan.Start select local; @@ -122,7 +121,7 @@ protected BoundStatement Rewrite() { // create proxies for user-defined variables and for lambda closures: Debug.Assert(local.RefKind == RefKind.None); - proxies.Add(local, MakeNonReusableLocalProxy(typeMap, captured, local)); + proxies.Add(local, MakeNonReusableLocalProxy(typeMap, variablesCaptured, local)); } } else diff --git a/Src/Compilers/CSharp/Test/Emit/CSharpCompilerEmitTest.csproj b/Src/Compilers/CSharp/Test/Emit/CSharpCompilerEmitTest.csproj index a526d0c064ba49ef6f7f3aefac923afef2c47985..a7aa203c24a0b8eba3d99527a53f8b1865297ffe 100644 --- a/Src/Compilers/CSharp/Test/Emit/CSharpCompilerEmitTest.csproj +++ b/Src/Compilers/CSharp/Test/Emit/CSharpCompilerEmitTest.csproj @@ -122,7 +122,7 @@ - + diff --git a/Src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenAsyncLocalsTests.cs b/Src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenAsyncLocalsTests.cs index 9b222f9b830f5059d54af5993247b49fa6b7a818..96e524d3f33137f60378d1ddd5fb15ae7b5eb58f 100644 --- a/Src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenAsyncLocalsTests.cs +++ b/Src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenAsyncLocalsTests.cs @@ -15,23 +15,23 @@ namespace Microsoft.CodeAnalysis.CSharp.UnitTests.CodeGen { public class CodeGenAsyncLocalsTests : EmitMetadataTestBase { - private CSharpCompilation CreateCompilation(string source, IEnumerable references = null, CSharpCompilationOptions compOptions = null) + private CSharpCompilation CreateCompilation(string source, IEnumerable references = null, CSharpCompilationOptions options = null) { SynchronizationContext.SetSynchronizationContext(null); - compOptions = compOptions ?? TestOptions.ReleaseExe; + options = options ?? TestOptions.ReleaseExe; IEnumerable asyncRefs = new[] { SystemRef_v4_0_30319_17929, SystemCoreRef_v4_0_30319_17929, CSharpRef }; references = (references != null) ? references.Concat(asyncRefs) : asyncRefs; - return CreateCompilationWithMscorlib45(source, options: compOptions, references: references); + return CreateCompilationWithMscorlib45(source, options: options, references: references); } - private CompilationVerifier CompileAndVerify(string source, string expectedOutput, IEnumerable references = null, EmitOptions emitOptions = EmitOptions.All, CSharpCompilationOptions compOptions = null) + private CompilationVerifier CompileAndVerify(string source, string expectedOutput, IEnumerable references = null, EmitOptions emitOptions = EmitOptions.All, CSharpCompilationOptions options = null) { SynchronizationContext.SetSynchronizationContext(null); - var compilation = this.CreateCompilation(source, references: references, compOptions: compOptions); + var compilation = this.CreateCompilation(source, references: references, options: options); return base.CompileAndVerify(compilation, expectedOutput: expectedOutput, emitOptions: emitOptions); } @@ -45,28 +45,6 @@ where pair.line2.Contains("ldfld") || pair.line2.Contains("stfld") select pair.line1.Trim() + Environment.NewLine + pair.line2.Trim()); } - [Fact] - public void Unhoisted_Used_Param() - { - var source = @" -using System.Collections.Generic; - -struct Test -{ - public static IEnumerable F(int x) - { - x = 2; - yield return 1; - } - - public static void Main() - { - F(1); - } -}"; - CompileAndVerify(source, ""); - } - [Fact] public void AsyncWithLocals() { @@ -440,7 +418,7 @@ class Test foreach (var x in GetEnum()) await F(5); } }"; - var c = CompileAndVerify(source, expectedOutput: null, compOptions: TestOptions.ReleaseDll); + var c = CompileAndVerify(source, expectedOutput: null, options: TestOptions.ReleaseDll); var actual = GetFieldLoadsAndStores(c, "Test.d__1.System.Runtime.CompilerServices.IAsyncStateMachine.MoveNext"); @@ -598,7 +576,7 @@ public static async void M() foreach (var x in GetObjectEnum()) await F(2); } }"; - var c = CompileAndVerify(source, expectedOutput: null, compOptions: TestOptions.ReleaseDll); + var c = CompileAndVerify(source, expectedOutput: null, options: TestOptions.ReleaseDll); var actual = GetFieldLoadsAndStores(c, "Test.d__1.System.Runtime.CompilerServices.IAsyncStateMachine.MoveNext"); diff --git a/Src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenExprLambda.cs b/Src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenExprLambdaTests.cs similarity index 100% rename from Src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenExprLambda.cs rename to Src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenExprLambdaTests.cs diff --git a/Src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenIterators.cs b/Src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenIterators.cs index 7e0394a9fe93f772970bf2f6f267eeaeff6fb242..15c1e5800cb346c0fe7588bf2c64305a969ea79e 100644 --- a/Src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenIterators.cs +++ b/Src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenIterators.cs @@ -1163,8 +1163,16 @@ public static IEnumerator M(IEnumerable items) yield return x; } }"; - CompileAndVerify(source). - VerifyIL("Program.M", @" + var c = CompileAndVerify(source, options: TestOptions.ReleaseDll.WithMetadataImportOptions(MetadataImportOptions.All), symbolValidator: module => + { + AssertEx.Equal(new[] + { + "<>1__state", + "<>2__current" + }, module.GetFieldNames("Program.d__0")); + }); + + c.VerifyIL("Program.M", @" { // Code size 7 (0x7) .maxstack 1 @@ -1173,6 +1181,77 @@ .maxstack 1 IL_0006: ret }"); } + + [Fact] + public void HoistedParameters_Enumerable() + { + var source = @" +using System.Collections.Generic; + +struct Test +{ + public static IEnumerable F(int x, int y, int z) + { + x = z; + yield return 1; + y = 1; + } + + public static void Main() + { + F(1, 2, 3); + } +}"; + var c = CompileAndVerify(source, options: TestOptions.ReleaseDll.WithMetadataImportOptions(MetadataImportOptions.All), symbolValidator: module => + { + // consider: we don't really need to hoist "x" and "z", we could store the values of "<>3__x" and "<>3__z" to locals at the beginning of MoveNext. + AssertEx.Equal(new[] + { + "<>1__state", + "<>2__current", + "<>l__initialThreadId", + "x", + "<>3__x", + "y", + "<>3__y", + "z", + "<>3__z", + }, module.GetFieldNames("Test.d__0")); + }); + } + + [Fact] + public void HoistedParameters_Enumerator() + { + var source = @" +using System.Collections.Generic; + +struct Test +{ + public static IEnumerator F(int x, int y, int z) + { + x = z; + yield return 1; + y = 1; + } + + public static void Main() + { + F(1, 2, 3); + } +}"; + var c = CompileAndVerify(source, options: TestOptions.ReleaseDll.WithMetadataImportOptions(MetadataImportOptions.All), symbolValidator: module => + { + AssertEx.Equal(new[] + { + "<>1__state", + "<>2__current", + "x", + "y", + "z", + }, module.GetFieldNames("Test.d__0")); + }); + } [Fact] public void IteratorForEach() diff --git a/Src/Compilers/Core/Portable/CodeAnalysis.csproj b/Src/Compilers/Core/Portable/CodeAnalysis.csproj index 3c209ca5bf087ee6388b3a42f35ee67b6ac77c30..5db68c03b9c556311afbca179ff5d413ddb65108 100644 --- a/Src/Compilers/Core/Portable/CodeAnalysis.csproj +++ b/Src/Compilers/Core/Portable/CodeAnalysis.csproj @@ -103,6 +103,7 @@ + diff --git a/Src/Compilers/Core/Portable/InternalUtilities/IReadOnlySet.cs b/Src/Compilers/Core/Portable/InternalUtilities/IReadOnlySet.cs new file mode 100644 index 0000000000000000000000000000000000000000..6f9eb9af11a14be8fab6a28463b0e47c26b448fc --- /dev/null +++ b/Src/Compilers/Core/Portable/InternalUtilities/IReadOnlySet.cs @@ -0,0 +1,10 @@ +// Copyright (c) Microsoft Open Technologies, Inc. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +namespace Roslyn.Utilities +{ + internal interface IReadOnlySet + { + int Count { get; } + bool Contains(T item); + } +} diff --git a/Src/Compilers/Core/Portable/InternalUtilities/MultiDictionary.cs b/Src/Compilers/Core/Portable/InternalUtilities/MultiDictionary.cs index 437949ed63fce327b1062ce64991124940ad8b59..266f5595e42f8eabee6578c5e683a98dec5b7969 100644 --- a/Src/Compilers/Core/Portable/InternalUtilities/MultiDictionary.cs +++ b/Src/Compilers/Core/Portable/InternalUtilities/MultiDictionary.cs @@ -1,5 +1,6 @@ // Copyright (c) Microsoft Open Technologies, Inc. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System; using System.Collections.Generic; using System.Collections.Immutable; using System.Diagnostics; @@ -7,7 +8,7 @@ namespace Roslyn.Utilities { // Note that this is not threadsafe for concurrent reading and writing. - internal sealed class MultiDictionary + internal sealed class MultiDictionary : IReadOnlySet { // store either a single V or an ImmutableHashSet private readonly Dictionary dictionary; @@ -123,5 +124,15 @@ internal void Clear() { this.dictionary.Clear(); } + + int IReadOnlySet.Count + { + get { return KeyCount; } + } + + bool IReadOnlySet.Contains(K item) + { + return ContainsKey(item); + } } } \ No newline at end of file diff --git a/Src/Compilers/Test/Utilities/CSharp/Extensions.cs b/Src/Compilers/Test/Utilities/CSharp/Extensions.cs index 081b246989b2e147bdfc3c53d0b54abc29993e60..0f736b7225a7f43993fa90b8a1e239b222d14cf8 100644 --- a/Src/Compilers/Test/Utilities/CSharp/Extensions.cs +++ b/Src/Compilers/Test/Utilities/CSharp/Extensions.cs @@ -206,6 +206,12 @@ public static NamespaceSymbol GetNamespace(this NamespaceSymbol symbol, string n return (NamespaceSymbol)symbol.GetMembers(name).Single(); } + public static string[] GetFieldNames(this ModuleSymbol module, string qualifiedTypeName) + { + var type = (NamedTypeSymbol)module.GlobalNamespace.GetMember(qualifiedTypeName); + return type.GetMembers().OfType().Select(f => f.Name).ToArray(); + } + public static IEnumerable GetAttributes(this Symbol @this, NamedTypeSymbol c) { return @this.GetAttributes().Where(a => a.AttributeClass == c); diff --git a/Src/Workspaces/Core/Workspaces.csproj b/Src/Workspaces/Core/Workspaces.csproj index fd7d5265e9bf1a24603fbc7871890ce498c4af65..ec256c46f935bae87a5393c0ad5e3a166174781c 100644 --- a/Src/Workspaces/Core/Workspaces.csproj +++ b/Src/Workspaces/Core/Workspaces.csproj @@ -90,6 +90,9 @@ InternalUtilities\MultiDictionary.cs + + InternalUtilities\IReadOnlySet.cs + InternalUtilities\PathKind.cs