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

Report warning for unannotated parameters that could be marked scoped #64344

Open
cston opened this issue Sep 28, 2022 · 2 comments
Open

Report warning for unannotated parameters that could be marked scoped #64344

cston opened this issue Sep 28, 2022 · 2 comments

Comments

@cston
Copy link
Member

cston commented Sep 28, 2022

Unscoped parameters are typically noticed at call-sites and only when an error is reported if the call-site has a mismatch of lifetimes. If we report a warning at the method declaration, that should catch cases where parameters were left unscoped unintentionally. The warning should also reduce the risk that a parameter needs to be marked scoped later, which is a potential API breaking change.

The warning would be reported for a ref, in, or ref struct parameter where references do not escape the method, and where:

  • The method returns a ref or ref readonly or ref struct or
  • The method has a ref or out parameter of ref struct type.

The warning could be a warning wave warning.

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Sep 28, 2022
@cston cston self-assigned this Sep 28, 2022
@cston cston added this to the C# 12.0 milestone Sep 28, 2022
@Sergio0694
Copy link
Contributor

Sergio0694 commented Sep 29, 2022

Related question on annotating parameters or not: now that we have the return-only scope, is there any value in annotating a ref or in or ref struct (value) parameter to a mutable method in a ref struct type as scoped? As in, given the escape scope of those (unannotated) parameters is return-only, that means that you would still not be able to capture them into the instance ref struct the method is invoked upon, so would that annotation do anything or is it just no longer needed at all? 🤔

As in (using the explicit ref this syntax for clarity):

ref struct Foo
{
    static void SomeMutatingMethod(scoped ref Foo @this, ref int x)
    {
    }

    static void AnotherMutatingMethod(scoped ref Foo @this, Span<byte> span)
    {
    }
}

Is there any reason to annotate x or span in those two methods as scoped?

EDIT: I guess this is the same as #64365?

@jaredpar
Copy link
Member

The scoped annotation is beneficial in that it helps the compiler understand what is or is not returnable. Returnable can take the form of explicit return or assignment to a ref / out parameter.

static void SomeMutatingMethod(scoped ref Foo @this, ref int x);

There is no benefit to annotating this method. It's void so nothing can be returned and because ref int is RSTE return only it can't be captured through @this. The defaults of this specific combination of parameters conspire to make nothing capturable.

However it can me made relevant with a very minor tweak

static ref int SomeMutatingMethod(scoped ref Foo @this, ref int x);

Now the scoped is beneficial because it tells the compiler which of the inputs is not part of the return.

Is there any reason to annotate x or span in those two methods as scoped?

There is no functional reason in terms of ref safety. My guidance though would be a bit different though:

Any time a method has a ref return or uses a ref struct then use scoped on any ref-like value that does not get returned from the method.

The rules around lifetime can be subtle in places and it's easy to make mistakes. Rather than trying to understand the full set of rules and only use scoped where required I would say use scoped when you know a value isn't returned.

@jaredpar jaredpar added Bug and removed untriaged Issues and PRs which have not yet been triaged by a lead labels Oct 10, 2022
@jaredpar jaredpar modified the milestones: C# 12.0, Backlog Sep 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants