Consider adding Half support to the BinaryPrimitives class

[eiriktsarpalis]

Background and Motivation

This came up while working to add Half support to the CBOR components. While not blocking to my work, these methods would be nice to have for the sake of completeness.

Proposed API

public static class BinaryPrimitives
{
    [MethodImpl(MethodImplOptions.AggressiveInlining)]
    public static Half ReadHalfLittleEndian(ReadOnlySpan<byte> source);
    [MethodImpl(MethodImplOptions.AggressiveInlining)]
    public static Half ReadHalfBigEndian(ReadOnlySpan<byte> source);
    [MethodImpl(MethodImplOptions.AggressiveInlining)]
    public static bool TryReadHalfLittleEndian(ReadOnlySpan<byte> source, out Half);
    [MethodImpl(MethodImplOptions.AggressiveInlining)]
    public static bool TryReadHalfBigEndian(ReadOnlySpan<byte> source, out Half);

    [MethodImpl(MethodImplOptions.AggressiveInlining)]
    public static void WriteHalfLittleEndian(Span<byte> destination, Half value);
    [MethodImpl(MethodImplOptions.AggressiveInlining)]
    public static void WriteHalfBigEndian(Span<byte> destination, Half value);
    [MethodImpl(MethodImplOptions.AggressiveInlining)]
    public static bool TryWriteHalfLittleEndian(Span<byte> destination, Half value);
    [MethodImpl(MethodImplOptions.AggressiveInlining)]
    public static bool TryWriteHalfBigEndian(Span<byte> destination, Half value);
}

[ghost]

Tagging subscribers to this area: @tannergooding
Notify danmosemsft if you want to be subscribed.

[GrabYourPitchforks]

Saw that you also mentioned this in #38288. Definitely should do both at the same time. :)

[GrabYourPitchforks]

Can we add these to BinaryReader and BinaryWriter as well? They're useful for throwing data across the wire.

namespace System.IO
{
    public class BinaryReader
    {
        // NEW virtual method on existing type
        // We'll give it a viable default implementation, just like the existing virtual Read* methods
        public virtual Half ReadHalf();
    }

    public class BinaryWriter
    {
        // NEW virtual method on existing type
        // We'll give it a viable default implementation, just like the existing virtual Write overloads
        public virtual void Write(Half value);
    }
}

[terrajobst]

Video

• Looks good
• But we should also do BinaryReader and BinaryWriter as they are the more high-level counterparts
• @tannergooding, please scout the framework for support of float and double and see what makes sense for Half
• We should consider Convert, but the interface dispatch due to lack of type code might be challenging.

namespace System.Buffers.Binary
{
    public static class BinaryPrimitives
    {
        public static Half ReadHalfLittleEndian(ReadOnlySpan<byte> source);
        public static Half ReadHalfBigEndian(ReadOnlySpan<byte> source);
        public static bool TryReadHalfLittleEndian(ReadOnlySpan<byte> source, out Half);
        public static bool TryReadHalfBigEndian(ReadOnlySpan<byte> source, out Half);

        public static void WriteHalfLittleEndian(Span<byte> destination, Half value);
        public static void WriteHalfBigEndian(Span<byte> destination, Half value);
        public static bool TryWriteHalfLittleEndian(Span<byte> destination, Half value);
        public static bool TryWriteHalfBigEndian(Span<byte> destination, Half value);
    }
}
namespace System.IO
{
    public partial class BinaryReader
    {
        public virtual Half ReadHalf();
    }
    public partial class BinaryWriter
    {
        public virtual Half Write(Half value);
    }
}

[akoeplinger]

Curious why we need separate *LittleEndian and *BigEndian methods? Is it because I might want to read something in big-endian encoding even though I'm on a little-endian machine so using BitConverter.IsLittleEndian internally wouldn't make sense?

[tannergooding]

Right, many file formats (for example) explicitly call out themselves as being big or little endian. These methods allow you to do the right thing regardless of what machine you are on.

[jeffhandley]

Moved to 6.0.0 per discussion on #40882. Thanks, @huoyaoyuan.

[xPaw]

Would you reconsider putting this into 5.0 release? Putting this into 6.0 release makes this new type harder to use imo.

I wanted to switch to using native half type, but the missing methods on BinaryReader make this a bit harder.

[jkotas]

.NET 5 API surface is done. We are not able to add this to 5.0 release. If you need the method on BinaryReader for your project, you can add it as internal extension method for now. It should be like ~20 lines.