Skip to content
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

REDUNDANT S.R.CS.Unsafe: Revise API to accomodate for ref readonly/in #24399

Closed
nietras opened this issue Dec 9, 2017 · 5 comments
Closed

REDUNDANT S.R.CS.Unsafe: Revise API to accomodate for ref readonly/in #24399

nietras opened this issue Dec 9, 2017 · 5 comments
Milestone

Comments

@nietras
Copy link
Contributor

nietras commented Dec 9, 2017

Already discussed, see https://github.com/dotnet/corefx/issues/23916 and use:

public static ref T AsRef<T>(in T source)

REDUNDANT!

With the introduction of ref readonly return values and in (i.e. ref readonly) parameters the existing Unsafe API should be considered for revision with respect to how to support these naturally.

Rationale and Usage

Naturally support ref readonly usage in the Unsafe API, to allow using this on readonly data.

Proposed API

A tentative proposal for discussion:

public static partial class Unsafe
{
    // Add new
    public static ref readonly T AddByteOffsetReadOnly<T>(in T source, System.IntPtr byteOffset) { throw null; }
    public static ref readonly T AddReadOnly<T>(in T source, int elementOffset) { throw null; }
    public static ref readonly T AddReadOnly<T>(in T source, System.IntPtr elementOffset) { throw null; }
    public static ref readonly TTo AsReadOnly<TFrom, TTo>(in TFrom source) { throw null; }
    public static ref readonly T SubtractByteOffsetReadOnly<T>(in T source, System.IntPtr byteOffset) { throw null; }
    public static ref readonly T SubtractReadOnly<T>(in T source, int elementOffset) { throw null; }
    public static ref readonly T SubtractReadOnly<T>(in T source, System.IntPtr elementOffset) { throw null; }

    // Consider changing existing 
    // (problem this will not require `ref` 
    //  when using due to `in` not requiring it, 
    //  which makes it a bit weird)
    public static bool AreSame<T>(in T left, in T right) { throw null; }
    public static System.IntPtr ByteOffset<T>(in T origin, in T target) { throw null; }
    public static void CopyBlock(ref byte destination, in byte source, uint byteCount) { }
    public static void CopyBlockUnaligned(ref byte destination, in byte source, uint byteCount) { }
    public unsafe static void Copy<T>(void* destination, in T source) { }
    public static T ReadUnaligned<T>(in byte source) { throw null; }
}

One could also add the new methods to a new class ReadOnlyUnsafe which would make methods less discoverable, but make naming easier.

Open Questions

How do we in general support adding new methods identical to existing methods but with ref readonly/in, do we simply suffix with ReadOnly if it is not possible to simply change the existing method?

Updates

Existing API

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

cc: @jkotas @mellinoe

@omariom
Copy link
Contributor

omariom commented Dec 9, 2017

Minimal alternative (though less convenient) is a method shaving off readonlyness:

private int value;

void Main()
{
    ref int refValue = ref Unsafe.Add(ref value, 5);
	
    ref readonly int roRefValue = ref Unsafe.Add(ref value, 5);

    //CS1510 A ref or out value must be an assignable variable
    ref int refValue2 = ref Unsafe.Add(ref roRefValue, 5);

    ref int refValue3 = ref Unsafe.Add(ref ShaveInOff(roRefValue), 5);
}

public static ref T ShaveInOff<T>(in T source) 
{
    throw null; 
}

cc @VSadov

@nietras
Copy link
Contributor Author

nietras commented Dec 9, 2017 via email

@jkotas
Copy link
Member

jkotas commented Dec 9, 2017

This was discussed in https://github.com/dotnet/corefx/issues/23916.

@nietras
Copy link
Contributor Author

nietras commented Dec 9, 2017

My mistake. Did not see when looking.

@nietras nietras closed this as completed Dec 9, 2017
@nietras nietras changed the title [WIP] S.R.CS.Unsafe: Revise API to accomodate for ref readonly/in REDUNDANT S.R.CS.Unsafe: Revise API to accomodate for ref readonly/in Dec 9, 2017
@VSadov
Copy link
Member

VSadov commented Dec 10, 2017

BTW the concerns about aliasing that might not be guaranteed as discussed in #23504 was alleviated by allowing ‘in’ at the call site.

Since AsRef relies on direct reference to the arg, it would make sense to always call it with explicit ‘in’. - to make sure there are no conversions, temporary copies, etc...

AsRef(in someReadonlyVariable)

@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 2.1.0 milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants