Preview version of C# language uses incorrect constructor overloading for arrays

[nsentinel]

Thanks for the great library!

---

First-class Span Types is currently under development. Recent changes, in particular Prefer converting T[] to ReadOnlySpan over Span in overload resolution, also affect the library.

See the following code for an example:

using System;
using System.Buffers;

class Program {
    static void Main()
    {
        S(ArrayPool<char>.Shared.Rent(16));
        S("abc");
    }

    static void S(Span<char> _) => Console.WriteLine("Span");
    static void S(ReadOnlySpan<char> _) => Console.WriteLine("ReadOnlySpan");
}

Old behavior vs New behavior

That is, by default, it's now easy to get into an invalid constructor overload when passing a buffer from an array. This is not something that will happen sometime in the future, to get problems you just need to enable the preview version of the language for .NET 9. For example, interesting issues: 1, 2

---

To resolve this issue, all you need to do now is add the OverloadResolutionPriority attribute

using System;
using System.Buffers;
using System.Runtime.CompilerServices;

class Program {
    static void Main()
    {
        S(ArrayPool<char>.Shared.Rent(16));
        S("abc");
    }

    [OverloadResolutionPriority(1)]
    static void S(Span<char> _) => Console.WriteLine("Span");
    static void S(ReadOnlySpan<char> _) => Console.WriteLine("ReadOnlySpan");
}

Demo

---

It might be more correct to remove overloading with ReadOnlySpan<char>, for example by adding a static method to create a ValueStringBuilder from a given buffer. For example, .NET without it.

[linkdotnet]

Ohhh that is a very very good point and thanks for the kind words! Thanks for bringing this up @nsentinel

It might be more correct to remove overloading with ReadOnlySpan

Maybe - it would be a breaking change for sure, so nothing I would consider for v1. If we move the ReadOnlySpan version to a static method, I fear that we have (maybe???) less discoverability of the API. I have to make myself some thoughts. But I am glad that you brought this up.

[nsentinel]

Thanks for the quick reply, and I'm glad you found my comments interesting and helpful!

---

About removing the overloading from ReadOnlySpan: yes, I realize that this will be a definite breaking change for everyone, which is usually something we would like to avoid. I mentioned it specifically in this case because the behavior changes dramatically depending on the argument type, which I think is generally unusual, but since ValueStringBuilder users are unusual people then it's probably not that big of a deal, I'll leave it up to you.

I agree that it is less noticeable, but you can always just call Append right after constructing with the initial value (exactly what is done in the constructor overload), and those who don't want to can use the special constructor option via static method.

Probably the best option from the current user's point of view would be to really just add the attribute and leave it as is, since this solves the problem for everyone and doesn't break anything. And the nuance of perceiving differences in constructor behavior seems more philosophical to me.

[linkdotnet]

Probably the best option from the current user's point of view would be to really just add the attribute and leave it as is, since this solves the problem for everyone and doesn't break anything. And the nuance of perceiving differences in constructor behavior seems more philosophical to me.

Indeed - for now that might be the most straightforward solution! I will put this "up for grabs" for folks.

[linkdotnet]

@nsentinel I "fixed" this on v2 (basically, there is no real breaking change besides not supporting net6.0 and net7.0 anymore).

[linkdotnet]

Thanks again for bringing this up!

[nsentinel]

Thank you for the quick fix, and glad to see that my findings came in handy!