Add annotations for ref fields to public API surface #71265
Conversation
|
Note regarding the This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change. |
ghost
assigned 
|
Tagging subscribers to this area: @dotnet/area-meta Issue DetailsThis adds the necessary annotations to facilitate updating to C# 11. They will all be removed and converted to use the The desire is to also backport this to Preview 6. /cc @stephentoub @jaredpar @cston
|
|
/backport to release/7.0-preview6 |
|
Started backporting to release/7.0-preview6: https://github.com/dotnet/runtime/actions/runs/2556869823 |
| @@ -671,7 +671,7 @@ public static void Write<T>(void* destination, T value) | |||
| // Mono:AsRef | |||
| [NonVersionable] | |||
| [MethodImpl(MethodImplOptions.AggressiveInlining)] | |||
| public static ref T AsRef<T>(in T source) | |||
| public static ref T AsRef<T>([LifetimeAnnotation(true, false)] in T source) | |||
There was a problem hiding this comment.
FYI: this is effectively encoding that AsRef is the single method for stripping all safety from a ref value. It now removes in (changes mutability) and scoped (changes lifetime). I'm 100% okay with this, it's the design I prefer, however IIRC there was some debate on whether we should do this or have a separate method for each type of safety we strip. Wanted to double check that this debate was had and we're not accidentally just making that decision here.
There was a problem hiding this comment.
BTW: don't think it necessarily needs to hold up this merge. Can fix this later. Just didn't want this to be accidentally enshrined as a decision if we hadn't had the proper discussion on it yet
There was a problem hiding this comment.
I'm 100% okay with this, it's the design I prefer,
+1
I recall discussions about fine grained Unsafe. For example, allow unsafe code to express difference between unsafe readonly refs and unsafe writeable refs. We would need to add a number of new methods to Unsafe to the fine grained Unsafe work well end-to-end.
There was a problem hiding this comment.
The main concern on my end is making unsafe code harder to reason about. There are both pros and cons to having a single method here and I think its overall fine for this scenario.
I think the bigger annoyance on my end is actually the IL bloat and readability issues around having to strip in when doing more complex Unsafe logic, even where the underlying value isn't mutated, but that's a separate issue and one that is ideally handled by the planned language to allow migration from ref to in and ref readonly without being a binary or source breaking change.
src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/MemoryMarshal.cs
Show resolved
Hide resolved
|
Wasm failure (1) is acknowledged at #70991 (comment). Failure (2) is #71096. |
This adds the necessary annotations to facilitate updating to C# 11. They will all be removed and converted to use the
scopedkeyword once we update to the latest Roslyn withreffield support.The desire is to also backport this to Preview 6.
/cc @stephentoub @jaredpar @cston