Async-iterator method can return `IAsyncEnumerator<T>` #31114

jcouv · 2018-11-12T06:48:16Z

Aside from not implementing the IAsyncEnumerable<T> interface (and its GetAsyncEnumerator method), the state machine also does not need to hold proxied/preserved parameters, or to preserve the original threadID.

Also integrated with #31072 (nullability analysis of yield return).

jcouv · 2018-11-19T15:55:53Z

@dotnet/roslyn-compiler for review. Thanks
This allows async-iterator methods to return IAsyncEnumerator<T> and in that case, we can produce a simplified state machine (removing some machinery that serves only for enumerable case). #Resolved

jcouv · 2018-11-26T15:25:08Z

@dotnet/roslyn-compiler for review. Thanks

cston · 2018-11-26T18:01:41Z

src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenAsyncIteratorTests.cs

+    async System.Collections.Generic.IAsyncEnumerator<int> M() { await Task.CompletedTask; yield return 3; }
+}
+";
+        private void VerifyMissingMember(WellKnownMember member, params DiagnosticDescription[] expected)


private [](start = 8, length = 7)

static #Resolved

cston · 2018-11-26T18:01:49Z

src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenAsyncIteratorTests.cs

+        private static void VerifyMissingMember(string source, WellKnownMember member, params DiagnosticDescription[] expected)
+        {
+            var lib = CreateCompilationWithTasksExtensions(AsyncStreamsTypes);
+            var lib_ref = lib.EmitToImageReference();
            var comp = CreateCompilationWithTasksExtensions(source, references: new[] { lib_ref });
            comp.MakeMemberMissing(member);
            comp.VerifyEmitDiagnostics(expected);
        }

        private void VerifyMissingType(WellKnownType type, params DiagnosticDescription[] expected)


private [](start = 8, length = 7)

static #Resolved

cston · 2018-11-26T18:09:35Z

src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenAsyncIteratorTests.cs

