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: minimal test for IOperation and CFG. Improve diagnostics #30363

Merged
merged 9 commits into from
Nov 9, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions docs/features/async-streams.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ async IAsyncEnumerable<int> GetValuesFromServer()

**open issue**: Design async LINQ

### Detailed design for `await using` statement

An asynchronous `using` is lowered just like a regular `using`, except that `Dispose()` is replaced with `await DisposeAsync()`.

### Detailed design for `await foreach` statement

An `await foreach` is lowered just like a regular `foreach`, except that:
Expand Down
5 changes: 1 addition & 4 deletions src/Compilers/CSharp/Portable/Binder/ForEachLoopBinder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1033,10 +1033,7 @@ private MethodSymbol PerformForEachPatternOverloadResolution(TypeSymbol patternT
}

overloadResolutionResult.Free();
if (arguments.Arguments.Count == 0)
{
arguments.Free();
}
arguments.Free();
333fred marked this conversation as resolved.
Show resolved Hide resolved
typeArguments.Free();

return result;
Expand Down
92 changes: 55 additions & 37 deletions src/Compilers/CSharp/Portable/Binder/UsingStatementBinder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -64,66 +64,38 @@ internal override BoundStatement BindUsingStatementParts(DiagnosticBag diagnosti
bool hasAwait = _syntax.AwaitKeyword.Kind() != default;

Debug.Assert((expressionSyntax == null) ^ (declarationSyntax == null)); // Can't have both or neither.
TypeSymbol disposableInterface = getDisposableInterface(hasAwait);

TypeSymbol iDisposable = hasAwait
? this.Compilation.GetWellKnownType(WellKnownType.System_IAsyncDisposable)
: this.Compilation.GetSpecialType(SpecialType.System_IDisposable);

Debug.Assert((object)iDisposable != null);
bool hasErrors = ReportUseSiteDiagnostics(iDisposable, diagnostics, hasAwait ? _syntax.AwaitKeyword : _syntax.UsingKeyword);
Debug.Assert((object)disposableInterface != null);
bool hasErrors = ReportUseSiteDiagnostics(disposableInterface, diagnostics, hasAwait ? _syntax.AwaitKeyword : _syntax.UsingKeyword);

Conversion iDisposableConversion = Conversion.NoConversion;
BoundMultipleLocalDeclarations declarationsOpt = null;
BoundExpression expressionOpt = null;
AwaitableInfo awaitOpt = null;
TypeSymbol declarationTypeOpt = null;

if (expressionSyntax != null)
{
expressionOpt = this.BindTargetExpression(diagnostics, originalBinder);

HashSet<DiagnosticInfo> useSiteDiagnostics = null;
iDisposableConversion = originalBinder.Conversions.ClassifyImplicitConversionFromExpression(expressionOpt, iDisposable, ref useSiteDiagnostics);
diagnostics.Add(expressionSyntax, useSiteDiagnostics);

if (!iDisposableConversion.IsImplicit)
{
TypeSymbol expressionType = expressionOpt.Type;
if ((object)expressionType == null || !expressionType.IsErrorType())
{
Error(diagnostics, hasAwait ? ErrorCode.ERR_NoConvToIAsyncDisp : ErrorCode.ERR_NoConvToIDisp, expressionSyntax, expressionOpt.Display);
}
hasErrors = true;
}
hasErrors |= !initConversion(fromExpression: true);
}
else
{
ImmutableArray<BoundLocalDeclaration> declarations;
originalBinder.BindForOrUsingOrFixedDeclarations(declarationSyntax, LocalDeclarationKind.UsingVariable, diagnostics, out declarations);

Debug.Assert(!declarations.IsEmpty);

declarationsOpt = new BoundMultipleLocalDeclarations(declarationSyntax, declarations);
declarationTypeOpt = declarations[0].DeclaredType.Type;

TypeSymbol declType = declarations[0].DeclaredType.Type;

if (declType.IsDynamic())
if (declarationTypeOpt.IsDynamic())
Copy link
Member

@333fred 333fred Nov 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

declarationTypeOpt [](start = 20, length = 18)

If this is optional, consider ?. instead. If it is not optional, consider renaming. #Resolved

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's optional in the method, but it's always set in this branch, so I don't see a great way. I could have an extra local for the branch, but that seems overkill :-(


In reply to: 232006228 [](ancestors = 232006228)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could assert, but I'm ok with this.


In reply to: 232067572 [](ancestors = 232067572,232006228)

{
iDisposableConversion = Conversion.ImplicitDynamic;
}
else
{
HashSet<DiagnosticInfo> useSiteDiagnostics = null;
iDisposableConversion = originalBinder.Conversions.ClassifyImplicitConversionFromType(declType, iDisposable, ref useSiteDiagnostics);
diagnostics.Add(declarationSyntax, useSiteDiagnostics);

if (!iDisposableConversion.IsImplicit)
{
if (!declType.IsErrorType())
{
Error(diagnostics, hasAwait ? ErrorCode.ERR_NoConvToIAsyncDisp : ErrorCode.ERR_NoConvToIDisp, declarationSyntax, declType);
}

hasErrors = true;
}
hasErrors |= !initConversion(fromExpression: false);
}
}

Expand All @@ -149,6 +121,52 @@ internal override BoundStatement BindUsingStatementParts(DiagnosticBag diagnosti
boundBody,
awaitOpt,
hasErrors);

bool initConversion(bool fromExpression)
{
HashSet<DiagnosticInfo> useSiteDiagnostics = null;

iDisposableConversion = classifyConversion(fromExpression, disposableInterface, ref useSiteDiagnostics);

diagnostics.Add(fromExpression ? (CSharpSyntaxNode)expressionSyntax : declarationSyntax, useSiteDiagnostics);

if (iDisposableConversion.IsImplicit)
{
return true;
}

TypeSymbol type = fromExpression ? expressionOpt.Type : declarationTypeOpt;
if (type is null || !type.IsErrorType())
{
// Retry with a different assumption about whether the `using` is async
TypeSymbol alternateInterface = getDisposableInterface(!hasAwait);
HashSet<DiagnosticInfo> ignored = null;
Conversion alternateConversion = classifyConversion(fromExpression, alternateInterface, ref ignored);

bool wrongAsync = alternateConversion.IsImplicit;
ErrorCode errorCode = wrongAsync
? (hasAwait ? ErrorCode.ERR_NoConvToIAsyncDispWrongAsync : ErrorCode.ERR_NoConvToIDispWrongAsync)
: (hasAwait ? ErrorCode.ERR_NoConvToIAsyncDisp : ErrorCode.ERR_NoConvToIDisp);

Error(diagnostics, errorCode, (CSharpSyntaxNode)declarationSyntax ?? expressionSyntax, declarationTypeOpt ?? expressionOpt.Display);
}

return false;
333fred marked this conversation as resolved.
Show resolved Hide resolved
}

Conversion classifyConversion(bool fromExpression, TypeSymbol targetInterface, ref HashSet<DiagnosticInfo> diag)
{
return fromExpression?
originalBinder.Conversions.ClassifyImplicitConversionFromExpression(expressionOpt, targetInterface, ref diag) :
originalBinder.Conversions.ClassifyImplicitConversionFromType(declarationTypeOpt, targetInterface, ref diag);
}

TypeSymbol getDisposableInterface(bool isAsync)
{
return isAsync
? this.Compilation.GetWellKnownType(WellKnownType.System_IAsyncDisposable)
: this.Compilation.GetSpecialType(SpecialType.System_IDisposable);
}
}

