-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Spec out as return only #6483
Spec out as return only #6483
Changes from 4 commits
ced7d7b
b74cee0
639e06b
ee380bf
32a22bf
99e3690
c25884f
a85f49c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -157,7 +157,7 @@ Next the rules for ref reassignment need to be adjusted for the presence of `ref | |
The left operand of the `= ref` operator must be an expression that binds to a ref local variable, a ref parameter (other than `this`), an out parameter, **or a ref field**. | ||
|
||
> For a ref reassignment in the form ... | ||
> 1. `x.e1 = ref e2`: where `x` is *safe-to-escape* to *calling method* then `e2` must be *ref-safe-to-escape* to the *calling method* | ||
> 1. `x.e1 = ref e2`: where `x` is *safe-to-escape* at least *return only* then `e2` must have *ref-safe-to-escape* at least as large as `x` | ||
> 2. `e1 = ref e2`: where `e1` is a `ref` local or `ref` parameter then `e2` must have a *safe-to-escape* equal to *safe-to-escape* for `e1` and `e2` must have *ref-safe-to-escape* at least as large as *ref-safe-to-escape* of the *ref-safe-to-escape* of `e1` | ||
|
||
That means the desired `Span<T>` constructor works without any extra annotation: | ||
|
@@ -171,8 +171,8 @@ readonly ref struct Span<T> | |
public Span(ref T value) | ||
{ | ||
// Falls into the `x.e1 = ref e2` case, where `x` is the implicit `this`. The | ||
// safe-to-escape of `this` and ref-safe-to-escape of `value` is *calling method* hence | ||
// this is legal. | ||
// safe-to-escape of `this` is *return only* and ref-safe-to-escape of `value` is | ||
// *calling method* hence this is legal. | ||
_field = ref value; | ||
_length = 1; | ||
} | ||
|
@@ -313,10 +313,14 @@ The design also requires that the introduction of a new escape scope: *return on | |
|
||
The details of *return only* is that it's a scope which is greater than *current method* but smaller than *calling method*. An expression provided to a `return` statement must be at least *return only*. As such most existing rules fall out. For example assignment into a `ref` parameter from an expression with a *safe-to-escape* of *return only* will fail because it's smaller than the `ref` parameter's *safe-to-escape* which is *calling method*. The need for this new escape scope will be discussed [below](#rules-unscoped). | ||
|
||
A `ref` or `in` parameter for a `ref struct` will have a *ref-safe-to-escape* of *return only*. This is necessary to prevent [silly cyclic assignment](#cyclic-assignment) issues. | ||
There are two locations which default to *return only*: | ||
- A `ref` or `in` parameter for a `ref struct` will have a *ref-safe-to-escape* of *return only*. This is necessary to prevent [silly cyclic assignment](#cyclic-assignment) issues. | ||
- A `out` parameter for a `ref struct` will have *safe-to-escape* of *return only*. This allows for return and `out` to be equally expressive. This does not have the silly cyclic assignment problem because `out` is implicitly `scoped` so the *ref-safe-to-escape* is still smaller than the *safe-to-escape*. | ||
|
||
Any expression or statement which explicitly returns a value from a method or lambda must have a *safe-to-escape*, and if applicable a *ref-safe-to-escape*, of at least *return only*. That includes `return` statements, expression bodied members and lambda expressions. | ||
|
||
Likewise any assignment to an `out` must have a *safe-to-escape* of at least *return only*. This is not a special case though, this just follows from the existing assignment rules. | ||
|
||
<a name="rules-method-invocation"></a> | ||
|
||
The span safety rules for method invocation will be updated in several ways. The first is by recognizing the impact that `scoped` has on arguments. For a given argument `a` that is passed to parameter `p`: | ||
|
@@ -383,11 +387,25 @@ The presence of `ref` fields means the rules around method arguments must match | |
|
||
> For any method invocation `e.M(a1, a2, ... aN)` | ||
> 1. Calculate the *safe-to-escape* of the method return. | ||
> - Ignore the *ref-safe-to-escape* of arguments to `in / ref / out` parameters of `ref struct` types. The corresponding parameters *ref-safe-to-escape* are at most *return only* and hence cannot be returned via `ref` or `out` parameters. | ||
> - Ignore the *ref-safe-to-escape* of arguments whose corresponding parameters have a *ref-safe-to-escape* of *return-only* or smaller. | ||
> - Assume the method has a `ref struct` return type. | ||
> 2. All `ref` or `out` arguments of `ref struct` types must be assignable by a value with that *safe-to-escape*. This applies even when the `ref` argument matches a `scoped ref` parameter. | ||
> 2. All `ref` arguments of `ref struct` types must be assignable by a value with that *safe-to-escape*. This applies even when the `ref` argument matches a `scoped ref` parameter. | ||
|
||
> For any method invocation `e.M(a1, a2, ... aN)` | ||
> 1. Calculate the minimum of the following: | ||
> - *calling method* | ||
> - The *safe-to-escape* of all arguments | ||
> - The *ref-safe-to-escape* of all ref arguments whose corresponding parameters have a *ref-safe-to-escape* of *calling method* or smaller. | ||
> 2. All `ref` arguments of `ref struct` types must be assignable by a value with that *safe-to-escape*. This applies even when the `ref` argument matches a `scoped ref` parameter. | ||
|
||
The presence of `scoped` allows developers to reduce the friction this rule creates by marking parameters which are not returned as `scoped`. This removes their arguments from (1) above and provides greater flexibility to callers. | ||
|
||
> Further for any method invocation `e.M(a1, a2, ... aN)` | ||
> 1. Calculate the *safe-to-escape* of the method return. | ||
> - Ignore the *ref-safe-to-escape* of arguments whose corresponding parameters are `scoped` | ||
> - Assume the method has a `ref struct` return type. | ||
> 2. All `out` arguments of `ref struct` types must be assignable by a value with that *safe-to-escape*. | ||
|
||
The presence of `scoped` allows developers to reduce the friction this rule creates by marking parameters which are not returned as `scoped`. This removes their arguments from (1) in both cases above and provides greater flexibility to callers. | ||
|
||
Impact of this change is discussed more deeply [below](#examples-method-arguments-must-match). Overall this will allow developers to make call sites more flexible by annotating non-escaping ref-like values with `scoped`. | ||
|
||
|
@@ -1211,7 +1229,7 @@ ref struct StackLinkedListNode<T> | |
} | ||
``` | ||
|
||
### Examples | ||
### Examples and Notes | ||
Below are a set of examples demonstrating how and why the rules work the way they do. Included are several examples showing dangerous behaviors and how the rules prevent them from happening. It's important to keep these in mind when making adjustments to the proposal. | ||
|
||
#### Ref reassignment and call sites | ||
|
@@ -1239,16 +1257,17 @@ ref struct RS | |
|
||
public ref int M1(RS rs) | ||
{ | ||
// The arguments contribute here: | ||
// The call site arguments for Prop contribute here: | ||
// - `rs` contributes no ref-safe-to-escape as the corresponding parameter, | ||
// which is `this`, is `scoped ref` | ||
// - `rs` contribute safe-to-escape of *calling method* | ||
// | ||
// This is an lvalue invocation and the arguments contribute only safe-to-escape | ||
// values of *calling method*. That means `local` is safe-to-escape to *calling method* | ||
// values of *calling method*. That means `local1` is ref-safe-to-escape to | ||
// *calling method* | ||
ref int local1 = ref rs.Prop; | ||
|
||
// Okay: this is legal because `local` has safe-to-escape of *calling method* | ||
// Okay: this is legal because `local` has ref-safe-to-escape of *calling method* | ||
return ref local1; | ||
|
||
// The arguments contribute here: | ||
|
@@ -1411,6 +1430,7 @@ class C | |
``` | ||
|
||
#### Preventing tricky ref assignment from readonly mutation | ||
<a name="tricky-ref-assignment"></a> | ||
When a `ref` is taken to a `readonly` field in a constructor or `init` member the type is `ref` not `ref readonly`. This is a long standing behavior that allows for code like the following: | ||
|
||
```c# | ||
|
@@ -1491,9 +1511,9 @@ S Usage() | |
} | ||
``` | ||
|
||
To make these APIs usable the compiler has to create separation between how freely `ref` and their values can be returned by default. This is the rational for having `ref` to `ref struct` and `out` be [implicitly scoped](#implicitly-scoped). | ||
To make these APIs usable the compiler ensures that the `ref` lifetime for a `ref` parameter is smaller than lifetime of any references in the associated parameter value. This is the rationale for having *ref-safe-to-escape* for `ref` to `ref struct` be *return only* and `out` be *containing method*. That prevents cyclic assignment because of the difference in lifetimes. | ||
|
||
It is also why `[UnscopedRef]` only promotes the *ref-safe-to-escape* of such values to *return only* and not *calling method*. Consider that using *calling method* forces a viral use of `[UnscopedRef]` for a `ref struct`: | ||
It is also why `[UnscopedRef]` only promotes the *ref-safe-to-escape* of any `ref` to `ref struct` values to *return only* and not *calling method*. Consider that using *calling method* allows for cyclic assignment and would force a viral use of `[UnscopedRef]` for a `ref struct`: | ||
|
||
```c# | ||
ref struct S | ||
|
@@ -1513,6 +1533,30 @@ void M(ref S s) | |
|
||
This is correctly illegal in that case because the compiler has to consider the pathological case that `S.Data` could cyclic assign via `this`. That forces methods all methods that call `S.Data` to further mark their `ref` parameters as `[UnscopedRef]`. This is viral until the method which creates the value as a local. This is why *return only* exists as an escape scope. It does complicate the spec / implementation but it serves to make the feature significantly more usable. | ||
|
||
Note: this cyclic assignment problem does continue to exist for `[UnscopedRef] out` to `ref struct` because that causes the *safe-to-escape* and *ref-safe-to-escape* to be equivalent. | ||
|
||
```c# | ||
ref struct RS | ||
{ | ||
int field; | ||
ref int refField; | ||
} | ||
|
||
void M1(out RS p) | ||
{ | ||
// Error: from method arguments must match: | ||
// Step 1 would calculate the narrowest escape as *containing method* | ||
// Step 2 would fail the assignment check because p safe-to-escape is *return only* | ||
M2(out p); | ||
} | ||
|
||
void M2([UnscopedRef] out RS p) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I gave a lot of consideration to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One thing I considered briefly was having the STE of |
||
{ | ||
// The lifetimes of LHS and RHS are equivalent here and hence this is legal | ||
p.refField = ref p.Field; | ||
} | ||
``` | ||
|
||
In terms of advanced annotations the `[UnscopedRef]` design creates the following: | ||
|
||
``` | ||
|
@@ -1569,6 +1613,42 @@ readonly ref struct SpanOfOne | |
|
||
This means we must choose the shallow interpretation of `readonly`. | ||
|
||
#### Modeling constructors | ||
One subtle design question is: How are constructors bodies modeled for ref safety? Essentially how is the following constructor analyzed? | ||
|
||
```c# | ||
ref struct S | ||
{ | ||
int field; | ||
|
||
public S(ref int f) | ||
{ | ||
field = ref f; | ||
} | ||
} | ||
``` | ||
|
||
There are roughly two approaches: | ||
|
||
1. Model as a `static` method where `this` is a local where it's *safe-to-escape* is *calling method* | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it has to be a local. Consider that a constructor body has assignments, method calls, etc ... All of those need to happen on a value. If it's return only when binding the method body how do we evaluate the calls? A local works because it can be STS return only and RSTS containing method. Yes that means there is a copy at the end. That would matter if we actually implemented ctors that way. Instead this is just how do we model ref safety analysis. A local mostly works.
jaredpar marked this conversation as resolved.
Show resolved
Hide resolved
|
||
2. Model as a `static` method where `this` is an `out` parameter. | ||
|
||
Further a constructor must meet the following invariants: | ||
|
||
1. Ensure that `ref` parameters can be captured as `ref` fields. | ||
2. Ensure that `ref` to fields of `this` cannot be escaped through `ref` parameters. That would violate [tricky ref assignment](#tricky-ref-assignment). | ||
|
||
The intent is to pick the form that satisfies our invariants without introduction of any special rules for constructors. Given that the best model for constructors is viewing `this` as an `out` parameter. The *return-only* nature of the `out` allows us to satisfy all the invariants above without any special casing: | ||
|
||
```c# | ||
public static void ctor(out S @this, ref int f) | ||
{ | ||
// The ref-safe-to-escape of `ref f` is *return-only* which is also the | ||
// safe-to-escape of `this.field` hence this assignment is allowed | ||
@this.field = ref f; | ||
} | ||
``` | ||
|
||
#### Method arguments must match | ||
The method arguments must match rule is a common source of confusion for developers. It's a rule which has a number of special cases that are hard to understand unless you are familiar with the reasoning behind the rule. For the sake of better understanding the reasons for the rule we will simplify ref-safe-to-escape* and *safe-to-escape* to simply *escape-scope*. | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a little confusing that we're calling this the "safe-to-escape of the method return", and also in the previous set of bullets, but we ignore different arguments in each case.
I'm not sure how to make it clearer though. Perhaps just add a "where:" to the end of bullet 1 in each case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was really trying to lean on method invocation rules here and essentially re-use them. Cause really that is what this is doing, using the method invocation rules to calculate the narrowest possible return and validate it's okay to assign to every
ref
/out
parameter. The method invocation rules are simple enough now, maybe just restate them here and note that this is effectively applyiyng method escape rules.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cston here is the version where I don't rely on re-using method invocation rules. Do you think this is clearer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That sounds good. One concern though is step 2 refers to "a value with that safe-to-escape" but step 1 doesn't call the result a safe-to-escape value. Perhaps step 1 could be:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@RikkiGibson are you okay with this change of the MAMM language?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think of calling method as the "minimum" scope, so I found the language confusing.
Consider using a term like "narrowest" instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general I'd like if we could avoid saying bigger, smaller, minimum, etc. for scope, and say narrower, wider, narrowest, widest instead. Otherwise mentally switching between the spec and implementation becomes very difficult.
I imagine that when we invest in this area of the compiler later on, we could consider introducing a scope type where "narrower" scopes are also considered "smaller" per the comparison operators. I think that's one of several aspects of the implementation which makes understanding it more difficult.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, updated this section.