Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Async streams #1730

Merged
merged 20 commits into from Nov 4, 2019

Conversation

@dgrunwald
Copy link
Member

dgrunwald commented Sep 29, 2019

This branch adds support for:

  • async IAsyncEnumerable<T> methods
  • await using
  • Test cases can now use .NET Core 3
  • Fixed some of the bugs discovered when trying to run all tests with .NET Core 3 (mostly wrt. nullable reference types) -- but then we decided to make this a per-test opt-in, as there is still some work remaining in this area.
  • Fixes #1640 and #1780
dgrunwald and others added 17 commits Sep 29, 2019
…s code now results in oblivious `string` instead of `string!`.
This could cause our overload resolution to consider an overload as not-applicable when it actually is applicable. This could cause us to miss some cases where we need to insert casts.
@dgrunwald

This comment has been minimized.

Copy link
Member Author

dgrunwald commented Sep 29, 2019

Screenshot of current status:
image
Note that there's a lot of stuff already happening in this example:

  • The state machine with a mixture of await and yield return yield points is turned back to control flow
  • Catch+rethrow is turned back into try-finally with await asyncEnumerator.DisposeAsync();
  • try-finally is converted into await using
    And only the last step reconstructing the await foreach loop is missing.
Remove [AsyncIteratorStateMachine] attribute and left-over ldc.i4 instructions.
@dgrunwald dgrunwald changed the title [WIP] Async streams Async streams Sep 30, 2019
@dgrunwald

This comment has been minimized.

Copy link
Member Author

dgrunwald commented Sep 30, 2019

This feature could use more tests; but otherwise I think it's ready to be merged.
await foreach can be left for a future PR.

return false;
if (!SemanticHelper.IsPure(stloc.Value.Flags))
return false;
}

This comment was marked as resolved.

Copy link
@yyjdelete

yyjdelete Sep 30, 2019

Contributor

Missing an pos --; here

@yyjdelete

This comment has been minimized.

Copy link
Contributor

yyjdelete commented Oct 1, 2019

Some others limited(just some enhancement). Attached below, incase also need them.

  1. yield break in async IAsyncEnumerable<T> is not supported;

Should not see "/TRY/END" and "/END" when Run with Test(0)

        async IAsyncEnumerable<int> Test(int i = 0)
        {
            try
            {
                Console.WriteLine("/TRY");
                try
                {
                    Console.WriteLine("/TRY/TRY");
                    if (i == 0)
                        yield break;
                }
                finally
                {
                    Console.WriteLine("/TRY/FINALLY");
                }
                Console.WriteLine("/TRY/END");
            }
            finally
            {
                Console.WriteLine("/FINALLY");
            }
            Console.WriteLine("/END");
        }
  1. EnumeratorCancellationAttribute is not supported.

See combinedTokens in the CompilerGenerated class, it break the check for SetResult and others.

        public static async IAsyncEnumerable<int> MainXZ2([EnumeratorCancellation] CancellationToken ct = default)
        {
            while (true) { ct.ThrowIfCancellationRequested(); }
            yield break;
        }
  1. ICSharpCode.ILSpy.AssertionFailedException: 'Should not insert using declaration for namespace that is missing from the superset: System.Runtime.ExceptionServices'

The same code with 4, it doesn't happen for Task, the `when (i > 0) is not needed for 3.

  1. Moved to separated issue #1749
    (Also for Task)OpCode not supported: BlockContainer for when()
> The `when (i > 0)` is needed for 4, and this also happen for Task.
        public static async IAsyncEnumerable<int> TestTethrow()
        {
            int i = 100;
            try
            {
                await Task.Yield();
            }
            catch (Exception ex) when (i > 0)
            {
                if (i == 100)
                    throw;
                await Task.Yield();
            }
            yield break;
        }

Is it possible to still show the code for block, just with some warn here? Also for some other thing unsupported like when (ex is XXException xxEx ? xxEx.A && xxEx.B : false)
=>

when(/*OpCode not supported: BlockContainer*/((Func<bool>)(()=>{
    //when
    var xxEx = ex as XXException;
    if (xxEx != null)
    {
        return xxEx.A && xxEx.B;
    }
    else
    {
        return false;
    }
})).Invoke())
{
    //catch
}
@yyjdelete

This comment has been minimized.

Copy link
Contributor

yyjdelete commented Oct 22, 2019

Not sure, but the logic of async IAsyncEnumerable<T> may changed after https://github.com/dotnet/roslyn/pull/39436 is applied.

@yyjdelete

This comment has been minimized.

Copy link
Contributor

yyjdelete commented Oct 22, 2019

var catchBlock = YieldReturnDecompiler.SingleBlock(handler.Body);
catchHandlerOffset = catchBlock.StartILOffset;
if (catchBlock?.Instructions.Count != 4)
throw new SymbolicAnalysisFailedException();

catchBlock is used before null check.

@dgrunwald

This comment has been minimized.

Copy link
Member Author

dgrunwald commented Oct 31, 2019

Thanks for the review. I've fixed the missing null checks.

yield break: The yield break; itself is fixed; but some if (stateVar == -1) checks in finally blocks remain.

EnumeratorCancellationAttribute: I think we'll handle this later; I've created #1772 for this feature.

dotnet/roslyn#39436: I'll leave this for later, when the Roslyn change is released and we can update our NuGet references to run our tests with the new compiler.

@dgrunwald dgrunwald merged commit cb301bb into master Nov 4, 2019
5 of 7 checks passed
5 of 7 checks passed
icsharpcode.ILSpy Build #6.0.0.5330-merge-alpha1-pr1730 failed
Details
icsharpcode.ILSpy (Build Config_Release_Store) Build Config_Release_Store failed
Details
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
icsharpcode.ILSpy (Build Config_Debug_Zip) Build Config_Debug_Zip succeeded
Details
icsharpcode.ILSpy (Build Config_Release_CI) Build Config_Release_CI succeeded
Details
icsharpcode.ILSpy (Build Config_Release_Zip) Build Config_Release_Zip succeeded
Details
@christophwille christophwille deleted the async-streams branch Nov 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.