internal override ImmutableArray<LocalSymbol> GetDeclaredLocalsForScope(SyntaxNode scopeDesignator)
Expand Down
33 changes: 25 additions & 8 deletions src/Compilers/CSharp/Portable/CSharpResources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 11 additions & 5 deletions src/Compilers/CSharp/Portable/CSharpResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -907,7 +907,7 @@
<value>foreach requires that the return type '{0}' of '{1}' must have a suitable public 'MoveNext' method and public 'Current' property</value>
</data>
<data name="ERR_BadGetAsyncEnumerator" xml:space="preserve">
<value>Async foreach requires that the return type '{0}' of '{1}' must have a suitable public 'MoveNextAsync' method and public 'Current' property</value>
<value>Asynchronous foreach requires that the return type '{0}' of '{1}' must have a suitable public 'MoveNextAsync' method and public 'Current' property</value>
</data>
<data name="ERR_TooManyLocals" xml:space="preserve">
<value>Only 65534 locals, including those generated by the compiler, are allowed</value>
Expand Down Expand Up @@ -2633,13 +2633,13 @@ A catch() block after a catch (System.Exception e) block can catch non-CLS excep
<value>foreach statement cannot operate on variables of type '{0}' because '{0}' does not contain a public instance definition for '{1}'</value>
</data>
<data name="ERR_AwaitForEachMissingMember" xml:space="preserve">
<value>Async foreach statement cannot operate on variables of type '{0}' because '{0}' does not contain a public instance definition for '{1}'</value>
<value>Asynchronous foreach statement cannot operate on variables of type '{0}' because '{0}' does not contain a public instance definition for '{1}'</value>
</data>
<data name="ERR_ForEachMissingMemberWrongAsync" xml:space="preserve">
<value>foreach statement cannot operate on variables of type '{0}' because '{0}' does not contain a public instance definition for '{1}'. Did you mean 'await foreach'?</value>
<value>foreach statement cannot operate on variables of type '{0}' because '{0}' does not contain a public instance definition for '{1}'. Did you mean 'await foreach' rather than 'foreach'?</value>
</data>
<data name="ERR_AwaitForEachMissingMemberWrongAsync" xml:space="preserve">
<value>Async foreach statement cannot operate on variables of type '{0}' because '{0}' does not contain a public instance definition for '{1}'. Did you mean 'foreach' rather than 'await foreach'?</value>
<value>Asynchronous foreach statement cannot operate on variables of type '{0}' because '{0}' does not contain a public instance definition for '{1}'. Did you mean 'foreach' rather than 'await foreach'?</value>
</data>
<data name="WRN_BadXMLRefParamType" xml:space="preserve">
<value>Invalid type for parameter {0} in XML comment cref attribute: '{1}'</value>
Expand Down Expand Up @@ -2828,7 +2828,7 @@ A catch() block after a catch (System.Exception e) block can catch non-CLS excep
<value>foreach statement cannot operate on variables of type '{0}' because it implements multiple instantiations of '{1}'; try casting to a specific interface instantiation</value>
</data>
<data name="ERR_MultipleIAsyncEnumOfT" xml:space="preserve">
<value>Async foreach statement cannot operate on variables of type '{0}' because it implements multiple instantiations of '{1}'; try casting to a specific interface instantiation</value>
<value>Asynchronous foreach statement cannot operate on variables of type '{0}' because it implements multiple instantiations of '{1}'; try casting to a specific interface instantiation</value>
</data>
<data name="ERR_FixedDimsRequired" xml:space="preserve">
<value>A fixed size buffer field must have the array size specifier after the field name</value>
Expand Down Expand Up @@ -2959,9 +2959,15 @@ A catch() block after a catch (System.Exception e) block can catch non-CLS excep
<data name="ERR_NoConvToIDisp" xml:space="preserve">
<value>'{0}': type used in a using statement must be implicitly convertible to 'System.IDisposable'</value>
</data>
<data name="ERR_NoConvToIDispWrongAsync" xml:space="preserve">
<value>'{0}': type used in a using statement must be implicitly convertible to 'System.IDisposable'. Did you mean 'await using' rather than 'using'?</value>
</data>
<data name="ERR_NoConvToIAsyncDisp" xml:space="preserve">
<value>'{0}': type used in an async using statement must be implicitly convertible to 'System.IAsyncDisposable'</value>
</data>
<data name="ERR_NoConvToIAsyncDispWrongAsync" xml:space="preserve">
<value>'{0}': type used in an async using statement must be implicitly convertible to 'System.IAsyncDisposable'. Did you mean 'using' rather than 'await using'?</value>
</data>
<data name="ERR_BadParamRef" xml:space="preserve">
<value>Parameter {0} must be declared with the '{1}' keyword</value>
</data>
Expand Down
2 changes: 2 additions & 0 deletions src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1592,6 +1592,8 @@ internal enum ErrorCode
ERR_ForEachMissingMemberWrongAsync = 8414,
ERR_AwaitForEachMissingMemberWrongAsync = 8415,
ERR_BadDynamicAwaitForEach = 8416,
ERR_NoConvToIAsyncDispWrongAsync = 8417,
ERR_NoConvToIDispWrongAsync = 8418,

WRN_ConvertingNullableToNonNullable = 8600,
WRN_NullReferenceAssignment = 8601,
Expand Down
Loading