-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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 main codegen #18472
Async main codegen #18472
Changes from 54 commits
e27fb34
b4cee90
61e4705
39a8600
0cb90e3
4af405a
5a3fbcf
b16987f
8f0e223
ed6271d
578e811
92d3bfc
af675d3
1f71b66
671c53b
e99bf98
1114921
21a892c
c692009
9144795
cd2488e
d0675b1
04b5380
112f4fe
f532f23
617c791
5f177ba
d1e3e72
39cf05d
41cddf9
90edc8f
0407c20
8acedcc
9cbf02c
fe158be
89a15d8
1465d2c
2fc8c62
7d01d2e
e9e2875
81b01e2
4cd16ea
8a8afe2
1c2c643
c8b7b07
d975ba2
bad2923
85de26c
f121bb4
0a84b1d
e4e355e
b290268
eb68cea
247e05d
8e60326
881dea0
1d2b4db
3e8237a
2047833
7187917
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1457,7 +1457,7 @@ private MethodSymbol FindEntryPoint(CancellationToken cancellationToken, out Imm | |
var viableEntryPoints = ArrayBuilder<MethodSymbol>.GetInstance(); | ||
foreach (var candidate in entryPointCandidates) | ||
{ | ||
if (!candidate.HasEntryPointSignature(this)) | ||
if (!HasEntryPointSignature(candidate, warnings)) | ||
{ | ||
// a single error for partial methods | ||
warnings.Add(ErrorCode.WRN_InvalidMainSig, candidate.Locations.First(), candidate); | ||
|
@@ -1471,10 +1471,13 @@ private MethodSymbol FindEntryPoint(CancellationToken cancellationToken, out Imm | |
continue; | ||
} | ||
|
||
// PROTOTYPE(async-main): CheckFeatureAvailability should be called on non-async methods | ||
// that return Task or Task<T> | ||
if (candidate.IsAsync) | ||
{ | ||
// PROTOTYPE(async-main): Get the diagnostic to point to a smaller syntax piece. | ||
CheckFeatureAvailability(candidate.DeclaringSyntaxReferences.Single().GetSyntax(), MessageID.IDS_FeatureAsyncMain, diagnostics); | ||
// PROTOTYPE(async-main): Switch diagnostics around if the type is not Task or Task<T> | ||
CheckFeatureAvailability(candidate.GetNonNullSyntaxNode(), MessageID.IDS_FeatureAsyncMain, diagnostics); | ||
} | ||
|
||
viableEntryPoints.Add(candidate); | ||
|
@@ -1525,6 +1528,99 @@ private MethodSymbol FindEntryPoint(CancellationToken cancellationToken, out Imm | |
} | ||
} | ||
|
||
internal bool ReturnsAwaitableToVoidOrInt(MethodSymbol method, DiagnosticBag diagnostics) | ||
{ | ||
// Common case optimization | ||
if (method.ReturnType.SpecialType == SpecialType.System_Void || method.ReturnType.SpecialType == SpecialType.System_Int32) | ||
{ | ||
return false; | ||
} | ||
|
||
if (!(method.ReturnType is NamedTypeSymbol namedType)) | ||
{ | ||
return false; | ||
} | ||
|
||
// Early bail so we only even check things that are System.Threading.Tasks.Task(<T>) | ||
if (!(namedType.ConstructedFrom == GetWellKnownType(WellKnownType.System_Threading_Tasks_Task) || | ||
namedType.ConstructedFrom == GetWellKnownType(WellKnownType.System_Threading_Tasks_Task_T))) | ||
{ | ||
return false; | ||
} | ||
|
||
var syntax = method.ExtractReturnTypeSyntax(); | ||
var dumbInstance = new BoundLiteral(syntax, ConstantValue.Null, method.ReturnType); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, swore I fixed this earlier. Done. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Were you going to change this to BoundDefaultExpression? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was, but the synthesized expression was emitting warnings about using "dot" after default, so I reverted it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok |
||
// PROTOTYPE(async-main): We might need to adjust the containing member of the binder to be the Main method | ||
var binder = GetBinder(syntax); | ||
BoundExpression result; | ||
var success = binder.GetAwaitableExpressionInfo(dumbInstance, out _, out _, out _, out result, syntax, diagnostics); | ||
|
||
return success && | ||
(result.Type.SpecialType == SpecialType.System_Void || result.Type.SpecialType == SpecialType.System_Int32); | ||
} | ||
|
||
/// <summary> | ||
/// Checks if the method has an entry point compatible signature, i.e. | ||
/// - the return type is either void, int, <see cref="System.Threading.Tasks.Task" />, | ||
/// or <see cref="System.Threading.Tasks.Task{T}" /> where T is an int. | ||
/// - has either no parameter or a single parameter of type string[] | ||
/// </summary> | ||
private bool HasEntryPointSignature(MethodSymbol method, DiagnosticBag bag) | ||
{ | ||
if (method.IsVararg) | ||
{ | ||
return false; | ||
} | ||
|
||
TypeSymbol returnType = method.ReturnType; | ||
bool returnsTaskOrTaskOfInt = false; | ||
if (returnType.SpecialType != SpecialType.System_Int32 && returnType.SpecialType != SpecialType.System_Void) | ||
{ | ||
// Never look for ReturnsAwaitableToVoidOrInt on int32 or void | ||
returnsTaskOrTaskOfInt = ReturnsAwaitableToVoidOrInt(method, bag); | ||
if (!returnsTaskOrTaskOfInt) | ||
{ | ||
return false; | ||
} | ||
} | ||
|
||
// Prior to 7.1, async methods were considered to "have the entrypoint signature". | ||
// In order to keep back-compat, we need to let these through if compiling using a previous language version. | ||
if (!returnsTaskOrTaskOfInt && method.IsAsync && this.LanguageVersion >= LanguageVersion.CSharp7_1) | ||
{ | ||
return false; | ||
} | ||
|
||
if (method.RefKind != RefKind.None) | ||
{ | ||
return false; | ||
} | ||
|
||
if (method.Parameters.Length == 0) | ||
{ | ||
return true; | ||
} | ||
|
||
if (method.Parameters.Length > 1) | ||
{ | ||
return false; | ||
} | ||
|
||
if (!method.ParameterRefKinds.IsDefault) | ||
{ | ||
return false; | ||
} | ||
|
||
var firstType = method.Parameters[0].Type; | ||
if (firstType.TypeKind != TypeKind.Array) | ||
{ | ||
return false; | ||
} | ||
|
||
var array = (ArrayTypeSymbol)firstType; | ||
return array.IsSZArray && array.ElementType.SpecialType == SpecialType.System_String; | ||
} | ||
|
||
internal override bool IsUnreferencedAssemblyIdentityDiagnosticCode(int code) | ||
=> code == (int)ErrorCode.ERR_NoTypeDef; | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -184,6 +184,8 @@ public static void CompileMethodBodies( | |
} | ||
} | ||
|
||
// Returns the MethodSymbol for the assembly entrypoint. If the user has a Task returning main, | ||
// this function returns the synthesized Main MethodSymbol. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to specify the "MoveNext" as an entry point for debug purposes? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we have to specify two entry points - actual and for debugging purposes, it would be a different change, I guess. In reply to: 111274974 [](ancestors = 111274974) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll manually test this and see if I run into anything. |
||
private static MethodSymbol GetEntryPoint(CSharpCompilation compilation, PEModuleBuilder moduleBeingBuilt, bool hasDeclarationErrors, DiagnosticBag diagnostics, CancellationToken cancellationToken) | ||
{ | ||
var entryPointAndDiagnostics = compilation.GetEntryPointAndDiagnostics(cancellationToken); | ||
|
@@ -194,9 +196,26 @@ private static MethodSymbol GetEntryPoint(CSharpCompilation compilation, PEModul | |
|
||
Debug.Assert(!entryPointAndDiagnostics.Diagnostics.IsDefault); | ||
diagnostics.AddRange(entryPointAndDiagnostics.Diagnostics); | ||
|
||
var entryPoint = entryPointAndDiagnostics.MethodSymbol; | ||
var synthesizedEntryPoint = entryPoint as SynthesizedEntryPointSymbol; | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should add a comment to the top of the function noting that this returns the real entry point vs. the user defined one. The name of the function is ambiguous with this feature now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
if ((object)entryPoint == null) | ||
{ | ||
Debug.Assert(entryPointAndDiagnostics.Diagnostics.HasAnyErrors() || !compilation.Options.Errors.IsDefaultOrEmpty); | ||
return null; | ||
} | ||
|
||
// entryPoint can be a SynthesizedEntryPointSymbol if a script is being compiled. | ||
SynthesizedEntryPointSymbol synthesizedEntryPoint = entryPoint as SynthesizedEntryPointSymbol; | ||
if ((object)synthesizedEntryPoint == null && compilation.ReturnsAwaitableToVoidOrInt(entryPoint, diagnostics)) | ||
{ | ||
synthesizedEntryPoint = new SynthesizedEntryPointSymbol.AsyncForwardEntryPoint(compilation, diagnostics, entryPoint.ContainingType, entryPoint); | ||
entryPoint = synthesizedEntryPoint; | ||
if ((object)moduleBeingBuilt != null) | ||
{ | ||
moduleBeingBuilt.AddSynthesizedDefinition(entryPoint.ContainingType, synthesizedEntryPoint); | ||
} | ||
} | ||
|
||
if (((object)synthesizedEntryPoint != null) && | ||
(moduleBeingBuilt != null) && | ||
!hasDeclarationErrors && | ||
|
@@ -221,7 +240,6 @@ private static MethodSymbol GetEntryPoint(CSharpCompilation compilation, PEModul | |
moduleBeingBuilt.SetMethodBody(synthesizedEntryPoint, emittedBody); | ||
} | ||
|
||
Debug.Assert((object)entryPoint != null || entryPointAndDiagnostics.Diagnostics.HasAnyErrors() || !compilation.Options.Errors.IsDefaultOrEmpty); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
It is not clear why this assert is removed.#Closed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The assert wasn't deleted, it was moved upwards in the file to line 203 |
||
return entryPoint; | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not understand the motivation behind the change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't make sense to attach the error to the return type, it is much nicer to have it on the entire method body.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you think it is much nice to squiggle entire method body for the feature error? I there something wrong with the code in the method? How is this going to interact with other errors in the body? Will the customer be able to see that location?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather squiggle the whole signature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Squiggling just the return type when the only real problem is the "async" isn't intuitive at all.