API proposal: string.Concat(ReadOnlySpan<char>, ...)

[stephentoub]

We have String.Concat APIs for concatenating objects and strings, but we have no way of concatenating ReadOnlySpan<char>s into a string that don't involve unnecessary copies or allocation. This shows up when optimizing APIs that already need to return strings, but as a result internally end up incurring more allocation or copies than is desirable, e.g. a call like String.Remove("some long string", "g") needs to concatenate two substrings to produce the final result.

We should add the following overloads to string:

public static string Concat(ReadOnlySpan<char> str0, ReadOnlySpan<char> str1);
public static string Concat(ReadOnlySpan<char> str0, ReadOnlySpan<char> str1, ReadOnlySpan<char> str2);
public static string Concat(ReadOnlySpan<char> str0, ReadOnlySpan<char> str1, ReadOnlySpan<char> str2, ReadOnlySpan<char> str3);

(these parameter names match the corresponding overloads that take strings).

We don't currently have a good way to pass an arbitrary number of spans into another Concat overload, so for now we would only support the above numbers of arguments, and in the future we could potentially expand to higher and/or arbitrary counts when such support is available.

[stephentoub]

These are now implemented and used in coreclr, they're just internal (for now hopefully) rather than public.
https://github.com/dotnet/coreclr/blob/8f91ac8f374d30cd1a1122f78a90c35d7827a64a/src/System.Private.CoreLib/shared/System/String.Manipulation.cs#L344-L391

[juliusfriedman]

+1 but I would have expected these methods as extensions just like the others in:
https://github.com/dotnet/corefx/issues/21395

I personally think they have a better home with those methods or in a separate Extensions class since they return string and do not force the string type itself to have reference on ROS, that is unless such a span type becomes implicit via using stackalloc.

[Tornhoof]

Does it make sense to have string.Concat for ReadOnlySequence? To concat each segment?

[stephentoub]

Does it make sense to have string.Concat for ReadOnlySequence? To concat each segment?

You can implement that with String.Create (which can't be done with span because it's a ref struct and thus can't be passed in to the delegate without pinning and unsafe code). String already has a dependency on span, in both public API and implementation, and span lives in Corelib. I don't think we should make string have a dependency on ReadOnlySequence, nor move it to Corelib, at least not at present.