S.R.CS.Unsafe and read-only references

[ektrah]

Most (if not all) APIs today are defined to accept references with ref rather than ref readonly. This includes all methods of the Unsafe class. This proposal is to provide a method to turn a ref readonly T into a ref T, so that read-only references can be passed to methods that use ref but are known to be only reading them.

 public static partial class Unsafe
 {
     ...
+    public static ref T AsRef<T>(ref readonly T source)
 }

Original proposals (click to expand)

A few proposals for APIs related to the System.Runtime.CompilerServices.Unsafe class and read-only references.

Proposal 1: Change ref T to ref readonly T where possible:

 public static class Unsafe
 {
     public static ref T AddByteOffset<T>(ref T source, System.IntPtr byteOffset)
     public static ref T Add<T>(ref T source, int elementOffset)
     public static void* Add<T>(void* source, int elementOffset)
     public static ref T Add<T>(ref T source, System.IntPtr elementOffset)
-    public static bool AreSame<T>(ref T left, ref T right)
+    public static bool AreSame<T>(ref readonly T left, ref readonly T right)
-    public static void* AsPointer<T>(ref T value)
+    public static void* AsPointer<T>(ref readonly T value)
     public static ref T AsRef<T>(void* source)
     public static T As<T>(object o) where T : class
     public static ref TTo As<TFrom, TTo>(ref TFrom source)
-    public static System.IntPtr ByteOffset<T>(ref T origin, ref T target)
+    public static System.IntPtr ByteOffset<T>(ref readonly T origin, ref readonly T target)
-    public static void CopyBlock(ref byte destination, ref byte source, uint byteCount)
+    public static void CopyBlock(ref byte destination, ref readonly byte source, uint byteCount)
     public static void CopyBlock(void* destination, void* source, uint byteCount)
-    public static void CopyBlockUnaligned(ref byte destination, ref byte source, uint byteCount)
+    public static void CopyBlockUnaligned(ref byte destination, ref readonly byte source, uint byteCount)
     public static void CopyBlockUnaligned(void* destination, void* source, uint byteCount)
-    public static void Copy<T>(void* destination, ref T source)
+    public static void Copy<T>(void* destination, ref readonly T source)
     public static void Copy<T>(ref T destination, void* source)
     public static void InitBlock(ref byte startAddress, byte value, uint byteCount)
     public static void InitBlock(void* startAddress, byte value, uint byteCount)
     public static void InitBlockUnaligned(ref byte startAddress, byte value, uint byteCount)
     public static void InitBlockUnaligned(void* startAddress, byte value, uint byteCount)
     public static T Read<T>(void* source)
     public static T ReadUnaligned<T>(void* source)
-    public static T ReadUnaligned<T>(ref byte source)
+    public static T ReadUnaligned<T>(ref readonly byte source)
     public static int SizeOf<T>()
     public static ref T SubtractByteOffset<T>(ref T source, System.IntPtr byteOffset)
     public static ref T Subtract<T>(ref T source, int elementOffset)
     public static void* Subtract<T>(void* source, int elementOffset)
     public static ref T Subtract<T>(ref T source, System.IntPtr elementOffset)
     public static void Write<T>(void* destination, T value)
     public static void WriteUnaligned<T>(void* destination, T value)
     public static void WriteUnaligned<T>(ref byte destination, T value)
 }

Proposal 2: Unsafe but every ref T is a ref readonly T:

+public static class UnsafeReadOnly
+{
+    public static ref readonly T AddByteOffset<T>(ref readonly T source, System.IntPtr byteOffset)
+    public static ref readonly T Add<T>(ref readonly T source, int elementOffset)
+    public static void* Add<T>(void* source, int elementOffset)
+    public static ref readonly T Add<T>(ref readonly T source, System.IntPtr elementOffset)
+    public static bool AreSame<T>(ref readonly T left, ref readonly T right)
+    public static void* AsPointer<T>(ref readonly T value)
+    public static ref readonly T AsRef<T>(void* source)
+    public static T As<T>(object o) where T : class
+    public static ref readonly TTo As<TFrom, TTo>(ref readonly TFrom source)
+    public static System.IntPtr ByteOffset<T>(ref readonly T origin, ref readonly T target)
+    public static void CopyBlock(ref readonly byte destination, ref readonly byte source, uint byteCount)
+    public static void CopyBlock(void* destination, void* source, uint byteCount)
+    public static void CopyBlockUnaligned(ref readonly byte destination, ref readonly byte source, uint byteCount)
+    public static void CopyBlockUnaligned(void* destination, void* source, uint byteCount)
+    public static void Copy<T>(void* destination, ref readonly T source)
+    public static void Copy<T>(ref readonly T destination, void* source)
+    public static void InitBlock(ref readonly byte startAddress, byte value, uint byteCount)
+    public static void InitBlock(void* startAddress, byte value, uint byteCount)
+    public static void InitBlockUnaligned(ref readonly byte startAddress, byte value, uint byteCount)
+    public static void InitBlockUnaligned(void* startAddress, byte value, uint byteCount)
+    public static T Read<T>(void* source)
+    public static T ReadUnaligned<T>(void* source)
+    public static T ReadUnaligned<T>(ref readonly byte source)
+    public static int SizeOf<T>()
+    public static ref readonly T SubtractByteOffset<T>(ref readonly T source, System.IntPtr byteOffset)
+    public static ref readonly T Subtract<T>(ref readonly T source, int elementOffset)
+    public static void* Subtract<T>(void* source, int elementOffset)
+    public static ref readonly T Subtract<T>(ref readonly T source, System.IntPtr elementOffset)
+    public static void Write<T>(void* destination, T value)
+    public static void WriteUnaligned<T>(void* destination, T value)
+    public static void WriteUnaligned<T>(ref readonly byte destination, T value)
+}

