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

ref fields updates #6285

Merged
merged 3 commits into from
Jul 19, 2022
Merged

ref fields updates #6285

merged 3 commits into from
Jul 19, 2022

Conversation

jaredpar
Copy link
Member

Updates based on our deployment experiences

Updates based on our deployment experiences
@jaredpar jaredpar requested a review from a team as a code owner July 15, 2022 23:03
@@ -245,8 +249,6 @@ Span<int> ScopedLocalExamples()

Other uses for `scoped` on locals are discussed [below](#examples-scoped-locals).

When `scoped` is applied to an instance method the `this` parameter will have the type `scoped ref T`. By default this is redundant as `scoped ref T` is the default type of `this`. It is useful in the case the `struct` is declared as `unscoped` (detailed [below](#return-fields-by-ref)).
Copy link
Member Author

Choose a reason for hiding this comment

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

As written felt like I was just mostly restating previous stuff. Expanded section above and removed this.

Overall there are three `ref` location which are implicitly declared as `scoped`:
- `this` on a `struct` instance method
- `out` parameters
- `ref` parameters which refer to a `ref struct`
Copy link
Member Author

Choose a reason for hiding this comment

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

This default flip is easy to add to rules and fixes the last usability hurdle we had with ref fields. There are other alternatives we are exploring that would remove this special casing. Short term though wanted to focus on getting the rules tightened up as teams are locking down on implementations.

Next week we'll be exploing the other options to see if we can remove this special case.

In addition to parameters the `scoped` annotation can be applied to locals or `struct` instance methods. These annotations have the same impact to lifetimes when applied to locals. For locals these annotations concretely define the lifetimes and override the lifetime that would be inferred via the initializer.
The `scoped` annotation can also be applied to the following locations:

- locals: This annotation sets the lifetime as *safe-to-escape*, or *ref-safe-to-escape* in case of a `ref` local, to of *current method* irrespective of the initializer lifetime.
Copy link
Member

Choose a reason for hiding this comment

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

to of

Typo?


The presence of `scoped` allows us to refine the rule to reduce the friction the rule creates. The `scoped` modifier lets us remove arguments from consideration as they cannot be returned from the method.
> For any method invocation
> 1. Calculate the *safe-to-escape* of the method return (for this rule assume it has a `ref struct` return)
Copy link
Member

Choose a reason for hiding this comment

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

assume it has a ref struct return

Because we're treating a ref struct return value as a placeholder for any ref or out ref struct argument? If so, could we state that somewhere?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's because if it's a non-ref struct return then safe-to-escape of the return is trivially calling method. Effectively this is saying to "pretend" there is a ref struct return, calculate that safe-to-escape scope and then validate it can be assigned to every ref or out argument.

@jaredpar jaredpar merged commit 3e479f8 into dotnet:main Jul 19, 2022
The `scoped` annotation can also be applied to the following locations:

- locals: This annotation sets the lifetime as *safe-to-escape*, or *ref-safe-to-escape* in case of a `ref` local, to of *current method* irrespective of the initializer lifetime.
- `struct` instance methods: This annotation sets the type of `this` to `scoped ref T`. This is the default value but it is useful in the case the overall `struct` type is declared as `unscoped` (detailed [below](#return-fields-by-ref)).
Copy link

@Sergio0694 Sergio0694 Jul 22, 2022

Choose a reason for hiding this comment

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

Is this still needed now that API review has disallowed adding [UnscopedRef] to a whole type? 🤔
See: dotnet/runtime#72074 (comment). I realize this PR has already been merged, but should we fix this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants