Skip to content

Commit

Permalink
Support UnscopedRefAttribute (#62783)
Browse files Browse the repository at this point in the history
  • Loading branch information
cston authored Aug 3, 2022
1 parent 4865344 commit 7ab83bb
Show file tree
Hide file tree
Showing 76 changed files with 2,012 additions and 48 deletions.
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
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;
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;
}
}
2 changes: 2 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/ErrorPropertySymbol.cs
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.

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

0 comments on commit 7ab83bb

Please sign in to comment.