Proposal 3: A method to turn a ref readonly T into a ref T:

 public static partial class Unsafe
 {
     ...
+    public static ref T AsRef<T>(ref readonly T source)
 }

Potential problems:

Passing a ref readonly T argument does not look different from passing a T argument, so the following code is legal but very surprising:

byte[] buffer = new byte[16];
Unsafe.CopyBlockUnaligned(ref buffer[0], 123, 16); // what does this copy?

[jkotas]

@VSadov Is changing ref T to ref readonly T in the method signatures a breaking change?

[jkotas]

Proposal 1 looks fine if it is not breaking change. Not sure what the rules are.
Proposal 2 is overkill, I think.
Proposal 3 looks good. It is the universal hammer that lets you do everything. We should do that for sure.

[VSadov]

Changing ref --> readonly ref is a source-breaker to callers. They no longer need/can use "ref" at the call site.

Generally "ref readonly" works well when we intend to read the value. Volatile.Read would be a good example.

When dealing with references, especially when reference types are allowed, it may be confusing.
IsSame(obj1, obj2) - are we comparing variables or their values?

The 3rd thing will allow most what you want.
AsRef(o.readonlyField) - cast away readonly. Could be useful/dangerous.
AsRef(42); - do something strange. Hardly useful.

Another concern is that "ref readonly" allows implicit conversions at the call site. Including those that do not preserve identity.
Generally one must be very careful when relying on aliasing. "ref readonly" does not guarantee that in the same way as "ref", only makes a good effort.

[jkotas]

Changing ref --> readonly ref is a source-breaker

Ok, so this takes 1st thing out.

@ektrah Could you please update the proposal to have the 3rd thing at the top to prep it for the API review?

[ektrah]

@jkotas - done.

[jkotas]

@ektrah Thanks!

[tannergooding]

Shouldn't there also be a method to directly convert a ref readonly to/from a pointer, so that you don't need to perform an intermediate conversion to ref first.

[jkotas]

You can say that about other Unsafe APIs that take ref as well ... the question is where to stop. The proposed API makes things possible; more APIs are just for convenience.

[terrajobst]

Video

Looks good as proposed.

[karelz]

FYI: The API review discussion was recorded - see https://youtu.be/m4BUM3nJZRw?t=5923 (5 min duration)

[ektrah]

I would grab this, but I have a few questions first.

I guess the implementation is the same as for the existing AsRef method...

https://github.com/dotnet/corefx/blob/883ce3820853a1dbff5902b9428369135c0d327a/src/System.Runtime.CompilerServices.Unsafe/src/System.Runtime.CompilerServices.Unsafe.il#L261-L283

... except for the method signature:

.method public hidebysig static !!T& AsRef<T>(!!T& source) cil managed aggressiveinlining
                                              ~~~~

• How are ref readonly in parameters denoted in IL?
• Is the #ifdef needed?
• Does any file outside https://github.com/dotnet/corefx/tree/master/src/System.Runtime.CompilerServices.Unsafe need to be modified?

[jkotas]

• They are marked with IsReadOnlyAttribute.

For netcoreapp builds, it should look like this:

  .method private hidebysig static !!T&  Test<T>(!!T& 'value') cil managed
  {
    .param [1]
    .custom instance void [CORE_ASSEMBLY]System.Runtime.CompilerServices.IsReadOnlyAttribute::.ctor() = ( 01 00 00 00 ) 

For non-netcoreapp builds, it should look like this:

  .method private hidebysig static !!T&  Test<T>(!!T& 'value') cil managed
  {
    .param [1]
    .custom instance void System.Runtime.CompilerServices.IsReadOnlyAttribute::.ctor() = ( 01 00 00 00 ) 

And there needs to be a local definition of IsReadOnlyAttribute included in the file for non-netcoreapp builds.

The best way to figure this out is to compile a small test program that has ref readonly method using Roslyn that has ref readonly support, run ildasm on it and copy&paste the local definition of the IsReadOnlyAttribute from that. You can use Roslyn compiler that is downloaded to build the repo, e.g. on Windows - tools\microsoft.net.compilers\2.6.0-beta1-62126-01\tools\csc /langversion:latest test.cs

• The ifdef is not needed. The short version will work everywhere.
• All files that need to be touched should be under src\System.Runtime.CompilerServices.Unsafe

[tannergooding]

@jkotas, I thought they additionally had a modreq? https://github.com/dotnet/csharplang/blob/ac995bd794be5a17acb52c30dfa4d61a1bfd15c9/proposals/csharp-7.2/readonly-ref.md#metadata-representation

CC @VSadov

[jcouv]

@tannergooding Yes, the modreq should be there and the compiler will require that it be there when loading [IsReadOnly]. We have a bug on that for 15.5 (the current implementation accepts such metadata, but it won't after the bug is fixed).