Remove scope variance exceptions

dotnet · Dec 6, 2024 · 493ac3a · 493ac3a
1 parent e6a3bd7
commit 493ac3a
Show file tree

Hide file tree

Showing 6 changed files with 428 additions and 118 deletions.
diff --git a/docs/compilers/CSharp/Compiler Breaking Changes - DotNet 10.md b/docs/compilers/CSharp/Compiler Breaking Changes - DotNet 10.md
@@ -118,3 +118,22 @@ class C
 }
 ```
 
+## Variance of `scoped` and `[UnscopedRef]` is more strict
+
+***Introduced in Visual Studio 2022 version 17.13***
+
+Scope can be changed when overriding a method, implementing an interface, or converting a lambda/method to a delegate under
+[some conditions](https://github.com/dotnet/csharplang/blob/05064c2a9567b7a58a07e526dff403ece1866541/proposals/csharp-11.0/low-level-struct-improvements.md#scoped-mismatch)
+(roughly, `scoped` can be added and `[UnscopedRef]` can be removed).
+Previously, the compiler did not report an error/warning for such mismatch under some circumstances, but it is now always reported.
+
+```cs
+D1 d1 = (ref int i) => { }; // previously no mismatch error reported, now:
+                            // error CS8986: The 'scoped' modifier of parameter 'i' doesn't match target 'D1'.
+
+D2 d2 = (ref int i) => ref i; // an error was and continues to be reported:
+                              // error CS8986: The 'scoped' modifier of parameter 'i' doesn't match target 'D2'.
+
+delegate void D1(scoped ref int x);
+delegate ref int D2(scoped ref int x);
+```
diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs
@@ -2274,26 +2274,23 @@ private static void CheckParameterModifierMismatchMethodConversion(SyntaxNode sy
                 return;
             }
 
-            if (SourceMemberContainerTypeSymbol.RequiresValidScopedOverrideForRefSafety(delegateMethod))
-            {
-                SourceMemberContainerTypeSymbol.CheckValidScopedOverride(
-                    delegateMethod,
-                    lambdaOrMethod,
-                    diagnostics,
-                    static (diagnostics, delegateMethod, lambdaOrMethod, parameter, _, typeAndSyntax) =>
-                    {
-                        diagnostics.Add(
-                            SourceMemberContainerTypeSymbol.ReportInvalidScopedOverrideAsError(delegateMethod, lambdaOrMethod) ?
-                                ErrorCode.ERR_ScopedMismatchInParameterOfTarget :
-                                ErrorCode.WRN_ScopedMismatchInParameterOfTarget,
-                            typeAndSyntax.Syntax.Location,
-                            new FormattedSymbol(parameter, SymbolDisplayFormat.ShortFormat),
-                            typeAndSyntax.Type);
-                    },
-                    (Type: targetType, Syntax: syntax),
-                    allowVariance: true,
-                    invokedAsExtensionMethod: invokedAsExtensionMethod);
-            }
+            SourceMemberContainerTypeSymbol.CheckValidScopedOverride(
+                delegateMethod,
+                lambdaOrMethod,
+                diagnostics,
+                static (diagnostics, delegateMethod, lambdaOrMethod, parameter, _, typeAndSyntax) =>
+                {
+                    diagnostics.Add(
+                        SourceMemberContainerTypeSymbol.ReportInvalidScopedOverrideAsError(delegateMethod, lambdaOrMethod) ?
+                            ErrorCode.ERR_ScopedMismatchInParameterOfTarget :
+                            ErrorCode.WRN_ScopedMismatchInParameterOfTarget,
+                        typeAndSyntax.Syntax.Location,
+                        new FormattedSymbol(parameter, SymbolDisplayFormat.ShortFormat),
+                        typeAndSyntax.Type);
+                },
+                (Type: targetType, Syntax: syntax),
+                allowVariance: true,
+                invokedAsExtensionMethod: invokedAsExtensionMethod);
 
             SourceMemberContainerTypeSymbol.CheckRefReadonlyInMismatch(
                 delegateMethod, lambdaOrMethod, diagnostics,

diff --git a/...pilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol_ImplementationChecks.cs b/...pilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol_ImplementationChecks.cs
@@ -1149,25 +1149,22 @@ static void checkValidMethodOverride(
                 MethodSymbol overridingMethod,
                 BindingDiagnosticBag diagnostics)
             {
-                if (RequiresValidScopedOverrideForRefSafety(overriddenMethod))
-                {
-                    CheckValidScopedOverride(
-                        overriddenMethod,
-                        overridingMethod,
-                        diagnostics,
-                        static (diagnostics, overriddenMethod, overridingMethod, overridingParameter, _, location) =>
-                            {
-                                diagnostics.Add(
-                                    ReportInvalidScopedOverrideAsError(overriddenMethod, overridingMethod) ?
-                                        ErrorCode.ERR_ScopedMismatchInParameterOfOverrideOrImplementation :
-                                        ErrorCode.WRN_ScopedMismatchInParameterOfOverrideOrImplementation,
-                                    location,
-                                    new FormattedSymbol(overridingParameter, SymbolDisplayFormat.ShortFormat));
-                            },
-                        overridingMemberLocation,
-                        allowVariance: true,
-                        invokedAsExtensionMethod: false);
-                }
+                CheckValidScopedOverride(
+                    overriddenMethod,
+                    overridingMethod,
+                    diagnostics,
+                    static (diagnostics, overriddenMethod, overridingMethod, overridingParameter, _, location) =>
+                        {
+                            diagnostics.Add(
+                                ReportInvalidScopedOverrideAsError(overriddenMethod, overridingMethod) ?
+                                    ErrorCode.ERR_ScopedMismatchInParameterOfOverrideOrImplementation :
+                                    ErrorCode.WRN_ScopedMismatchInParameterOfOverrideOrImplementation,
+                                location,
+                                new FormattedSymbol(overridingParameter, SymbolDisplayFormat.ShortFormat));
+                        },
+                    overridingMemberLocation,
+                    allowVariance: true,
+                    invokedAsExtensionMethod: false);
 
                 CheckValidNullableMethodOverride(overridingMethod.DeclaringCompilation, overriddenMethod, overridingMethod, diagnostics,
                                                  ReportBadReturn,
@@ -1369,53 +1366,6 @@ static bool isValidNullableConversion(
         }
 
 #nullable enable
-        /// <summary>
-        /// Returns true if the method signature must match, with respect to scoped for ref safety,
-        /// in overrides, interface implementations, or delegate conversions.
-        /// </summary>
-        internal static bool RequiresValidScopedOverrideForRefSafety(MethodSymbol? method)
-        {
-            if (method is null)
-            {
-                return false;
-            }
-
-            var parameters = method.Parameters;
-
-            // https://github.com/dotnet/csharplang/blob/main/proposals/csharp-11.0/low-level-struct-improvements.md#scoped-mismatch
-            // The compiler will report a diagnostic for _unsafe scoped mismatches_ across overrides, interface implementations, and delegate conversions when:
-            // - The method returns a `ref struct` or returns a `ref` or `ref readonly`, or the method has a `ref` or `out` parameter of `ref struct` type, and
-            // ...
-            int nRefParametersRequired;
-            if (method.ReturnType.IsRefLikeOrAllowsRefLikeType() ||
-                (method.RefKind is RefKind.Ref or RefKind.RefReadOnly))
-            {
-                nRefParametersRequired = 1;
-            }
-            else if (parameters.Any(p => (p.RefKind is RefKind.Ref or RefKind.Out) && p.Type.IsRefLikeOrAllowsRefLikeType()))
-            {
-                nRefParametersRequired = 2; // including the parameter found above
-            }
-            else
-            {
-                return false;
-            }
-
-            // ...
-            // - The method has at least one additional `ref`, `in`, `ref readonly`, or `out` parameter, or a parameter of `ref struct` type.
-            int nRefParameters = parameters.Count(p => p.RefKind is RefKind.Ref or RefKind.In or RefKind.RefReadOnlyParameter or RefKind.Out);
-            if (nRefParameters >= nRefParametersRequired)
-            {
-                return true;
-            }
-            else if (parameters.Any(p => p.RefKind == RefKind.None && p.Type.IsRefLikeOrAllowsRefLikeType()))
-            {
-                return true;
-            }
-
-            return false;
-        }
-
         /// <summary>
         /// Returns true if a scoped mismatch should be reported as an error rather than a warning.
         /// </summary>

diff --git a/src/Compilers/CSharp/Portable/Symbols/TypeSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/TypeSymbol.cs
@@ -1855,25 +1855,23 @@ static void checkMethodOverride(
                             reportMismatchInParameterType,
                             (implementingType, isExplicit));
 
-                        if (SourceMemberContainerTypeSymbol.RequiresValidScopedOverrideForRefSafety(implementedMethod))
-                        {
-                            SourceMemberContainerTypeSymbol.CheckValidScopedOverride(
-                                implementedMethod,
-                                implementingMethod,
-                                diagnostics,
-                                static (diagnostics, implementedMethod, implementingMethod, implementingParameter, _, arg) =>
-                                    {
-                                        diagnostics.Add(
-                                            SourceMemberContainerTypeSymbol.ReportInvalidScopedOverrideAsError(implementedMethod, implementingMethod) ?
-                                                ErrorCode.ERR_ScopedMismatchInParameterOfOverrideOrImplementation :
-                                                ErrorCode.WRN_ScopedMismatchInParameterOfOverrideOrImplementation,
-                                            GetImplicitImplementationDiagnosticLocation(implementedMethod, arg.implementingType, implementingMethod),
-                                            new FormattedSymbol(implementingParameter, SymbolDisplayFormat.ShortFormat));
-                                    },
-                                (implementingType, isExplicit),
-                                allowVariance: true,
-                                invokedAsExtensionMethod: false);
-                        }
+                        SourceMemberContainerTypeSymbol.CheckValidScopedOverride(
+                            implementedMethod,
+                            implementingMethod,
+                            diagnostics,
+                            static (diagnostics, implementedMethod, implementingMethod, implementingParameter, _, arg) =>
+                                {
+                                    diagnostics.Add(
+                                        SourceMemberContainerTypeSymbol.ReportInvalidScopedOverrideAsError(implementedMethod, implementingMethod) ?
+                                            ErrorCode.ERR_ScopedMismatchInParameterOfOverrideOrImplementation :
+                                            ErrorCode.WRN_ScopedMismatchInParameterOfOverrideOrImplementation,
+                                        GetImplicitImplementationDiagnosticLocation(implementedMethod, arg.implementingType, implementingMethod),
+                                        new FormattedSymbol(implementingParameter, SymbolDisplayFormat.ShortFormat));
+                                },
+                            (implementingType, isExplicit),
+                            allowVariance: true,
+                            invokedAsExtensionMethod: false);
+
                         SourceMemberContainerTypeSymbol.CheckRefReadonlyInMismatch(
                             implementedMethod, implementingMethod, diagnostics,
                             static (diagnostics, implementedMethod, implementingMethod, implementingParameter, _, arg) =>