Reevaluate `params ReadOnlySpan<string>` overloads from #77873

[jozkee]

Existing compiler rules make that types that define implicit operators for string and string[] incur in a source breaking change.
Example:

class Program
{
    static void Main()
    {
        var c = new C();
        Join(",", c); // error CS0121: The call is ambiguous between the following methods or properties: 'Program.Join(string, params string[])' and 'Program.Join(string, params ReadOnlySpan<string>)'
    }

    public static string Join(string separator, params string[] value) => "array";

    public static string Join(string separator, params ReadOnlySpan<string> value) => "span";
}

class C
{
    public static implicit operator string(C c) => "c";

    public static implicit operator string[](C c) => ["c"];
}

One real-world example is Microsoft.Extensions.Primitives.StringValues which is passed into string.Join(string, string[]) on aspnetcore, and that it failed to compile when the changes in #100898 flowed into their repo.

API Proposal

Not quite a new API proposal, but a proposal for keeping as is the APIs on #77873.
These APIs would make existing callsites of their analogous params string[] overloads to fail if a StringValues is being passed as argument.

System.String.Concat(params ReadOnlySpan<string?> values)
System.String.Join(char separator, params ReadOnlySpan<string?> value)
System.String.Join(string? separator, params ReadOnlySpan<string?> value)
System.IO.Path.Combine(params ReadOnlySpan<string> paths)
System.IO.Path.Join(params ReadOnlySpan<string?> paths)
System.Text.StringBuilder.AppendJoin(string? separator, params ReadOnlySpan<string?> values)
System.Text.StringBuilder.AppendJoin(char separator, params ReadOnlySpan<string?> values)

Considering that having the ambiguity of string and string[] implicit operators feels like is against FDGs 5.7.2 Conversion Operators

DO NOT provide a conversion operator if such a conversion is not clearly expected by the end users.

I think the best path would be to accept the source breaking change and expect users to update their codebase, preferably picking the new ReadOnlySpan<string> overloads.

Alternatively

@dotnet/roslyn could the compiler rules be adjusted to accommodate StringValues? Why adding a public static implicit operator ReadonlySpan<string> to StringValues doesn't make the compiler select string.Join(string, ReadonlySpan<string>)? It does make the error go away, though, by selecting the string[] overload.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-memory
See info in area-owners.md if you want to be subscribed.

[RikkiGibson]

could the compiler rules be adjusted to accommodate StringValues? Why adding a public static implicit operator ReadonlySpan<string> to StringValues doesn't make the compiler select string.Join(string, ReadOnlySpan<string>)? It does make the error go away, though, by selecting the string[] overload.

string[] is considered better because implicit conversion from string[] to ReadOnlySpan<string> exists, and not the other way around. Thus the compiler thinks of string[] as "more derived/specific" than ReadOnlySpan<string>.

It seems like when overload resolution is comparing a conversion from StringValues->string to invoke Join(ReadOnlySpan) in expanded form, with a conversion from StringValues->string[] to invoke Join(string[]) in normal form, there's no rule which indicates the conversion to string is better than the conversion to string[].

However once an operator StringValues->ReadOnlySpan<string> is added, now the overloads can be compared as: oh, I can either invoke in Join(ReadOnlySpan) in normal form, or Join(string[]) in normal form, and the conversion from string[] to ReadOnlySpan indicates that string[] is better.

I'm not sure what change is most appropriate to fix the case in this issue, though. I will say that choosing ReadOnlySpan<string> in expanded form feels undesirable for this case as it converts the StringValues to string and implicitly joins the inner strings as part of that.

[jozkee]

However once an operator StringValues->ReadOnlySpan is added, now the overloads can be compared as: oh, I can either invoke in Join(ReadOnlySpan) in normal form, or Join(string[]) in normal form, and the conversion from string[] to ReadOnlySpan indicates that string[] is better.

I would expect conversion of StringValues->ReadOnlySpan<string> to be better than StringValues->string[] and avoid weight-in the array_type->span_type conversion in such indirect cases. If that would be possible, I think we could add the implicit ReadOnlySpan<string> operator to fix the issue on our end.

[jozkee]

cc @dotnet/fxdc

[RikkiGibson]

I would expect conversion of StringValues->ReadOnlySpan<string> to be better than StringValues->string[]

It looks like "overload resolution priority" was designed to address this specific kind of problem. dotnet/csharplang#7906

[CyrusNajmabadi]

could the compiler rules be adjusted to accommodate StringValues? Why adding a public static implicit operator ReadonlySpan<string> to StringValues doesn't make the compiler select string.Join(string, ReadOnlySpan<string>)? It does make the error go away, though, by selecting the string[] overload.

string[] is considered better because implicit conversion from string[] to ReadOnlySpan<string> exists, and not the other way around. Thus the compiler thinks of string[] as "more derived/specific" than ReadOnlySpan<string>.

It seems like when overload resolution is comparing a conversion from StringValues->string to invoke Join(ReadOnlySpan) in expanded form, with a conversion from StringValues->string[] to invoke Join(string[]) in normal form, there's no rule which indicates the conversion to string is better than the conversion to string[].

However once an operator StringValues->ReadOnlySpan<string> is added, now the overloads can be compared as: oh, I can either invoke in Join(ReadOnlySpan) in normal form, or Join(string[]) in normal form, and the conversion from string[] to ReadOnlySpan indicates that string[] is better.

I'm not sure what change is most appropriate to fix the case in this issue, though. I will say that choosing ReadOnlySpan<string> in expanded form feels undesirable for this case as it converts the StringValues to string and implicitly joins the inner strings as part of that.

Isn't this was @333fred had a proposal for?

[AlekseyTs]

I would expect conversion of StringValues->ReadOnlySpan<string> to be better than StringValues->string[] and avoid weight-in the array_type->span_type conversion in such indirect cases.

What is the basis for these expectations? Usually, expectations should be based on something.

[jozkee]

What is the basis for these expectations?

1. Given that we implicitly convert arrays into [ReadOnly]Spans in other contexts, feels counter-intuitive to prefer the opposite here.
2. When Span & the array conversion were introduced, I doubt it was considered that it would resolve conversion fights in other types in favor of the array.
3. The number of efforts put into spreading Span benefits, e.g. although I'm not sure if such benefits exist in conversions.

[AlekseyTs]

Given that we implicitly convert arrays into [ReadOnly]Spans in other contexts, feels counter-intuitive to prefer the opposite here.

Based on what? I would expect exactly the behavior that we have, and that expectation is based on language rules, not on feelings. From your perspective span types are unconditionally "better" than array types, but compiler cannot get into one's head and read one's mind/feelings. There is a rational behind the language rules. To simplify, language prefers a candidate that takes a "narrower" type for a given argument. For example, an int parameter is preferred over a long parameter. The process of determining what type is "narrower" is based on the set of available implicit conversions. If type A is implicitly convertible to type B and B is not implicitly convertible to A, type A is "narrower" than B. Thus, it is "better" than B. By this rule, int is better than long, and array is "better"/"narrower" than span.

I guess, the point that I am trying to make. There is nothing wrong to have a desire that compiler behaves a certain way. But the desire itself is not a good basis (I would even say it is not a basis at all) to have an expectation that compiler is supposed to behave this way. Basically, there is an analogy between a contract for an API and language rules. The contract defines the expectations, not the other way around. It is, obviously, fine to ask to adjust the language rules to align expected behavior of the compiler with the desired outcome. But, in my opinion, that is quite different from simply expecting things to work a certain way just because there is a desire for that at given moment in time.