Runtime: Proposal: Trim/TrimStart/TrimEnd methods for Memory and Span with trim element

[schungx]

Currently, the overloaded versions of Trim, TrimStart and TrimEnd that accept a trimChar or trimChars only works with ReadOnlySpan<char>. There is no reason why these versions cannot be extended to any ReadOnlySpan<T>. For example:

c# public static ReadOnlySpan<T> TrimEnd<T>(this ReadOnlySpan<T> span, T trimElement) { int end = span.Length - 1; for (; end >= 0; end--) { if (span[end] != trimElement) break; } return span.Slice(0, end + 1); }

This will allow, for example, quick processing of a byte array to trim off zero paddings before parsing.

[gfoidl]

@schungx can you update your code sample so that the return is outside the loop?

Edit: to be correct: remove the extra }?

[stephentoub]

if (span[end] != trimElement)

You can't use != with arbitrary Ts like this. You would instead, for example, need to constraint T with where T : IEquatable<T> and then use a comparison like:
C# if (trimElement == null) { if (span[end] != null) break; } else { if (span[end] != null && !span[end].Equals(trimElement)) break; }
or something like that.

[ahsonkhan]

To confirm, these are the APIs being proposed, correct?

C# public static ReadOnlySpan<T> Trim<T>(this ReadOnlySpan<T> span) where T : IEquatable<T> { throw null; } public static ReadOnlySpan<T> Trim<T>(this ReadOnlySpan<T> span, T trimElement) where T : IEquatable<T> { throw null; } public static ReadOnlySpan<T> Trim<T>(this ReadOnlySpan<T> span, System.ReadOnlySpan<T> trimElements) where T : IEquatable<T> { throw null; } public static ReadOnlySpan<T> TrimEnd<T>(this ReadOnlySpan<T> span) where T : IEquatable<T> { throw null; } public static ReadOnlySpan<T> TrimEnd<T>(this ReadOnlySpan<T> span, T trimElement) where T : IEquatable<T> { throw null; } public static ReadOnlySpan<T> TrimEnd<T>(this ReadOnlySpan<T> span, System.ReadOnlySpan<T> trimElements) where T : IEquatable<T> { throw null; } public static ReadOnlySpan<T> TrimStart<T>(this ReadOnlySpan<T> span) where T : IEquatable<T> { throw null; } public static ReadOnlySpan<T> TrimStart<T>(this ReadOnlySpan<T> span, T trimElement) where T : IEquatable<T> { throw null; } public static ReadOnlySpan<T> TrimStart<T>(this ReadOnlySpan<T> span, System.ReadOnlySpan<T> trimElements) where T : IEquatable<T> { throw null; }

[terrajobst]

Video

We can probably cut the APIs that don't specify a element (because default(T) doesn't seem reasonable) but it's still 6 APIs. We feel like we need a bit more justification to add these APIs.

@schungx what are the scenarios where you needed this API?

[karelz]

Also, the original ask (from title) is also for Memory and Span. Do we need those? If not, let's change the title.

[ahsonkhan]

Also, the original ask (from title) is also for Memory and Span. Do we need those? If not, let's change the title.

We already have the T=char Trim APIs approved for ReadOnlyMemory. I would assume the same rationale would apply here for the generic ReadOnlyMemory. So, if we add the span-based ones, we should add the memory-based ones too.

https://github.com/dotnet/corefx/issues/30592

[schungx]

@schungx what are the scenarios where you needed this API?

Well, I came about this when needing to process zero-padded byte data obtained via a Socket. So naturally something like TrimEnd for Span<byte> where I can specify the trimElement to be zero is immensely useful.

I can see this being useful in cases where you need to handle fixed-sized records with some default padding value.

Therefore, if 6 API's are too many, I would recommend just adding the two TrimEnd variations:

~
public static ReadOnlySpan TrimEnd(this ReadOnlySpan span, T trimElement) where T : IEquatable { throw null; }
public static ReadOnlySpan TrimEnd(this ReadOnlySpan span, System.ReadOnlySpan trimElements) where T : IEquatable { throw null; }
~

