diff --git a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.Analysis.cs b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.Analysis.cs index 2e3965882f636405eafe712d1383ef92254cd652..da84ddcd93b0ea92cd1eea7c80bb14dd4282de7c 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.Analysis.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.Analysis.cs @@ -73,7 +73,16 @@ internal sealed class Analysis : BoundTreeWalkerWithStackGuardWithoutRecursionOn public readonly HashSet MethodsConvertedToDelegates = new HashSet(); /// - /// Any scope that a method in closes over. If a scope is in this set, don't use a struct closure. + /// True if the method signature can't be rewritten to contain ref/out parameters. + /// + public bool CanTakeRefParameters(MethodSymbol closure) => !(closure.IsAsync + || closure.IsIterator + // We can't rewrite delegate signatures + || MethodsConvertedToDelegates.Contains(closure)); + + /// + /// Any scope that a method that doesn't close over. + /// If a scope is in this set, don't use a struct closure. /// public readonly HashSet ScopesThatCantBeStructs = new HashSet(); @@ -325,7 +334,7 @@ internal void ComputeLambdaScopesAndFrameCaptures() LambdaScopes.Add(lambda, innermostScope); // Disable struct closures on methods converted to delegates, as well as on async and iterator methods. - var markAsNoStruct = MethodsConvertedToDelegates.Contains(lambda) || lambda.IsAsync || lambda.IsIterator; + var markAsNoStruct = !CanTakeRefParameters(lambda); if (markAsNoStruct) { ScopesThatCantBeStructs.Add(innermostScope); diff --git a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.cs b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.cs index 233c4b00b8e79b0b0453a38b2842482d17a8ad05..8e3d3bb7fab1f52adb52505f840d80c2358367c9 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.cs @@ -1311,35 +1311,15 @@ private DebugId GetLambdaId(SyntaxNode syntax, ClosureKind closureKind, int clos if (_analysis.LambdaScopes.TryGetValue(node.Symbol, out lambdaScope)) { containerAsFrame = _frames[lambdaScope]; - var structClosureParamBuilder = ArrayBuilder.GetInstance(); - while (containerAsFrame != null && containerAsFrame.IsValueType) + structClosures = _analysis.CanTakeRefParameters(node.Symbol) + ? GetStructClosures(containerAsFrame, lambdaScope) + : default(ImmutableArray); + + if (containerAsFrame?.IsValueType == true) { - structClosureParamBuilder.Add(containerAsFrame); - if (this._analysis.NeedsParentFrame.Contains(lambdaScope)) - { - var found = false; - while (this._analysis.ScopeParent.TryGetValue(lambdaScope, out lambdaScope)) - { - if (_frames.TryGetValue(lambdaScope, out containerAsFrame)) - { - found = true; - break; - } - } - if (found) - { - continue; - } - } - // can happen when scope no longer needs parent frame, or we're at the outermost level and the "parent frame" is top level "this". - lambdaScope = null; + // Lower directly onto the containing type containerAsFrame = null; - } - // Reverse it because we're going from inner to outer, and parameters are in order of outer to inner - structClosureParamBuilder.ReverseContents(); - structClosures = structClosureParamBuilder.ToImmutableAndFree(); - if (containerAsFrame == null) - { + lambdaScope = null; closureKind = ClosureKind.Static; // not exactly... but we've rewritten the receiver to be a by-ref parameter translatedLambdaContainer = _topLevelMethod.ContainingType; closureOrdinal = LambdaDebugInfo.StaticClosureOrdinal; @@ -1440,6 +1420,47 @@ private DebugId GetLambdaId(SyntaxNode syntax, ClosureKind closureKind, int clos return synthesizedMethod; } + /// + /// Builds a list of all the struct-based closure environments that will + /// need to be passed as arguments to the method. + /// + private ImmutableArray GetStructClosures(LambdaFrame containerAsFrame, BoundNode lambdaScope) + { + var structClosureParamBuilder = ArrayBuilder.GetInstance(); + while (containerAsFrame?.IsValueType == true || + _analysis.NeedsParentFrame.Contains(lambdaScope)) + { + if (containerAsFrame?.IsValueType == true) + { + structClosureParamBuilder.Add(containerAsFrame); + } + if (_analysis.NeedsParentFrame.Contains(lambdaScope) + && FindParentFrame(ref containerAsFrame, ref lambdaScope)) + { + continue; + } + + // can happen when scope no longer needs parent frame, or we're at the outermost level and the "parent frame" is top level "this". + break; + } + + // Reverse it because we're going from inner to outer, and parameters are in order of outer to inner + structClosureParamBuilder.ReverseContents(); + return structClosureParamBuilder.ToImmutableAndFree(); + + bool FindParentFrame(ref LambdaFrame container, ref BoundNode scope) + { + while (_analysis.ScopeParent.TryGetValue(scope, out scope)) + { + if (_frames.TryGetValue(scope, out container)) + { + return true; + } + } + return false; + } + } + private void AddSynthesizedMethod(MethodSymbol method, BoundStatement body) { if (_synthesizedMethods == null) diff --git a/src/Compilers/CSharp/Portable/Lowering/StateMachineRewriter/IteratorAndAsyncCaptureWalker.cs b/src/Compilers/CSharp/Portable/Lowering/StateMachineRewriter/IteratorAndAsyncCaptureWalker.cs index bda2852d633431dd132d1d3905427a5c986f825c..7434e57f0918d59c9837d02c0c2e3280937ee844 100644 --- a/src/Compilers/CSharp/Portable/Lowering/StateMachineRewriter/IteratorAndAsyncCaptureWalker.cs +++ b/src/Compilers/CSharp/Portable/Lowering/StateMachineRewriter/IteratorAndAsyncCaptureWalker.cs @@ -212,6 +212,9 @@ private void CaptureVariable(Symbol variable, SyntaxNode syntax) protected override void EnterParameter(ParameterSymbol parameter) { + // Async and iterators should never have ref parameters aside from `this` + Debug.Assert(parameter.IsThis || parameter.RefKind == RefKind.None); + // parameters are NOT initially assigned here - if that is a problem, then // the parameters must be captured. GetOrCreateSlot(parameter); diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenLocalFunctionTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenLocalFunctionTests.cs index ed7ea887bc0e1798218913dd42fa3e20a119dc58..ea370963ca807221f9f0fcff795f16437500e378 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenLocalFunctionTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenLocalFunctionTests.cs @@ -30,6 +30,54 @@ public static IMethodSymbol FindLocalFunction(this CommonTestBase.CompilationVer [CompilerTrait(CompilerFeature.LocalFunctions)] public class CodeGenLocalFunctionTests : CSharpTestBase { + [Fact] + [WorkItem(17890, "https://github.com/dotnet/roslyn/issues/17890")] + public void Repro17890() + { + var comp = CreateCompilationWithMscorlib46(@" +using System; +using System.Collections.Generic; +using System.Linq; + +public class Class +{ + public class Item + { + public int Id { get; set; } + } + + public class ItemsContainer : IDisposable + { + public List Items { get; set; } + + public void Dispose() + { + } + } + + public static void CompilerError() + { + using (var itemsContainer = new ItemsContainer()) + { + Item item = null; + + itemsContainer.Items.Where(x => x.Id == item.Id); + + void Local1() + { + itemsContainer.Items = null; + } + + void Local2() + { + Local1(); + } + } + } +}", references: new[] { LinqAssemblyRef }); + CompileAndVerify(comp); + } + [Fact(Skip = "https://github.com/dotnet/roslyn/issues/16783")] [WorkItem(16783, "https://github.com/dotnet/roslyn/issues/16783")] public void GenericDefaultParams() @@ -104,6 +152,312 @@ void Local(string s = nameof(Local)) CompileAndVerify(comp, expectedOutput: "Local"); } + [Fact] + [WorkItem(16895, "https://github.com/dotnet/roslyn/issues/16895")] + public void CaptureVarNestedLambdaSkipScope() + { + var src = @" +using System; +class C +{ + + public static void Main() + { + var d = """"; + { + int x = 0; + void M() + { + if (d != null) + { + Action a = () => x++; + a(); + } + } + M(); + Console.WriteLine(x); + } + } +}"; + CompileAndVerify(src, expectedOutput: "1"); + } + + [Fact] + [WorkItem(16895, "https://github.com/dotnet/roslyn/issues/16895")] + public void CaptureVarNestedLambdaSkipScope2() + { + var src = @" +using System; +class C +{ + class D : IDisposable { public void Dispose() {} } + + public static void Main() + { + using (var d = new D()) + { + int x = 0; + void M() + { + if (d != null) + { + Action a = () => x++; + a(); + } + } + M(); + Console.WriteLine(x); + } + } +}"; + CompileAndVerify(src, expectedOutput: "1"); + } + + [Fact] + [WorkItem(16895, "https://github.com/dotnet/roslyn/issues/16895")] + public void CaptureVarNestedLambdaSkipScope3() + { + var src = @" +using System; +class C +{ + + public static void Main() + { + var d = """"; + { + int x = 0; + void M() + { + if (d != null) + { + void Local() => x++; + Action a = Local; + a(); + } + } + M(); + Console.WriteLine(x); + } + } +}"; + CompileAndVerify(src, expectedOutput: "1"); + } + + [Fact] + [WorkItem(16895, "https://github.com/dotnet/roslyn/issues/16895")] + public void CaptureVarNestedLambdaSkipScope4() + { + var src = @" +using System; +class C +{ + + public static void Main() + { + var d = """"; + { + int y = 0; + { + int x = 0; + void M() + { + if (d != null) + { + Action a = () => x++;; + a(); + } + } + M(); + Console.WriteLine(x); + } + y++; + } + } +}"; + CompileAndVerify(src, expectedOutput: "1"); + } + + [Fact] + [WorkItem(16895, "https://github.com/dotnet/roslyn/issues/16895")] + public void CaptureVarNestedLambdaSkipScope5() + { + var src = @" +using System; +class C +{ + + public static void Main() + { + int x = 0; + { + int y = 0; + void L() + { + int z = 0; + void L2() + { + if (x == 0 && z == 0) + { + Action a = () => y++; + a(); + } + } + L2(); + } + L(); + Console.WriteLine(y); + } + } +}"; + CompileAndVerify(src, expectedOutput: "1"); + } + + [Fact] + [WorkItem(16895, "https://github.com/dotnet/roslyn/issues/16895")] + public void CaptureVarNestedLambdaSkipScope6() + { + var src = @" +using System; +class C +{ + + public static void Main() + { + int x = 0; + { + int y = 0; + void L() + { + int z = 0; + void L2() + { + if (x == 0 && y == 0) + { + Action a = () => z++; + a(); + } + y++; + } + L2(); + Console.WriteLine(z); + } + L(); + Console.WriteLine(y); + } + ((Action)(() => x++))(); + Console.WriteLine(x); + } +}"; + CompileAndVerify(src, expectedOutput: @"1 +1 +1"); + } + + [Fact] + [WorkItem(16895, "https://github.com/dotnet/roslyn/issues/16895")] + public void CaptureVarNestedLambdaSkipScope7() + { + var src = @" +using System; +using System.Threading.Tasks; +class C +{ + + public static void Main() + { + int x = 0; + { + int y = 0; + void L() + { + if (x == 0) + { + async Task L2() + { + await Task.Delay(1); + y++; + } + L2().Wait(); + } + } + L(); + Console.WriteLine(y); + } + Console.WriteLine(x); + } +}"; + CompileAndVerify(src, + additionalRefs: new[] { MscorlibRef_v46 }, + expectedOutput: @"1 +0"); + } + + [Fact] + [WorkItem(16895, "https://github.com/dotnet/roslyn/issues/16895")] + public void CaptureVarNestedLambdaSkipScope8() + { + var src = @" +using System; +using System.Collections.Generic; +class C +{ + + public static void Main() + { + int x = 0; + { + int y = 0; + void L() + { + if (x == 0) + { + IEnumerable L2() + { + yield return 0; + y++; + } + foreach (var i in L2()) { } + } + } + L(); + Console.WriteLine(y); + } + Console.WriteLine(x); + } +}"; + CompileAndVerify(src, + additionalRefs: new[] { MscorlibRef_v46 }, + expectedOutput: @"1 +0"); + } + + [Fact] + [WorkItem(16895, "https://github.com/dotnet/roslyn/issues/16895")] + public void LocalFunctionCaptureSkipScope() + { + var src = @" +using System; +class C +{ + public static void Main(string[] args) + { + { + int uncaptured = 0; + uncaptured++; + + { + int x = 0; + bool Local(int y) => x == 0 && args == null && y == 0; + Local(0); + } + } + } +}"; + CompileAndVerify(src); + } + + [Fact] [WorkItem(16399, "https://github.com/dotnet/roslyn/issues/16399")] public void RecursiveGenericLocalFunctionIterator()