Skip to content

Commit

Permalink
Support UnscopedRefAttribute
Browse files Browse the repository at this point in the history
  • Loading branch information
cston committed Jul 28, 2022
1 parent 498db64 commit 7884b57
Show file tree
Hide file tree
Showing 37 changed files with 1,161 additions and 42 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
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 @@ -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
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 @@ -294,7 +294,11 @@ private PEParameterSymbol(
typeWithAnnotations = NullableTypeDecoder.TransformType(typeWithAnnotations, handle, moduleSymbol, accessSymbol: accessSymbol, nullableContext: nullableContext);
typeWithAnnotations = TupleTypeDecoder.DecodeTupleTypesIfApplicable(typeWithAnnotations, handle, moduleSymbol);

if (_moduleSymbol.Module.HasLifetimeAnnotationAttribute(_handle, out var pair))
if (_moduleSymbol.Module.HasUnscopedRefAttribute(_handle))
{
scope = DeclarationScope.Unscoped;
}
else if (_moduleSymbol.Module.HasLifetimeAnnotationAttribute(_handle, out var pair))
{
var scopeOpt = GetScope(refKind, typeWithAnnotations.Type, pair.IsRefScoped, pair.IsValueScoped);
if (scopeOpt is null)
Expand Down Expand Up @@ -978,7 +982,9 @@ public override ImmutableArray<SyntaxReference> DeclaringSyntaxReferences
}
}

internal sealed override DeclarationScope Scope => _packedFlags.Scope;
internal sealed override DeclarationScope DeclaredScope => _packedFlags.Scope;

internal sealed override DeclarationScope EffectiveScope => DeclaredScope;

