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 UnscopedRef attribute on interface methods and properties #72171

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 84 additions & 3 deletions src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2157,7 +2157,7 @@ private void GetInvocationArgumentsForEscape(
Debug.Assert(receiverIsSubjectToCloning != ThreeState.Unknown);
if (receiverIsSubjectToCloning == ThreeState.True)
{
Debug.Assert(receiver is not BoundValuePlaceholderBase && method is not null && receiver.Type?.IsValueType == true);
Debug.Assert(receiver is not BoundValuePlaceholderBase && method is not null && receiver.Type?.IsReferenceType == false);
#if DEBUG
AssertVisited(receiver);
#endif
Expand Down Expand Up @@ -2251,6 +2251,12 @@ static EscapeArgument getReceiver(MethodSymbol? method, BoundExpression receiver
method.TryGetThisParameter(out thisParameter) &&
thisParameter is not null)
{
if (receiver.Type is TypeParameterSymbol typeParameter)
{
// Pretend that the type of the parameter is the type parameter
thisParameter = new TypeParameterThisParameterSymbol(thisParameter, typeParameter);
}

refKind = thisParameter.RefKind;
}

Expand Down Expand Up @@ -2723,12 +2729,87 @@ private static bool AllParametersConsideredInEscapeAnalysisHaveArguments(
}
#endif

#nullable disable

private static ErrorCode GetStandardCallEscapeError(bool checkingReceiver)
{
return checkingReceiver ? ErrorCode.ERR_EscapeCall2 : ErrorCode.ERR_EscapeCall;
}

private sealed class TypeParameterThisParameterSymbol : ThisParameterSymbolBase
{
private readonly TypeParameterSymbol _type;
private readonly ParameterSymbol _underlyingParameter;

internal TypeParameterThisParameterSymbol(ParameterSymbol underlyingParameter, TypeParameterSymbol type)
{
Debug.Assert(underlyingParameter.IsThis);
Debug.Assert(underlyingParameter.RefKind != RefKind.Out); // Shouldn't get here for a constructor
Debug.Assert(underlyingParameter.ContainingSymbol is MethodSymbol);

_underlyingParameter = underlyingParameter;
_type = type;
}

public override TypeWithAnnotations TypeWithAnnotations
=> TypeWithAnnotations.Create(_type, NullableAnnotation.NotAnnotated);

public override RefKind RefKind
{
get
{
if (_underlyingParameter.RefKind is not RefKind.None and var underlyingRefKind)
{
return underlyingRefKind;
}

if (!_underlyingParameter.ContainingType.IsInterface || _type.IsReferenceType)
{
return RefKind.None;
}

// Receiver of an interface method could possibly be a structure.
// Let's treat it as by ref parameter for the purpose of ref safety analysis.
return RefKind.Ref;
}
}

public override ImmutableArray<Location> Locations
{
get { return _underlyingParameter.Locations; }
}

public override Symbol ContainingSymbol
{
get { return _underlyingParameter.ContainingSymbol; }
}

internal override ScopedKind EffectiveScope
{
get
{
if (HasUnscopedRefAttribute)
{
return ScopedKind.None;
}

if (!_underlyingParameter.ContainingType.IsInterface || _type.IsReferenceType)
{
return ScopedKind.None;
}

// Receiver of an interface method could possibly be a structure.
// Let's treat it as scoped ref by ref parameter for the purpose of ref safety analysis.
Copy link
Member

@jjonescz jjonescz Feb 20, 2024

Choose a reason for hiding this comment

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

Suggested change
// Let's treat it as scoped ref by ref parameter for the purpose of ref safety analysis.
// Let's treat it as scoped ref parameter for the purpose of ref safety analysis.
``` #WontFix

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This isn't actually a typo. the value that we return is ScopedRef and the parameter is ref parameter

return ScopedKind.ScopedRef;
}
}

internal override bool HasUnscopedRefAttribute
=> _underlyingParameter.HasUnscopedRefAttribute;

internal sealed override bool UseUpdatedEscapeRules
=> _underlyingParameter.UseUpdatedEscapeRules;
}

#nullable disable
}

internal partial class Binder
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1211,7 +1211,7 @@ internal ThreeState ReceiverIsSubjectToCloning(BoundExpression? receiver, Proper

internal ThreeState ReceiverIsSubjectToCloning(BoundExpression? receiver, MethodSymbol method)
{
if (receiver is BoundValuePlaceholderBase || receiver?.Type?.IsValueType != true)
if (receiver is BoundValuePlaceholderBase || receiver?.Type is null or { IsReferenceType: true })
{
return ThreeState.False;
}
Expand Down
7 changes: 5 additions & 2 deletions src/Compilers/CSharp/Portable/CSharpResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -7480,10 +7480,10 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<value>UnscopedRefAttribute cannot be applied to this parameter because it is unscoped by default.</value>
</data>
<data name="ERR_UnscopedRefAttributeUnsupportedMemberTarget" xml:space="preserve">
<value>UnscopedRefAttribute can only be applied to struct instance methods and properties, and cannot be applied to constructors or init-only members.</value>
<value>UnscopedRefAttribute can only be applied to struct or virtual interface instance methods and properties, and cannot be applied to constructors or init-only members.</value>
</data>
<data name="ERR_UnscopedRefAttributeInterfaceImplementation" xml:space="preserve">
<value>UnscopedRefAttribute cannot be applied to an interface implementation.</value>
<value>UnscopedRefAttribute cannot be applied to an interface implementation because implemented member '{0}' doesn't have this attribute.</value>
</data>
<data name="ERR_UnrecognizedRefSafetyRulesAttributeVersion" xml:space="preserve">
<value>'{0}' is defined in a module with an unrecognized RefSafetyRulesAttribute version, expecting '11'.</value>
Expand Down Expand Up @@ -7845,4 +7845,7 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="IDS_ImplicitIndexerInitializer" xml:space="preserve">
<value>implicit indexer initializer</value>
</data>
<data name="IDS_RefStructInterfaces" xml:space="preserve">
<value>ref struct interfaces</value>
</data>
</root>
3 changes: 3 additions & 0 deletions src/Compilers/CSharp/Portable/Errors/MessageID.cs
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,8 @@ internal enum MessageID
IDS_StringEscapeCharacter = MessageBase + 12839,

IDS_ImplicitIndexerInitializer = MessageBase + 12840,

IDS_RefStructInterfaces = MessageBase + 12950, // PROTOTYPE(RefStructInterfaces): Pack numbers
}

// Message IDs may refer to strings that need to be localized.
Expand Down Expand Up @@ -461,6 +463,7 @@ internal static LanguageVersion RequiredVersion(this MessageID feature)
// C# preview features.
case MessageID.IDS_StringEscapeCharacter:
case MessageID.IDS_ImplicitIndexerInitializer:
case MessageID.IDS_RefStructInterfaces:
return LanguageVersion.Preview;

// C# 12.0 features.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,8 @@ internal static CSharpSyntaxNode ExtractReturnTypeSyntax(this MethodSymbol metho
internal static bool IsValidUnscopedRefAttributeTarget(this MethodSymbol method)
{
return !method.IsStatic &&
method.ContainingType?.IsStructType() == true &&
method.ContainingType is NamedTypeSymbol containingType &&
(containingType.IsStructType() == true || (containingType.IsInterface && method.IsImplementable())) &&
method.MethodKind is (MethodKind.Ordinary or MethodKind.ExplicitInterfaceImplementation or MethodKind.PropertyGet or MethodKind.PropertySet) &&
!method.IsInitOnly;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -593,6 +593,11 @@ private void DecodeWellKnownAttributeAppliedToMethod(ref DecodeWellKnownAttribut
if (this.IsValidUnscopedRefAttributeTarget())
{
arguments.GetOrCreateData<MethodWellKnownAttributeData>().HasUnscopedRefAttribute = true;

if (ContainingType.IsInterface || IsExplicitInterfaceImplementation)
{
MessageID.IDS_RefStructInterfaces.CheckFeatureAvailability(diagnostics, arguments.AttributeSyntaxOpt);
}
}
else
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1311,6 +1311,11 @@ protected override void DecodeWellKnownAttributeImpl(ref DecodeWellKnownAttribut
if (this.IsValidUnscopedRefAttributeTarget())
{
arguments.GetOrCreateData<PropertyWellKnownAttributeData>().HasUnscopedRefAttribute = true;

if (ContainingType.IsInterface || IsExplicitInterfaceImplementation)
{
MessageID.IDS_RefStructInterfaces.CheckFeatureAvailability(diagnostics, arguments.AttributeSyntaxOpt);
}
}
else
{
Expand Down
Loading
Loading