-            VerifyMissingType(WellKnownType.System_Collections_Generic_IAsyncEnumerable_T,
+            VerifyMissingMember(_enumerator, WellKnownMember.System_Collections_Generic_IAsyncEnumerable_T__GetAsyncEnumerator);
+
+            VerifyMissingType(_enumerable, WellKnownType.System_Collections_Generic_IAsyncEnumerable_T,
                // (5,60): error CS1983: The return type of an async method must be void, Task, Task<T>, a task-like type, or IAsyncEnumerable<T>


or IAsyncEnumerable [](start = 123, length = 22)

Should the message include IAsyncEnumerator<T> as well? #Resolved

cston · 2018-11-26T18:14:19Z

src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenAsyncIteratorTests.cs

+            comp.VerifyDiagnostics();
+            CompileAndVerify(comp, expectedOutput: "Value:0 1 2 Value:3 4 Value:5 Done");
+
+            var comp2 = CreateCompilationWithTasksExtensions("", references: new[] { comp.EmitToImageReference() });


CreateCompilationWithTasksExtensions [](start = 24, length = 36)

Are we creating a second compilation to check PE symbols? If so, consider using CompileAndVerify(..., symbolValidator=...) instead, for consistency with other tests. #Resolved

cston · 2018-11-26T18:20:26Z

src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenAsyncIteratorTests.cs

@@ -1008,6 +1302,37 @@ await using (var enumerator2 = enumerable.GetAsyncEnumerator())
            var comp = CreateCompilationWithTasksExtensions(new[] { source, AsyncStreamsTypes }, options: TestOptions.DebugExe);
            comp.VerifyDiagnostics();
            CompileAndVerify(comp, expectedOutput: "1 2 Stream1:3 1 2 Stream2:3 4 2 4 2 Done");
+
+            var comp2 = CreateCompilationWithTasksExtensions("", references: new[] { comp.EmitToImageReference() });


CreateCompilationWithTasksExtensions("", [](start = 24, length = 40)

Consider adding symbolValidator=... to the CompileAndVerify call above instead. #Resolved

cston · 2018-11-26T18:29:40Z

src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AsyncStateMachine.cs

@@ -31,8 +31,12 @@ public AsyncStateMachine(VariableSlotAllocator variableAllocatorOpt, TypeCompila
                var elementType = TypeMap.SubstituteType(asyncMethod.IteratorElementType).TypeSymbol;
                this.IteratorElementType = elementType;

-                // IAsyncEnumerable<TResult>
-                interfaces.Add(compilation.GetWellKnownType(WellKnownType.System_Collections_Generic_IAsyncEnumerable_T).Construct(elementType));
+                bool isEnumerable = asyncMethod.IsIAsyncEnumerableReturningAsync(asyncMethod.DeclaringCompilation);


asyncMethod.DeclaringCompilation [](start = 81, length = 32)

compilation #Resolved

jcouv · 2018-11-27T15:47:28Z

@dotnet/roslyn-compiler for second review. Thanks

agocke · 2018-11-27T21:36:56Z

src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenAsyncIteratorTests.cs

@@ -9,6 +9,7 @@

 namespace Microsoft.CodeAnalysis.CSharp.UnitTests.CodeGen
 {
+    using System.Linq;


I believe our style guidelines specify that all usings must go outside the namespace. #Resolved

agocke · 2018-11-27T21:52:45Z

src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenAsyncIteratorTests.cs

+            VerifyMissingMember(_enumerator, WellKnownMember.System_Collections_Generic_IAsyncEnumerable_T__GetAsyncEnumerator);
+
+            VerifyMissingType(_enumerable, WellKnownType.System_Collections_Generic_IAsyncEnumerable_T,
+                // (5,60): error CS1983: The return type of an async method must be void, Task, Task<T>, a task-like type, IAsyncEnumerable<T>, or IAsyncEnumerator<T>


Is this testing what happens when you have something like

using System.Threading.Tasks; class C { async System.Collections.Generic.IAsyncEnumerable<int> M() { await Task.CompletedTask; yield return 3; } }

but IAsyncEnumerable<int> is missing? If so, is this error message useful? Should we just skip it if the return type is an error type? #Resolved

Yes that's what this tests.
Unfortunately, the MakeTypeMissing infrastructure doesn't fully simulate a type being missing. It only makes the type disappear from GetWellKnownType.

I'll adjust the test to show what users actually would see. #Resolved

agocke

LGTM aside from minor cascading error question

jcouv added Area-Compilers PR For Personal Review Only The PR doesn’t require anyone other than the developer to review it. New Feature - Async Streams Async Streams labels Nov 12, 2018

jcouv added this to the 16.0.P2 milestone Nov 12, 2018

jcouv self-assigned this Nov 12, 2018

jcouv requested a review from a team as a code owner November 12, 2018 06:48

jcouv force-pushed the async-enumerator branch from a96458c to dd58e42 Compare November 12, 2018 07:07

This was referenced Nov 12, 2018

[Umbrella] Work items and test plan for async streams #24037

Closed

Nullable analysis of yield return #31072

Merged

jasonmalinowski changed the base branch from dev16.0-preview2 to master November 16, 2018 19:40

Async-iterator method can return IAsyncEnumerator<T>

09ed675

jcouv force-pushed the async-enumerator branch from dd58e42 to 09ed675 Compare November 18, 2018 04:28

jcouv removed the PR For Personal Review Only The PR doesn’t require anyone other than the developer to review it. label Nov 18, 2018

cston reviewed Nov 26, 2018

View reviewed changes

cston approved these changes Nov 26, 2018

View reviewed changes

Address feedback from Chuck

2de8af0

agocke reviewed Nov 27, 2018

View reviewed changes

agocke approved these changes Nov 27, 2018

View reviewed changes

Address feedback from Andy

75d985d

jcouv merged commit 259cae0 into dotnet:master Nov 28, 2018

jcouv deleted the async-enumerator branch November 28, 2018 02:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Async-iterator method can return `IAsyncEnumerator<T>` #31114

Async-iterator method can return `IAsyncEnumerator<T>` #31114

jcouv commented Nov 12, 2018 •

edited

Loading

jcouv commented Nov 19, 2018 •

edited

Loading

jcouv commented Nov 26, 2018

cston Nov 26, 2018 •

edited by jcouv

Loading

cston Nov 26, 2018 •

edited by jcouv

Loading

cston Nov 26, 2018 •

edited by jcouv

Loading

cston Nov 26, 2018 •

edited by jcouv

Loading

cston Nov 26, 2018 •

edited by jcouv

Loading

cston Nov 26, 2018 •

edited by jcouv

Loading

jcouv commented Nov 27, 2018

agocke Nov 27, 2018 •

edited by jcouv

Loading

agocke Nov 27, 2018 •

edited by jcouv

Loading

jcouv Nov 28, 2018 •

edited

Loading

agocke left a comment

Async-iterator method can return IAsyncEnumerator<T> #31114

Async-iterator method can return IAsyncEnumerator<T> #31114

Conversation

jcouv commented Nov 12, 2018 • edited Loading

jcouv commented Nov 19, 2018 • edited Loading

jcouv commented Nov 26, 2018

cston Nov 26, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

cston Nov 26, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

cston Nov 26, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

cston Nov 26, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

cston Nov 26, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

cston Nov 26, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

jcouv commented Nov 27, 2018

agocke Nov 27, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

agocke Nov 27, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

jcouv Nov 28, 2018 • edited Loading

Choose a reason for hiding this comment

agocke left a comment

Choose a reason for hiding this comment

Async-iterator method can return `IAsyncEnumerator<T>` #31114

Async-iterator method can return `IAsyncEnumerator<T>` #31114

jcouv commented Nov 12, 2018 •

edited

Loading

jcouv commented Nov 19, 2018 •

edited

Loading

cston Nov 26, 2018 •

edited by jcouv

Loading

cston Nov 26, 2018 •

edited by jcouv

Loading

cston Nov 26, 2018 •

edited by jcouv

Loading

cston Nov 26, 2018 •

edited by jcouv

Loading

cston Nov 26, 2018 •

edited by jcouv

Loading

cston Nov 26, 2018 •

edited by jcouv

Loading

agocke Nov 27, 2018 •

edited by jcouv

Loading

agocke Nov 27, 2018 •

edited by jcouv

Loading

jcouv Nov 28, 2018 •

edited

Loading