This is because TrimStart and Trim are really not very meaningful except for strings.

[ahsonkhan]

If we are going to add TrimEnd, we should add TrimStart too.

C# public static ReadOnlySpan<T> TrimEnd<T>(this ReadOnlySpan<T> span, T trimElement) where T : IEquatable<T> { throw null; } public static ReadOnlySpan<T> TrimEnd<T>(this ReadOnlySpan<T> span, System.ReadOnlySpan<T> trimElements) where T : IEquatable<T> { throw null; } public static ReadOnlySpan<T> TrimStart<T>(this ReadOnlySpan<T> span, T trimElement) where T : IEquatable<T> { throw null; } public static ReadOnlySpan<T> TrimStart<T>(this ReadOnlySpan<T> span, System.ReadOnlySpan<T> trimElements) where T : IEquatable<T> { throw null; }

[terrajobst]

• We should replace the already approved methods for ReadOnlyMemory<char> to be over T (except for the one that doesn't take values and assumes whistespace, which should remain to be over char).

```C#
// Existing, effectively dead methods:

Trim(ReadOnlySpan, Char)
Trim(ReadOnlySpan, ReadOnlySpan)
TrimEnd(ReadOnlySpan, Char)
TrimEnd(ReadOnlySpan, ReadOnlySpan)
TrimStart(ReadOnlySpan, Char)
TrimStart(ReadOnlySpan, ReadOnlySpan)

// Effective method set

//Trim(ReadOnlySpan)
Trim(ReadOnlySpan, T)
Trim(ReadOnlySpan, ReadOnlySpan)
//TrimEnd(ReadOnlySpan)
TrimEnd(ReadOnlySpan, T)
TrimEnd(ReadOnlySpan, ReadOnlySpan)
//TrimStart(ReadOnlySpan)
TrimStart(ReadOnlySpan, T)
TrimStart(ReadOnlySpan, ReadOnlySpan)
Trim(Span)
Trim(Span, T)
Trim(Span, ReadOnlySpan)
TrimEnd(Span)
TrimEnd(Span, T)
TrimEnd(Span, ReadOnlySpan)
TrimStart(Span)
TrimStart(Span, T)
TrimStart(Span, ReadOnlySpan)
Trim(ReadOnlyMemory)
Trim(ReadOnlyMemory, T)
Trim(ReadOnlyMemory, ReadOnlySpan)
TrimEnd(ReadOnlyMemory)
TrimEnd(ReadOnlyMemory, T)
TrimEnd(ReadOnlyMemory, ReadOnlySpan)
TrimStart(ReadOnlyMemory)
TrimStart(ReadOnlyMemory, T)
TrimStart(ReadOnlyMemory, ReadOnlySpan)
Trim(Memory)
Trim(Memory, T)
Trim(Memory, ReadOnlySpan)
TrimEnd(Memory)
TrimEnd(Memory, T)
TrimEnd(Memory, ReadOnlySpan)
TrimStart(Memory)
TrimStart(Memory, T)
TrimStart(Memory, ReadOnlySpan)
```

[ahsonkhan]

We should replace the already approved methods for ReadOnlyMemory<char> to be over T (except for the one that doesn't take values and assumes whistespace, which should remain to be over char).

i.e. https://github.com/dotnet/corefx/issues/30592, for reference.

[grant-d]

or something like that.

I submitted the above PR, using the suggested comparison but moving it outside the loop to make it cheaper.

Not sure what the final set of required signatures is, it's a confusing thread above.
Is @terrajobst's list the final say?
I have implemented Trim, TrimStart and TrimEnd on RoS only.
Let me know if I need to add any of Span, Memory and RoM

[Edited]

• [x] Implement changes in CoreCLR (https://github.com/dotnet/coreclr/pull/22798)
• [x] Merge CoreCLR
• [x] Implement ref assembly and units in CoreFX (https://github.com/dotnet/corefx/pull/35576)
• [x] Merge CoreFX

[ahsonkhan]

Fixed in https://github.com/dotnet/coreclr/pull/22798 / https://github.com/dotnet/corefx/pull/35576