-
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
Entrypoint detection stages #19093
Entrypoint detection stages #19093
Changes from 15 commits
83c2f0d
ee1a204
412ca9e
d4234c1
e9d9041
8328ea4
06bf4a2
6d96516
aae7023
49154bd
f173a7b
56ad35b
ba23bae
6ffa8ab
b5968a6
0f43ac2
4ac1006
f6a9818
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1453,42 +1453,101 @@ private MethodSymbol FindEntryPoint(CancellationToken cancellationToken, out Imm | |
} | ||
} | ||
|
||
DiagnosticBag warnings = DiagnosticBag.GetInstance(); | ||
var viableEntryPoints = ArrayBuilder<MethodSymbol>.GetInstance(); | ||
foreach (var candidate in entryPointCandidates) | ||
// Validity and diagnostics are also tracked because they must be conditionally handled | ||
// if there are not any "traditional" entrypoints found. | ||
var taskEntryPoints = ArrayBuilder<(bool IsValid, MethodSymbol Candidate, DiagnosticBag SpecificDiagnostics)>.GetInstance(); | ||
|
||
// These diagnostics (warning only) are added to the compilation only if | ||
// there were not any main methods found. | ||
DiagnosticBag noMainFoundDiagnostics = DiagnosticBag.GetInstance(); | ||
|
||
bool CheckValid(MethodSymbol candidate, bool isCandidate, DiagnosticBag specificDiagnostics) | ||
{ | ||
if (!HasEntryPointSignature(candidate, warnings)) | ||
if (!isCandidate) | ||
{ | ||
// a single error for partial methods | ||
warnings.Add(ErrorCode.WRN_InvalidMainSig, candidate.Locations.First(), candidate); | ||
continue; | ||
noMainFoundDiagnostics.Add(ErrorCode.WRN_InvalidMainSig, candidate.Locations.First(), candidate); | ||
noMainFoundDiagnostics.AddRange(specificDiagnostics); | ||
return false; | ||
} | ||
|
||
if (candidate.IsGenericMethod || candidate.ContainingType.IsGenericType) | ||
{ | ||
// a single error for partial methods: | ||
warnings.Add(ErrorCode.WRN_MainCantBeGeneric, candidate.Locations.First(), candidate); | ||
continue; | ||
noMainFoundDiagnostics.Add(ErrorCode.WRN_MainCantBeGeneric, candidate.Locations.First(), candidate); | ||
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. Perhaps check
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. Looks good! Done |
||
return true; | ||
} | ||
|
||
var viableEntryPoints = ArrayBuilder<MethodSymbol>.GetInstance(); | ||
|
||
foreach (var candidate in entryPointCandidates) | ||
{ | ||
var perCandidateBag = DiagnosticBag.GetInstance(); | ||
var (IsCandidate, IsTaskLike) = HasEntryPointSignature(candidate, perCandidateBag); | ||
|
||
// PROTOTYPE(async-main): CheckFeatureAvailability should be called on non-async methods | ||
// that return Task or Task<T> | ||
if (candidate.IsAsync) | ||
if (IsTaskLike) | ||
{ | ||
// PROTOTYPE(async-main): Get the diagnostic to point to a smaller syntax piece. | ||
// PROTOTYPE(async-main): Switch diagnostics around if the type is not Task or Task<T> | ||
CheckFeatureAvailability(candidate.GetNonNullSyntaxNode(), MessageID.IDS_FeatureAsyncMain, diagnostics); | ||
taskEntryPoints.Add((IsCandidate, candidate, perCandidateBag)); | ||
} | ||
else | ||
{ | ||
if (CheckValid(candidate, IsCandidate, perCandidateBag)) | ||
{ | ||
if (candidate.IsAsync) | ||
{ | ||
diagnostics.Add(ErrorCode.ERR_NonTaskMainCantBeAsync, candidate.Locations.First(), candidate); | ||
} | ||
else | ||
{ | ||
diagnostics.AddRange(perCandidateBag); | ||
viableEntryPoints.Add(candidate); | ||
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. Assert 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. Or alternatively, add 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 like |
||
} | ||
} | ||
perCandidateBag.Free(); | ||
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.
Please assert the bag is empty. #Closed |
||
} | ||
} | ||
|
||
viableEntryPoints.Add(candidate); | ||
if (viableEntryPoints.Count == 0) | ||
{ | ||
foreach (var (IsValid, Candidate, SpecificDiagnostics) in taskEntryPoints) | ||
{ | ||
// PROTOTYPE(async-main): Get the diagnostic to point to a smaller syntax piece. | ||
if (CheckValid(Candidate, IsValid, SpecificDiagnostics) && | ||
CheckFeatureAvailability(Candidate.GetNonNullSyntaxNode(), MessageID.IDS_FeatureAsyncMain, diagnostics)) | ||
{ | ||
diagnostics.AddRange(SpecificDiagnostics); | ||
viableEntryPoints.Add(Candidate); | ||
} | ||
} | ||
} | ||
|
||
if ((object)mainType == null || viableEntryPoints.Count == 0) | ||
foreach (var (_, _, SpecificDiagnostics) in taskEntryPoints) | ||
{ | ||
diagnostics.AddRange(warnings); | ||
SpecificDiagnostics.Free(); | ||
} | ||
|
||
warnings.Free(); | ||
if (viableEntryPoints.Count == 0) | ||
{ | ||
diagnostics.AddRange(noMainFoundDiagnostics); | ||
} | ||
else if ((object)mainType == null) | ||
{ | ||
// Filters out diagnostics so that only InvalidMainSig and MainCant'BeGeneric are left. | ||
// The reason that Error diagnostics can end up in `noMainFoundDiagnostics` is when | ||
// HasEntryPointSignature yields some Error Diagnostics when people implement Task or Task<T> incorrectly. | ||
// | ||
// We can't add those Errors to the general diagnostics bag because it would break previously-working programs. | ||
// The fact that these warnings are not added when csc is invoked with /main is possibly a bug, and is tracked at | ||
// https://github.com/dotnet/roslyn/issues/18964 | ||
foreach (var diagnostic in noMainFoundDiagnostics.AsEnumerable()) | ||
{ | ||
if (diagnostic.Code == (int)ErrorCode.WRN_InvalidMainSig || diagnostic.Code == (int)ErrorCode.WRN_MainCantBeGeneric) | ||
{ | ||
diagnostics.Add(diagnostic); | ||
} | ||
} | ||
} | ||
|
||
MethodSymbol entryPoint = null; | ||
if (viableEntryPoints.Count == 0) | ||
|
@@ -1518,7 +1577,9 @@ private MethodSymbol FindEntryPoint(CancellationToken cancellationToken, out Imm | |
entryPoint = viableEntryPoints[0]; | ||
} | ||
|
||
taskEntryPoints.Free(); | ||
viableEntryPoints.Free(); | ||
noMainFoundDiagnostics.Free(); | ||
return entryPoint; | ||
} | ||
finally | ||
|
@@ -1561,15 +1622,16 @@ internal bool ReturnsAwaitableToVoidOrInt(MethodSymbol method, DiagnosticBag dia | |
|
||
/// <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. | ||
/// - the return type is either void, int, or returns a <see cref="System.Threading.Tasks.Task" />, | ||
/// or <see cref="System.Threading.Tasks.Task{T}" /> where the return type of GetAwaiter().GetResult() | ||
/// is either void or int. | ||
/// - has either no parameter or a single parameter of type string[] | ||
/// </summary> | ||
private bool HasEntryPointSignature(MethodSymbol method, DiagnosticBag bag) | ||
private (bool IsCandidate, bool IsTaskLike) HasEntryPointSignature(MethodSymbol method, DiagnosticBag bag) | ||
{ | ||
if (method.IsVararg) | ||
{ | ||
return false; | ||
return (false, false); | ||
} | ||
|
||
TypeSymbol returnType = method.ReturnType; | ||
|
@@ -1580,45 +1642,38 @@ private bool HasEntryPointSignature(MethodSymbol method, DiagnosticBag bag) | |
returnsTaskOrTaskOfInt = ReturnsAwaitableToVoidOrInt(method, bag); | ||
if (!returnsTaskOrTaskOfInt) | ||
{ | ||
return false; | ||
return (false, 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; | ||
return (false, returnsTaskOrTaskOfInt); | ||
} | ||
|
||
if (method.Parameters.Length == 0) | ||
{ | ||
return true; | ||
return (true, returnsTaskOrTaskOfInt); | ||
} | ||
|
||
if (method.Parameters.Length > 1) | ||
{ | ||
return false; | ||
return (false, returnsTaskOrTaskOfInt); | ||
} | ||
|
||
if (!method.ParameterRefKinds.IsDefault) | ||
{ | ||
return false; | ||
return (false, returnsTaskOrTaskOfInt); | ||
} | ||
|
||
var firstType = method.Parameters[0].Type; | ||
if (firstType.TypeKind != TypeKind.Array) | ||
{ | ||
return false; | ||
return (false, returnsTaskOrTaskOfInt); | ||
} | ||
|
||
var array = (ArrayTypeSymbol)firstType; | ||
return array.IsSZArray && array.ElementType.SpecialType == SpecialType.System_String; | ||
return (array.IsSZArray && array.ElementType.SpecialType == SpecialType.System_String, returnsTaskOrTaskOfInt); | ||
} | ||
|
||
internal override bool IsUnreferencedAssemblyIdentityDiagnosticCode(int code) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1104,7 +1104,7 @@ internal enum ErrorCode | |
ERR_VarargsAsync = 4006, | ||
ERR_ByRefTypeAndAwait = 4007, | ||
ERR_BadAwaitArgVoidCall = 4008, | ||
// ERR_MainCantBeAsync = 4009, | ||
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.
Why comment out instead of delete? 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. Seemed like the convention. Should it be removed? 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 convention, at least as well as I can surmise, is to comment out when there is a gap in the ordering. For instance ERR_BadAwaitWithoutAsyncAnonMeth (20-ish lines down). It was removed but the error slot 4035 still exists. In this case we're replacing an error hence 4009 is not a gap anymore. This convention makes it easy to find places to put new errors. Vs having to scan through te file looking for subtle changes in the number. In reply to: 114816031 [](ancestors = 114816031) 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 (as discussed in person) |
||
ERR_NonTaskMainCantBeAsync = 4009, | ||
ERR_CantConvAsyncAnonFuncReturns = 4010, | ||
ERR_BadAwaiterPattern = 4011, | ||
ERR_BadSpecialByRefLocal = 4012, | ||
|
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.
intOrVoidEntryPoints
andtaskEntryPoints
are not freed.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.