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

[API Proposal]: System.Diagnostics.CodeAnalysis.UnscopedRefAttribute #72074

Closed
cston opened this issue Jul 13, 2022 · 10 comments · Fixed by #72499
Closed

[API Proposal]: System.Diagnostics.CodeAnalysis.UnscopedRefAttribute #72074

cston opened this issue Jul 13, 2022 · 10 comments · Fixed by #72499
Labels
api-approved API was approved in API review, it can be implemented area-System.Runtime.CompilerServices blocking Marks issues that we want to fast track in order to unblock other important work
Milestone

Comments

@cston
Copy link
Member

cston commented Jul 13, 2022

Background and motivation

There are several cases where the C# compiler treats a ref as implicitly scoped, where the compiler does not allow the ref to escape the method.

  1. this for struct instance methods
  2. ref parameters that refer to ref struct types
  3. out parameters

For specific instances where the ref should be allowed to escape, we should provide a well-known attribute that the compiler will recognize that indicates the ref is not scoped.

See low-level-struct-improvements.md.

API Proposal

namespace System.Diagnostics.CodeAnalysis
{
    [AttributeUsage(
        AttributeTargets.Struct | AttributeTargets.Method | AttributeTargets.Property | AttributeTargets.Parameter,
        AllowMultiple = false,
        Inherited = false)]
    public sealed class RefEscapesAttribute : Attribute
    {
    }
}

API Usage

When applied to struct instance methods and properties, this ref is unscoped.

struct S1
{
    private int _field;
    [RefEscapes] public ref int Property => ref _field; // ok
    [RefEscapes] public ref int GetField() => ref _field; // ok
}

When applied to a struct type, this ref is unscoped for all instance methods and properties.

[RefEscapes]
struct S2
{
    private int _field;
    public ref int Property => ref _field; // ok
    public ref int GetField() => ref _field; // ok
}

[RefEscapes]
struct S3
{
    private S2 _child;
    public ref int Value => ref _child.Property; // ok
}

When applied to a ref parameter, the ref is unscoped.

ref struct R { }

ref R ReturnRefStructRef(bool b, ref R x, [RefEscapes] ref R y)
{
    if (b)
        return ref x; // error: Cannot return parameter by reference
    else
        return ref y; // ok
}

When applied to an out parameter, the ref is unscoped.

ref int ReturnOut(bool b, out int x, [RefEscapes] out int y)
{
    x = 1;
    y = 2;
    if (b)
        return ref x; // error: Cannot return parameter by reference
    else
        return ref y; // ok
}

Alternative Designs

Without a well-known attribute recognized by the compiler, callers will need to use Unsafe.AsRef() and similar methods to return a scoped ref.

An alternative attribute name: UnscopedAttribute.

Risks

No response

@cston cston added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Jul 13, 2022
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jul 13, 2022
@ghost
Copy link

ghost commented Jul 13, 2022

Tagging subscribers to this area: @dotnet/area-system-runtime-compilerservices
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

There are several cases where the C# compiler treats a ref as implicitly scoped, where the compiler does not allow the ref to escape the method.

  1. this for struct instance methods
  2. ref parameters that refer to ref struct types
  3. out parameters

For specific instances where the ref should be allowed to escape, we should provide a well-known attribute that the compiler will recognize that indicates the ref is not scoped.

See low-level-struct-improvements.md.

API Proposal

namespace System.Diagnostics.CodeAnalysis
{
    [AttributeUsage(
        AttributeTargets.Struct | AttributeTargets.Method | AttributeTargets.Property | AttributeTargets.Parameter,
        AllowMultiple = false,
        Inherited = false)]
    public sealed class RefEscapesAttribute : Attribute
    {
    }
}

API Usage

When applied to struct instance methods and properties, this ref is unscoped.

struct S1
{
    private int _field;
    [RefEscapes] public ref int Property => ref _field; // ok
    [RefEscapes] public ref int GetField() => ref _field; // ok
}

When applied to a struct type, this ref is unscoped for all instance methods and properties.

[RefEscapes]
struct S2
{
    private int _field;
    public ref int Property => ref _field; // ok
    public ref int GetField() => ref _field; // ok
}

[RefEscapes]
struct S3
{
    private S2 _child;
    public ref int Value => ref _child.Property; // ok
}

When applied to a ref parameter, the ref is unscoped.

ref struct R { }

ref R ReturnRefStructRef(bool b, ref R x, [RefEscapes] ref R y)
{
    if (b)
        return ref x; // error: Cannot return parameter by reference
    else
        return ref y; // ok
}

