Organize the local function work

Reorganizes the local function documents into a spec document and a worklist document. Everything in the worklist has been given a separate bug number and been linked to the worklist. The spec has had a preliminary grammar added to it. There has also been a skipped test added for one of the bugs that was filed.
dotnet · Apr 6, 2016 · 78a3324 · 78a3324
1 parent 5d48684
commit 78a3324
Show file tree

Hide file tree

Showing 6 changed files with 210 additions and 72 deletions.
diff --git a/docs/features/local-functions.md b/docs/features/local-functions.md
@@ -3,56 +3,59 @@ Local Functions
 
 This feature is to support the definition of functions in block scope.
 
-Local functions may use variables defined in the enclosing scope. The current implementation requires that every variable read inside a local function be definitely assigned, as if executing the local function at its point of definition. Also, the local function definition must have been "executed" at any use point.
-
-After experimenting with that a bit (for example, it is not possible to define two mutually recursive local functions), we've since revised how we want the definite assignment to work. The revision (not yet implemented) is that all local variables read in a local function must be definitely assigned at each invocation of the local function. That's actually more subtle than it sounds, and there is a bunch of work remaining to make it work. Once it is done you'll be able to move your local functions to the end of its enclosing block.
-
-The new definite assignment rules are incompatible with inferring the return type of a local function, so we'll likely be removing support for inferring the return type.
-
-Unless you convert a local function to a delegate, capturing is done into frames that are value types. That means you don't get any GC pressure from using local functions with capturing.
-
-We don't have a spec yet, but the feature is fairly straightforward.
-
---------------------
-
-Below is a checklist of work on the feature
-- [ ] Parser ambiguity research
-	- Some thought done, not complete
-	- Currently thought to be unambiguous past the parameter list (starting at `{` or `=>`), but that requires lots of lookahead.
-	- See LanguageParser.cs for comments on both ambiguity in standard parsing, and ambiguity in error recovery.
-- [x] N-level nested local functions
-- [x] Capture
-	- Works alongside lambdas and behaves very similarly in fallback cases
-	- Has zero-allocation closures (struct frames by ref) on functions never converted to a delegate and are not an iterator/async
-- [x] Standard parameter features
-	- params
-	- ref/out
-	- named/optional
-- [x] Visibility
-	- May revisit design (currently shadows, may do overloads)
-- [x] Generics
-	- Nongeneric local functions in generic methods (same as lambdas).
-	- Generic local functions in nongeneric methods.
-	- Generic local functions in generic methods.
-	- Arbitrary nesting of generic local functions.
-	- Generic local functions with constraints.
-- [x] Inferred return type
-- [x] Iterators
-- [x] Async
-- [ ] API
-- [ ] Editor
-	- [x] Basic features (variable highlight, rename, etc.)
-	- [ ] Advanced features (refactorings, analyzers)
-
-Intentionally disabled features that might eventually be in the end result:
-- Calling a local function with a dynamic argument (due to name mangling and potential conflicts with closures and possibly overloads/shadows)
-- Referring to a local function in an expression tree (note that defining a local function itself in an expression tree is impossible)
-- Definite assignment rules: Currently, all captured variables must be assigned at point of local function declaration. Might change to requiring assignment at point of local function use, not declaration (would allow mutually recursive local functions without nesting, etc.). Related to "Visibility" point in main list.
-
-TODO:
-
-- Update error messages.
-	- `LocalScopeBinder.ReportConflictWithLocal()` (twice)
-- `LocalScopeBinder.EnsureSingleDefinition()`, handle case where 'name' exists in both `localsMap` and `localFunctionsMap`. Might be related to `LocalFunctionTests.NameConflictLocalVarLast()`
-- Defining a local function with a dynamic parameter doesn't work at runtime.
-- Return type of `var` is broken - see LocalFunctionSymbol.cs for an explanation. Fixing it will require a large rewrite of much of return type analysis, as the current system assumes that all return types are known (mostly) without examining the method body.
+TODO: _WRITE SPEC_
+
+
+Syntax Grammar
+==============
+
+This grammar is represented as a diff from the current spec grammar.
+
+```diff
+declaration-statement
+    : local-variable-declaration ';'
+    | local-constant-declaration ';'
++   | local-function-declaration
+    ;
+
++local-function-declaration
++   : local-function-header local-function-body
++   ;
+
++local-function-header
++   : local-function-modifiers? return-type identifier type-parameter-list?
++       ( formal-parameter-list? ) type-parameter-constraints-clauses
++   ;
+
++local-function-modifiers
++   : async
++   ;
+
++local-function-body
++   : block
++   | arrow-expression-body
++   ;
+```
+
+Local functions may use variables defined in the enclosing scope. The current
+implementation requires that every variable read inside a local function be
+definitely assigned, as if executing the local function at its point of
+definition. Also, the local function definition must have been "executed" at
+any use point.
+
+After experimenting with that a bit (for example, it is not possible to define
+two mutually recursive local functions), we've since revised how we want the
+definite assignment to work. The revision (not yet implemented) is that all
+local variables read in a local function must be definitely assigned at each
+invocation of the local function. That's actually more subtle than it sounds,
+and there is a bunch of work remaining to make it work. Once it is done you'll
+be able to move your local functions to the end of its enclosing block.
+
+The new definite assignment rules are incompatible with inferring the return
+type of a local function, so we'll likely be removing support for inferring the
+return type.
+
+Unless you convert a local function to a delegate, capturing is done into
+frames that are value types. That means you don't get any GC pressure from
+using local functions with capturing.
+
diff --git a/docs/features/local-functions.work.md b/docs/features/local-functions.work.md
@@ -0,0 +1,65 @@
+Local Function Status
+=====================
+
+This is a checklist for current and outstanding work on the local functions
+feature, as spec'd in [local-functions.md](./local-functions.md).
+
+--------------------
+
+Known issues
+============
+
+Compiler:
+
+- [ ] Parser builds nodes for local functions when feature not enabled (#9940)
+- [ ] Compiler crash: base call to state machine method, in state machine
+  method (#9872)
+- [ ] Need custom warning for unused local function (#9661)
+- [ ] Generate quick action does not offer to generate local (#9352)
+- [ ] Parser ambiguity research (#10388)
+- [ ] Dynamic support (#10389)
+- [ ] Referring to local function in expression tree (#10390)
+- [ ] Resolve definite assignment rules (#10391)
+- [ ] Remove support for `var` return type (#10392)
+- [ ] Update error messages (#10393)
+
+IDE:
+
+- [ ] Some selections around local functions fail extract method refactoring
+  [ ] (#8719)
+- [ ] Extracting local function passed to an Action introduces compiler error
+  [ ] (#8718)
+- [ ] Ctrl+. on a delegate invocation crashes VS (via Extract method) (#8717)
+- [ ] Inline temp introductes compiler error (#8716)
+- [ ] Call hierarchy search never terminates on local functions (#8654)
+- [ ] Nav bar doesn't support local functions (#8648)
+- [ ] No outlining for local functions (#8647)
+- [ ] Squiggles all over the place when using an unsupported modifier (#8645)
+- [ ] Peek definition errors out on local function (#8644)
+- [ ] Void keyword not recommended while declaring local function (#8616)
+- [ ] Change signature doesn't update the signature of the local function (#8539)
+
+
+Feature Completeness Progress
+=============================
+
+- [x] N-level nested local functions
+- [x] Capture
+    - Works alongside lambdas and behaves very similarly in fallback cases
+    - Has zero-allocation closures (struct frames by ref) on functions never
+      converted to a delegate and are not an iterator/async
+- [x] Standard parameter features
+    - params
+    - ref/out
+    - named/optional
+- [x] Visibility
+    - May revisit design (currently shadows, may do overloads)
+- [x] Generics
+    - Nongeneric local functions in generic methods (same as lambdas).
+    - Generic local functions in nongeneric methods.
+    - Generic local functions in generic methods.
+    - Arbitrary nesting of generic local functions.
+    - Generic local functions with constraints.
+- [x] Inferred return type
+- [x] Iterators
+- [x] Async
diff --git a/src/Compilers/CSharp/Test/Syntax/CSharpCompilerSyntaxTest.csproj b/src/Compilers/CSharp/Test/Syntax/CSharpCompilerSyntaxTest.csproj
@@ -89,6 +89,7 @@
     <Compile Include="Parsing\CrefParsingTests.cs" />
     <Compile Include="Parsing\DeclarationParsingTests.cs" />
     <Compile Include="Parsing\ExpressionParsingTests.cs" />
+    <Compile Include="Parsing\LocalFunctionParsingTests.cs" />
     <Compile Include="Parsing\ParserRegressionTests.cs" />
     <Compile Include="Parsing\ScriptParsingTests.cs" />
     <Compile Include="Parsing\LambdaParameterParsingTests.cs" />
@@ -149,4 +150,4 @@
     <Import Project="..\..\..\..\..\build\Targets\VSL.Imports.targets" />
     <Import Project="..\..\..\..\..\build\Targets\Roslyn.Toolsets.Xunit.targets" />
   </ImportGroup>
-</Project>
+</Project>
diff --git a/src/Compilers/CSharp/Test/Syntax/Parsing/DeclarationParsingTests.cs b/src/Compilers/CSharp/Test/Syntax/Parsing/DeclarationParsingTests.cs
@@ -14,17 +14,7 @@ public class DeclarationParsingTests : ParsingTests
     {
         protected override SyntaxTree ParseTree(string text, CSharpParseOptions options)
         {
-            return SyntaxFactory.ParseSyntaxTree(text);
-        }
-
-        private CompilationUnitSyntax ParseFile(string text, CSharpParseOptions parseOptions = null)
-        {
-            return SyntaxFactory.ParseCompilationUnit(text, options: parseOptions);
-        }
-
-        private CompilationUnitSyntax ParseFileExperimental(string text)
-        {
-            return ParseFile(text, parseOptions: TestOptions.ExperimentalParseOptions);
+            return SyntaxFactory.ParseSyntaxTree(text, options);
         }
 
         [Fact]

diff --git a/src/Compilers/CSharp/Test/Syntax/Parsing/LocalFunctionParsingTests.cs b/src/Compilers/CSharp/Test/Syntax/Parsing/LocalFunctionParsingTests.cs
@@ -0,0 +1,75 @@
+// 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 Microsoft.CodeAnalysis.CSharp.Syntax;
+using System.Collections.Generic;
+using System.Linq;
+using Xunit;
+
+namespace Microsoft.CodeAnalysis.CSharp.UnitTests
+{
+    public class LocalFunctionParsingTests : ParsingTests
+    {
+        [Fact(Skip = "https://github.com/dotnet/roslyn/issues/10388")]
+        public void LocalFunctionsWithAwait()
+        {
+            var file = ParseFileExperimental(@"class c
+{
+    void m1() { await await() => new await(); }
+    void m2() { await () => new await(); }
+    async void m3() { await () => new await(); }
+    void m4() { async await() => new await(); }
+}");
+
+            Assert.NotNull(file);
+            var c = (ClassDeclarationSyntax)file.Members.Single();
+            Assert.Equal(4, c.Members.Count);
+
+            {
+                Assert.Equal(SyntaxKind.MethodDeclaration, c.Members[0].Kind());
+                var m1 = (MethodDeclarationSyntax)c.Members[0];
+                Assert.Equal(0, m1.Modifiers.Count);
+                Assert.Equal(1, m1.Body.Statements.Count);
+                Assert.Equal(SyntaxKind.LocalFunctionStatement, m1.Body.Statements[0].Kind());
+                var s1 = (LocalFunctionStatementSyntax)m1.Body.Statements[0];
+                Assert.False(s1.HasErrors);
+                Assert.Equal(0, s1.Modifiers.Count);
+                Assert.Equal("await", s1.ReturnType.ToString());
+                Assert.Equal("await", s1.Identifier.ToString());
+                Assert.Null(s1.TypeParameterList);
+                Assert.Equal(0, s1.ParameterList.ParameterCount);
+                Assert.Null(s1.Body);
+                Assert.NotNull(s1.ExpressionBody);
+            }
+
+            {
+                Assert.Equal(SyntaxKind.MethodDeclaration, c.Members[1].Kind());
+                var m2 = (MethodDeclarationSyntax)c.Members[1];
+                Assert.Equal(0, m2.Modifiers.Count);
+                Assert.Equal(2, m2.Body.Statements.Count);
+                Assert.Equal(SyntaxKind.ExpressionStatement, m2.Body.Statements[0].Kind());
+                var s1 = (ExpressionStatementSyntax)m2.Body.Statements[0];
+                Assert.Equal(SyntaxKind.InvocationExpression, s1.Expression.Kind());
+                var e1 = (InvocationExpressionSyntax)s1.Expression;
+                Assert.Equal("await", e1.Expression.ToString());
+                Assert.Equal(0, e1.ArgumentList.Arguments.Count);
+                Assert.True(s1.SemicolonToken.IsMissing);
+                Assert.Equal("=> ", s1.GetTrailingTrivia().ToFullString());
+            }
+
+            {
+                Assert.Equal(SyntaxKind.MethodDeclaration, c.Members[2].Kind());
+                var m3 = (MethodDeclarationSyntax)c.Members[2];
+                Assert.Equal(1, m3.Modifiers.Count);
+                Assert.Equal("async", m3.Modifiers.Single().ToString());
+                Assert.Equal(2, m3.Body.Statements.Count);
+                Assert.Equal(SyntaxKind.ExpressionStatement, m3.Body.Statements[0].Kind());
+                var s1 = (ExpressionStatementSyntax)m3.Body.Statements[0];
+                Assert.Equal(SyntaxKind.AwaitExpression, s1.Expression.Kind());
+                var e1 = (AwaitExpressionSyntax)s1.Expression;
+                Assert.Equal(SyntaxKind.SimpleLambdaExpression, e1.Expression.Kind());
+                Assert.True(s1.SemicolonToken.IsMissing);
+                Assert.Equal("=> ", s1.GetTrailingTrivia().ToFullString());
+            }
+        }
+    }
+}
diff --git a/src/Compilers/CSharp/Test/Syntax/Parsing/ParsingTests.cs b/src/Compilers/CSharp/Test/Syntax/Parsing/ParsingTests.cs
@@ -1,7 +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.
 
-//#define PARSING_TESTS_DUMP
-
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.CSharp.Test.Utilities;
 using System.Collections.Generic;
 using System.Diagnostics;
 using Xunit;
@@ -12,12 +12,16 @@ public abstract class ParsingTests
     {
         private IEnumerator<SyntaxNodeOrToken> _treeEnumerator;
 
-        protected abstract SyntaxTree ParseTree(string text, CSharpParseOptions options);
+        protected virtual SyntaxTree ParseTree(string text, CSharpParseOptions options) => SyntaxFactory.ParseSyntaxTree(text, options);
 
-        protected virtual CSharpSyntaxNode ParseNode(string text, CSharpParseOptions options)
-        {
-            return ParseTree(text, options).GetCompilationUnitRoot();
-        }
+        public CompilationUnitSyntax ParseFile(string text, CSharpParseOptions parseOptions = null) =>
+            SyntaxFactory.ParseCompilationUnit(text, options: parseOptions);
+
+        public CompilationUnitSyntax ParseFileExperimental(string text) =>
+            ParseFile(text, parseOptions: TestOptions.ExperimentalParseOptions);
+
+        protected virtual CSharpSyntaxNode ParseNode(string text, CSharpParseOptions options) =>
+            ParseTree(text, options).GetCompilationUnitRoot();
 
         /// <summary>
         /// Parses given string and initializes a depth-first preorder enumerator.