-
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
enable main methods to be asynchronous #18157
Changes from 6 commits
88542e5
909560f
b90d121
2868b64
d7e9e7f
2678fbe
3228961
3d4363a
c3f2482
d88c6e4
c113421
c6c641c
c96708b
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 |
---|---|---|
|
@@ -597,18 +597,48 @@ internal bool IsEntryPointCandidate | |
|
||
/// <summary> | ||
/// Checks if the method has an entry point compatible signature, i.e. | ||
/// - the return type is either void or int | ||
/// - 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. | ||
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.
Nit: capitalization on "where" 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 #Resolved |
||
/// - has either no parameter or a single parameter of type string[] | ||
/// </summary> | ||
internal bool HasEntryPointSignature() | ||
internal bool HasEntryPointSignature(CSharpCompilation compilation) | ||
{ | ||
if (this.IsVararg) | ||
|
||
bool IsAsyncMainReturnType(TypeSymbol type) | ||
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. Should the new logic be conditional on language version? 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. Added a version of each of these tests that uses langversion 7 |
||
{ | ||
var namedType = type as NamedTypeSymbol; | ||
if (namedType == null) | ||
{ | ||
return false; | ||
} | ||
else if (namedType.ConstructedFrom == compilation.GetWellKnownType(WellKnownType.System_Threading_Tasks_Task)) | ||
{ | ||
// Change this to `namedType.IsNonGenericTaskType` if you want to support "task-like" objects. | ||
return true; | ||
} | ||
else if (namedType.ConstructedFrom == compilation.GetWellKnownType(WellKnownType.System_Threading_Tasks_Task_T)) | ||
{ | ||
// Change this to `namedType.IsGenericTaskType` if you want to support "task-like" objects. | ||
return namedType.TypeArguments[0].SpecialType == SpecialType.System_Int32; | ||
} | ||
else | ||
{ | ||
return false; | ||
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. is there a concern that ValueTask is not matched here? 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. For now we are just supporting Task, however, the comments in this method describe how to (very easily) support any "task-like" object including ValueTask. |
||
} | ||
} | ||
|
||
if (IsVararg) | ||
{ | ||
return false; | ||
} | ||
|
||
TypeSymbol returnType = ReturnType; | ||
if (returnType.SpecialType != SpecialType.System_Int32 && returnType.SpecialType != SpecialType.System_Void) | ||
if (returnType.SpecialType != SpecialType.System_Int32 && returnType.SpecialType != SpecialType.System_Void && !IsAsyncMainReturnType(returnType)) | ||
{ | ||
return false; | ||
} | ||
|
||
if (!IsAsyncMainReturnType(returnType) && IsAsync) | ||
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 test case for this too. 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. Already had an 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.
Consider pulling 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 #Closed |
||
{ | ||
return false; | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ | |
|
||
using System.Collections.Generic; | ||
using System.Linq; | ||
using System.Threading; | ||
using Microsoft.CodeAnalysis.CSharp.Symbols; | ||
using Microsoft.CodeAnalysis.CSharp.Syntax; | ||
using Microsoft.CodeAnalysis.CSharp.Test.Utilities; | ||
|
@@ -1221,7 +1222,31 @@ interface IInterface | |
} | ||
|
||
[Fact] | ||
public void MainCantBeAsync() | ||
public void MainCanBeAsync() | ||
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. Note: please add a test verifying we don't allow variants where main has a ref return. static ref Task Main() { ... } Context is that we incorrectly allow this for 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 cherry-pick your PR into my branch and add a test. |
||
{ | ||
var origSource = @" | ||
using System.Threading.Tasks; | ||
|
||
class A | ||
{ | ||
async static Task Main() | ||
{ | ||
await Task.Factory.StartNew(() => { }); | ||
} | ||
}"; | ||
var sources = new string[] { origSource, origSource.Replace("async ", "").Replace("await", "return") }; | ||
foreach (var source in sources) | ||
{ | ||
var compilation = CreateCompilationWithMscorlib45(source, options: TestOptions.DebugExe); | ||
compilation.VerifyDiagnostics(); | ||
var entry = compilation.GetEntryPoint(CancellationToken.None); | ||
Assert.NotNull(entry); | ||
Assert.Equal("System.Threading.Tasks.Task A.Main()", entry.ToTestDisplayString()); | ||
} | ||
} | ||
|
||
[Fact] | ||
public void MainCantBeAsyncVoid() | ||
{ | ||
var source = @" | ||
using System.Threading.Tasks; | ||
|
@@ -1233,10 +1258,86 @@ async static void Main() | |
await Task.Factory.StartNew(() => { }); | ||
} | ||
}"; | ||
CreateCompilationWithMscorlib45(source, options: TestOptions.ReleaseExe).VerifyDiagnostics( | ||
// (4,23): error CS4009: 'A.Main()': an entry point cannot be marked with the 'async' modifier | ||
var compilation = CreateCompilationWithMscorlib45(source, options: TestOptions.DebugExe); | ||
compilation.VerifyDiagnostics( | ||
// (6,23): warning CS0028: 'A.Main()' has the wrong signature to be an entry point | ||
// async static void Main() | ||
Diagnostic(ErrorCode.WRN_InvalidMainSig, "Main").WithArguments("A.Main()").WithLocation(6, 23), | ||
// error CS5001: Program does not contain a static 'Main' method suitable for an entry point | ||
Diagnostic(ErrorCode.ERR_NoEntryPoint).WithLocation(1, 1)); | ||
var entry = compilation.GetEntryPoint(CancellationToken.None); | ||
Assert.Null(entry); | ||
} | ||
|
||
[Fact] | ||
public void MainCantBeAsyncInt() | ||
{ | ||
var source = @" | ||
using System.Threading.Tasks; | ||
|
||
class A | ||
{ | ||
async static int Main() | ||
{ | ||
await Task.Factory.StartNew(() => 5); | ||
} | ||
}"; | ||
var compilation = CreateCompilationWithMscorlib45(source, options: TestOptions.DebugExe); | ||
compilation.VerifyDiagnostics( | ||
// (6,23): warning CS0028: 'A.Main()' has the wrong signature to be an entry point | ||
// async static void Main() | ||
Diagnostic(ErrorCode.ERR_MainCantBeAsync, "Main").WithArguments("A.Main()")); | ||
Diagnostic(ErrorCode.WRN_InvalidMainSig, "Main").WithArguments("A.Main()").WithLocation(6, 23), | ||
// error CS5001: Program does not contain a static 'Main' method suitable for an entry point | ||
Diagnostic(ErrorCode.ERR_NoEntryPoint).WithLocation(1, 1)); | ||
var entry = compilation.GetEntryPoint(CancellationToken.None); | ||
Assert.Null(entry); | ||
} | ||
|
||
[Fact] | ||
public void MainCanBeAsyncAndGenericOnInt() | ||
{ | ||
var origSource = @" | ||
using System.Threading.Tasks; | ||
|
||
class A | ||
{ | ||
async static Task<int> Main() | ||
{ | ||
return await Task.Factory.StartNew(() => 5); | ||
} | ||
}"; | ||
var sources = new string[] { origSource, origSource.Replace("async ", "").Replace("await", "") }; | ||
foreach (var source in sources) | ||
{ | ||
var compilation = CreateCompilationWithMscorlib45(source, options: TestOptions.DebugExe); | ||
compilation.VerifyDiagnostics(); | ||
var entry = compilation.GetEntryPoint(CancellationToken.None); | ||
Assert.NotNull(entry); | ||
Assert.Equal("System.Threading.Tasks.Task<System.Int32> A.Main()", entry.ToTestDisplayString()); | ||
} | ||
} | ||
|
||
[Fact] | ||
public void MainCantBeAsyncAndGenericOverFloats() | ||
{ | ||
var source = @" | ||
using System.Threading.Tasks; | ||
|
||
class A | ||
{ | ||
async static Task<float> Main() | ||
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. Should add variants that check for 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 |
||
{ | ||
await Task.Factory.StartNew(() => { }); | ||
return 0; | ||
} | ||
}"; | ||
var compilation = CreateCompilationWithMscorlib45(source, options: TestOptions.ReleaseExe); | ||
compilation.VerifyDiagnostics( | ||
// (6,30): warning CS0028: 'A.Main()' has the wrong signature to be an entry point | ||
// async static Task<float> Main() | ||
Diagnostic(ErrorCode.WRN_InvalidMainSig, "Main").WithArguments("A.Main()").WithLocation(6, 30), | ||
// error CS5001: Program does not contain a static 'Main' method suitable for an entry point | ||
Diagnostic(ErrorCode.ERR_NoEntryPoint).WithLocation(1, 1) ); | ||
} | ||
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. Consider adding a test without 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. Never mind, I see that you use string replacement to achieve that. In reply to: 108235403 [](ancestors = 108235403) |
||
|
||
[Fact] | ||
|
@@ -1255,7 +1356,7 @@ async static void Main<T>() | |
CreateCompilationWithMscorlib45(source, options: TestOptions.ReleaseExe).VerifyDiagnostics( | ||
// (4,23): warning CS0402: 'A.Main<T>()': an entry point cannot be generic or in a generic type | ||
// async static void Main<T>() | ||
Diagnostic(ErrorCode.WRN_MainCantBeGeneric, "Main").WithArguments("A.Main<T>()"), | ||
Diagnostic(ErrorCode.WRN_InvalidMainSig, "Main").WithArguments("A.Main<T>()"), | ||
// error CS5001: Program does not contain a static 'Main' method suitable for an entry point | ||
Diagnostic(ErrorCode.ERR_NoEntryPoint)); | ||
} | ||
|
@@ -3486,19 +3587,17 @@ public static int Main() | |
public void Repro_17885() | ||
{ | ||
var source = @" | ||
using System.Threading.Tasks; | ||
class Test | ||
{ | ||
async public static void Main() | ||
async public static Task Main() | ||
{ | ||
} | ||
}"; | ||
CreateCompilationWithMscorlib45(source, options: TestOptions.ReleaseExe).VerifyDiagnostics( | ||
// (4,30): warning CS1998: This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread. | ||
// async public static void Main() | ||
Diagnostic(ErrorCode.WRN_AsyncLacksAwaits, "Main"), | ||
// (4,30): error CS4009: 'Test.Main()': an entry point cannot be marked with the 'async' modifier | ||
// async public static void Main() | ||
Diagnostic(ErrorCode.ERR_MainCantBeAsync, "Main").WithArguments("Test.Main()")); | ||
Diagnostic(ErrorCode.WRN_AsyncLacksAwaits, "Main")); | ||
} | ||
|
||
[Fact, WorkItem(547088, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/547088")] | ||
|
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.
Should we delete the corresponding resource string?
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.
Done