From 9af0651cb5ad1d7d99d3e69d4778789a17cd5964 Mon Sep 17 00:00:00 2001 From: Heejae Chang Date: Mon, 7 Aug 2017 14:32:16 -0700 Subject: [PATCH 01/41] removed left out from deleted esent code. --- .../PersistentStorage/AbstractPersistentStorageTests.cs | 7 +++---- .../Host/PersistentStorage/PersistentStorageOptions.cs | 2 -- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/VisualStudio/CSharp/Test/PersistentStorage/AbstractPersistentStorageTests.cs b/src/VisualStudio/CSharp/Test/PersistentStorage/AbstractPersistentStorageTests.cs index aa66cd0d8b7c3..5e3a8e97eee0c 100644 --- a/src/VisualStudio/CSharp/Test/PersistentStorage/AbstractPersistentStorageTests.cs +++ b/src/VisualStudio/CSharp/Test/PersistentStorage/AbstractPersistentStorageTests.cs @@ -29,8 +29,7 @@ private enum Size private readonly Encoding _encoding = Encoding.UTF8; internal readonly IOptionService _persistentEnabledOptionService = new OptionServiceMock(new Dictionary { - { PersistentStorageOptions.Enabled, true }, - { PersistentStorageOptions.EsentPerformanceMonitor, false } + { PersistentStorageOptions.Enabled, true } }); private readonly string _persistentFolder; @@ -388,13 +387,13 @@ protected Solution CreateOrOpenSolution(bool nullPaths = false) { var projectFile = Path.Combine(Path.GetDirectoryName(solutionFile), "Project1.csproj"); File.WriteAllText(projectFile, ""); - solution = solution.AddProject(ProjectInfo.Create(ProjectId.CreateNewId(), VersionStamp.Create(), "Project1", "Project1", LanguageNames.CSharp, + solution = solution.AddProject(ProjectInfo.Create(ProjectId.CreateNewId(), VersionStamp.Create(), "Project1", "Project1", LanguageNames.CSharp, filePath: nullPaths ? null : projectFile)); var project = solution.Projects.Single(); var documentFile = Path.Combine(Path.GetDirectoryName(projectFile), "Document1.cs"); File.WriteAllText(documentFile, ""); - solution = solution.AddDocument(DocumentInfo.Create(DocumentId.CreateNewId(project.Id), "Document1", + solution = solution.AddDocument(DocumentInfo.Create(DocumentId.CreateNewId(project.Id), "Document1", filePath: nullPaths ? null : documentFile)); } diff --git a/src/Workspaces/Core/Portable/Workspace/Host/PersistentStorage/PersistentStorageOptions.cs b/src/Workspaces/Core/Portable/Workspace/Host/PersistentStorage/PersistentStorageOptions.cs index df43eafbb4228..6160590e5739f 100644 --- a/src/Workspaces/Core/Portable/Workspace/Host/PersistentStorage/PersistentStorageOptions.cs +++ b/src/Workspaces/Core/Portable/Workspace/Host/PersistentStorage/PersistentStorageOptions.cs @@ -9,7 +9,5 @@ internal static class PersistentStorageOptions public const string OptionName = "FeatureManager/Persistence"; public static readonly Option Enabled = new Option(OptionName, "Enabled", defaultValue: true); - - public static readonly Option EsentPerformanceMonitor = new Option(OptionName, "Esent PerfMon", defaultValue: false); } } From 12b2937d3dcbd424b65ca2775b995ae373dc9d43 Mon Sep 17 00:00:00 2001 From: Jason Malinowski Date: Wed, 26 Jul 2017 14:00:44 -0700 Subject: [PATCH 02/41] Replace project reference with linked file ExpressionEvaluatorPackage was depending upon ServicesVisualStudioImpl to get the references for ProvideRoslynBindingRedirection.cs that it was building itself. This is somewhat terrifying as that's an assembly dependency that at runtime isn't allowed to exist, so let's be explicit. --- .../Package/ExpressionEvaluatorPackage.csproj | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/ExpressionEvaluator/Package/ExpressionEvaluatorPackage.csproj b/src/ExpressionEvaluator/Package/ExpressionEvaluatorPackage.csproj index d1999b4d911a7..db22322a3eff2 100644 --- a/src/ExpressionEvaluator/Package/ExpressionEvaluatorPackage.csproj +++ b/src/ExpressionEvaluator/Package/ExpressionEvaluatorPackage.csproj @@ -21,10 +21,6 @@ Microsoft\ManagedLanguages\VBCSharp\ExpressionEvaluators - - ServicesVisualStudioImpl - false - VisualStudioSetup false @@ -104,10 +100,14 @@ Designer + + + + ProvideRoslynBindingRedirection.cs - \ No newline at end of file + From a12306e472c96b110091da9b410606c3ba9b572f Mon Sep 17 00:00:00 2001 From: John Doe Date: Mon, 7 Aug 2017 16:50:40 -0700 Subject: [PATCH 03/41] Typo --- src/Tools/Source/RunTests/Cache/AssemblyUtil.cs | 2 +- .../FindSymbols/SymbolTree/SymbolTreeInfo_Serialization.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Tools/Source/RunTests/Cache/AssemblyUtil.cs b/src/Tools/Source/RunTests/Cache/AssemblyUtil.cs index 0044e6862a448..3e87d25c63923 100644 --- a/src/Tools/Source/RunTests/Cache/AssemblyUtil.cs +++ b/src/Tools/Source/RunTests/Cache/AssemblyUtil.cs @@ -26,7 +26,7 @@ internal AssemblyUtil(string binariesPath) } /// - /// There are some DLLs whose abscence is expected and should not be considered an error. These + /// There are some DLLs whose absence is expected and should not be considered an error. These /// are assemblies which are either light up components or are a part of the VS reference graph /// which are never deployed for our tests. /// diff --git a/src/Workspaces/Core/Portable/FindSymbols/SymbolTree/SymbolTreeInfo_Serialization.cs b/src/Workspaces/Core/Portable/FindSymbols/SymbolTree/SymbolTreeInfo_Serialization.cs index 3455d29e8753c..4efd12dab159a 100644 --- a/src/Workspaces/Core/Portable/FindSymbols/SymbolTree/SymbolTreeInfo_Serialization.cs +++ b/src/Workspaces/Core/Portable/FindSymbols/SymbolTree/SymbolTreeInfo_Serialization.cs @@ -89,7 +89,7 @@ internal partial class SymbolTreeInfo : IObjectWritable cancellationToken.ThrowIfCancellationRequested(); // Couldn't read from the persistence service. If we've been asked to only load - // data and not create new instances in their absense, then there's nothing left + // data and not create new instances in their absence, then there's nothing left // to do at this point. if (loadOnly) { From 6a2133c4192f416e2296512a831b84a000bbadbc Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Mon, 7 Aug 2017 17:07:25 -0700 Subject: [PATCH 04/41] Enable skipped tests and fix them (#21335) --- .../Emit/CodeGen/CodeGenRefReturnTests.cs | 7 +- .../Test/Emit/CodeGen/CodeGenThrowTests.cs | 129 ++++++++++++------ .../CSharp/Test/Emit/PDB/PDBTests.cs | 42 +++--- .../IOperation/IOperationTests_IArgument.cs | 4 +- .../Syntax/Parsing/ExpressionParsingTests.cs | 14 +- 5 files changed, 119 insertions(+), 77 deletions(-) diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenRefReturnTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenRefReturnTests.cs index 343d59aa45ac7..ac91af109dc49 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenRefReturnTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenRefReturnTests.cs @@ -2900,7 +2900,7 @@ public void M() ); } - [Fact(Skip = "https://github.com/dotnet/roslyn/issues/21079")] + [Fact] [WorkItem(16947, "https://github.com/dotnet/roslyn/issues/16947")] public void Dynamic003() { @@ -2930,13 +2930,12 @@ public void M() "; CreateCompilationWithMscorlib45AndCSruntime(source).VerifyEmitDiagnostics( - // (14,28): error CS8156: An expression cannot be used in this context because it may not be returned by reference + // (14,26): error CS8156: An expression cannot be used in this context because it may not be returned by reference // return ref G(ref d.Length); - Diagnostic(ErrorCode.ERR_RefReturnLvalueExpected, "d.Length").WithLocation(14, 28), + Diagnostic(ErrorCode.ERR_RefReturnLvalueExpected, "d.Length").WithLocation(14, 26), // (14,20): error CS8164: Cannot return by reference a result of 'C.G(ref dynamic)' because the argument passed to parameter 'd' cannot be returned by reference // return ref G(ref d.Length); Diagnostic(ErrorCode.ERR_RefReturnCall, "G(ref d.Length)").WithArguments("C.G(ref dynamic)", "d").WithLocation(14, 20) - ); } diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenThrowTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenThrowTests.cs index 02c6882e386e4..e5202862b7e07 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenThrowTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenThrowTests.cs @@ -81,7 +81,7 @@ .maxstack 1 "); } - [Fact(Skip = "https://github.com/dotnet/roslyn/issues/21079")] + [Fact] public void TestRethrowImplicit() { var source = @" @@ -91,6 +91,7 @@ static void Main() { try { + System.Console.WriteLine(); } catch { @@ -101,28 +102,24 @@ static void Main() var compilation = CompileAndVerify(source); compilation.VerifyIL("C.Main", @"{ - // Code size 11 (0xb) + // Code size 11 (0xb) .maxstack 1 - IL_0000: nop .try { - IL_0001: nop - IL_0002: nop - IL_0003: leave.s IL_0009 - } // end .try - catch [mscorlib]System.Object + IL_0000: call ""void System.Console.WriteLine()"" + IL_0005: leave.s IL_000a + } + catch object { - IL_0005: pop - IL_0006: nop - IL_0007: rethrow - } // end handler - IL_0009: nop - IL_000a: ret + IL_0007: pop + IL_0008: rethrow + } + IL_000a: ret } "); } - [Fact(Skip = "https://github.com/dotnet/roslyn/issues/21079")] + [Fact] public void TestRethrowTyped() { var source = @" @@ -142,28 +139,52 @@ static void Main() var compilation = CompileAndVerify(source); compilation.VerifyIL("C.Main", @"{ - // Code size 11 (0xb) + // Code size 1 (0x1) + .maxstack 0 + IL_0000: ret +} +"); + } + + [Fact] + public void TestRethrowTyped2() + { + var source = @" +class C +{ + static void Main() + { + try + { + System.Console.WriteLine(); + } + catch (System.Exception) + { + throw; + } + } +}"; + var compilation = CompileAndVerify(source); + + compilation.VerifyIL("C.Main", @"{ + // Code size 11 (0xb) .maxstack 1 - IL_0000: nop .try { - IL_0001: nop - IL_0002: nop - IL_0003: leave.s IL_0009 - } // end .try - catch [mscorlib]System.Exception + IL_0000: call ""void System.Console.WriteLine()"" + IL_0005: leave.s IL_000a + } + catch System.Exception { - IL_0005: pop - IL_0006: nop - IL_0007: rethrow - } // end handler - IL_0009: nop - IL_000a: ret + IL_0007: pop + IL_0008: rethrow + } + IL_000a: ret } "); } - [Fact(Skip = "https://github.com/dotnet/roslyn/issues/21079")] + [Fact] public void TestRethrowNamed() { var source = @" @@ -183,23 +204,47 @@ static void Main() var compilation = CompileAndVerify(source); compilation.VerifyIL("C.Main", @"{ - // Code size 11 (0xb) + // Code size 1 (0x1) + .maxstack 0 + IL_0000: ret +} +"); + } + + [Fact] + public void TestRethrowNamed2() + { + var source = @" +class C +{ + static void Main() + { + try + { + System.Console.WriteLine(); + } + catch (System.Exception e) + { + throw; + } + } +}"; + var compilation = CompileAndVerify(source); + + compilation.VerifyIL("C.Main", @"{ + // Code size 11 (0xb) .maxstack 1 - IL_0000: nop .try { - IL_0001: nop - IL_0002: nop - IL_0003: leave.s IL_0009 - } // end .try - catch [mscorlib]System.Exception + IL_0000: call ""void System.Console.WriteLine()"" + IL_0005: leave.s IL_000a + } + catch System.Exception { - IL_0005: stloc.0 - IL_0006: nop - IL_0007: rethrow - } // end handler - IL_0009: nop - IL_000a: ret + IL_0007: pop + IL_0008: rethrow + } + IL_000a: ret } "); } diff --git a/src/Compilers/CSharp/Test/Emit/PDB/PDBTests.cs b/src/Compilers/CSharp/Test/Emit/PDB/PDBTests.cs index d902e87ef9bdd..4747987146f2d 100644 --- a/src/Compilers/CSharp/Test/Emit/PDB/PDBTests.cs +++ b/src/Compilers/CSharp/Test/Emit/PDB/PDBTests.cs @@ -6795,32 +6795,32 @@ static void M() #region Patterns - [Fact(Skip = "https://github.com/dotnet/roslyn/issues/21079")] + [Fact] public void SyntaxOffset_Pattern() { var source = @"class C { bool F(object o) => o is int i && o is 3 && o is bool; }"; var c = CreateCompilationWithMscorlibAndSystemCore(source, options: TestOptions.DebugDll); c.VerifyPdb("C.F", @" - - - - - - - - - - - - - - - - - - - - "); + + + + + + + + + + + + + + + + + + + +"); } #endregion diff --git a/src/Compilers/CSharp/Test/Semantic/IOperation/IOperationTests_IArgument.cs b/src/Compilers/CSharp/Test/Semantic/IOperation/IOperationTests_IArgument.cs index 6fc4cad506991..995f757cd2744 100644 --- a/src/Compilers/CSharp/Test/Semantic/IOperation/IOperationTests_IArgument.cs +++ b/src/Compilers/CSharp/Test/Semantic/IOperation/IOperationTests_IArgument.cs @@ -136,7 +136,7 @@ static void M1() VerifyOperationTreeAndDiagnosticsForTest(source, expectedOperationTree, expectedDiagnostics); } - [Fact(Skip = "https://github.com/dotnet/roslyn/issues/21079")] + [Fact] public void NamedArgumentInParameterOrderWithDefaultValue() { string source = @" @@ -157,7 +157,7 @@ static void M1() IArgument (ArgumentKind.Explicit, Matching Parameter: z) (OperationKind.Argument) (Syntax: '2') ILiteralExpression (Text: 2) (OperationKind.LiteralExpression, Type: System.Int32, Constant: 2) (Syntax: '2') IArgument (ArgumentKind.DefaultValue, Matching Parameter: x) (OperationKind.Argument) (Syntax: 'M2(y: 0, z: 2)') - ILiteralExpression (Text: 1) (OperationKind.LiteralExpression, Type: System.Int32, Constant: 1) (Syntax: 'M2(y: 0, z: 2)') + ILiteralExpression (OperationKind.LiteralExpression, Type: System.Int32, Constant: 1) (Syntax: 'M2(y: 0, z: 2)') "; var expectedDiagnostics = DiagnosticDescription.None; diff --git a/src/Compilers/CSharp/Test/Syntax/Parsing/ExpressionParsingTests.cs b/src/Compilers/CSharp/Test/Syntax/Parsing/ExpressionParsingTests.cs index ae58b2e9c8967..dcbd0b7df0cc8 100644 --- a/src/Compilers/CSharp/Test/Syntax/Parsing/ExpressionParsingTests.cs +++ b/src/Compilers/CSharp/Test/Syntax/Parsing/ExpressionParsingTests.cs @@ -1852,7 +1852,7 @@ public void TestFromOrderByDescendingSelect() Assert.Null(qs.Body.Continuation); } - [Fact(Skip = "https://github.com/dotnet/roslyn/issues/21079")] + [Fact] public void TestFromGroupBy() { var text = "from a in A group b by c"; @@ -1864,10 +1864,9 @@ public void TestFromGroupBy() Assert.Equal(0, expr.Errors().Length); var qs = (QueryExpressionSyntax)expr; - Assert.Equal(1, qs.Body.Clauses.Count); - Assert.Equal(SyntaxKind.FromClause, qs.Body.Clauses[0].Kind()); + Assert.Equal(0, qs.Body.Clauses.Count); - var fs = (FromClauseSyntax)qs.Body.Clauses[0]; + var fs = qs.FromClause; Assert.NotNull(fs.FromKeyword); Assert.False(fs.FromKeyword.IsMissing); Assert.Null(fs.Type); @@ -1892,7 +1891,7 @@ public void TestFromGroupBy() Assert.Null(qs.Body.Continuation); } - [Fact(Skip = "https://github.com/dotnet/roslyn/issues/21079")] + [Fact] public void TestFromGroupByIntoSelect() { var text = "from a in A group b by c into d select e"; @@ -1904,10 +1903,9 @@ public void TestFromGroupByIntoSelect() Assert.Equal(0, expr.Errors().Length); var qs = (QueryExpressionSyntax)expr; - Assert.Equal(1, qs.Body.Clauses.Count); - Assert.Equal(SyntaxKind.FromClause, qs.Body.Clauses[0].Kind()); + Assert.Equal(0, qs.Body.Clauses.Count); - var fs = (FromClauseSyntax)qs.Body.Clauses[0]; + var fs = qs.FromClause; Assert.NotNull(fs.FromKeyword); Assert.False(fs.FromKeyword.IsMissing); Assert.Null(fs.Type); From 995b158e9c27059e9d7714731f7449c7056c8561 Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Mon, 7 Aug 2017 19:04:14 -0700 Subject: [PATCH 05/41] Use langver=latest for scripting (#21331) --- src/Scripting/CSharp/CSharpScriptCompiler.cs | 4 +- .../CSharpTest/CommandLineRunnerTests.cs | 43 +++++++++++++++++++ .../VisualBasic/VisualBasicScriptCompiler.vb | 2 +- 3 files changed, 45 insertions(+), 4 deletions(-) diff --git a/src/Scripting/CSharp/CSharpScriptCompiler.cs b/src/Scripting/CSharp/CSharpScriptCompiler.cs index a0080d2f48215..1139f12b7364b 100644 --- a/src/Scripting/CSharp/CSharpScriptCompiler.cs +++ b/src/Scripting/CSharp/CSharpScriptCompiler.cs @@ -1,9 +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; -using System.Text; using System.Threading; -using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.Scripting; using Microsoft.CodeAnalysis.Text; @@ -13,7 +11,7 @@ internal sealed class CSharpScriptCompiler : ScriptCompiler { public static readonly ScriptCompiler Instance = new CSharpScriptCompiler(); - private static readonly CSharpParseOptions s_defaultOptions = new CSharpParseOptions(kind: SourceCodeKind.Script); + private static readonly CSharpParseOptions s_defaultOptions = new CSharpParseOptions(kind: SourceCodeKind.Script, languageVersion: LanguageVersion.Latest); private CSharpScriptCompiler() { diff --git a/src/Scripting/CSharpTest/CommandLineRunnerTests.cs b/src/Scripting/CSharpTest/CommandLineRunnerTests.cs index ce537bfea90e2..755f34b024840 100644 --- a/src/Scripting/CSharpTest/CommandLineRunnerTests.cs +++ b/src/Scripting/CSharpTest/CommandLineRunnerTests.cs @@ -947,5 +947,48 @@ public void PreservingDeclarationsOnException() Bang!", runner.Console.Error.ToString()); } + + [Fact] + [WorkItem(21327, "https://github.com/dotnet/roslyn/issues/21327")] + public void DefaultLiteral() + { + var runner = CreateRunner(input: +@"int i = default; +Print(i); +"); + runner.RunInteractive(); + + AssertEx.AssertEqualToleratingWhitespaceDifferences( +$@"Microsoft (R) Visual C# Interactive Compiler version {s_compilerVersion} +Copyright (C) Microsoft Corporation. All rights reserved. + +Type ""#help"" for more information. +> int i = default; +> Print(i); +0 +> ", runner.Console.Out.ToString()); + } + + [Fact] + [WorkItem(21327, "https://github.com/dotnet/roslyn/issues/21327")] + public void InferredTupleNames() + { + var runner = CreateRunner(input: +@"var a = 1; +var t = (a, 2); +Print(t.a); +"); + runner.RunInteractive(); + + AssertEx.AssertEqualToleratingWhitespaceDifferences( +$@"Microsoft (R) Visual C# Interactive Compiler version {s_compilerVersion} +Copyright (C) Microsoft Corporation. All rights reserved. +Type ""#help"" for more information. +> var a = 1; +> var t = (a, 2); +> Print(t.a); +1 +> ", runner.Console.Out.ToString()); + } } } diff --git a/src/Scripting/VisualBasic/VisualBasicScriptCompiler.vb b/src/Scripting/VisualBasic/VisualBasicScriptCompiler.vb index 2bf6ebeae2f1c..aa81d2a8627ec 100644 --- a/src/Scripting/VisualBasic/VisualBasicScriptCompiler.vb +++ b/src/Scripting/VisualBasic/VisualBasicScriptCompiler.vb @@ -13,7 +13,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Scripting Public Shared ReadOnly Instance As ScriptCompiler = New VisualBasicScriptCompiler() - Private Shared ReadOnly s_defaultOptions As VisualBasicParseOptions = New VisualBasicParseOptions(kind:=SourceCodeKind.Script) + Private Shared ReadOnly s_defaultOptions As VisualBasicParseOptions = New VisualBasicParseOptions(kind:=SourceCodeKind.Script, languageVersion:=LanguageVersion.Latest) Private Shared ReadOnly s_vbRuntimeReference As MetadataReference = MetadataReference.CreateFromAssemblyInternal(GetType(CompilerServices.NewLateBinding).GetTypeInfo().Assembly) Private Sub New() From 012c7fe9f105ccb20443414249a2ece5407c2818 Mon Sep 17 00:00:00 2001 From: Andy Gocke Date: Sun, 23 Jul 2017 22:47:28 -0700 Subject: [PATCH 06/41] Move MakeFrames logic into Analysis This is part of the effort to move as much logic from LambdaRewriter into Analysis as possible. The more logic there is in the Rewriter, the more difficult it is to find calculation problems until the last possible moment, and it's also very difficult to calculate useful information for later analysis passes, like the final debug and closure IDs for each environment. --- .../CSharp/Portable/CSharpParseOptions.cs | 2 +- .../Portable/Compiler/TypeCompilationState.cs | 2 +- .../LambdaRewriter/LambdaCapturedVariable.cs | 4 +- .../Lowering/LambdaRewriter/LambdaFrame.cs | 102 ++--- .../LambdaRewriter/LambdaFrameConstructor.cs | 2 +- .../LambdaRewriter.Analysis.Tree.cs | 74 +++- .../LambdaRewriter/LambdaRewriter.Analysis.cs | 210 +++++++-- ...Rewriter.LocalFunctionReferenceRewriter.cs | 6 +- .../Lowering/LambdaRewriter/LambdaRewriter.cs | 402 +++++------------- .../LambdaRewriter/SynthesizedLambdaMethod.cs | 37 +- .../ImmutableArrayExtensions.cs | 18 +- .../SetWithInsertionOrder.cs | 48 +-- .../Symbols/Source/SourceNamedTypeSymbol.vb | 2 +- .../Portable/VisualBasicParseOptions.vb | 2 +- 14 files changed, 432 insertions(+), 479 deletions(-) diff --git a/src/Compilers/CSharp/Portable/CSharpParseOptions.cs b/src/Compilers/CSharp/Portable/CSharpParseOptions.cs index 25cd68f968b69..66ad37bc0006e 100644 --- a/src/Compilers/CSharp/Portable/CSharpParseOptions.cs +++ b/src/Compilers/CSharp/Portable/CSharpParseOptions.cs @@ -61,7 +61,7 @@ public override IEnumerable PreprocessorSymbolNames LanguageVersion languageVersion, DocumentationMode documentationMode, SourceCodeKind kind, - IEnumerable preprocessorSymbols, + ImmutableArray preprocessorSymbols, IReadOnlyDictionary features) : base(kind, documentationMode) { diff --git a/src/Compilers/CSharp/Portable/Compiler/TypeCompilationState.cs b/src/Compilers/CSharp/Portable/Compiler/TypeCompilationState.cs index a81b758e6f017..2898d62ca6289 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 LambdaFrame StaticLambdaFrame; + public ClosureEnvironment 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 80f1261743776..a67f98a89bffe 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(LambdaFrame frame, Symbol captured, ref int uniqueId) + public static LambdaCapturedVariable Create(ClosureEnvironment frame, Symbol captured, ref int uniqueId) { Debug.Assert(captured is LocalSymbol || captured is ParameterSymbol); @@ -87,7 +87,7 @@ private static TypeSymbol GetCapturedVariableFieldType(SynthesizedContainer fram if ((object)local != null) { // if we're capturing a generic frame pointer, construct it with the new frame's type parameters - var lambdaFrame = local.Type.OriginalDefinition as LambdaFrame; + var lambdaFrame = local.Type.OriginalDefinition as ClosureEnvironment; if ((object)lambdaFrame != null) { // lambdaFrame may have less generic type parameters than frame, so trim them down (the first N will always match) diff --git a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaFrame.cs b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaFrame.cs index 43c5bd3a8729e..5ef934ebdc892 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaFrame.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaFrame.cs @@ -1,44 +1,58 @@ // Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System.Collections.Generic; using System.Collections.Immutable; using System.Diagnostics; using Microsoft.CodeAnalysis.CodeGen; using Microsoft.CodeAnalysis.CSharp.Symbols; -using Microsoft.CodeAnalysis.CSharp.Syntax; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.CSharp { /// /// A class that represents the set of variables in a scope that have been - /// captured by lambdas within that scope. + /// captured by nested functions within that scope. /// - internal sealed class LambdaFrame : SynthesizedContainer, ISynthesizedMethodBodyImplementationSymbol + internal sealed class ClosureEnvironment : SynthesizedContainer, ISynthesizedMethodBodyImplementationSymbol { - private readonly TypeKind _typeKind; private readonly MethodSymbol _topLevelMethod; - private readonly MethodSymbol _containingMethod; - private readonly MethodSymbol _constructor; - private readonly MethodSymbol _staticConstructor; - private readonly FieldSymbol _singletonCache; internal readonly SyntaxNode ScopeSyntaxOpt; internal readonly int ClosureOrdinal; - - internal LambdaFrame(MethodSymbol topLevelMethod, MethodSymbol containingMethod, bool isStruct, SyntaxNode scopeSyntaxOpt, DebugId methodId, DebugId closureId) + /// + /// The closest method/lambda that this frame is originally from. Null if nongeneric static closure. + /// Useful because this frame's type parameters are constructed from this method and all methods containing this method. + /// + 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, + MethodSymbol topLevelMethod, + MethodSymbol containingMethod, + bool isStruct, + SyntaxNode scopeSyntaxOpt, + DebugId methodId, + DebugId closureId) : base(MakeName(scopeSyntaxOpt, methodId, closureId), containingMethod) { - _typeKind = isStruct ? TypeKind.Struct : TypeKind.Class; + CapturedVariables = capturedVariables; + TypeKind = isStruct ? TypeKind.Struct : TypeKind.Class; _topLevelMethod = topLevelMethod; - _containingMethod = containingMethod; - _constructor = isStruct ? null : new LambdaFrameConstructor(this); + OriginalContainingMethodOpt = containingMethod; + Constructor = isStruct ? null : new LambdaFrameConstructor(this); this.ClosureOrdinal = closureId.Ordinal; // static lambdas technically have the class scope so the scope syntax is null if (scopeSyntaxOpt == null) { - _staticConstructor = new SynthesizedStaticConstructor(this); + StaticConstructor = new SynthesizedStaticConstructor(this); var cacheVariableName = GeneratedNames.MakeCachedFrameInstanceFieldName(); - _singletonCache = new SynthesizedLambdaCacheFieldSymbol(this, this, cacheVariableName, topLevelMethod, isReadOnly: true, isStatic: true); + SingletonCache = new SynthesizedLambdaCacheFieldSymbol(this, this, cacheVariableName, topLevelMethod, isReadOnly: true, isStatic: true); } AssertIsClosureScopeSyntax(scopeSyntaxOpt); @@ -77,69 +91,25 @@ private static void AssertIsClosureScopeSyntax(SyntaxNode syntaxOpt) throw ExceptionUtilities.UnexpectedValue(syntaxOpt.Kind()); } - public override TypeKind TypeKind - { - get { return _typeKind; } - } - - internal override MethodSymbol Constructor - { - get { return _constructor; } - } - - internal MethodSymbol StaticConstructor - { - get { return _staticConstructor; } - } - - /// - /// The closest method/lambda that this frame is originally from. Null if nongeneric static closure. - /// Useful because this frame's type parameters are constructed from this method and all methods containing this method. - /// - internal MethodSymbol ContainingMethod - { - get { return _containingMethod; } - } - public override ImmutableArray GetMembers() { var members = base.GetMembers(); - if ((object)_staticConstructor != null) + if ((object)StaticConstructor != null) { - members = ImmutableArray.Create(_staticConstructor, _singletonCache).AddRange(members); + members = ImmutableArray.Create(StaticConstructor, SingletonCache).AddRange(members); } return members; } - internal FieldSymbol SingletonCache - { - get { return _singletonCache; } - } - // display classes for static lambdas do not have any data and can be serialized. - internal override bool IsSerializable - { - get { return (object)_singletonCache != null; } - } + internal override bool IsSerializable => (object)SingletonCache != null; - public override Symbol ContainingSymbol - { - get { return _topLevelMethod.ContainingSymbol; } - } + public override Symbol ContainingSymbol => _topLevelMethod.ContainingSymbol; - bool ISynthesizedMethodBodyImplementationSymbol.HasMethodBodyDependency - { - get - { - // the lambda method contains user code from the lambda: - return true; - } - } + // The lambda method contains user code from the lambda + bool ISynthesizedMethodBodyImplementationSymbol.HasMethodBodyDependency => true; - IMethodSymbol ISynthesizedMethodBodyImplementationSymbol.Method - { - get { return _topLevelMethod; } - } + IMethodSymbol ISynthesizedMethodBodyImplementationSymbol.Method => _topLevelMethod; } } diff --git a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaFrameConstructor.cs b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaFrameConstructor.cs index b5e90ce0f5a28..b64c048f75971 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaFrameConstructor.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaFrameConstructor.cs @@ -6,7 +6,7 @@ namespace Microsoft.CodeAnalysis.CSharp { internal sealed class LambdaFrameConstructor : SynthesizedInstanceConstructor, ISynthesizedMethodBodyImplementationSymbol { - internal LambdaFrameConstructor(LambdaFrame frame) + internal LambdaFrameConstructor(ClosureEnvironment frame) : base(frame) { } 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 386dc38534b59..f776ab258519f 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.Analysis.Tree.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.Analysis.Tree.cs @@ -4,6 +4,7 @@ using System.Collections.Generic; using System.Collections.Immutable; using System.Diagnostics; +using Microsoft.CodeAnalysis.CodeGen; using Microsoft.CodeAnalysis.CSharp.Symbols; using Microsoft.CodeAnalysis.PooledObjects; using Roslyn.Utilities; @@ -25,14 +26,14 @@ internal sealed partial class Analysis : BoundTreeWalkerWithStackGuardWithoutRec [DebuggerDisplay("{ToString(), nq}")] public sealed class Scope { - public Scope Parent { get; } + public readonly Scope Parent; - public ArrayBuilder NestedScopes { get; } = ArrayBuilder.GetInstance(); + public readonly ArrayBuilder NestedScopes = ArrayBuilder.GetInstance(); /// /// A list of all closures (all lambdas and local functions) declared in this scope. /// - public ArrayBuilder Closures { get; } = ArrayBuilder.GetInstance(); + public readonly ArrayBuilder Closures = ArrayBuilder.GetInstance(); /// /// A list of all locals or parameters that were declared in this scope and captured @@ -45,7 +46,7 @@ public sealed class Scope /// non-deterministic compilation, and if we generate duplicate proxies we'll generate /// wasteful code in the best case and incorrect code in the worst. /// - public SetWithInsertionOrder DeclaredVariables { get; } = new SetWithInsertionOrder(); + public readonly SetWithInsertionOrder DeclaredVariables = new SetWithInsertionOrder(); /// /// The bound node representing this scope. This roughly corresponds to the bound @@ -53,13 +54,19 @@ public sealed class Scope /// methods/closures are introduced into their Body's scope and do not get their /// own scope. /// - public BoundNode BoundNode { get; } + public readonly BoundNode BoundNode; /// /// The closure that this scope is nested inside. Null if this scope is not nested /// inside a closure. /// - public Closure ContainingClosureOpt { get; } + public readonly Closure ContainingClosureOpt; + + /// + /// Environments created in this scope to hold . + /// + public readonly ArrayBuilder DeclaredEnvironments + = ArrayBuilder.GetInstance(); public Scope(Scope parent, BoundNode boundNode, Closure containingClosure) { @@ -83,6 +90,7 @@ public void Free() closure.Free(); } Closures.Free(); + DeclaredEnvironments.Free(); } public override string ToString() => BoundNode.Syntax.GetText().ToString(); @@ -102,9 +110,14 @@ public sealed class Closure /// /// The method symbol for the original lambda or local function. /// - public MethodSymbol OriginalMethodSymbol { get; } + public readonly MethodSymbol OriginalMethodSymbol; + + public readonly PooledHashSet CapturedVariables = PooledHashSet.GetInstance(); - public PooledHashSet CapturedVariables { get; } = PooledHashSet.GetInstance(); + public readonly ArrayBuilder CapturedEnvironments + = ArrayBuilder.GetInstance(); + + public ClosureEnvironment ContainingEnvironmentOpt; public Closure(MethodSymbol symbol) { @@ -115,6 +128,7 @@ public Closure(MethodSymbol symbol) public void Free() { CapturedVariables.Free(); + CapturedEnvironments.Free(); } } @@ -164,17 +178,36 @@ private void RemoveUnneededReferences(ParameterSymbol thisParam) } } + // True if there are any closures in the tree which + // capture 'this' and another variable + bool captureMoreThanThis = false; + VisitClosures(ScopeTree, (scope, closure) => { if (!capturesVariable.Contains(closure.OriginalMethodSymbol)) { closure.CapturedVariables.Clear(); } + if (capturesThis.Contains(closure.OriginalMethodSymbol)) { closure.CapturedVariables.Add(thisParam); + if (closure.CapturedVariables.Count > 1) + { + captureMoreThanThis |= true; + } } }); + + if (!captureMoreThanThis && capturesThis.Count > 0) + { + // If we have closures which capture 'this', and nothing else, we can + // remove 'this' from the declared variables list, since we don't need + // to create an environment to hold 'this' (since we can emit the + // lowered methods directly onto the containing class) + bool removed = ScopeTree.DeclaredVariables.Remove(thisParam); + Debug.Assert(removed); + } } /// @@ -193,6 +226,19 @@ public static void VisitClosures(Scope scope, Action action) } } + /// + /// Visit the tree with the given root and run the + /// + public static void VisitScopeTree(Scope treeRoot, Action action) + { + action(treeRoot); + + foreach (var nested in treeRoot.NestedScopes) + { + VisitScopeTree(nested, action); + } + } + /// /// Builds a tree of nodes corresponding to a given method. /// @@ -267,6 +313,12 @@ private void Build() { // Set up the current method locals DeclareLocals(_currentScope, _topLevelMethod.Parameters); + // Treat 'this' as a formal parameter of the top-level method + if (_topLevelMethod.TryGetThisParameter(out var thisParam)) + { + DeclareLocals(_currentScope, ImmutableArray.Create(thisParam)); + } + Visit(_currentScope.BoundNode); } @@ -468,12 +520,6 @@ private void AddIfCaptured(Symbol symbol, SyntaxNode syntax) return; } - // The 'this' parameter isn't declared in method scope - if (symbol is ParameterSymbol param && param.IsThis) - { - return; - } - if (_localToScope.TryGetValue(symbol, out var declScope)) { declScope.DeclaredVariables.Add(symbol); diff --git a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.Analysis.cs b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.Analysis.cs index 8e3e0d936d140..4c77933cd3c64 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.Analysis.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.Analysis.cs @@ -1,6 +1,9 @@ // Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System.Collections.Generic; +using System.Diagnostics; using System.Linq; +using Microsoft.CodeAnalysis.CodeGen; using Microsoft.CodeAnalysis.CSharp.Symbols; using Microsoft.CodeAnalysis.PooledObjects; using Roslyn.Utilities; @@ -29,12 +32,6 @@ internal sealed partial class Analysis // We can't rewrite delegate signatures || MethodsConvertedToDelegates.Contains(closure)); - /// - /// 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 PooledHashSet ScopesThatCantBeStructs = PooledHashSet.GetInstance(); - /// /// Blocks that are positioned between a block declaring some lifted variables /// and a block that contains the lambda that lifts said variables. @@ -44,29 +41,44 @@ internal sealed partial class Analysis /// public readonly PooledHashSet NeedsParentFrame = PooledHashSet.GetInstance(); - /// - /// Optimized locations of lambdas. - /// - /// Lambda does not need to be placed in a frame that corresponds to its lexical scope if lambda does not reference any local state in that scope. - /// It is advantageous to place lambdas higher in the scope tree, ideally in the innermost scope of all scopes that contain variables captured by a given lambda. - /// Doing so reduces indirections needed when captured locals are accessed. For example locals from the innermost scope can be accessed with no indirection at all. - /// needs to be called to compute this. - /// - public readonly SmallDictionary LambdaScopes = - new SmallDictionary(ReferenceEqualityComparer.Instance); - /// /// The root of the scope tree for this method. /// public readonly Scope ScopeTree; - private Analysis(Scope scopeTree, PooledHashSet methodsConvertedToDelegates) + private readonly MethodSymbol _topLevelMethod; + private readonly int _topLevelMethodOrdinal; + private readonly MethodSymbol _substitutedSourceMethod; + private readonly VariableSlotAllocator _slotAllocatorOpt; + private readonly TypeCompilationState _compilationState; + + private Analysis( + Scope scopeTree, + PooledHashSet methodsConvertedToDelegates, + MethodSymbol topLevelMethod, + int topLevelMethodOrdinal, + MethodSymbol substitutedSourceMethod, + VariableSlotAllocator slotAllocatorOpt, + TypeCompilationState compilationState) { ScopeTree = scopeTree; MethodsConvertedToDelegates = methodsConvertedToDelegates; + _topLevelMethod = topLevelMethod; + _topLevelMethodOrdinal = topLevelMethodOrdinal; + _substitutedSourceMethod = substitutedSourceMethod; + _slotAllocatorOpt = slotAllocatorOpt; + _compilationState = compilationState; } - public static Analysis Analyze(BoundNode node, MethodSymbol method, DiagnosticBag diagnostics) + public static Analysis Analyze( + BoundNode node, + MethodSymbol method, + int topLevelMethodOrdinal, + MethodSymbol substitutedSourceMethod, + VariableSlotAllocator slotAllocatorOpt, + TypeCompilationState compilationState, + ArrayBuilder closureDebugInfo, + DiagnosticBag diagnostics) { var methodsConvertedToDelegates = PooledHashSet.GetInstance(); var scopeTree = ScopeTreeBuilder.Build( @@ -74,7 +86,21 @@ public static Analysis Analyze(BoundNode node, MethodSymbol method, DiagnosticBa method, methodsConvertedToDelegates, diagnostics); - return new Analysis(scopeTree, methodsConvertedToDelegates); + Debug.Assert(scopeTree != null); + + var analysis = new Analysis( + scopeTree, + methodsConvertedToDelegates, + method, + topLevelMethodOrdinal, + substitutedSourceMethod, + slotAllocatorOpt, + compilationState); + + analysis.RemoveUnneededReferences(method.ThisParameter); + analysis.MakeAndAssignEnvironments(closureDebugInfo); + analysis.ComputeLambdaScopesAndFrameCaptures(method.ThisParameter); + return analysis; } private static BoundNode FindNodeToAnalyze(BoundNode node) @@ -107,12 +133,17 @@ private static BoundNode FindNodeToAnalyze(BoundNode node) } /// - /// Create the optimized plan for the location of lambda methods and whether scopes need access to parent scopes - /// - internal void ComputeLambdaScopesAndFrameCaptures(ParameterSymbol thisParam) + /// Must be called only after + /// has been calculated. + /// + /// Finds the most optimal capture environment to place a closure in. + /// This roughly corresponds to the 'highest' Scope in the tree where all + /// the captured variables for this closure are in scope. This minimizes + /// the number of indirections we may have to traverse to access captured + /// variables. + /// + private void ComputeLambdaScopesAndFrameCaptures(ParameterSymbol thisParam) { - RemoveUnneededReferences(thisParam); - VisitClosures(ScopeTree, (scope, closure) => { if (closure.CapturedVariables.Count > 0) @@ -124,6 +155,15 @@ internal void ComputeLambdaScopesAndFrameCaptures(ParameterSymbol thisParam) (Scope innermost, Scope outermost) FindLambdaScopeRange(Closure closure, Scope closureScope) { + // If the closure only captures this, put the method directly in the + // top-level method's containing type + if (closure.CapturedVariables.Count == 1 && + closure.CapturedVariables.Single() is ParameterSymbol param && + param.IsThis) + { + return (null, null); + } + Scope innermost = null; Scope outermost = null; @@ -149,8 +189,8 @@ internal void ComputeLambdaScopesAndFrameCaptures(ParameterSymbol thisParam) curScope != null && capturedVars.Count > 0; curScope = curScope.Parent) { - if (!(capturedVars.Overlaps(curScope.DeclaredVariables) || - capturedVars.Overlaps(curScope.Closures.Select(c => c.OriginalMethodSymbol)))) + if (!(capturedVars.RemoveAll(curScope.DeclaredVariables) || + capturedVars.RemoveAll(curScope.Closures.Select(c => c.OriginalMethodSymbol)))) { continue; } @@ -160,9 +200,6 @@ internal void ComputeLambdaScopesAndFrameCaptures(ParameterSymbol thisParam) { innermost = curScope; } - - capturedVars.RemoveAll(curScope.DeclaredVariables); - capturedVars.RemoveAll(curScope.Closures.Select(c => c.OriginalMethodSymbol)); } // If any captured variables are left, they're captured above method scope @@ -183,33 +220,119 @@ void RecordClosureScope(Scope innermost, Scope outermost, Closure closure) // // Example: // if a lambda captures a method's parameter and `this`, - // its innermost scope depth is 0 (method locals and parameters) - // and outermost scope is -1 + // its innermost scope is the root Scope (method locals and parameters) + // and outermost Scope is null // Such lambda will be placed in a closure frame that corresponds to the method's outer block // and this frame will also lift original `this` as a field when created by its parent. // Note that it is completely irrelevant how deeply the lexical scope of the lambda was originally nested. if (innermost != null) { - LambdaScopes.Add(closure.OriginalMethodSymbol, innermost.BoundNode); - - // Disable struct closures on methods converted to delegates, as well as on async and iterator methods. - var markAsNoStruct = !CanTakeRefParameters(closure.OriginalMethodSymbol); - if (markAsNoStruct) - { - ScopesThatCantBeStructs.Add(innermost.BoundNode); - } + closure.ContainingEnvironmentOpt = innermost.DeclaredEnvironments[0]; while (innermost != outermost) { NeedsParentFrame.Add(innermost.BoundNode); innermost = innermost.Parent; - if (markAsNoStruct && innermost != null) + } + } + } + } + + private void MakeAndAssignEnvironments(ArrayBuilder closureDebugInfo) + { + VisitScopeTree(ScopeTree, scope => + { + if (scope.DeclaredVariables.Count > 0) + { + // First walk the nested scopes to find all closures which + // capture variables from this scope. They all need to capture + // this environment. This includes closures which captured local + // functions that capture those variables, so multiple passes may + // be needed. This will also decide if the environment is a struct + // or a class. + bool isStruct = true; + var closures = new SetWithInsertionOrder(); + bool addedItem; + do + { + addedItem = false; + VisitClosures(scope, (closureScope, closure) => { - ScopesThatCantBeStructs.Add(innermost.BoundNode); - } + if (!closures.Contains(closure) && + (closure.CapturedVariables.Overlaps(scope.DeclaredVariables) || + closure.CapturedVariables.Overlaps(closures.Select(c => c.OriginalMethodSymbol)))) + { + closures.Add(closure); + addedItem = true; + isStruct &= CanTakeRefParameters(closure.OriginalMethodSymbol); + } + }); + } while (addedItem == true); + + // Next create the environment and add it to the declaration scope + // Currently all variables declared in the same scope are added + // to the same closure environment + var env = MakeEnvironment(scope, scope.DeclaredVariables, isStruct); + scope.DeclaredEnvironments.Add(env); + + foreach (var closure in closures) + { + closure.CapturedEnvironments.Add(env); } } + }); + + ClosureEnvironment MakeEnvironment(Scope scope, IEnumerable capturedVariables, bool isStruct) + { + var scopeBoundNode = scope.BoundNode; + + var syntax = scopeBoundNode.Syntax; + Debug.Assert(syntax != null); + + DebugId methodId = GetTopLevelMethodId(); + DebugId closureId = GetClosureId(syntax, closureDebugInfo); + + var containingMethod = scope.ContainingClosureOpt?.OriginalMethodSymbol ?? _topLevelMethod; + if ((object)_substitutedSourceMethod != null && containingMethod == _topLevelMethod) + { + containingMethod = _substitutedSourceMethod; + } + + return new ClosureEnvironment( + capturedVariables, + _topLevelMethod, + containingMethod, + isStruct, + syntax, + methodId, + closureId); + } + } + + internal DebugId GetTopLevelMethodId() + { + return _slotAllocatorOpt?.MethodId ?? new DebugId(_topLevelMethodOrdinal, _compilationState.ModuleBuilderOpt.CurrentGenerationOrdinal); + } + + private DebugId GetClosureId(SyntaxNode syntax, ArrayBuilder closureDebugInfo) + { + Debug.Assert(syntax != null); + + DebugId closureId; + DebugId previousClosureId; + if (_slotAllocatorOpt != null && _slotAllocatorOpt.TryGetPreviousClosure(syntax, out previousClosureId)) + { + closureId = previousClosureId; } + else + { + closureId = new DebugId(closureDebugInfo.Count, _compilationState.ModuleBuilderOpt.CurrentGenerationOrdinal); + } + + int syntaxOffset = _topLevelMethod.CalculateLocalSyntaxOffset(syntax.SpanStart, syntax.SyntaxTree); + closureDebugInfo.Add(new ClosureDebugInfo(syntaxOffset, closureId)); + + return closureId; } /// @@ -347,7 +470,6 @@ Closure Helper(Scope scope) public void Free() { MethodsConvertedToDelegates.Free(); - ScopesThatCantBeStructs.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 ecd59c21f1718..ffc7842e98836 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 LambdaFrame; + var containerAsFrame = synthesizedLambda.ContainingType as ClosureEnvironment; // Includes type parameters from the containing type iff // the containing type is a frame. If it is a frame then @@ -201,11 +201,11 @@ public BoundStatement RewriteLocalFunctionReferences(BoundStatement loweredBody) // will always be a LambdaFrame, it's always a capture frame var frameType = (NamedTypeSymbol)loweredSymbol.Parameters[i].Type.OriginalDefinition; - Debug.Assert(frameType is LambdaFrame); + Debug.Assert(frameType is ClosureEnvironment); if (frameType.Arity > 0) { - var typeParameters = ((LambdaFrame)frameType).ConstructedFromTypeParameters; + var typeParameters = ((ClosureEnvironment)frameType).ConstructedFromTypeParameters; Debug.Assert(typeParameters.Length == frameType.Arity); var subst = this.TypeMap.SubstituteTypeParameters(typeParameters); frameType = frameType.Construct(subst); diff --git a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.cs b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.cs index b0e9ffca56a38..0b00ee2c34454 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 LambdaFrame _lazyStaticLambdaFrame; + private ClosureEnvironment _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. @@ -187,7 +187,8 @@ public MappedLocalFunction(SynthesizedLambdaMethod symbol, ClosureKind closureKi _assignLocals = assignLocals; _currentTypeParameters = method.TypeParameters; _currentLambdaBodyTypeMap = TypeMap.Empty; - _innermostFramePointer = _currentFrameThis = thisParameterOpt; + _innermostFramePointer = null; + _currentFrameThis = thisParameterOpt; _framePointers[thisType] = thisParameterOpt; _seenBaseCall = method.MethodKind != MethodKind.Constructor; // only used for ctors _synthesizedFieldNameIdDispenser = 1; @@ -243,7 +244,15 @@ protected override bool NeedsProxy(Symbol localOrParameter) Debug.Assert(((object)thisParameter == null) || (thisParameter.Type == thisType)); Debug.Assert(compilationState.ModuleBuilderOpt != null); - var analysis = Analysis.Analyze(loweredBody, method, diagnostics); + var analysis = Analysis.Analyze( + loweredBody, + method, + methodOrdinal, + substitutedSourceMethod, + slotAllocatorOpt, + compilationState, + closureDebugInfoBuilder, + diagnostics); CheckLocalsDefined(loweredBody); var rewriter = new LambdaRewriter( @@ -259,8 +268,7 @@ protected override bool NeedsProxy(Symbol localOrParameter) diagnostics, assignLocals); - analysis.ComputeLambdaScopesAndFrameCaptures(thisParameter); - rewriter.MakeFrames(closureDebugInfoBuilder); + rewriter.SynthesizeClosureEnvironments(); // First, lower everything but references (calls, delegate conversions) // to local functions @@ -333,161 +341,47 @@ protected override NamedTypeSymbol ContainingType static partial void CheckLocalsDefined(BoundNode node); /// - /// Create the frame types. + /// Adds synthesized types to the compilation state + /// and creates hoisted fields for all locals captured by the environments. /// - private void MakeFrames(ArrayBuilder closureDebugInfo) + private void SynthesizeClosureEnvironments() { - Analysis.VisitClosures(_analysis.ScopeTree, (scope, closure) => + Analysis.VisitScopeTree(_analysis.ScopeTree, scope => { - var capturedVars = closure.CapturedVariables; - MethodSymbol closureSymbol = closure.OriginalMethodSymbol; - bool canTakeRefParams = _analysis.CanTakeRefParameters(closureSymbol); - - if (canTakeRefParams && OnlyCapturesThis(closure, scope)) + if (scope.DeclaredEnvironments.Count > 0) { - return; - } + Debug.Assert(!_frames.ContainsKey(scope.BoundNode)); + // At the moment, all variables declared in the same + // scope always get assigned to the same environment + Debug.Assert(scope.DeclaredEnvironments.Count == 1); - foreach (var captured in capturedVars) - { - var declarationScope = Analysis.GetVariableDeclarationScope(scope, captured); - if (declarationScope == null) - { - continue; - } - - // If this is a local function that can take ref params, skip - // frame creation for local function calls. This is semantically - // important because otherwise we may create a struct frame which - // is empty, which crashes in emit. - // This is not valid for lambdas or local functions which can't take - // take ref params since they will be lowered into their own frames. - if (canTakeRefParams && captured.Kind == SymbolKind.Method) - { - continue; - } - - LambdaFrame frame = GetFrameForScope(declarationScope, closureDebugInfo); - - if (captured.Kind != SymbolKind.Method && !proxies.ContainsKey(captured)) - { - var hoistedField = LambdaCapturedVariable.Create(frame, captured, ref _synthesizedFieldNameIdDispenser); - proxies.Add(captured, new CapturedToFrameSymbolReplacement(hoistedField, isReusable: false)); - CompilationState.ModuleBuilderOpt.AddSynthesizedDefinition(frame, hoistedField); - } - } - }); - } - - private SmallDictionary _onlyCapturesThisMemoTable; - /// - /// Helper for determining whether a local function transitively - /// only captures this (only captures this or other local functions - /// which only capture this). - /// - private bool OnlyCapturesThis( - Analysis.Closure closure, - Analysis.Scope scope, - PooledHashSet localFuncsInProgress = null) - { - Debug.Assert(closure != null); - Debug.Assert(scope != null); - - bool result = false; - if (_onlyCapturesThisMemoTable?.TryGetValue(closure, out result) == true) - { - return result; - } - - result = true; - foreach (var captured in closure.CapturedVariables) - { - var param = captured as ParameterSymbol; - if (param != null && param.IsThis) - { - continue; - } + var env = scope.DeclaredEnvironments[0]; - var localFunc = captured as LocalFunctionSymbol; - if (localFunc != null) - { - bool freePool = false; - if (localFuncsInProgress == null) - { - localFuncsInProgress = PooledHashSet.GetInstance(); - freePool = true; - } - else if (localFuncsInProgress.Contains(localFunc)) + CompilationState.ModuleBuilderOpt.AddSynthesizedDefinition(ContainingType, env); + if (env.Constructor != null) { - continue; + AddSynthesizedMethod( + env.Constructor, + FlowAnalysisPass.AppendImplicitReturn( + MethodCompiler.BindMethodBody(env.Constructor, CompilationState, null), + env.Constructor)); } - localFuncsInProgress.Add(localFunc); - var (found, foundScope) = Analysis.GetVisibleClosure(scope, localFunc); - bool transitivelyTrue = OnlyCapturesThis(found, foundScope, localFuncsInProgress); - - if (freePool) + foreach (var captured in env.CapturedVariables) { - localFuncsInProgress.Free(); - localFuncsInProgress = null; - } + Debug.Assert(!proxies.ContainsKey(captured)); - if (transitivelyTrue) - { - continue; + var hoistedField = LambdaCapturedVariable.Create(env, captured, ref _synthesizedFieldNameIdDispenser); + proxies.Add(captured, new CapturedToFrameSymbolReplacement(hoistedField, isReusable: false)); + CompilationState.ModuleBuilderOpt.AddSynthesizedDefinition(env, hoistedField); } - } - result = false; - break; - } - - if (_onlyCapturesThisMemoTable == null) - { - _onlyCapturesThisMemoTable = new SmallDictionary(); - } - - _onlyCapturesThisMemoTable[closure] = result; - return result; - } - - private LambdaFrame GetFrameForScope(Analysis.Scope scope, ArrayBuilder closureDebugInfo) - { - var scopeBoundNode = scope.BoundNode; - LambdaFrame frame; - if (!_frames.TryGetValue(scopeBoundNode, out frame)) - { - var syntax = scopeBoundNode.Syntax; - Debug.Assert(syntax != null); - - DebugId methodId = GetTopLevelMethodId(); - DebugId closureId = GetClosureId(syntax, closureDebugInfo); - - var canBeStruct = !_analysis.ScopesThatCantBeStructs.Contains(scopeBoundNode); - - var containingMethod = scope.ContainingClosureOpt?.OriginalMethodSymbol ?? _topLevelMethod; - if (_substitutedSourceMethod != null && containingMethod == _topLevelMethod) - { - containingMethod = _substitutedSourceMethod; - } - frame = new LambdaFrame(_topLevelMethod, containingMethod, canBeStruct, syntax, methodId, closureId); - _frames.Add(scopeBoundNode, frame); - - CompilationState.ModuleBuilderOpt.AddSynthesizedDefinition(ContainingType, frame); - if (frame.Constructor != null) - { - AddSynthesizedMethod( - frame.Constructor, - FlowAnalysisPass.AppendImplicitReturn( - MethodCompiler.BindMethodBody(frame.Constructor, CompilationState, null), - frame.Constructor)); + _frames.Add(scope.BoundNode, env); } - } - - return frame; + }); } - private LambdaFrame GetStaticFrame(DiagnosticBag diagnostics, IBoundLambdaOrFunction lambda) + private ClosureEnvironment GetStaticFrame(DiagnosticBag diagnostics, IBoundLambdaOrFunction lambda) { if (_lazyStaticLambdaFrame == null) { @@ -506,13 +400,20 @@ private LambdaFrame GetStaticFrame(DiagnosticBag diagnostics, IBoundLambdaOrFunc } else { - methodId = GetTopLevelMethodId(); + methodId = _analysis.GetTopLevelMethodId(); } 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 LambdaFrame(_topLevelMethod, containingMethod, isStruct: false, scopeSyntaxOpt: null, methodId: methodId, closureId: closureId); + _lazyStaticLambdaFrame = new ClosureEnvironment( + SpecializedCollections.EmptyEnumerable(), + _topLevelMethod, + containingMethod, + isStruct: false, + scopeSyntaxOpt: null, + methodId: methodId, + closureId: closureId); // non-generic static lambdas can share the frame if (isNonGeneric) @@ -632,7 +533,7 @@ private static void InsertAndFreePrologue(ArrayBuilder result, A /// The frame 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, LambdaFrame frame, Func, ArrayBuilder, BoundNode> F) + private BoundNode IntroduceFrame(BoundNode node, ClosureEnvironment frame, Func, ArrayBuilder, BoundNode> F) { var frameTypeParameters = ImmutableArray.Create(StaticCast.From(_currentTypeParameters).SelectAsArray(TypeMap.TypeSymbolAsTypeWithModifiers), 0, frame.Arity); NamedTypeSymbol frameType = frame.ConstructIfGeneric(frameTypeParameters); @@ -646,7 +547,7 @@ private BoundNode IntroduceFrame(BoundNode node, LambdaFrame frame, Func.GetInstance(); - if (frame.Constructor != null) + if ((object)frame.Constructor != null) { MethodSymbol constructor = frame.Constructor.AsMember(frameType); Debug.Assert(frameType == constructor.ContainingType); @@ -675,20 +576,7 @@ private BoundNode IntroduceFrame(BoundNode node, LambdaFrame frame, Func.GetInstance(); addedLocals.Add(framePointer); _framePointers.Add(frame, framePointer); @@ -775,7 +664,20 @@ private void InitVariableProxy(SyntaxNode syntax, Symbol symbol, LocalSymbol fra var left = proxy.Replacement(syntax, frameType1 => new BoundLocal(syntax, framePointer, null, framePointer.Type)); var assignToProxy = new BoundAssignmentOperator(syntax, left, value, value.Type); - prologue.Add(assignToProxy); + if (_currentMethod.MethodKind == MethodKind.Constructor && + symbol == _currentMethod.ThisParameter && + !_seenBaseCall) + { + // Containing method is a constructor + // Initialization statement for the "this" proxy must be inserted + // after the constructor initializer statement block + Debug.Assert(_thisProxyInitDeferred == null); + _thisProxyInitDeferred = assignToProxy; + } + else + { + prologue.Add(assignToProxy); + } } } @@ -826,7 +728,7 @@ public override BoundNode VisitBaseReference(BoundBaseReference node) out NamedTypeSymbol constructedFrame) { var translatedLambdaContainer = synthesizedMethod.ContainingType; - var containerAsFrame = translatedLambdaContainer as LambdaFrame; + var containerAsFrame = translatedLambdaContainer as ClosureEnvironment; // 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)); @@ -915,17 +817,20 @@ public override BoundNode VisitCall(BoundCall node) // Check if we need to init the 'this' proxy in a ctor call if (!_seenBaseCall) { - _seenBaseCall = _currentMethod == _topLevelMethod && node.IsConstructorInitializer(); - if (_seenBaseCall && _thisProxyInitDeferred != null) + if (_currentMethod == _topLevelMethod && node.IsConstructorInitializer()) { - // Insert the this proxy assignment after the ctor call. - // Create bound sequence: { ctor call, thisProxyInitDeferred } - return new BoundSequence( - syntax: node.Syntax, - locals: ImmutableArray.Empty, - sideEffects: ImmutableArray.Create(rewritten), - value: _thisProxyInitDeferred, - type: rewritten.Type); + _seenBaseCall = true; + if (_thisProxyInitDeferred != null) + { + // Insert the this proxy assignment after the ctor call. + // Create bound sequence: { ctor call, thisProxyInitDeferred } + return new BoundSequence( + syntax: node.Syntax, + locals: ImmutableArray.Empty, + sideEffects: ImmutableArray.Create(rewritten), + value: _thisProxyInitDeferred, + type: rewritten.Type); + } } } @@ -961,7 +866,7 @@ private BoundSequence RewriteSequence(BoundSequence node, ArrayBuilder prologue, ArrayBuilder newLocals) => @@ -1086,7 +991,7 @@ private BoundNode RewriteCatch(BoundCatchBlock node, ArrayBuilder closureDebugInfo) - { - Debug.Assert(syntax != null); - - DebugId closureId; - DebugId previousClosureId; - if (slotAllocatorOpt != null && slotAllocatorOpt.TryGetPreviousClosure(syntax, out previousClosureId)) - { - closureId = previousClosureId; - } - else - { - closureId = new DebugId(closureDebugInfo.Count, CompilationState.ModuleBuilderOpt.CurrentGenerationOrdinal); - } - - int syntaxOffset = _topLevelMethod.CalculateLocalSyntaxOffset(syntax.SpanStart, syntax.SyntaxTree); - closureDebugInfo.Add(new ClosureDebugInfo(syntaxOffset, closureId)); - - return closureId; - } - private DebugId GetLambdaId(SyntaxNode syntax, ClosureKind closureKind, int closureOrdinal) { Debug.Assert(syntax != null); @@ -1299,19 +1178,19 @@ private DebugId GetLambdaId(SyntaxNode syntax, ClosureKind closureKind, int clos IBoundLambdaOrFunction node, out ClosureKind closureKind, out NamedTypeSymbol translatedLambdaContainer, - out LambdaFrame containerAsFrame, + out ClosureEnvironment containerAsFrame, out BoundNode lambdaScope, out DebugId topLevelMethodId, out DebugId lambdaId) { - ImmutableArray structClosures; + Analysis.Closure closure = Analysis.GetClosureInTree(_analysis.ScopeTree, node.Symbol); + + var structClosures = closure.CapturedEnvironments + .Where(env => env.IsStructType()).AsImmutable(); int closureOrdinal; - if (_analysis.LambdaScopes.TryGetValue(node.Symbol, out lambdaScope)) + if (closure.ContainingEnvironmentOpt != null) { - containerAsFrame = _frames[lambdaScope]; - structClosures = _analysis.CanTakeRefParameters(node.Symbol) - ? GetStructClosures(containerAsFrame, lambdaScope) - : default(ImmutableArray); + containerAsFrame = closure.ContainingEnvironmentOpt; if (containerAsFrame?.IsValueType == true) { @@ -1327,9 +1206,20 @@ private DebugId GetLambdaId(SyntaxNode syntax, ClosureKind closureKind, int clos 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)) + { + tmpScope = scope.BoundNode; + } + }); + Debug.Assert(tmpScope != null); + lambdaScope = tmpScope; } } - else if (Analysis.GetClosureInTree(_analysis.ScopeTree, node.Symbol).CapturedVariables.Count == 0) + else if (closure.CapturedVariables.Count == 0) { if (_analysis.MethodsConvertedToDelegates.Contains(node.Symbol)) { @@ -1344,28 +1234,11 @@ private DebugId GetLambdaId(SyntaxNode syntax, ClosureKind closureKind, int clos closureKind = ClosureKind.Static; closureOrdinal = LambdaDebugInfo.StaticClosureOrdinal; } - structClosures = default; + lambdaScope = null; } else { - // GetStructClosures is currently overly aggressive in gathering - // closures since the only information it has at this point is - // NeedsParentFrame, which doesn't say what exactly is needed from - // the parent frame. If `this` is captured, that's enough to mark - // NeedsParentFrame for the current closure, so we need to gather - // struct closures for all intermediate frames, even if they only - // strictly need `this`. - if (_analysis.CanTakeRefParameters(node.Symbol)) - { - lambdaScope = Analysis.GetScopeParent(_analysis.ScopeTree, node.Body)?.BoundNode; - _ = _frames.TryGetValue(lambdaScope, out containerAsFrame); - structClosures = GetStructClosures(containerAsFrame, lambdaScope); - } - else - { - structClosures = default; - } - + lambdaScope = null; containerAsFrame = null; translatedLambdaContainer = _topLevelMethod.ContainingType; closureKind = ClosureKind.ThisOnly; @@ -1373,7 +1246,7 @@ private DebugId GetLambdaId(SyntaxNode syntax, ClosureKind closureKind, int clos } // Move the body of the lambda to a freshly generated synthetic method on its frame. - topLevelMethodId = GetTopLevelMethodId(); + topLevelMethodId = _analysis.GetTopLevelMethodId(); lambdaId = GetLambdaId(node.Syntax, closureKind, closureOrdinal); var synthesizedMethod = new SynthesizedLambdaMethod(translatedLambdaContainer, structClosures, closureKind, _topLevelMethod, topLevelMethodId, node, lambdaId); @@ -1411,7 +1284,6 @@ private DebugId GetLambdaId(SyntaxNode syntax, ClosureKind closureKind, int clos else { _currentFrameThis = synthesizedMethod.ThisParameter; - _innermostFramePointer = null; _framePointers.TryGetValue(translatedLambdaContainer, out _innermostFramePointer); } @@ -1435,52 +1307,6 @@ 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 (true) - { - scope = Analysis.GetScopeParent(_analysis.ScopeTree, scope)?.BoundNode; - if (scope == null) - { - return false; - } - - if (_frames.TryGetValue(scope, out container)) - { - return true; - } - } - } - } - private void AddSynthesizedMethod(MethodSymbol method, BoundStatement body) { if (_synthesizedMethods == null) @@ -1512,7 +1338,7 @@ private BoundNode RewriteLambdaConversion(BoundLambda node) ClosureKind closureKind; NamedTypeSymbol translatedLambdaContainer; - LambdaFrame containerAsFrame; + ClosureEnvironment containerAsFrame; BoundNode lambdaScope; DebugId topLevelMethodId; DebugId lambdaId; diff --git a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/SynthesizedLambdaMethod.cs b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/SynthesizedLambdaMethod.cs index f8cd67bdc9ccc..1eee93f9df38b 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/SynthesizedLambdaMethod.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/SynthesizedLambdaMethod.cs @@ -15,11 +15,11 @@ namespace Microsoft.CodeAnalysis.CSharp internal sealed class SynthesizedLambdaMethod : SynthesizedMethodBaseSymbol, ISynthesizedMethodBodyImplementationSymbol { private readonly MethodSymbol _topLevelMethod; - private readonly ImmutableArray _structClosures; + private readonly ImmutableArray _structEnvironments; internal SynthesizedLambdaMethod( NamedTypeSymbol containingType, - ImmutableArray structClosures, + ImmutableArray structEnvironments, ClosureKind closureKind, MethodSymbol topLevelMethod, DebugId topLevelMethodId, @@ -43,15 +43,15 @@ internal sealed class SynthesizedLambdaMethod : SynthesizedMethodBaseSymbol, ISy TypeMap typeMap; ImmutableArray typeParameters; ImmutableArray constructedFromTypeParameters; - LambdaFrame lambdaFrame; + ClosureEnvironment lambdaFrame; - lambdaFrame = this.ContainingType as LambdaFrame; + lambdaFrame = this.ContainingType as ClosureEnvironment; switch (closureKind) { case ClosureKind.Singleton: // all type parameters on method (except the top level method's) case ClosureKind.General: // only lambda's type parameters on method (rest on class) Debug.Assert(lambdaFrame != null); - typeMap = lambdaFrame.TypeMap.WithConcatAlphaRename(lambdaNode.Symbol, this, out typeParameters, out constructedFromTypeParameters, lambdaFrame.ContainingMethod); + typeMap = lambdaFrame.TypeMap.WithConcatAlphaRename(lambdaNode.Symbol, this, out typeParameters, out constructedFromTypeParameters, lambdaFrame.OriginalContainingMethodOpt); break; case ClosureKind.ThisOnly: // all type parameters on method case ClosureKind.Static: @@ -62,28 +62,30 @@ internal sealed class SynthesizedLambdaMethod : SynthesizedMethodBaseSymbol, ISy throw ExceptionUtilities.UnexpectedValue(closureKind); } - if (!structClosures.IsDefaultOrEmpty && typeParameters.Length != 0) + if (!structEnvironments.IsDefaultOrEmpty && typeParameters.Length != 0) { - var constructedStructClosures = ArrayBuilder.GetInstance(); - foreach (var closure in structClosures) + var constructedStructClosures = ArrayBuilder.GetInstance(); + foreach (var env in structEnvironments) { - var frame = (LambdaFrame)closure; NamedTypeSymbol constructed; - if (frame.Arity == 0) + if (env.Arity == 0) { - constructed = frame; + constructed = env; } else { - var originals = frame.ConstructedFromTypeParameters; + var originals = env.ConstructedFromTypeParameters; var newArgs = typeMap.SubstituteTypeParameters(originals); - constructed = frame.Construct(newArgs); + constructed = env.Construct(newArgs); } constructedStructClosures.Add(constructed); } - structClosures = constructedStructClosures.ToImmutableAndFree(); + _structEnvironments = constructedStructClosures.ToImmutableAndFree(); + } + else + { + _structEnvironments = ImmutableArray.CastUp(structEnvironments); } - _structClosures = structClosures; AssignTypeMapAndTypeParameters(typeMap, typeParameters); } @@ -125,8 +127,9 @@ private static string MakeName(string topLevelMethodName, DebugId topLevelMethod // UNDONE: names from the delegate. Does it really matter? protected override ImmutableArray BaseMethodParameters => this.BaseMethod.Parameters; - protected override ImmutableArray ExtraSynthesizedRefParameters => _structClosures; - internal int ExtraSynthesizedParameterCount => this._structClosures.IsDefault ? 0 : this._structClosures.Length; + protected override ImmutableArray ExtraSynthesizedRefParameters + => ImmutableArray.CastUp(_structEnvironments); + internal int ExtraSynthesizedParameterCount => this._structEnvironments.IsDefault ? 0 : this._structEnvironments.Length; internal override bool GenerateDebugInfo => !this.IsAsync; internal override bool IsExpressionBodied => false; diff --git a/src/Compilers/Core/Portable/InternalUtilities/ImmutableArrayExtensions.cs b/src/Compilers/Core/Portable/InternalUtilities/ImmutableArrayExtensions.cs index 2eee158e23f08..34ca8ee7671fa 100644 --- a/src/Compilers/Core/Portable/InternalUtilities/ImmutableArrayExtensions.cs +++ b/src/Compilers/Core/Portable/InternalUtilities/ImmutableArrayExtensions.cs @@ -9,24 +9,10 @@ namespace Roslyn.Utilities internal static class ImmutableArrayExtensions { internal static ImmutableArray ToImmutableArrayOrEmpty(this IEnumerable items) - { - if (items == null) - { - return ImmutableArray.Create(); - } - - return ImmutableArray.CreateRange(items); - } + => items == null ? ImmutableArray.Empty : ImmutableArray.CreateRange(items); internal static ImmutableArray ToImmutableArrayOrEmpty(this ImmutableArray items) - { - if (items.IsDefault) - { - return ImmutableArray.Create(); - } - - return items; - } + => items.IsDefault ? ImmutableArray.Empty : items; // same as Array.BinarySearch but the ability to pass arbitrary value to the comparer without allocation internal static int BinarySearch(this ImmutableArray array, TValue value, Func comparer) diff --git a/src/Compilers/Core/Portable/InternalUtilities/SetWithInsertionOrder.cs b/src/Compilers/Core/Portable/InternalUtilities/SetWithInsertionOrder.cs index 9fc12c3d1f692..97145d508a68e 100644 --- a/src/Compilers/Core/Portable/InternalUtilities/SetWithInsertionOrder.cs +++ b/src/Compilers/Core/Portable/InternalUtilities/SetWithInsertionOrder.cs @@ -7,6 +7,7 @@ using System.Linq; using System.Text; using System.Threading.Tasks; +using Microsoft.CodeAnalysis.PooledObjects; namespace Roslyn.Utilities { @@ -16,46 +17,45 @@ namespace Roslyn.Utilities /// internal sealed class SetWithInsertionOrder : IEnumerable, IReadOnlySet { - private HashSet _set = new HashSet(); - private uint _nextElementValue = 0; - private T[] _elements = null; + private HashSet _set = null; + private ArrayBuilder _elements = null; public bool Add(T value) { - if (!_set.Add(value)) return false; - var thisValue = _nextElementValue++; - if (_elements == null) + if (_set == null) { - _elements = new T[10]; + _set = new HashSet(); + _elements = new ArrayBuilder(); } - else if (_elements.Length <= thisValue) + + if (!_set.Add(value)) { - Array.Resize(ref _elements, _elements.Length * 2); + return false; } - _elements[thisValue] = value; + _elements.Add(value); + return true; + } + + public bool Remove(T value) + { + if (!_set.Remove(value)) + { + return false; + } + _elements.RemoveAt(_elements.IndexOf(value)); return true; } - public int Count => (int)_nextElementValue; + public int Count => _elements?.Count ?? 0; - public bool Contains(T value) => _set.Contains(value); + public bool Contains(T value) => _set?.Contains(value) ?? false; public IEnumerator GetEnumerator() - { - for (int i = 0; i < _nextElementValue; i++) yield return _elements[i]; - } + => _elements?.GetEnumerator() ?? SpecializedCollections.EmptyEnumerator(); IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); - /// - /// An enumerable that yields the set's elements in insertion order. - /// - public SetWithInsertionOrder InInsertionOrder => this; - - public ImmutableArray AsImmutable() - { - return (_elements == null) ? ImmutableArray.Empty : ImmutableArray.Create(_elements, 0, (int)_nextElementValue); - } + public ImmutableArray AsImmutable() => _elements.ToImmutableArrayOrEmpty(); } } diff --git a/src/Compilers/VisualBasic/Portable/Symbols/Source/SourceNamedTypeSymbol.vb b/src/Compilers/VisualBasic/Portable/Symbols/Source/SourceNamedTypeSymbol.vb index 9f2d11c928cf2..f78d6cff48547 100644 --- a/src/Compilers/VisualBasic/Portable/Symbols/Source/SourceNamedTypeSymbol.vb +++ b/src/Compilers/VisualBasic/Portable/Symbols/Source/SourceNamedTypeSymbol.vb @@ -1312,7 +1312,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols MakeDeclaredInterfacesInPart(syntaxRef.SyntaxTree, syntaxRef.GetVisualBasicSyntax(), interfaces, basesBeingResolved, diagnostics) Next - Return interfaces.InInsertionOrder.AsImmutable + Return interfaces.AsImmutable End Function Private Function GetInheritsLocation(base As NamedTypeSymbol) As Location diff --git a/src/Compilers/VisualBasic/Portable/VisualBasicParseOptions.vb b/src/Compilers/VisualBasic/Portable/VisualBasicParseOptions.vb index 180e6e5e39bcf..841805d3e18f0 100644 --- a/src/Compilers/VisualBasic/Portable/VisualBasicParseOptions.vb +++ b/src/Compilers/VisualBasic/Portable/VisualBasicParseOptions.vb @@ -47,7 +47,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic languageVersion As LanguageVersion, documentationMode As DocumentationMode, kind As SourceCodeKind, - preprocessorSymbols As IEnumerable(Of KeyValuePair(Of String, Object)), + preprocessorSymbols As ImmutableArray(Of KeyValuePair(Of String, Object)), features As ImmutableDictionary(Of String, String)) MyBase.New(kind, documentationMode) From 343ada1bc119ad8daf4476bf2b5e1ab376695f45 Mon Sep 17 00:00:00 2001 From: Andy Gocke Date: Mon, 24 Jul 2017 11:29:55 -0700 Subject: [PATCH 07/41] Re-baseline some emit tests The frame creation pass now considers scope of captured variable introduction instead of closure creation, so the ordering can change if two or more nested functions access variables 'outside-in', e.g. ```csharp { int x = 0; { int y = 0; int L1() => y; { int z = 0; int L2() => x; } } } ``` If we visit closures in-order in the previous example, we create frames for L1, then L2. If we visit captured variables scopes, however, we visit `x` (captured by L2), then `y` (captued by L1). --- .../Emit/CodeGen/CodeGenClosureLambdaTests.cs | 32 +++++++++---------- .../Emit/CodeGen/CodeGenLocalFunctionTests.cs | 22 +++++++++++++ .../EditAndContinueClosureTests.cs | 4 +-- .../CSharp/Test/Emit/PDB/PDBLambdaTests.cs | 8 ++--- .../CSharp/Test/Emit/PDB/PDBTests.cs | 10 +++--- 5 files changed, 49 insertions(+), 27 deletions(-) diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenClosureLambdaTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenClosureLambdaTests.cs index dc31451ef4438..434f863cea41a 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenClosureLambdaTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenClosureLambdaTests.cs @@ -1015,17 +1015,17 @@ static void Main() { // Code size 131 (0x83) .maxstack 3 - .locals init (Program.<>c__DisplayClass1_2 V_0, //CS$<>8__locals0 - Program.<>c__DisplayClass1_1 V_1, //CS$<>8__locals1 + .locals init (Program.<>c__DisplayClass1_1 V_0, //CS$<>8__locals0 + Program.<>c__DisplayClass1_2 V_1, //CS$<>8__locals1 T V_2) - IL_0000: newobj ""Program.<>c__DisplayClass1_2..ctor()"" + IL_0000: newobj ""Program.<>c__DisplayClass1_1..ctor()"" IL_0005: stloc.0 IL_0006: ldloc.0 IL_0007: ldarg.0 - IL_0008: stfld ""Program.<>c__DisplayClass1_0 Program.<>c__DisplayClass1_2.CS$<>8__locals1"" + IL_0008: stfld ""Program.<>c__DisplayClass1_0 Program.<>c__DisplayClass1_1.CS$<>8__locals1"" IL_000d: ldloc.0 IL_000e: ldstr ""y"" - IL_0013: stfld ""string Program.<>c__DisplayClass1_2.y"" + IL_0013: stfld ""string Program.<>c__DisplayClass1_1.y"" .try { IL_0018: ldstr ""xy"" @@ -1041,17 +1041,17 @@ .maxstack 3 IL_002c: ldc.i4.0 IL_002d: br.s IL_005d IL_002f: unbox.any ""T"" - IL_0034: newobj ""Program.<>c__DisplayClass1_1..ctor()"" + IL_0034: newobj ""Program.<>c__DisplayClass1_2..ctor()"" IL_0039: stloc.1 IL_003a: ldloc.1 IL_003b: ldloc.0 - IL_003c: stfld ""Program.<>c__DisplayClass1_2 Program.<>c__DisplayClass1_1.CS$<>8__locals2"" + IL_003c: stfld ""Program.<>c__DisplayClass1_1 Program.<>c__DisplayClass1_2.CS$<>8__locals2"" IL_0041: stloc.2 IL_0042: ldloc.1 IL_0043: ldloc.2 - IL_0044: stfld ""T Program.<>c__DisplayClass1_1.e"" + IL_0044: stfld ""T Program.<>c__DisplayClass1_2.e"" IL_0049: ldloc.1 - IL_004a: ldftn ""bool Program.<>c__DisplayClass1_1.b__1()"" + IL_004a: ldftn ""bool Program.<>c__DisplayClass1_2.b__1()"" IL_0050: newobj ""System.Func..ctor(object, System.IntPtr)"" IL_0055: callvirt ""bool System.Func.Invoke()"" IL_005a: ldc.i4.0 @@ -1064,8 +1064,8 @@ .maxstack 3 IL_0065: ldarg.0 IL_0066: ldfld ""string Program.<>c__DisplayClass1_0.x"" IL_006b: ldloc.1 - IL_006c: ldfld ""Program.<>c__DisplayClass1_2 Program.<>c__DisplayClass1_1.CS$<>8__locals2"" - IL_0071: ldfld ""string Program.<>c__DisplayClass1_2.y"" + IL_006c: ldfld ""Program.<>c__DisplayClass1_1 Program.<>c__DisplayClass1_2.CS$<>8__locals2"" + IL_0071: ldfld ""string Program.<>c__DisplayClass1_1.y"" IL_0076: call ""string string.Concat(string, string, string)"" IL_007b: call ""void System.Console.Write(string)"" IL_0080: leave.s IL_0082 @@ -3646,12 +3646,12 @@ private bool T() { // Code size 89 (0x59) .maxstack 2 - .locals init (Program.c1.<>c__DisplayClass1_1 V_0) //CS$<>8__locals0 - IL_0000: newobj ""Program.c1.<>c__DisplayClass1_1..ctor()"" + .locals init (Program.c1.<>c__DisplayClass1_0 V_0) //CS$<>8__locals0 + IL_0000: newobj ""Program.c1.<>c__DisplayClass1_0..ctor()"" IL_0005: stloc.0 IL_0006: ldloc.0 IL_0007: ldarg.0 - IL_0008: stfld ""Program.c1 Program.c1.<>c__DisplayClass1_1.<>4__this"" + IL_0008: stfld ""Program.c1 Program.c1.<>c__DisplayClass1_0.<>4__this"" IL_000d: ldarg.0 IL_000e: call ""bool Program.c1.T()"" IL_0013: brfalse.s IL_002e @@ -3664,12 +3664,12 @@ .maxstack 2 IL_002d: ret IL_002e: ldloc.0 IL_002f: ldc.i4.s 42 - IL_0031: stfld ""int Program.c1.<>c__DisplayClass1_1.aaa"" + IL_0031: stfld ""int Program.c1.<>c__DisplayClass1_0.aaa"" IL_0036: ldarg.0 IL_0037: call ""bool Program.c1.T()"" IL_003c: brfalse.s IL_0057 IL_003e: ldloc.0 - IL_003f: ldftn ""bool Program.c1.<>c__DisplayClass1_1.b__2(int)"" + IL_003f: ldftn ""bool Program.c1.<>c__DisplayClass1_0.b__2(int)"" IL_0045: newobj ""System.Func..ctor(object, System.IntPtr)"" IL_004a: ldc.i4.s 42 IL_004c: 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 8e36dd14c2848..39818e6069820 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenLocalFunctionTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenLocalFunctionTests.cs @@ -76,6 +76,28 @@ void L5() }", expectedOutput: @"0 1"); + verifier.VerifyIL("C.M()", @" +{ + // Code size 46 (0x2e) + .maxstack 2 + .locals init (C.<>c__DisplayClass2_0 V_0) //CS$<>8__locals0 + IL_0000: ldloca.s V_0 + IL_0002: ldarg.0 + IL_0003: stfld ""C C.<>c__DisplayClass2_0.<>4__this"" + IL_0008: ldloca.s V_0 + IL_000a: ldc.i4.0 + IL_000b: stfld ""int C.<>c__DisplayClass2_0.var1"" + 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 +}"); + // L1 verifier.VerifyIL("C.g__L12_0(ref C.<>c__DisplayClass2_0)", @" { diff --git a/src/Compilers/CSharp/Test/Emit/Emit/EditAndContinue/EditAndContinueClosureTests.cs b/src/Compilers/CSharp/Test/Emit/Emit/EditAndContinue/EditAndContinueClosureTests.cs index dd7108b21d47c..bea69b33f55b3 100644 --- a/src/Compilers/CSharp/Test/Emit/Emit/EditAndContinue/EditAndContinueClosureTests.cs +++ b/src/Compilers/CSharp/Test/Emit/Emit/EditAndContinue/EditAndContinueClosureTests.cs @@ -271,8 +271,8 @@ int F(int a) // no new synthesized members generated (with #1 in names): diff1.VerifySynthesizedMembers( - "C: {<>c__DisplayClass0_0}", - "C.<>c__DisplayClass0_0: {a, <>4__this, b__0}"); + "C.<>c__DisplayClass0_0: {<>4__this, a, b__0}", + "C: {<>c__DisplayClass0_0}"); var md1 = diff1.GetMetadata(); var reader1 = md1.Reader; diff --git a/src/Compilers/CSharp/Test/Emit/PDB/PDBLambdaTests.cs b/src/Compilers/CSharp/Test/Emit/PDB/PDBLambdaTests.cs index 7a3358dbe1da4..a63ab728ce7e1 100644 --- a/src/Compilers/CSharp/Test/Emit/PDB/PDBLambdaTests.cs +++ b/src/Compilers/CSharp/Test/Emit/PDB/PDBLambdaTests.cs @@ -1128,11 +1128,11 @@ void F() 1 - - - - + + + + diff --git a/src/Compilers/CSharp/Test/Emit/PDB/PDBTests.cs b/src/Compilers/CSharp/Test/Emit/PDB/PDBTests.cs index 4747987146f2d..96e1fef0d7624 100644 --- a/src/Compilers/CSharp/Test/Emit/PDB/PDBTests.cs +++ b/src/Compilers/CSharp/Test/Emit/PDB/PDBTests.cs @@ -2948,12 +2948,12 @@ class Student : Person { public double GPA; } 2 - - - - - + + + + + From 414d7d604d1c881b058a6f5b018f0105fe29ca15 Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Tue, 8 Aug 2017 10:07:41 -0700 Subject: [PATCH 08/41] Turn on ref assemblies in all projects (#21346) --- build/Targets/Settings.props | 1 + build/scripts/build.ps1 | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/build/Targets/Settings.props b/build/Targets/Settings.props index 8d144472bc757..c9a244786f664 100644 --- a/build/Targets/Settings.props +++ b/build/Targets/Settings.props @@ -22,6 +22,7 @@ net46;netcoreapp2.0 strict,IOperation + true Debug true