[API Proposal]: MemoryExtensions.Replace(Span<T>, T, T)

[stephentoub]

Background and motivation

string.Replace(char, char) is useful for replacing every occurrence of a character value. When using spans/arrays to build up strings, that capability is also useful, but today it needs to be hand-rolled, e.g.

Span<char> tmp = ...;
int pos;
while ((pos = tmp.IndexOf(oldValue)) >= 0)
{
    tmp[pos] = newValue;
    tmp = tmp.Slice(pos + 1);
}

It'd be good to have this as a reusable helper on MemoryExtensions so that such open-coded loops can be replaced. There are other more complicated variations of this as well, but this is a simple and common one we can include.

e.g.

runtime/src/libraries/System.Private.Uri/src/System/Uri.cs

Lines 1037 to 1043 in e8b1491

	for (ushort i = 0; i < (ushort)count; ++i)
	{
	if (result[i] == '/')
	{
	result[i] = '\\';
	}
	}

runtime/src/libraries/Common/src/System/IO/Archiving.Utils.Windows.cs

Lines 79 to 84 in e8b1491

	int pos;
	while ((pos = dest.IndexOf('\\')) >= 0)
	{
	dest[pos] = '/';
	dest = dest.Slice(pos + 1);
	}

API Proposal

namespace System
{
    public static class MemoryExtensions
    {
+        public static void Replace<T>(this Span<T> span, T oldValue, T newValue) where T : IEquatable<T>?;
    }
}

API Usage

char[] result = ...;
result.AsSpan().Replace('\\', '/');
return new string(result);

Alternative Designs

No response

Risks

No response

[ghost]

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

Issue Details

---

Background and motivation

string.Replace(char, char) is useful for replacing every occurrence of a character value. When using spans/arrays to build up strings, that capability is also useful, but today it needs to be hand-rolled, e.g.

Span<char> tmp = ...;
int pos;
while ((pos = tmp.IndexOf(oldValue)) >= 0)
{
    tmp[pos] = newValue;
    tmp = tmp.Slice(pos + 1);
}

It'd be good to have this as a reusable helper on MemoryExtensions so that such open-coded loops can be replaced. There are other more complicated variations of this as well, but this is a simple and common one we can include.

e.g.

runtime/src/libraries/System.Private.Uri/src/System/Uri.cs

Lines 1037 to 1043 in e8b1491

	for (ushort i = 0; i < (ushort)count; ++i)
	{
	if (result[i] == '/')
	{
	result[i] = '\\';
	}
	}

runtime/src/libraries/Common/src/System/IO/Archiving.Utils.Windows.cs

Lines 79 to 84 in e8b1491

	int pos;
	while ((pos = dest.IndexOf('\\')) >= 0)
	{
	dest[pos] = '/';
	dest = dest.Slice(pos + 1);
	}

API Proposal

namespace System
{
    public static class MemoryExtensions
    {
+        public static void Replace<T>(this Span<T> span, T oldValue, T newValue) where T : IEquatable<T>?;
    }
}

API Usage

char[] result = ...;
result.AsSpan().Replace('\\', '/');
return new string(result);

Alternative Designs

No response

Risks

No response

Author:	stephentoub
Assignees:	-
Labels:	api-suggestion, area-System.Memory
Milestone:	8.0.0

[gfoidl]

I'm looking forward that this API gets approved -- so I can implement it 😉

[bartonjs]

Video

• We discussed whether Replace should also return the replacement count, and the answer was no, because it has a nontrivial cost that exceeds its value (a counting variant could be added in the future, if needed)
  • If someone wants to try experimenting with this and show it's actually negligible, please change it to returning int.
• We discussed whether we needed a more complicated name, like ReplaceInPlace, to avoid a confusing overload with future potential replaces. The feeling was "no".

namespace System
{
    public static class MemoryExtensions
    {
        public static void Replace<T>(this Span<T> span, T oldValue, T newValue) where T : IEquatable<T>?;
    }
}

[stephentoub]

One issue we didn't discuss today but should:
This is doing in-place replacement. What do we do for code that wants to replace into something else, e.g. to be able to use this from string.Replace? Currently string.Replace uses a conditional select to copy/replace from the source into the destination in one pass. With the approved shape of this API, for string to use it, presumably it would need to copy to the destination and then in-place replace in the destination. Will that perform measurably worse? Of course, the other approach is have Replace take two spans, a source and a destination, and we'd allow the input to be the same for each, but then we have the complication of what do we do if the arguments are overlapping but not the same, do we need to check for that or just say it's undefined behavior, etc. We could also consider having both overloads.

We should come up with a pattern we want to use for these sorts of things, as it's going to start showing up more often, e.g. in #75901, in #26651, etc.

[stephentoub]

I'm looking forward that this API gets approved -- so I can implement it 😉

Ok 😄 I assigned it to you.