-
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
Expanding ref support #7555
Expanding ref support #7555
Conversation
This proposal expands the `ref` support in the compiler to include: 1. `ref struct` as `ref` fields 2. `ref scoped` parameters and fields 3. Fully sunsetting restricted types At the moment I'm not proposing that LDM move forward with the design. I'm still deciding if the complexity / cost is worth the gain it provides. Yet recent discussions crystalized in my mind how this would all work hence I wanted to get it written down and out for review. The intent here is to socialize these ideas, get feedback on where this would or would not help and then make a decision on whether to push this through LDM / future .NET release.
Confirmed that I see this, but currently a bit underwater and it may take me some time to sit down and digest it |
No rush on this. Was just paged in from other discussions so I wanted to get it written out. |
|
||
Once `ref` fields are available and extended to support `ref struct` these types can be fully rationalized within those rules. As such the compiler will no longer have the notion of restricted types when using a language version that supports `ref` fields of `ref struct`. | ||
|
||
To support this our `ref` safety rules will be updated as follows: |
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.
Can we assume updates to the following don't indicate anything other than clarity on their semantics rather than changing anything about their "officially supported" state.
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.
Correct. This is just about getting clarity on the semantics. Particularly I know the runtime team has wanted to leverage TypedReference
more and this would provide clarity on what can and cannot be done safely.
- The `ref` of a variable has a lifetime of the | ||
- For a ref local, parameter, field or return of type `ref<$a> T` the lifetime is `$a` | ||
- For a `ref` field defined as `ref<$l0> T<$l1, $l2, ... $ln>`: | ||
- All lifetimes `$l1` through `$ln` must be invariant. |
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.
Is this true even for a ref readonly
field?
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 possible that readonly
could allow us to relax the restriction a bit here. At the same time it's something we'd have to really map out to make sure we weren't missing anything.
## Detailed Design | ||
The rules for `ref struct` safety are defined in the following documents: | ||
|
||
- [ref safety proposal](https://github.com/dotnet/csharplang/blob/master/proposals/csharp-7.2/span-safety.md). |
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 it's hard to get use out of this document now because you have to have the other document at hand to cross-reference everything that has changed in later versions of C#.
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.
At some point I do need to put together a unified doc that we can reference.
|
||
This is accomplished by giving every `ref scoped` parameter a new escape scope named _current parameter N_ where _N_ is the numeric order of the parameter. For example the first parameter has a _safe-to-escape_ of _current parameter 1_. An escape scope of _current parameter N_ can be converted to _current method_ but has no other defined relationship. That serves to restrict their usage to the current method. | ||
|
||
It's important to note each parameter has a different _current parameter N_ scope. That means they cannot be assigned to each other. This is necessary to prevent `ref scoped` parameters from returning each others data. |
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 this is a critical concept. The implementation currently, by representing lifetimes as uint
, requires every lifetime to be comparable to every other lifetime. It's equivalent to it being impossible to declare void M<T, U>() {}
without also saying where T : U
or where U : T
.
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.
Is there a need to do something similar for value-scoped
parameters, once we permit RTRS fields? Since it feels like it will be possible to declare a similar signature, just using void Swap(RefSpanWrapper<int> p1, RefSpanWrapper<int> p2)
.
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 agree our current implementation does not lend itself to this type of validation. This was one of the motivations I had in wanting us to refactor the implementation significantly in C# 12.0 to better match the existing spec. That would've made the transition to this much easier. As is though we effectively need to do that work before we can do this feature.
proposals/expand-ref.md
Outdated
} | ||
``` | ||
|
||
This is accomplished by giving every `ref scoped` field a new two new escape scopes named _current field N_ and _current ref field N_ where _N_ is the numeric order of the field. For example the first field has a _safe-to-escape_ of _current field 1_ and a _ref-safe-to-escape_ of _current ref field N_. Both escape scopes can be converted to _current method_, and _current field N_ can be converted to _current ref field N_, but no other defined relationships exist. That serves to restrict their usage to the current method where the containing value is used. This escape scope applies to both |
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 that all lifetimes $current_field_N
and $current_ref_field_N
must also be convertible to lifetime $this
. In the absence of this constraint, it would be possible for the this
variable to hold references to fields with narrower lifetimes.
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.
Or, depending on how you look at it, it simply wouldn't be possible to declare any 'ref scoped' fields in ref structs, because the field's lifetime would not be convertible to the lifetime of the containing 'this' variable.
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 disagree. Logically yes I agree that the lifetimes must be convertible to $this
. The moment you allow that though in all contexts, suddenly it's not scoped
anymore, it's returnable
struct S<$this, $refField1, $field1>
where $refField : $this
where $field1: $this, $refField1 {
ref<$refField1> Span<$field1, byte> span;
}
Span<$cm> M<$cm, $l1, $l2>(S<$cm, $l1, $l2> s) =>
// works because $l2 is convertible to $cm
s.span;
}
The way this works is that at construction time the rules ensure that the lifetimes of both the ref
and the value are convertible to $this
. That just falls out from our existing method args rules. Given that the fields lifetimes are invariant there is no need to keep that relationship alive for correctness after that. The only thing we have to worry about is how they are interpreted in the context of a method.
Note: if these were ref unscoped
fields then you are 100% correct we'd put that relationship in place at the decl of the type.
Co-authored-by: Jan Jones <[email protected]> Co-authored-by: Aaron Robinson <[email protected]>
Co-authored-by: Charles Stoner <[email protected]>
This proposal expands the
ref
support in the compiler to include:ref struct
asref
fieldsref scoped
parameters and fieldsAt the moment I'm not proposing that LDM move forward with the design. I'm still deciding if the complexity / cost is worth the gain it provides. Yet recent discussions crystalized in my mind how this would all work hence I wanted to get it written down and out for review.
The intent here is to socialize these ideas, get feedback on where this would or would not help and then make a decision on whether to push this through LDM / future .NET release.