When applied to an out parameter, the ref is unscoped.

ref int ReturnOut(bool b, out int x, [RefEscapes] out int y)
{
    x = 1;
    y = 2;
    if (b)
        return ref x; // error: Cannot return parameter by reference
    else
        return ref y; // ok
}

Alternative Designs

Without a well-known attribute recognized by the compiler, callers will need to use Unsafe.AsRef() and similar methods to return a scoped ref.

An alternative attribute name: UnscopedAttribute.

Risks

No response

Author: cston
Assignees: -
Labels:

api-suggestion, area-System.Runtime.CompilerServices

Milestone: -

@cston
Copy link
Member Author

cston commented Jul 13, 2022

cc @jaredpar, @AaronRobinsonMSFT

@AaronRobinsonMSFT AaronRobinsonMSFT removed the untriaged New issue has not been triaged by the area owner label Jul 13, 2022
@AaronRobinsonMSFT AaronRobinsonMSFT added this to the 8.0.0 milestone Jul 13, 2022
@AaronRobinsonMSFT
Copy link
Member

@cston Is this expected for .NET 8 or do we need it for .NET 7?

@Sergio0694
Copy link
Contributor

Why is the attribute needed here:

ref struct R { }

ref R ReturnRefStructRef(bool b, ref R x, [RefEscapes] ref R y)
{
    if (b)
        return ref x; // error: Cannot return parameter by reference
    else
        return ref y; // ok
}

Wouldn't those refs all be escaping, and in order to get only the second one to be so, the first should be marked as scoped ref instead? 🤔

@stephentoub
Copy link
Member

@cston, is this instead of the proposed "unscoped" keyword?

@cston
Copy link
Member Author

cston commented Jul 13, 2022

@Sergio0694, it's likely that ref parameters to ref struct types will be considered implicitly scoped ref.

@cston
Copy link
Member Author

cston commented Jul 13, 2022

@stephentoub, yes, this attribute is instead of the proposed unscoped keyword.

@cston
Copy link
Member Author

cston commented Jul 13, 2022

@AaronRobinsonMSFT, we should add this in .NET 7 if possible.

The attribute will be especially important after the compiler has been updated to treat ref parameters to ref struct types as scoped ref.

@Sergio0694
Copy link
Contributor

"it's likely that ref parameters to ref struct types will be considered implicitly scoped ref."

Q: wouldn't that make it harder to read and understand code, if the scope of refs will now depend on how the type itself is defined rather than just what's visible at the parameter declaration? As in, I can no longer just see whether I'm taking a ref or a scoped ref, but now I have to CTRL + click on that type to double check whether it's a struct or a ref struct to understand whether that ref has been secretly downgraded to a scoped ref. Not sure I like that change, personally 🥲

"yes, this attribute is instead of the proposed unscoped keyword."

Just so I understand, this is only so that we can enable this in C# 11 already by manually using the attribute (and allow people to escape fields by ref), while language support isn't there yet, but the plan is still to eventually get unscoped in C# 12, right?

@stephentoub stephentoub modified the milestones: 8.0.0, 7.0.0 Jul 13, 2022
@stephentoub stephentoub added blocking Marks issues that we want to fast track in order to unblock other important work api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Jul 13, 2022
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jul 20, 2022
@terrajobst
Copy link
Member

terrajobst commented Jul 21, 2022

Video

  • We'd like to align the names between the synthesized attribute, the language keyword, and this new attribute
    • Could we go for [ScopedRef] and [UnscopedRef]?
  • We don't believe we should have AttributeTargets.Struct
    • This should be rare, so consumers should be selective anyway
namespace System.Diagnostics.CodeAnalysis;

[AttributeUsage(AttributeTargets.Method |
                AttributeTargets.Property |
                AttributeTargets.Parameter,
                AllowMultiple = false,
                Inherited = false)]
public sealed class UnscopedRefAttribute : Attribute
{
    public UnscopedRefAttribute();
}

@terrajobst terrajobst added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Jul 21, 2022
@AaronRobinsonMSFT AaronRobinsonMSFT changed the title [API Proposal]: System.Diagnostics.CodeAnalysis.RefEscapesAttribute [API Proposal]: System.Diagnostics.CodeAnalysis.UnscopedRefAttribute Jul 21, 2022
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jul 22, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Aug 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Runtime.CompilerServices blocking Marks issues that we want to fast track in order to unblock other important work
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants