Skip to content

Commit

Permalink
Merge "ref in async" feature into main (#73488)
Browse files Browse the repository at this point in the history
  • Loading branch information
jjonescz authored May 15, 2024
2 parents 07d80d7 + 80c8b00 commit fd9a371
Show file tree
Hide file tree
Showing 61 changed files with 10,695 additions and 1,426 deletions.
27 changes: 27 additions & 0 deletions docs/compilers/CSharp/Compiler Breaking Changes - DotNet 9.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# This document lists known breaking changes in Roslyn after .NET 8 all the way to .NET 9.

## Iterators introduce safe context in C# 13 and newer

***Introduced in Visual Studio 2022 version 17.11***

Although the language spec states that iterators introduce a safe context, Roslyn does not implement that in C# 12 and lower.
This will change in C# 13 as part of [a feature which allows unsafe code in iterators](https://github.com/dotnet/roslyn/issues/72662).
The change does not break normal scenarios as it was disallowed to use unsafe constructs directly in iterators anyway.
However, it can break scenarios where an unsafe context was previously inherited into nested local functions, for example:

```cs
unsafe class C // unsafe context
{
System.Collections.Generic.IEnumerable<int> M() // an iterator
{
yield return 1;
local();
void local()
{
int* p = null; // allowed in C# 12; error in C# 13
}
}
}
```

You can work around the break simply by adding the `unsafe` modifier to the local function.
8 changes: 8 additions & 0 deletions docs/compilers/CSharp/Warnversion Warning Waves.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,14 @@ In a typical project, this setting is controlled by the `AnalysisLevel` property
which determines the `WarningLevel` property (passed to the `Csc` task).
For more information on `AnalysisLevel`, see https://devblogs.microsoft.com/dotnet/automatically-find-latent-bugs-in-your-code-with-net-5/

## Warning level 9

The compiler shipped with .NET 9 (the C# 13 compiler) contains the following warnings which are reported only under `/warn:9` or higher.

| Warning ID | Description |
|------------|-------------|
| CS9237 | ['yield return' should not be used in the body of a lock statement](https://github.com/dotnet/roslyn/issues/72443) |

## Warning level 8

The compiler shipped with .NET 8 (the C# 12 compiler) contains the following warnings which are reported only under `/warn:8` or higher.
Expand Down
24 changes: 17 additions & 7 deletions src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs
Original file line number Diff line number Diff line change
Expand Up @@ -872,12 +872,24 @@ private static bool CheckNotNamespaceOrType(BoundExpression expr, BindingDiagnos
}
}

private bool CheckLocalValueKind(SyntaxNode node, BoundLocal local, BindValueKind valueKind, bool checkingReceiver, BindingDiagnosticBag diagnostics)
private void CheckAddressOfInAsyncOrIteratorMethod(SyntaxNode node, BindValueKind valueKind, BindingDiagnosticBag diagnostics)
{
if (valueKind == BindValueKind.AddressOf && this.IsInAsyncMethod())
if (valueKind == BindValueKind.AddressOf)
{
Error(diagnostics, ErrorCode.WRN_AddressOfInAsync, node);
if (this.IsInAsyncMethod())
{
Error(diagnostics, ErrorCode.WRN_AddressOfInAsync, node);
}
else if (this.IsDirectlyInIterator && Compilation.IsFeatureEnabled(MessageID.IDS_FeatureRefUnsafeInIteratorAsync))
{
Error(diagnostics, ErrorCode.ERR_AddressOfInIterator, node);
}
}
}

private bool CheckLocalValueKind(SyntaxNode node, BoundLocal local, BindValueKind valueKind, bool checkingReceiver, BindingDiagnosticBag diagnostics)
{
CheckAddressOfInAsyncOrIteratorMethod(node, valueKind, diagnostics);

// Local constants are never variables. Local variables are sometimes
// not to be treated as variables, if they are fixed, declared in a using,
Expand Down Expand Up @@ -968,10 +980,8 @@ internal partial class Binder
private bool CheckParameterValueKind(SyntaxNode node, BoundParameter parameter, BindValueKind valueKind, bool checkingReceiver, BindingDiagnosticBag diagnostics)
{
Debug.Assert(!RequiresAssignableVariable(BindValueKind.AddressOf));
if (valueKind == BindValueKind.AddressOf && this.IsInAsyncMethod())
{
Error(diagnostics, ErrorCode.WRN_AddressOfInAsync, node);
}

CheckAddressOfInAsyncOrIteratorMethod(node, valueKind, diagnostics);

ParameterSymbol parameterSymbol = parameter.ParameterSymbol;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ public override Binder VisitMethodDeclaration(MethodDeclarationSyntax methodDecl
}

SourceMemberMethodSymbol method = null;
bool isIteratorBody = false;

if (usage != NodeUsage.Normal && methodDecl.TypeParameterList != null)
{
Expand All @@ -181,10 +182,11 @@ public override Binder VisitMethodDeclaration(MethodDeclarationSyntax methodDecl
if (usage == NodeUsage.MethodBody)
{
method = method ?? GetMethodSymbol(methodDecl, resultBinder);
isIteratorBody = method.IsIterator;
resultBinder = new InMethodBinder(method, resultBinder);
}

resultBinder = resultBinder.WithUnsafeRegionIfNecessary(methodDecl.Modifiers);
resultBinder = resultBinder.SetOrClearUnsafeRegionIfNecessary(methodDecl.Modifiers, isIteratorBody: isIteratorBody);
binderCache.TryAdd(key, resultBinder);
}

Expand Down Expand Up @@ -222,7 +224,7 @@ public override Binder VisitConstructorDeclaration(ConstructorDeclarationSyntax
}
}

resultBinder = resultBinder.WithUnsafeRegionIfNecessary(parent.Modifiers);
resultBinder = resultBinder.SetOrClearUnsafeRegionIfNecessary(parent.Modifiers);

binderCache.TryAdd(key, resultBinder);
}
Expand All @@ -249,7 +251,7 @@ public override Binder VisitDestructorDeclaration(DestructorDeclarationSyntax pa
SourceMemberMethodSymbol method = GetMethodSymbol(parent, resultBinder);
resultBinder = new InMethodBinder(method, resultBinder);

resultBinder = resultBinder.WithUnsafeRegionIfNecessary(parent.Modifiers);
resultBinder = resultBinder.SetOrClearUnsafeRegionIfNecessary(parent.Modifiers);

binderCache.TryAdd(key, resultBinder);
}
Expand Down Expand Up @@ -311,6 +313,10 @@ public override Binder VisitAccessorDeclaration(AccessorDeclarationSyntax parent
if ((object)accessor != null)
{
resultBinder = new InMethodBinder(accessor, resultBinder);

resultBinder = resultBinder.SetOrClearUnsafeRegionIfNecessary(
modifiers: default,
isIteratorBody: accessor.IsIterator);
}
}

Expand Down Expand Up @@ -338,12 +344,14 @@ private Binder VisitOperatorOrConversionDeclaration(BaseMethodDeclarationSyntax
resultBinder = VisitCore(parent.Parent);

MethodSymbol method = GetMethodSymbol(parent, resultBinder);
bool isIteratorBody = false;
if ((object)method != null && inBody)
{
isIteratorBody = method.IsIterator;
resultBinder = new InMethodBinder(method, resultBinder);
}

resultBinder = resultBinder.WithUnsafeRegionIfNecessary(parent.Modifiers);
resultBinder = resultBinder.SetOrClearUnsafeRegionIfNecessary(parent.Modifiers, isIteratorBody: isIteratorBody);

binderCache.TryAdd(key, resultBinder);
}
Expand All @@ -363,24 +371,24 @@ public override Binder VisitConversionOperatorDeclaration(ConversionOperatorDecl

public override Binder VisitFieldDeclaration(FieldDeclarationSyntax parent)
{
return VisitCore(parent.Parent).WithUnsafeRegionIfNecessary(parent.Modifiers);
return VisitCore(parent.Parent).SetOrClearUnsafeRegionIfNecessary(parent.Modifiers);
}

public override Binder VisitEventDeclaration(EventDeclarationSyntax parent)
{
return VisitCore(parent.Parent).WithUnsafeRegionIfNecessary(parent.Modifiers);
return VisitCore(parent.Parent).SetOrClearUnsafeRegionIfNecessary(parent.Modifiers);
}

public override Binder VisitEventFieldDeclaration(EventFieldDeclarationSyntax parent)
{
return VisitCore(parent.Parent).WithUnsafeRegionIfNecessary(parent.Modifiers);
return VisitCore(parent.Parent).SetOrClearUnsafeRegionIfNecessary(parent.Modifiers);
}

public override Binder VisitPropertyDeclaration(PropertyDeclarationSyntax parent)
{
if (!LookupPosition.IsInBody(_position, parent))
{
return VisitCore(parent.Parent).WithUnsafeRegionIfNecessary(parent.Modifiers);
return VisitCore(parent.Parent).SetOrClearUnsafeRegionIfNecessary(parent.Modifiers);
}

return VisitPropertyOrIndexerExpressionBody(parent);
Expand All @@ -390,7 +398,7 @@ public override Binder VisitIndexerDeclaration(IndexerDeclarationSyntax parent)
{
if (!LookupPosition.IsInBody(_position, parent))
{
return VisitCore(parent.Parent).WithUnsafeRegionIfNecessary(parent.Modifiers);
return VisitCore(parent.Parent).SetOrClearUnsafeRegionIfNecessary(parent.Modifiers);
}

return VisitPropertyOrIndexerExpressionBody(parent);
Expand All @@ -403,12 +411,16 @@ private Binder VisitPropertyOrIndexerExpressionBody(BasePropertyDeclarationSynta
Binder resultBinder;
if (!binderCache.TryGetValue(key, out resultBinder))
{
resultBinder = VisitCore(parent.Parent).WithUnsafeRegionIfNecessary(parent.Modifiers);
resultBinder = VisitCore(parent.Parent).SetOrClearUnsafeRegionIfNecessary(parent.Modifiers);

var propertySymbol = GetPropertySymbol(parent, resultBinder);
var accessor = propertySymbol.GetMethod;
if ((object)accessor != null)
{
// Expression body cannot be an iterator, otherwise we would need to pass
// `isIteratorBody` to `SetOrClearUnsafeRegionIfNecessary` above.
Debug.Assert(!accessor.IsIterator);

resultBinder = new InMethodBinder(accessor, resultBinder);
}

Expand Down Expand Up @@ -663,7 +675,7 @@ public override Binder VisitDelegateDeclaration(DelegateDeclarationSyntax parent
resultBinder = new WithClassTypeParametersBinder(container, resultBinder);
}

resultBinder = resultBinder.WithUnsafeRegionIfNecessary(parent.Modifiers);
resultBinder = resultBinder.SetOrClearUnsafeRegionIfNecessary(parent.Modifiers);

binderCache.TryAdd(key, resultBinder);
}
Expand Down Expand Up @@ -691,7 +703,7 @@ public override Binder VisitEnumDeclaration(EnumDeclarationSyntax parent)

resultBinder = new InContainerBinder(container, outer);

resultBinder = resultBinder.WithUnsafeRegionIfNecessary(parent.Modifiers);
resultBinder = resultBinder.SetOrClearUnsafeRegionIfNecessary(parent.Modifiers);

binderCache.TryAdd(key, resultBinder);
}
Expand Down Expand Up @@ -773,7 +785,7 @@ internal Binder VisitTypeDeclarationCore(TypeDeclarationSyntax parent, NodeUsage
}
}

resultBinder = resultBinder.WithUnsafeRegionIfNecessary(parent.Modifiers);
resultBinder = resultBinder.SetOrClearUnsafeRegionIfNecessary(parent.Modifiers);

binderCache.TryAdd(key, resultBinder);
}
Expand Down
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
21 changes: 17 additions & 4 deletions src/Compilers/CSharp/Portable/Binder/Binder_Flags.cs
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,24 @@ internal Binder WithAdditionalFlagsAndContainingMemberOrLambda(BinderFlags flags
return new BinderWithContainingMemberOrLambda(this, this.Flags | flags, containing);
}

internal Binder WithUnsafeRegionIfNecessary(SyntaxTokenList modifiers)
internal Binder SetOrClearUnsafeRegionIfNecessary(SyntaxTokenList modifiers, bool isIteratorBody = false)
{
return (this.Flags.Includes(BinderFlags.UnsafeRegion) || !modifiers.Any(SyntaxKind.UnsafeKeyword))
? this
: new Binder(this, this.Flags | BinderFlags.UnsafeRegion);
// In C# 13 and above, iterator bodies define a safe context even when nested in an unsafe context.
// In C# 12 and below, we keep the (spec violating) behavior that iterator bodies inherit the safe/unsafe context
// from their containing scope. Since there are errors for unsafe constructs directly in iterators,
// this inherited unsafe context can be observed only in nested non-iterator local functions.
var withoutUnsafe = isIteratorBody && this.Compilation.IsFeatureEnabled(MessageID.IDS_FeatureRefUnsafeInIteratorAsync);

if (this.Flags.Includes(BinderFlags.UnsafeRegion))
{
return withoutUnsafe
? new Binder(this, this.Flags & ~BinderFlags.UnsafeRegion)
: this;
}

return !withoutUnsafe && modifiers.Any(SyntaxKind.UnsafeKeyword)
? new Binder(this, this.Flags | BinderFlags.UnsafeRegion)
: this;
}

internal Binder WithCheckedOrUncheckedRegion(bool @checked)
Expand Down
23 changes: 12 additions & 11 deletions src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs
Original file line number Diff line number Diff line change
Expand Up @@ -176,9 +176,7 @@ private BoundStatement BindUnsafeStatement(UnsafeStatementSyntax node, BindingDi
}
else if (this.IsIndirectlyInIterator) // called *after* we know the binder map has been created.
{
// Spec 8.2: "An iterator block always defines a safe context, even when its declaration
// is nested in an unsafe context."
Error(diagnostics, ErrorCode.ERR_IllegalInnerUnsafe, node.UnsafeKeyword);
CheckFeatureAvailability(node.UnsafeKeyword, MessageID.IDS_FeatureRefUnsafeInIteratorAsync, diagnostics);
}

return BindEmbeddedBlock(node.Block, diagnostics);
Expand Down Expand Up @@ -268,6 +266,15 @@ private BoundStatement BindYieldReturnStatement(YieldStatementSyntax node, Bindi
{
Error(diagnostics, ErrorCode.ERR_YieldNotAllowedInScript, node.YieldKeyword);
}
else if (InUnsafeRegion && Compilation.IsFeatureEnabled(MessageID.IDS_FeatureRefUnsafeInIteratorAsync))
{
Error(diagnostics, ErrorCode.ERR_BadYieldInUnsafe, node.YieldKeyword);
}
// NOTE: Error conditions should be checked above this point; only warning conditions below.
else if (this.Flags.Includes(BinderFlags.InLockBody))
{
Error(diagnostics, ErrorCode.WRN_BadYieldInLock, node.YieldKeyword);
}

CheckRequiredLangVersionForIteratorMethods(node, diagnostics);
return new BoundYieldReturnStatement(node, argument);
Expand Down Expand Up @@ -1165,15 +1172,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
10 changes: 4 additions & 6 deletions src/Compilers/CSharp/Portable/Binder/Binder_Unsafe.cs
Original file line number Diff line number Diff line change
Expand Up @@ -56,18 +56,16 @@ private CSDiagnosticInfo GetUnsafeDiagnosticInfo(TypeSymbol sizeOfTypeOpt)
{
return null;
}
else if (this.IsIndirectlyInIterator)
{
// Spec 8.2: "An iterator block always defines a safe context, even when its declaration
// is nested in an unsafe context."
return new CSDiagnosticInfo(ErrorCode.ERR_IllegalInnerUnsafe);
}
else if (!this.InUnsafeRegion)
{
return ((object)sizeOfTypeOpt == null)
? new CSDiagnosticInfo(ErrorCode.ERR_UnsafeNeeded)
: new CSDiagnosticInfo(ErrorCode.ERR_SizeofUnsafe, sizeOfTypeOpt);
}
else if (this.IsIndirectlyInIterator && MessageID.IDS_FeatureRefUnsafeInIteratorAsync.GetFeatureAvailabilityDiagnosticInfo(Compilation) is { } unsafeInIteratorDiagnosticInfo)
{
return unsafeInIteratorDiagnosticInfo;
}
else
{
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ public static void ValidateIteratorMethod(CSharpCompilation compilation, MethodS
if (((iterator as SourceMemberMethodSymbol)?.IsUnsafe == true || (iterator as LocalFunctionSymbol)?.IsUnsafe == true)
&& compilation.Options.AllowUnsafe) // Don't cascade
{
diagnostics.Add(ErrorCode.ERR_IllegalInnerUnsafe, errorLocation);
MessageID.IDS_FeatureRefUnsafeInIteratorAsync.CheckFeatureAvailability(diagnostics, compilation, errorLocation);
}

var returnType = iterator.ReturnType;
Expand Down
4 changes: 3 additions & 1 deletion src/Compilers/CSharp/Portable/Binder/LocalBinderFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,9 @@ public override void VisitLocalFunctionStatement(LocalFunctionStatementSyntax no
? new WithMethodTypeParametersBinder(match, _enclosing)
: _enclosing;

binder = binder.WithUnsafeRegionIfNecessary(node.Modifiers);
binder = binder.SetOrClearUnsafeRegionIfNecessary(node.Modifiers,
isIteratorBody: match.IsIterator);

binder = new InMethodBinder(match, binder);
}

Expand Down
Loading

0 comments on commit fd9a371

Please sign in to comment.