Spec out as return only #6483
Spec out as return only #6483
Conversation
| M1(out p); | ||
| } | ||
|
|
||
| void M2([UnscopedRef] out RS p) |
There was a problem hiding this comment.
I gave a lot of consideration to [UnscopedRef] out being a compilation error. Basically just cut off the problem. But it is realistic, although very very corner, that someone would have assigned and returned an out to a ref struct by ref so decided to leave it.
There was a problem hiding this comment.
One thing I considered briefly was having the STE of [UnscopedRef] out be CallingMethod, to ensure that outParam = M(ref outParam) is an error. But it just started to feel too complicated. I'm happy with play stupid games, win stupid prizes as a resolution here.
| > 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. | ||
|
|
||
| > Further for any method invocation `e.M(a1, a2, ... aN)` | ||
| > 1. Calculate the *safe-to-escape* of the method return. |
There was a problem hiding this comment.
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.
- Calculate the safe-to-escape of the method return where:
There was a problem hiding this comment.
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.
@cston here is the version where I don't rely on re-using method invocation rules. Do you think this is clearer?
For any method invocation
e.M(a1, a2, ... aN)
- Calculate a safe-to-escape from the minimum of:
- 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
- All
refarguments ofref structtypes must be assignable by a value with that safe-to-escape. This analysis does not considerinoroutarguments.
There was a problem hiding this comment.
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:
- Calculate a safe-to-escape from the minimum of:
There was a problem hiding this comment.
@RikkiGibson are you okay with this change of the MAMM language?
There was a problem hiding this comment.
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.
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.
Okay, updated this section.
|
|
||
| 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.
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.
Implementation side of dotnet/csharplang#6483 This unifies `out` parameters with return values with respect to ref safety. A value that can be returned through one can be returned through the other. closes #62094 closes #64155 closes #64365 **Note** this does not implement having `[UnscopedRef] ref` on a parameter widen the *ref-safe-to-escape*. That will be handled as a follow up change. That is strictly non-breaking, hence lower risk and can be considered in ask mode or post RTM.
|
@cston think it would be fine to just commit the changes you suggested to the PR and merge. |
Co-authored-by: Charles Stoner <10732005+cston@users.noreply.github.com>
Considering how the spec changes would look if we went forward with
outparameters having safe-to-return of return onlydotnet/roslyn#64155