-
Notifications
You must be signed in to change notification settings - Fork 2.6k
CallSites for MemoryExtensions.Contains (CoreCLR) #19874
Conversation
ahsonkhan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missed one:
| return IndexOf(value) != -1; |
Otherwise, LGTM. Thanks :)
|
@ahsonkhan, not sure what you are proposing with that one - it is an instance method on |
No, that isn't what I was suggesting.
Right now
Is it possible for it to instead call the following, directly?
So, something like: public bool Contains(char value) => SpanHelpers.Contains(ref _firstChar, value, Length); |
|
Sorry, must be a Friday, that makes sense. |
|
@dotnet-bot test Ubuntu x64 Checked CoreFX Tests |
1 similar comment
|
@dotnet-bot test Ubuntu x64 Checked CoreFX Tests |
|
@ahsonkhan would you mind taking another look at this, I have added a couple more call sites since your previous approval so it would be great if you could confirm it's kosher (& hopefully merge it)? |
* Update additional callsites for MemoryExtensions.Contains * One more callsite * More callsites * CR fixes Signed-off-by: dotnet-bot <[email protected]>
* Update additional callsites for MemoryExtensions.Contains * One more callsite * More callsites * CR fixes Signed-off-by: dotnet-bot <[email protected]>
* Update additional callsites for MemoryExtensions.Contains * One more callsite * More callsites * CR fixes Signed-off-by: dotnet-bot <[email protected]>
* Update additional callsites for MemoryExtensions.Contains * One more callsite * More callsites * CR fixes Signed-off-by: dotnet-bot <[email protected]>
Containsis optimized overIndexOfSpan<T>.Containsunsafe