Runtime: Add Span / ReadOnlySpan IndexOf extensions taking in a start index and a count

[cdmihai]

```c#
int IndexOf(T value, int startIndex);
int IndexOf(T value, int startIndex, int count);

int IndexOf(ReadOnlySpan value, int index);
int IndexOf(ReadOnlySpan value, int startIndex, int count);
```

I could reslice the Span so that I don't need the start index. But MSBuild has a lot of "delicate", hard to read index bookkeeping code, so reslicing to avoid the start index would need offseting everything, which is probably error prone.

[cdmihai]

Workaround implementation:
```c#
public static int IndexOf(this ReadOnlySpan aSpan, char aChar, int startIndex)
{
var indexInSlice = aSpan.Slice(startIndex).IndexOf(aChar);

        if (indexInSlice == -1)
        {
            return -1;
        }

        return startIndex + indexInSlice;
    }

```

[bartonjs]

Since (ReadOnly)Span already takes the start and length into account, the overloads just create clutter. Caller-slicing seems much better.

[ahsonkhan]

But MSBuild has a lot of "delicate", hard to read index bookkeeping code

Can you please share a sample code snippet where you are seeing the complexity with slicing? We opt'd out of such API overloads, in general, since span is cheaply sliceable and that is the pattern we encourage unless there is a strong reason to deviate.

[cdmihai]

The reason would be the ability to quickly and mechanically port to Span / Memory. If all of the sudden users who have index massaging legacy code have to now reverse engineer the +1s, -2s, etc, it might slow down adoption, as something that could have took 1 second now takes half an hour to figure out why tests are failing.

For example, MSBuild's Expander looks like a good candidate for Span / Memory, but there's index bookkeeping littered all over the place. For now I just went with the extension method I posted above.

[cdmihai]

Also, regarding the ease of porting, I wanted to quickly write some benchmarkdotnet tests on Span / Memory to see which one to use (as I hit some issues when having to split a Span and return a collection of some splits and started considering to use Memory instead) and realized that ReadOnlyMemory does not have the IndexOf and Trim extensions.

I can workaround IndexOf because I can convert the Memory to Span (unfortunately) but then how do I trim a ReadOnlyMemory without allocations?

[bartonjs]

Trim for ReadOnlyMemory<char> is dotnet/runtime#26570.

[ahsonkhan]

The reason would be the ability to quickly and mechanically port to Span / Memory. If all of the sudden users who have index massaging legacy code have to now reverse engineer the +1s, -2s, etc, it might slow down adoption, as something that could have took 1 second now takes half an hour to figure out why tests are failing.

That is a good point, but it would mean we would have to add such overloads to all other string-like APIs as well, like Compare, {Last}IndexOf{Any}, CopyTo. And as @bartonjs mentioned, that would result in quite a lot of clutter in the API surface (which are redundant to span slicing). In general, the caller slicing the spans has been good enough. For extenuating circumstances, writing your own extension method as a workaround (like you did), seems sufficient. It may be worth re-considering it for general ease of porting, but since the user will undoubtedly have to make other changes anyway (like slice, arg changes, etc.), I am not sure how much we will be benefiting from this.

For example, MSBuild's Expander looks like a good candidate for Span / Memory, but there's index bookkeeping littered all over the place. For now I just went with the extension method I posted above.

Looking specifically at your example, some of the uses of IndexOf seem unnecessary:
https://github.com/Microsoft/msbuild/blob/901c77484df3eece289829c431542d69c0294999/src/Build/Evaluation/Expander.cs#L3027-L3029
Re-write it as follows:
```C#
if (propertyValue == null && expressionRoot[0] == '[')
{
// We already checked that the first element is '[', so there is no difference in the end result if we did IndexOf(']') or IndexOf(']', 1)
var typeEndIndex = expressionRoot.IndexOf(']');

Same here:
https://github.com/Microsoft/msbuild/blob/901c77484df3eece289829c431542d69c0294999/src/Build/Evaluation/Expander.cs#L3064-L3066


Similarly, it looks like using the IndexOf overload results in unnecessary index fix-up complexity that we can avoid if we port to using existing span-based APIs and slicing.

https://github.com/Microsoft/msbuild/blob/901c77484df3eece289829c431542d69c0294999/src/Build/Evaluation/Expander.cs#L4326-L4342
```C#
                    var span = expressionFunction.AsSpan(methodStartIndex);
                    int nextMethodIndex = span.IndexOf('.');
                    int methodLength = span.Length; // remove unnecessary subtraction
                    int indexerIndex = span.IndexOf('[');

                    // We don't want to consume the indexer
                    if (indexerIndex >= 0 && indexerIndex < nextMethodIndex)
                    {
                        nextMethodIndex = indexerIndex;
                    }

                    functionArguments = Array.Empty<string>();

                    if (nextMethodIndex > 0)
                    {
// We can remove the need to subtract methodStartIndex from nextMethodIndex here
                        methodLength = nextMethodIndex;
 // This is just an example where I wanted to minimize the changes. We shouldn't be calling ToString here.
                        remainder = span.Slice(nextMethodIndex).Trim().ToString();
                    }

[cdmihai]

True, there's many opportunities for improvement in that piece of code. I guess for me the expectation was to quickly to convert to Span as an exploration and get some quick numbers so I can gauge how much extra effort to put into it.

[stephentoub]

I guess for me the expectation was to quickly to convert to Span as an exploration and get some quick numbers so I can gauge how much extra effort to put into it.

Seems like you could do that via your own extension methods, such as the one you shared earlier.

I agree with @bartonjs and @ahsonkhan that our design with span is very much geared around avoiding adding such overloads, preferring to have callers slice and pass in just the portion they need. It would take a lot I think for us to move away from that design, and if we did, it would entail a massive review/set of additions across the framework, not just focused on a few IndexOf overloads.

Given that, I'm going to close this out. But if you feel there's more that needs to be discussed, please feel free to re-open.