Skip to content

Commit

Permalink
Allow ref-like locals in iterators and async methods (#72664)
Browse files Browse the repository at this point in the history
* Allow ref-like locals in iterators and async methods

* Return check of ref-like locals in async methods

* Move declaration to block where it's used

* Add todo for improving error message of ERR_SpecialByRefInLambda

* Test `ref` in `await foreach` and iterator

* Extend tests

* Skip incompatible desktop tests

* Test `yield break` in the new `lock`

* Extend tests

* Mark removed unreleased error code as available

* Improve check for ref locals that can be hoisted

* Return error `ERR_BadSpecialByRefIterator`

* Extend tests

* Uncapitalize feature name to be like others

* Use better errors for refs across awaits

* Improve errors for spilled ref locals across awaits

* Replace remaining old error messages

* Return wrongly removed fact condition

* Extend tests

* Report errors close to problematic usage where possible

* Add more tests

* Test foreach on ref local with iterator inside

* Report spill local await boundary errors at their declaration

* Revert accidentally changed test name

* Test ref struct Current of async enumerator
  • Loading branch information
jjonescz authored Apr 16, 2024
1 parent 111ec28 commit 8ff156f
Show file tree
Hide file tree
Showing 43 changed files with 4,835 additions and 1,076 deletions.
12 changes: 2 additions & 10 deletions src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3182,21 +3182,13 @@ private BoundExpression BindOutVariableDeclarationArgument(
/// <summary>
/// Reports an error when a bad special by-ref local was found.
/// </summary>
internal static void CheckRestrictedTypeInAsyncMethod(Symbol containingSymbol, TypeSymbol type, BindingDiagnosticBag diagnostics, SyntaxNode syntax, ErrorCode errorCode = ErrorCode.ERR_BadSpecialByRefLocal)
internal static void CheckRestrictedTypeInAsyncMethod(Symbol containingSymbol, TypeSymbol type, BindingDiagnosticBag diagnostics, SyntaxNode syntax)
{
Debug.Assert(errorCode is ErrorCode.ERR_BadSpecialByRefLocal or ErrorCode.ERR_BadSpecialByRefUsing or ErrorCode.ERR_BadSpecialByRefLock);
if (containingSymbol.Kind == SymbolKind.Method
&& ((MethodSymbol)containingSymbol).IsAsync
&& type.IsRestrictedType())
{
if (errorCode == ErrorCode.ERR_BadSpecialByRefLock)
{
Error(diagnostics, errorCode, syntax);
}
else
{
Error(diagnostics, errorCode, syntax, type);
}
CheckFeatureAvailability(syntax, MessageID.IDS_FeatureRefUnsafeInIteratorAsync, diagnostics);
}
}

Expand Down
10 changes: 2 additions & 8 deletions src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1170,15 +1170,9 @@ protected BoundLocalDeclaration BindVariableDeclaration(

protected bool CheckRefLocalInAsyncOrIteratorMethod(SyntaxToken identifierToken, BindingDiagnosticBag diagnostics)
{
if (IsInAsyncMethod())
{
Error(diagnostics, ErrorCode.ERR_BadAsyncLocalType, identifierToken);
return true;
}
else if (IsDirectlyInIterator)
if (IsDirectlyInIterator || IsInAsyncMethod())
{
Error(diagnostics, ErrorCode.ERR_BadIteratorLocalType, identifierToken);
return true;
return !CheckFeatureAvailability(identifierToken, MessageID.IDS_FeatureRefUnsafeInIteratorAsync, diagnostics);
}

return false;
Expand Down
7 changes: 0 additions & 7 deletions src/Compilers/CSharp/Portable/Binder/LockBinder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,6 @@ internal override BoundStatement BindLockStatementParts(BindingDiagnosticBag dia
_ = diagnostics.ReportUseSite(lockTypeInfo.EnterScopeMethod, exprSyntax) ||
diagnostics.ReportUseSite(lockTypeInfo.ScopeType, exprSyntax) ||
diagnostics.ReportUseSite(lockTypeInfo.ScopeDisposeMethod, exprSyntax);

CheckRestrictedTypeInAsyncMethod(
originalBinder.ContainingMemberOrLambda,
lockTypeInfo.ScopeType,
diagnostics,
exprSyntax,
errorCode: ErrorCode.ERR_BadSpecialByRefLock);
}

BoundStatement stmt = originalBinder.BindPossibleEmbeddedStatement(_syntax.Statement, diagnostics);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ internal static BoundStatement BindUsingStatementOrDeclarationFromParts(SyntaxNo
Debug.Assert(expressionOpt is not null);
if (expressionOpt.Type is not null)
{
CheckRestrictedTypeInAsyncMethod(originalBinder.ContainingMemberOrLambda, expressionOpt.Type, diagnostics, expressionOpt.Syntax, errorCode: ErrorCode.ERR_BadSpecialByRefUsing);
CheckRestrictedTypeInAsyncMethod(originalBinder.ContainingMemberOrLambda, expressionOpt.Type, diagnostics, expressionOpt.Syntax);
}
}
else
Expand Down
26 changes: 10 additions & 16 deletions src/Compilers/CSharp/Portable/CSharpResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -3810,7 +3810,7 @@ Give the compiler some way to differentiate the methods. For example, you can gi
<value>__arglist is not allowed in the parameter list of async methods</value>
</data>
<data name="ERR_ByRefTypeAndAwait" xml:space="preserve">
<value>'await' cannot be used in an expression containing the type '{0}'</value>
<value>Instance of type '{0}' cannot be preserved across 'await' or 'yield' boundary.</value>
</data>
<data name="ERR_UnsafeAsyncArgType" xml:space="preserve">
<value>Async methods cannot have pointer type parameters</value>
Expand Down Expand Up @@ -3851,15 +3851,12 @@ Give the compiler some way to differentiate the methods. For example, you can gi
<data name="ERR_BadAsyncLacksBody" xml:space="preserve">
<value>The 'async' modifier can only be used in methods that have a body.</value>
</data>
<data name="ERR_BadSpecialByRefLocal" xml:space="preserve">
<value>Parameters or locals of type '{0}' cannot be declared in async methods or async lambda expressions.</value>
</data>
<data name="ERR_BadSpecialByRefUsing" xml:space="preserve">
<value>A using statement resource of type '{0}' cannot be used in async methods or async lambda expressions.</value>
</data>
<data name="ERR_BadSpecialByRefIterator" xml:space="preserve">
<value>foreach statement cannot operate on enumerators of type '{0}' in async or iterator methods because '{0}' is a ref struct.</value>
</data>
<data name="ERR_BadSpecialByRefParameter" xml:space="preserve">
<value>Parameters of type '{0}' cannot be declared in async methods or async lambda expressions.</value>
</data>
<data name="ERR_SecurityCriticalOrSecuritySafeCriticalOnAsync" xml:space="preserve">
<value>Security attribute '{0}' cannot be applied to an Async method.</value>
</data>
Expand Down Expand Up @@ -5290,12 +5287,6 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="ERR_AnonDelegateCantUseLocal" xml:space="preserve">
<value>Cannot use ref local '{0}' inside an anonymous method, lambda expression, or query expression</value>
</data>
<data name="ERR_BadIteratorLocalType" xml:space="preserve">
<value>Iterators cannot have by-reference locals</value>
</data>
<data name="ERR_BadAsyncLocalType" xml:space="preserve">
<value>Async methods cannot have by-reference locals</value>
</data>
<data name="ERR_RefReturningCallAndAwait" xml:space="preserve">
<value>A reference returned by a call to '{0}' cannot be preserved across 'await' or 'yield' boundary.</value>
</data>
Expand Down Expand Up @@ -7851,9 +7842,6 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="WRN_ConvertingLock_Title" xml:space="preserve">
<value>A value of type 'System.Threading.Lock' converted to a different type will use likely unintended monitor-based locking in 'lock' statement.</value>
</data>
<data name="ERR_BadSpecialByRefLock" xml:space="preserve">
<value>A lock statement on a value of type 'System.Threading.Lock' cannot be used in async methods or async lambda expressions.</value>
</data>
<data name="IDS_FeatureLockObject" xml:space="preserve">
<value>Lock object</value>
</data>
Expand Down Expand Up @@ -7911,4 +7899,10 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="WRN_BadYieldInLock_Title" xml:space="preserve">
<value>'yield return' should not be used in the body of a lock statement</value>
</data>
<data name="IDS_FeatureRefUnsafeInIteratorAsync" xml:space="preserve">
<value>ref and unsafe in async and iterator methods</value>
</data>
<data name="ERR_RefLocalAcrossAwait" xml:space="preserve">
<value>A 'ref' local cannot be preserved across 'await' or 'yield' boundary.</value>
</data>
</root>
10 changes: 5 additions & 5 deletions src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1111,7 +1111,7 @@ internal enum ErrorCode
ERR_NonTaskMainCantBeAsync = 4009,
ERR_CantConvAsyncAnonFuncReturns = 4010,
ERR_BadAwaiterPattern = 4011,
ERR_BadSpecialByRefLocal = 4012,
ERR_BadSpecialByRefParameter = 4012,
ERR_SpecialByRefInLambda = 4013,
WRN_UnobservedAwaitableExpression = 4014,
ERR_SynchronizedAsyncMethod = 4015,
Expand Down Expand Up @@ -1429,8 +1429,8 @@ internal enum ErrorCode
ERR_RefAssignmentMustHaveIdentityConversion = 8173,
ERR_ByReferenceVariableMustBeInitialized = 8174,
ERR_AnonDelegateCantUseLocal = 8175,
ERR_BadIteratorLocalType = 8176,
ERR_BadAsyncLocalType = 8177,
// ERR_BadIteratorLocalType = 8176,
// ERR_BadAsyncLocalType = 8177,
ERR_RefReturningCallAndAwait = 8178,
#endregion diagnostics for ref locals and ref returns introduced in C# 7

Expand Down Expand Up @@ -2161,7 +2161,7 @@ internal enum ErrorCode
ERR_UnscopedRefAttributeUnsupportedMemberTarget = 9101,
ERR_UnscopedRefAttributeInterfaceImplementation = 9102,
ERR_UnrecognizedRefSafetyRulesAttributeVersion = 9103,
ERR_BadSpecialByRefUsing = 9104,
// ERR_BadSpecialByRefUsing = 9104,

ERR_InvalidPrimaryConstructorParameterReference = 9105,
ERR_AmbiguousPrimaryConstructorParameterAsColorColorReceiver = 9106,
Expand Down Expand Up @@ -2286,7 +2286,7 @@ internal enum ErrorCode
ERR_CollectionExpressionMissingAdd = 9215,

WRN_ConvertingLock = 9216,
ERR_BadSpecialByRefLock = 9217,
ERR_RefLocalAcrossAwait = 9217,

ERR_CantInferMethTypeArgs_DynamicArgumentWithParamsCollections = 9218,
ERR_ParamsCollectionAmbiguousDynamicArgument = 9219,
Expand Down
7 changes: 2 additions & 5 deletions src/Compilers/CSharp/Portable/Errors/ErrorFacts.cs
Original file line number Diff line number Diff line change
Expand Up @@ -626,6 +626,7 @@ internal static bool IsBuildOnlyDiagnostic(ErrorCode code)
case ErrorCode.ERR_InterceptableMethodMustBeOrdinary:
case ErrorCode.ERR_PossibleAsyncIteratorWithoutYield:
case ErrorCode.ERR_PossibleAsyncIteratorWithoutYieldOrAwait:
case ErrorCode.ERR_RefLocalAcrossAwait:
// Update src\EditorFeatures\CSharp\LanguageServer\CSharpLspBuildOnlyDiagnostics.cs
// whenever new values are added here.
return true;
Expand Down Expand Up @@ -1537,7 +1538,7 @@ internal static bool IsBuildOnlyDiagnostic(ErrorCode code)
case ErrorCode.ERR_NonTaskMainCantBeAsync:
case ErrorCode.ERR_CantConvAsyncAnonFuncReturns:
case ErrorCode.ERR_BadAwaiterPattern:
case ErrorCode.ERR_BadSpecialByRefLocal:
case ErrorCode.ERR_BadSpecialByRefParameter:
case ErrorCode.WRN_UnobservedAwaitableExpression:
case ErrorCode.ERR_SynchronizedAsyncMethod:
case ErrorCode.ERR_BadAsyncReturnExpression:
Expand Down Expand Up @@ -1780,8 +1781,6 @@ internal static bool IsBuildOnlyDiagnostic(ErrorCode code)
case ErrorCode.ERR_RefAssignmentMustHaveIdentityConversion:
case ErrorCode.ERR_ByReferenceVariableMustBeInitialized:
case ErrorCode.ERR_AnonDelegateCantUseLocal:
case ErrorCode.ERR_BadIteratorLocalType:
case ErrorCode.ERR_BadAsyncLocalType:
case ErrorCode.ERR_PredefinedValueTupleTypeNotFound:
case ErrorCode.ERR_SemiOrLBraceOrArrowExpected:
case ErrorCode.ERR_NewWithTupleTypeSyntax:
Expand Down Expand Up @@ -2333,7 +2332,6 @@ internal static bool IsBuildOnlyDiagnostic(ErrorCode code)
case ErrorCode.ERR_UnscopedRefAttributeUnsupportedMemberTarget:
case ErrorCode.ERR_UnscopedRefAttributeInterfaceImplementation:
case ErrorCode.ERR_UnrecognizedRefSafetyRulesAttributeVersion:
case ErrorCode.ERR_BadSpecialByRefUsing:
case ErrorCode.ERR_InvalidPrimaryConstructorParameterReference:
case ErrorCode.ERR_AmbiguousPrimaryConstructorParameterAsColorColorReceiver:
case ErrorCode.WRN_CapturedPrimaryConstructorParameterPassedToBase:
Expand Down Expand Up @@ -2422,7 +2420,6 @@ internal static bool IsBuildOnlyDiagnostic(ErrorCode code)
case ErrorCode.ERR_CollectionExpressionMissingConstructor:
case ErrorCode.ERR_CollectionExpressionMissingAdd:
case ErrorCode.WRN_ConvertingLock:
case ErrorCode.ERR_BadSpecialByRefLock:
case ErrorCode.ERR_CantInferMethTypeArgs_DynamicArgumentWithParamsCollections:
case ErrorCode.ERR_ParamsCollectionAmbiguousDynamicArgument:
case ErrorCode.WRN_DynamicDispatchToParamsCollectionMethod:
Expand Down
3 changes: 3 additions & 0 deletions src/Compilers/CSharp/Portable/Errors/MessageID.cs
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,8 @@ internal enum MessageID
IDS_FeatureLockObject = MessageBase + 12841,

IDS_FeatureParamsCollections = MessageBase + 12842,

IDS_FeatureRefUnsafeInIteratorAsync = MessageBase + 12843,
}

// Message IDs may refer to strings that need to be localized.
Expand Down Expand Up @@ -466,6 +468,7 @@ internal static LanguageVersion RequiredVersion(this MessageID feature)
case MessageID.IDS_FeatureImplicitIndexerInitializer:
case MessageID.IDS_FeatureLockObject:
case MessageID.IDS_FeatureParamsCollections:
case MessageID.IDS_FeatureRefUnsafeInIteratorAsync:
return LanguageVersion.Preview;

// C# 12.0 features.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ namespace Microsoft.CodeAnalysis.CSharp
/// </summary>
/// <remarks>
/// Data flow analysis is used to calculate the locals. At yield/await we mark all variables as "unassigned".
/// When a read from an unassigned variables is reported we add the variable to the captured set.
/// When a read from an unassigned variable is reported we add the variable to the captured set.
/// "this" parameter is captured if a reference to "this", "base" or an instance field is encountered.
/// Variables used in finally also need to be captured if there is a yield in the corresponding try block.
/// </remarks>
Expand Down Expand Up @@ -76,19 +76,33 @@ public static OrderedSet<Symbol> Analyze(CSharpCompilation compilation, MethodSy
foreach (var kvp in lazyDisallowedCaptures)
{
var variable = kvp.Key;
var type = (variable.Kind == SymbolKind.Local) ? ((LocalSymbol)variable).Type : ((ParameterSymbol)variable).Type;

if (variable is SynthesizedLocal local && local.SynthesizedKind == SynthesizedLocalKind.Spill)
if (variable is LocalSymbol local)
{
Debug.Assert(local.TypeWithAnnotations.IsRestrictedType());
diagnostics.Add(ErrorCode.ERR_ByRefTypeAndAwait, local.GetFirstLocation(), local.TypeWithAnnotations);
foreach (var syntax in kvp.Value)
{
if (local.TypeWithAnnotations.IsRestrictedType())
{
// CS4007: Instance of type '{0}' cannot be preserved across 'await' or 'yield' boundary.
diagnostics.Add(ErrorCode.ERR_ByRefTypeAndAwait, syntax.Location, local.TypeWithAnnotations);
}
else
{
Debug.Assert(local.RefKind != RefKind.None);
// CS9217: A 'ref' local cannot be preserved across 'await' or 'yield' boundary.
diagnostics.Add(ErrorCode.ERR_RefLocalAcrossAwait, syntax.Location);
}
}
}
else
{
foreach (CSharpSyntaxNode syntax in kvp.Value)
var parameter = (ParameterSymbol)variable;
Debug.Assert(parameter.TypeWithAnnotations.IsRestrictedType());

foreach (var syntax in kvp.Value)
{
// CS4013: Instance of type '{0}' cannot be used inside an anonymous function, query expression, iterator block or async method
diagnostics.Add(ErrorCode.ERR_SpecialByRefInLambda, syntax.Location, type);
// CS4007: Instance of type '{0}' cannot be preserved across 'await' or 'yield' boundary.
diagnostics.Add(ErrorCode.ERR_ByRefTypeAndAwait, syntax.Location, parameter.TypeWithAnnotations);
}
}
}
Expand Down Expand Up @@ -195,14 +209,23 @@ protected override ImmutableArray<PendingBranch> Scan(ref bool badRegion)
private void CaptureVariable(Symbol variable, SyntaxNode syntax)
{
var type = (variable.Kind == SymbolKind.Local) ? ((LocalSymbol)variable).Type : ((ParameterSymbol)variable).Type;
if (type.IsRestrictedType())
if (type.IsRestrictedType() ||
(variable is LocalSymbol { RefKind: not RefKind.None } refLocal && !canRefLocalBeHoisted(refLocal)))
{
(_lazyDisallowedCaptures ??= new MultiDictionary<Symbol, SyntaxNode>()).Add(variable, syntax);
}
else
{
if (_variablesToHoist.Add(variable) && variable is LocalSymbol local && _boundRefLocalInitializers.TryGetValue(local, out var variableInitializer))
CaptureRefInitializer(variableInitializer, syntax);
CaptureRefInitializer(variableInitializer, local.SynthesizedKind != SynthesizedLocalKind.UserDefined ? variableInitializer.Syntax : syntax);
}

static bool canRefLocalBeHoisted(LocalSymbol refLocal)
{
return refLocal.SynthesizedKind == SynthesizedLocalKind.Spill ||
(refLocal.SynthesizedKind == SynthesizedLocalKind.ForEachArray &&
refLocal.Type.HasInlineArrayAttribute(out _) &&
refLocal.Type.TryGetInlineArrayElementField() is not null);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ internal void ReportAsyncParameterErrors(BindingDiagnosticBag diagnostics, Locat
}
else if (parameter.Type.IsRestrictedType())
{
diagnostics.Add(ErrorCode.ERR_BadSpecialByRefLocal, getLocation(parameter, location), parameter.Type);
diagnostics.Add(ErrorCode.ERR_BadSpecialByRefParameter, getLocation(parameter, location), parameter.Type);
}
}

Expand Down
Loading

0 comments on commit 8ff156f

Please sign in to comment.