private static DeclarationScope? GetScope(RefKind refKind, TypeSymbol type, bool isRefScoped, bool isValueScoped)
{
Expand Down
12 changes: 11 additions & 1 deletion src/Compilers/CSharp/Portable/Symbols/ParameterSymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,17 @@ internal sealed override ObsoleteAttributeData? ObsoleteAttributeData
/// </summary>
internal abstract bool HasInterpolatedStringHandlerArgumentError { get; }

internal abstract DeclarationScope Scope { get; }
/// <summary>
/// The declared scope. From source, this is from the <c>scope</c> keyword
/// and any implicit scope, ignoring any <c>UnscopedRefAttribute</c>.
/// </summary>
internal abstract DeclarationScope DeclaredScope { get; }

/// <summary>
/// The effective scope. This is from the declared scope and any
/// <c>UnscopedRefAttribute</c>.
/// </summary>
internal abstract DeclarationScope EffectiveScope { get; }

protected sealed override bool IsHighestPriorityUseSiteErrorCode(int code) => code is (int)ErrorCode.ERR_UnsupportedCompilerFeature or (int)ErrorCode.ERR_BogusType;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ public SignatureOnlyParameterSymbol(

public override bool IsDiscard { get { return false; } }

internal override DeclarationScope Scope => DeclarationScope.Unscoped;
internal override DeclarationScope DeclaredScope => DeclarationScope.Unscoped;
internal override DeclarationScope EffectiveScope => DeclaredScope;

#region Not used by MethodSignatureComparer

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ internal static bool RequiresLifetimeAnnotationAttribute(ParameterSymbol paramet
{
Debug.Assert(!parameter.IsThis);

var scope = parameter.Scope;
var scope = parameter.DeclaredScope;
if (scope == DeclarationScope.Unscoped)
{
return false;
Expand Down Expand Up @@ -648,7 +648,7 @@ private static void ReportParameterErrors(
diagnostics.Add(ErrorCode.ERR_MethodArgCantBeRefAny, parameterSyntax.Location, parameter.Type);
}

if (parameter.Scope == DeclarationScope.ValueScoped && !parameter.Type.IsErrorTypeOrRefLikeType())
if (parameter.DeclaredScope == DeclarationScope.ValueScoped && !parameter.Type.IsErrorTypeOrRefLikeType())
{
diagnostics.Add(ErrorCode.ERR_ScopedRefAndRefStructOnly, parameterSyntax.Location);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ internal override bool IsMetadataOptional
}
}

internal sealed override DeclarationScope Scope => _originalParam.Scope;
internal sealed override DeclarationScope DeclaredScope => _originalParam.DeclaredScope;
internal sealed override DeclarationScope EffectiveScope => _originalParam.EffectiveScope;

internal override ConstantValue ExplicitDefaultConstantValue
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,22 @@ internal bool HasEnumeratorCancellationAttribute

#nullable enable

internal sealed override DeclarationScope EffectiveScope
{
get
{
var scope = DeclaredScope;
if (scope != DeclarationScope.Unscoped &&
HasUnscopedRefAttribute)
{
return DeclarationScope.Unscoped;
}
return scope;
}
}

private bool HasUnscopedRefAttribute => GetEarlyDecodedWellKnownAttributeData()?.HasUnscopedRefAttribute == true;

internal static SyntaxNode? GetDefaultValueSyntaxForIsNullableAnalysisEnabled(ParameterSyntax? parameterSyntax) =>
parameterSyntax?.Default?.Value;

Expand Down Expand Up @@ -593,6 +609,13 @@ internal override (CSharpAttributeData?, BoundAttribute?) EarlyDecodeWellKnownAt
{
return EarlyDecodeAttributeForDefaultParameterValue(AttributeDescription.DateTimeConstantAttribute, ref arguments);
}
else if (CSharpAttributeData.IsTargetEarlyAttribute(arguments.AttributeType, arguments.AttributeSyntax, AttributeDescription.UnscopedRefAttribute))
{
// We can't bind the attribute here because that might lead to a cycle.
// Instead, simply record that the attribute exists and bind later.
arguments.GetOrCreateData<ParameterEarlyWellKnownAttributeData>().HasUnscopedRefAttribute = true;
return (null, null);
}
else if (!IsOnPartialImplementation(arguments.AttributeSyntax))
{
if (CSharpAttributeData.IsTargetEarlyAttribute(arguments.AttributeType, arguments.AttributeSyntax, AttributeDescription.CallerLineNumberAttribute))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1392,7 +1392,7 @@ internal static bool CheckValidScopedOverride<TArg>(
{
var baseParameter = baseParameters[i];
var overrideParameter = overrideParameters[i + overrideParameterOffset];
if (baseParameter.Scope != overrideParameter.Scope)
if (baseParameter.EffectiveScope != overrideParameter.EffectiveScope)
{
reportMismatchInParameterType(diagnostics, baseMethod, overrideMethod, overrideParameter, topLevel: true, extraArgument);
hasErrors = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -570,6 +570,10 @@ private void DecodeWellKnownAttributeAppliedToMethod(ref DecodeWellKnownAttribut
{
DecodeUnmanagedCallersOnlyAttribute(ref arguments);
}
else if (attribute.IsTargetAttribute(this, AttributeDescription.UnscopedRefAttribute))
{
arguments.GetOrCreateData<MethodWellKnownAttributeData>().HasUnscopedRefAttribute = true;
}
else
{
var compilation = this.DeclaringCompilation;
Expand Down Expand Up @@ -600,6 +604,8 @@ public override FlowAnalysisAnnotations FlowAnalysisAnnotations
private static FlowAnalysisAnnotations DecodeFlowAnalysisAttributes(MethodWellKnownAttributeData attributeData)
=> attributeData?.HasDoesNotReturnAttribute == true ? FlowAnalysisAnnotations.DoesNotReturn : FlowAnalysisAnnotations.None;

internal bool HasUnscopedRefAttribute => GetDecodedWellKnownAttributeData()?.HasUnscopedRefAttribute == true;

private bool VerifyObsoleteAttributeAppliedToMethod(
ref DecodeWellKnownAttributeArguments<AttributeSyntax, CSharpAttributeData, AttributeLocation> arguments,
AttributeDescription description)
Expand Down
Loading

0 comments on commit 7884b57

Please sign in to comment.