Merge 'main' into RefStructInterfaces

.github/ISSUE_TEMPLATE/bug-report.md

@@ -15,7 +15,7 @@ A minimal repro, with source-code provided, is ideal.  Using [sharplab](https://
 
 **Diagnostic Id**:
 
-If this is a report about a bug in an analyzer, please include the diagnostic if possible (e.g. `"IDE0030"`).
+If this is a report about a bug in an analyzer, please include the diagnostic ID and message if possible (e.g. `"IDE0030: Use coalesce expression"`).
 
 **Expected Behavior**:
 

docs/compilers/CSharp/Compiler Breaking Changes - DotNet 9.md

@@ -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.

docs/compilers/CSharp/Warnversion Warning Waves.md

@@ -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.

src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs

@@ -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, 
@@ -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;
 

src/Compilers/CSharp/Portable/Binder/BinderFactory.BinderFactoryVisitor.cs

@@ -171,6 +171,7 @@ public override Binder VisitMethodDeclaration(MethodDeclarationSyntax methodDecl
                     }
 
                     SourceMemberMethodSymbol method = null;
+                    bool isIteratorBody = false;
 
                     if (usage != NodeUsage.Normal && methodDecl.TypeParameterList != null)
                     {
@@ -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);
                 }
 
@@ -222,7 +224,7 @@ public override Binder VisitConstructorDeclaration(ConstructorDeclarationSyntax
                         }
                     }
 
-                    resultBinder = resultBinder.WithUnsafeRegionIfNecessary(parent.Modifiers);
+                    resultBinder = resultBinder.SetOrClearUnsafeRegionIfNecessary(parent.Modifiers);
 
                     binderCache.TryAdd(key, resultBinder);
                 }
@@ -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);
                 }
@@ -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);
                         }
                     }
 
@@ -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);
                 }
@@ -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);
@@ -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);
@@ -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);
                     }
 
@@ -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);
                 }
@@ -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);
                 }
@@ -773,7 +785,7 @@ internal Binder VisitTypeDeclarationCore(TypeDeclarationSyntax parent, NodeUsage
                         }
                     }
 
-                    resultBinder = resultBinder.WithUnsafeRegionIfNecessary(parent.Modifiers);
+                    resultBinder = resultBinder.SetOrClearUnsafeRegionIfNecessary(parent.Modifiers);
 
                     binderCache.TryAdd(key, resultBinder);
                 }

src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs

@@ -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);
             }
         }
 

src/Compilers/CSharp/Portable/Binder/Binder_Flags.cs

@@ -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)

src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs

@@ -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);
@@ -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);
@@ -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;

src/Compilers/CSharp/Portable/Binder/Binder_Unsafe.cs

@@ -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;

src/Compilers/CSharp/Portable/Binder/ExecutableCodeBinder.cs

@@ -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;

src/Compilers/CSharp/Portable/Binder/LocalBinderFactory.cs

@@ -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);
             }