Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support UnscopedRefAttribute #62783

Merged
merged 14 commits into from
Aug 3, 2022
25 changes: 17 additions & 8 deletions docs/compilers/CSharp/Compiler Breaking Changes - DotNet 7.md
Original file line number Diff line number Diff line change
Expand Up @@ -128,15 +128,24 @@ static ref T ReturnOutParamByRef<T>(out T t)
}
```

A possible workaround is to change the method signature to pass the parameter by `ref` instead.
Possible workarounds are:
1. Use `System.Diagnostics.CodeAnalysis.UnscopedRefAttribute` to mark the reference as unscoped.
```csharp
Copy link
Member

@jcouv jcouv Jul 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: other csharp blocks are left-indented. Not sure whether it's material (does that affect markup rendering on GitHub or docs.microsoft.com), but I'd stick with that to be safe. #Closed

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without indenting, subsequent bullets start at 1.

static ref T ReturnOutParamByRef<T>([UnscopedRef] out T t)
{
t = default;
return ref t; // ok
}
```

```csharp
static ref T ReturnRefParamByRef<T>(ref T t)
{
t = default;
return ref t; // ok
}
```
1. Change the method signature to pass the parameter by `ref`.
```csharp
static ref T ReturnRefParamByRef<T>(ref T t)
{
t = default;
return ref t; // ok
}
```

## Method ref struct return escape analysis depends on ref escape of ref arguments

Expand Down
30 changes: 26 additions & 4 deletions src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs
Original file line number Diff line number Diff line change
Expand Up @@ -782,7 +782,7 @@ private uint GetParameterValEscape(ParameterSymbol parameter)
{
if (UseUpdatedEscapeRules)
{
return parameter.Scope == DeclarationScope.ValueScoped ?
return parameter.EffectiveScope == DeclarationScope.ValueScoped ?
Binder.TopLevelScope :
Binder.ExternalScope;
}
Expand All @@ -796,7 +796,7 @@ private uint GetParameterRefEscape(ParameterSymbol parameter)
{
if (UseUpdatedEscapeRules)
{
return parameter.RefKind is RefKind.None || parameter.Scope != DeclarationScope.Unscoped ? Binder.TopLevelScope : Binder.ExternalScope;
return parameter.RefKind is RefKind.None || parameter.EffectiveScope != DeclarationScope.Unscoped ? Binder.TopLevelScope : Binder.ExternalScope;
}
else
{
Expand Down Expand Up @@ -1822,7 +1822,7 @@ bool considerParameter(ParameterSymbol? parameter)
// SPEC: For a given argument `a` that is passed to parameter `p`:
// SPEC: 1. ...
// SPEC: 2. If `p` is `scoped` then `a` does not contribute *safe-to-escape* when considering arguments.
if (parameter?.Scope == DeclarationScope.ValueScoped)
if (parameter?.EffectiveScope == DeclarationScope.ValueScoped)
{
return false;
}
Expand Down Expand Up @@ -1909,7 +1909,7 @@ private static RefKind GetEffectiveRefKind(
}
}

scope = paramIndex >= 0 ? parameters[paramIndex].Scope : DeclarationScope.Unscoped;
scope = paramIndex >= 0 ? parameters[paramIndex].EffectiveScope : DeclarationScope.Unscoped;
return effectiveRefKind;
}

Expand Down Expand Up @@ -2227,6 +2227,8 @@ internal uint GetRefEscape(BoundExpression expr, uint scopeOfTheContainingExpres
return ((BoundLocal)expr).LocalSymbol.RefEscapeScope;

case BoundKind.ThisReference:
Debug.Assert(this.ContainingMember() is MethodSymbol { ThisParameter: not null });

var thisref = (BoundThisReference)expr;

// "this" is an RValue, unless in a struct.
Expand All @@ -2235,6 +2237,15 @@ internal uint GetRefEscape(BoundExpression expr, uint scopeOfTheContainingExpres
break;
}

if (UseUpdatedEscapeRules)
{
if (this.ContainingMember() is MethodSymbol { ThisParameter: var thisParameter } &&
thisParameter.EffectiveScope == DeclarationScope.Unscoped)
{
return Binder.ExternalScope;
}
}

//"this" is not returnable by reference in a struct.
// can ref escape to any other level
return Binder.TopLevelScope;
Expand Down Expand Up @@ -2481,6 +2492,8 @@ internal bool CheckRefEscape(SyntaxNode node, BoundExpression expr, uint escapeF
return CheckLocalRefEscape(node, local, escapeTo, checkingReceiver, diagnostics);

case BoundKind.ThisReference:
Debug.Assert(this.ContainingMember() is MethodSymbol { ThisParameter: not null });

var thisref = (BoundThisReference)expr;

// "this" is an RValue, unless in a struct.
Expand All @@ -2492,6 +2505,15 @@ internal bool CheckRefEscape(SyntaxNode node, BoundExpression expr, uint escapeF
//"this" is not returnable by reference in a struct.
if (escapeTo == Binder.ExternalScope)
{
if (UseUpdatedEscapeRules)
{
if (this.ContainingMember() is MethodSymbol { ThisParameter: var thisParameter } &&
thisParameter.EffectiveScope == DeclarationScope.Unscoped)
{
// can ref escape to any other level
return true;
}
}
Error(diagnostics, ErrorCode.ERR_RefReturnStructThis, node);
return false;
}
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8846,8 +8846,8 @@ private MethodGroupResolution ResolveDefaultMethodGroup(
}

var parameters = method.Parameters;
var parameterScopes = parameters.Any(p => p.Scope != DeclarationScope.Unscoped) ?
parameters.SelectAsArray(p => p.Scope) :
var parameterScopes = parameters.Any(p => p.EffectiveScope != DeclarationScope.Unscoped) ?
parameters.SelectAsArray(p => p.EffectiveScope) :
default;
return GetMethodGroupOrLambdaDelegateType(node.Syntax, method.RefKind, method.ReturnTypeWithAnnotations, method.ParameterRefKinds, parameterScopes, method.ParameterTypesWithAnnotations);
}
Expand Down
3 changes: 3 additions & 0 deletions src/Compilers/CSharp/Portable/CSharpResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -7205,4 +7205,7 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="ERR_ScopedTypeNameDisallowed" xml:space="preserve">
<value>Types and aliases cannot be named 'scoped'.</value>
</data>
<data name="ERR_UnscopedRefAttributeUnsupportedTarget" xml:space="preserve">
<value>UnscopedRefAttribute can only be applied to 'out' parameters, 'ref' parameters that refer to 'ref struct' types, and instance methods and properties on 'struct' types other than constructors and 'init' accessors.</value>
</data>
</root>
1 change: 1 addition & 0 deletions src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2111,6 +2111,7 @@ internal enum ErrorCode
ERR_CannotMatchOnINumberBase = 9060,
ERR_MisplacedScoped = 9061,
ERR_ScopedTypeNameDisallowed = 9062,
ERR_UnscopedRefAttributeUnsupportedTarget = 9063,

#endregion

Expand Down
1 change: 1 addition & 0 deletions src/Compilers/CSharp/Portable/Errors/ErrorFacts.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2212,6 +2212,7 @@ internal static bool IsBuildOnlyDiagnostic(ErrorCode code)
case ErrorCode.ERR_CannotMatchOnINumberBase:
case ErrorCode.ERR_MisplacedScoped:
case ErrorCode.ERR_ScopedTypeNameDisallowed:
case ErrorCode.ERR_UnscopedRefAttributeUnsupportedTarget:
return false;
default:
// NOTE: All error codes must be explicitly handled in this switch statement
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,8 @@ public override bool IsExtern

internal override bool IsRequired => false;

internal sealed override bool HasUnscopedRefAttribute => false;

internal override ObsoleteAttributeData ObsoleteAttributeData
{
get { return null; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ private ImmutableArray<ParameterSymbol> MakeParameters()
ordinal++,
p.RefKind,
p.Name,
p.Scope,
p.DeclaredScope,
// the synthesized parameter doesn't need to have the same ref custom modifiers as the base
refCustomModifiers: default,
inheritAttributes ? p as SourceComplexParameterSymbolBase : null));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -612,7 +612,7 @@ void visitFunctionPointerSignature(IMethodSymbol symbol)
foreach (var param in symbol.Parameters)
{
// https://github.com/dotnet/roslyn/issues/61647: Use public API.
Debug.Assert((param as Symbols.PublicModel.ParameterSymbol)?.GetSymbol<ParameterSymbol>().Scope switch
Debug.Assert((param as Symbols.PublicModel.ParameterSymbol)?.GetSymbol<ParameterSymbol>().EffectiveScope switch
{
null => true,
DeclarationScope.Unscoped => param.RefKind != RefKind.Out,
Expand Down Expand Up @@ -780,7 +780,7 @@ public override void VisitParameter(IParameterSymbol symbol)
AddCustomModifiersIfNeeded(symbol.RefCustomModifiers, leadingSpace: false, trailingSpace: true);

// https://github.com/dotnet/roslyn/issues/61647: Use public API.
if ((symbol as Symbols.PublicModel.ParameterSymbol)?.GetSymbol<ParameterSymbol>().Scope == DeclarationScope.ValueScoped &&
if ((symbol as Symbols.PublicModel.ParameterSymbol)?.GetSymbol<ParameterSymbol>().EffectiveScope == DeclarationScope.ValueScoped &&
format.CompilerInternalOptions.IncludesOption(SymbolDisplayCompilerInternalOptions.IncludeScoped))
{
AddKeyword(SyntaxKind.ScopedKeyword);
Expand Down Expand Up @@ -1079,7 +1079,7 @@ private void AddParameterRefKindIfNeeded(IParameterSymbol symbol)
if (format.ParameterOptions.IncludesOption(SymbolDisplayParameterOptions.IncludeParamsRefOut))
{
// https://github.com/dotnet/roslyn/issues/61647: Use public API.
if ((symbol as Symbols.PublicModel.ParameterSymbol)?.GetSymbol<ParameterSymbol>().Scope == DeclarationScope.RefScoped &&
if ((symbol as Symbols.PublicModel.ParameterSymbol)?.GetSymbol<ParameterSymbol>().EffectiveScope == DeclarationScope.RefScoped &&
symbol.RefKind != RefKind.Out &&
format.CompilerInternalOptions.IncludesOption(SymbolDisplayCompilerInternalOptions.IncludeScoped))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,8 @@ public override bool IsAbstract

internal override bool IsRequired => false;

internal sealed override bool HasUnscopedRefAttribute => false;

internal sealed override ObsoleteAttributeData ObsoleteAttributeData
{
get { return null; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,22 @@ public bool HasSkipLocalsInitAttribute
}
}

private bool _hasUnscopedRefAttribute;
public bool HasUnscopedRefAttribute
{
get
{
VerifySealed(expected: true);
return _hasUnscopedRefAttribute;
}
set
{
VerifySealed(expected: false);
_hasUnscopedRefAttribute = value;
SetDataStored();
}
}

private ImmutableArray<string> _memberNotNullAttributeData = ImmutableArray<string>.Empty;

public void AddNotNullMember(string memberName)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,20 @@ namespace Microsoft.CodeAnalysis.CSharp.Symbols
/// </summary>
internal sealed class ParameterEarlyWellKnownAttributeData : CommonParameterEarlyWellKnownAttributeData
{
private bool _hasUnscopedRefAttribute;
333fred marked this conversation as resolved.
Show resolved Hide resolved
public bool HasUnscopedRefAttribute
{
get
{
VerifySealed(expected: true);
return _hasUnscopedRefAttribute;
}
set
{
VerifySealed(expected: false);
_hasUnscopedRefAttribute = value;
SetDataStored();
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,22 @@ public bool HasSkipLocalsInitAttribute
}
}

private bool _hasUnscopedRefAttribute;
public bool HasUnscopedRefAttribute
{
get
{
VerifySealed(expected: true);
return _hasUnscopedRefAttribute;
}
set
{
VerifySealed(expected: false);
_hasUnscopedRefAttribute = value;
SetDataStored();
}
}

private ImmutableArray<string> _memberNotNullAttributeData = ImmutableArray<string>.Empty;

public void AddNotNullMember(string memberName)
Expand Down
2 changes: 2 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/ErrorMethodSymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -284,5 +284,7 @@ protected override bool HasSetsRequiredMembersImpl
return false;
}
}

internal sealed override bool HasUnscopedRefAttribute => false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ public ErrorPropertySymbol(Symbol containingSymbol, TypeSymbol type, string name

internal override bool IsRequired => false;

internal sealed override bool HasUnscopedRefAttribute => false;

internal sealed override ObsoleteAttributeData ObsoleteAttributeData { get { return null; } }

public override ImmutableArray<ParameterSymbol> Parameters { get { return ImmutableArray<ParameterSymbol>.Empty; } }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -831,5 +831,6 @@ public override bool IsVararg
internal override IEnumerable<SecurityAttribute> GetSecurityInformation() => throw ExceptionUtilities.Unreachable;
internal sealed override bool IsNullableAnalysisEnabled() => throw ExceptionUtilities.Unreachable;
protected sealed override bool HasSetsRequiredMembersImpl => throw ExceptionUtilities.Unreachable;
internal sealed override bool HasUnscopedRefAttribute => false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ public FunctionPointerParameterSymbol(TypeWithAnnotations typeWithAnnotations, R
public override int Ordinal { get; }
public override Symbol ContainingSymbol => _containingSymbol;
public override ImmutableArray<CustomModifier> RefCustomModifiers { get; }
internal override DeclarationScope Scope => RefKind == RefKind.Out ? DeclarationScope.RefScoped : DeclarationScope.Unscoped;
internal override DeclarationScope DeclaredScope => RefKind == RefKind.Out ? DeclarationScope.RefScoped : DeclarationScope.Unscoped;
internal override DeclarationScope EffectiveScope => DeclaredScope;

public override bool Equals(Symbol other, TypeCompareKind compareKind)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ private struct PackedFlags
{
// We currently pack everything into a 32-bit int with the following layout:
//
// |w|v|u|t|s|r|q|p|ooo|n|m|l|k|j|i|h|g|f|e|d|c|b|aaaaa|
// |y|x|w|v|u|t|s|r|q|p|ooo|n|m|l|k|j|i|h|g|f|e|d|c|b|aaaaa|
//
// a = method kind. 5 bits.
// b = method kind populated. 1 bit.
Expand All @@ -74,7 +74,9 @@ private struct PackedFlags
// u = IsUnmanagedCallersOnlyAttributePopulated. 1 bit.
// v = IsSetsRequiredMembersBit. 1 bit.
// w = IsSetsRequiredMembersPopulated. 1 bit.
// 4 bits remain for future purposes.
// x = IsUnscopedRef. 1 bit.
// y = IsUnscopedRefPopulated. 1 bit.
// 2 bits remain for future purposes.
333fred marked this conversation as resolved.
Show resolved Hide resolved

private const int MethodKindOffset = 0;
private const int MethodKindMask = 0x1F;
Expand Down Expand Up @@ -102,6 +104,8 @@ private struct PackedFlags
private const int IsUnmanagedCallersOnlyAttributePopulatedBit = 0x1 << 26;
private const int HasSetsRequiredMembersBit = 0x1 << 27;
private const int HasSetsRequiredMembersPopulatedBit = 0x1 << 28;
private const int IsUnscopedRefBit = 0x1 << 29;
private const int IsUnscopedRefPopulatedBit = 0x1 << 30;

private int _bits;

Expand Down Expand Up @@ -140,6 +144,8 @@ public MethodKind MethodKind
public bool IsUnmanagedCallersOnlyAttributePopulated => (_bits & IsUnmanagedCallersOnlyAttributePopulatedBit) != 0;
public bool HasSetsRequiredMembers => (_bits & HasSetsRequiredMembersBit) != 0;
public bool HasSetsRequiredMembersPopulated => (_bits & HasSetsRequiredMembersPopulatedBit) != 0;
public bool IsUnscopedRef => (_bits & IsUnscopedRefBit) != 0;
public bool IsUnscopedRefPopulated => (_bits & IsUnscopedRefPopulatedBit) != 0;

#if DEBUG
static PackedFlags()
Expand Down Expand Up @@ -254,6 +260,14 @@ public bool InitializeSetsRequiredMembersBit(bool value)

return ThreadSafeFlagOperations.Set(ref _bits, bitsToSet);
}

public bool InitializeIsUnscopedRef(bool value)
{
int bitsToSet = IsUnscopedRefPopulatedBit;
if (value) bitsToSet |= IsUnscopedRefBit;

return ThreadSafeFlagOperations.Set(ref _bits, bitsToSet);
}
}

/// <summary>
Expand Down Expand Up @@ -1610,5 +1624,20 @@ public override bool AreLocalsZeroed
internal bool TestIsExtensionBitTrue => _packedFlags.IsExtensionMethod;

internal sealed override bool IsNullableAnalysisEnabled() => throw ExceptionUtilities.Unreachable;

internal sealed override bool HasUnscopedRefAttribute
{
get
{
if (!_packedFlags.IsUnscopedRefPopulated)
{
var moduleSymbol = _containingType.ContainingPEModule;
bool unscopedRef = moduleSymbol.Module.HasUnscopedRefAttribute(_handle);
_packedFlags.InitializeIsUnscopedRef(unscopedRef);
}

return _packedFlags.IsUnscopedRef;
}
}
}
}
Loading