Skip to content

Commit

Permalink
Revert "Remove scope variance exceptions (dotnet#76296)"
Browse files Browse the repository at this point in the history 
This reverts commit b86d831.
  • Loading branch information
@jjonescz
jjonescz committed Jan 7, 2025
1 parent 57ba28e commit b785fbf
Show file tree
Hide file tree
Showing 7 changed files with 127 additions and 524 deletions.
34 changes: 0 additions & 34 deletions docs/compilers/CSharp/Compiler Breaking Changes - DotNet 10.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,40 +172,6 @@ struct S
}
```

## 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.
Note that the error is downgraded to a warning in `unsafe` contexts and also (in scenarios where it would be a breaking change) with LangVersion 12 or lower.

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

```cs
using System.Diagnostics.CodeAnalysis;

D1 d1 = ([UnscopedRef] ref int i) => { }; // previously no mismatch error reported, now:
// error CS8986: The 'scoped' modifier of parameter 'i' doesn't match target 'D1'.
D2 d2 = ([UnscopedRef] 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(ref int x);
delegate ref int D2(ref int x);
```

## `Microsoft.CodeAnalysis.EmbeddedAttribute` is validated on declaration

***Introduced in Visual Studio 2022 version 17.13***
Expand Down
37 changes: 20 additions & 17 deletions src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2274,23 +2274,26 @@ private static void CheckParameterModifierMismatchMethodConversion(SyntaxNode sy
return;
}

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);
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.CheckRefReadonlyInMismatch(
delegateMethod, lambdaOrMethod, diagnostics,
Expand Down
126 changes: 67 additions & 59 deletions ...pilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol_ImplementationChecks.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1149,22 +1149,25 @@ static void checkValidMethodOverride(
MethodSymbol overridingMethod,
BindingDiagnosticBag diagnostics)
{
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);
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);
}

CheckValidNullableMethodOverride(overridingMethod.DeclaringCompilation, overriddenMethod, overridingMethod, diagnostics,
ReportBadReturn,
Expand Down Expand Up @@ -1367,55 +1370,60 @@ static bool isValidNullableConversion(

#nullable enable
/// <summary>
/// Returns true if a scoped mismatch should be reported as an error rather than a warning.
/// 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 ReportInvalidScopedOverrideAsError(MethodSymbol baseMethod, MethodSymbol overrideMethod)
internal static bool RequiresValidScopedOverrideForRefSafety(MethodSymbol? method)
{
// https://github.com/dotnet/csharplang/blob/main/proposals/csharp-11.0/low-level-struct-improvements.md#scoped-mismatch
// The diagnostic is reported as an error if the mismatched signatures are both using C#11 ref safety rules; otherwise, the diagnostic is a warning.
return baseMethod.UseUpdatedEscapeRules && overrideMethod.UseUpdatedEscapeRules &&
// We have removed exceptions to the scoped mismatch error reporting, but to avoid breaks
// we report the new scenarios (previously exempted) as warnings in C# 12 and earlier.
// https://github.com/dotnet/roslyn/issues/76100
(overrideMethod.DeclaringCompilation.LanguageVersion > LanguageVersion.CSharp12 || usedToBeReported(baseMethod));

static bool usedToBeReported(MethodSymbol method)
{
var parameters = method.Parameters;

// https://github.com/dotnet/csharplang/blob/1f7f23f/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;
}
if (method is null)
{
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;
}
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>
internal static bool ReportInvalidScopedOverrideAsError(MethodSymbol baseMethod, MethodSymbol overrideMethod)
{
// https://github.com/dotnet/csharplang/blob/main/proposals/csharp-11.0/low-level-struct-improvements.md#scoped-mismatch
// The diagnostic is reported as an error if the mismatched signatures are both using C#11 ref safety rules; otherwise, the diagnostic is a warning.
return baseMethod.UseUpdatedEscapeRules && overrideMethod.UseUpdatedEscapeRules;
}

/// <summary>
Expand Down
36 changes: 19 additions & 17 deletions src/Compilers/CSharp/Portable/Symbols/TypeSymbol.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1855,23 +1855,25 @@ static void checkMethodOverride(
reportMismatchInParameterType,
(implementingType, isExplicit));

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

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.CheckRefReadonlyInMismatch(
implementedMethod, implementingMethod, diagnostics,
static (diagnostics, implementedMethod, implementingMethod, implementingParameter, _, arg) =>
Expand Down
12 changes: 3 additions & 9 deletions src/Compilers/CSharp/Test/Emit3/RefStructInterfacesTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21358,7 +21358,7 @@ class Helper<T>
delegate void D2(T x);

static D1 d11 = M1;
static D1 d12 = M2; // 1
static D1 d12 = M2;
static D2 d21 = M1;
static D2 d22 = M2;

Expand All @@ -21372,7 +21372,7 @@ class Helper
delegate void D2(Span<int> x);

static D1 d11 = M1;
static D1 d12 = M2; // 2
static D1 d12 = M2;
static D2 d21 = M1;
static D2 d22 = M2;

Expand All @@ -21381,13 +21381,7 @@ static void M2(Span<int> x) {}
}
";
var comp = CreateCompilation(src, targetFramework: s_targetFrameworkSupportingByRefLikeGenerics);
comp.VerifyEmitDiagnostics(
// (11,21): error CS8986: The 'scoped' modifier of parameter 'x' doesn't match target 'Helper<T>.D1'.
// static D1 d12 = M2; // 1
Diagnostic(ErrorCode.ERR_ScopedMismatchInParameterOfTarget, "M2").WithArguments("x", "Helper<T>.D1").WithLocation(11, 21),
// (25,21): error CS8986: The 'scoped' modifier of parameter 'x' doesn't match target 'Helper.D1'.
// static D1 d12 = M2; // 2
Diagnostic(ErrorCode.ERR_ScopedMismatchInParameterOfTarget, "M2").WithArguments("x", "Helper.D1").WithLocation(25, 21));
comp.VerifyEmitDiagnostics();
}

[Fact]
Expand Down
Loading

0 comments on commit b785fbf

Please # to comment.