-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Fix IndexOf Optimization Code #108499
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix IndexOf Optimization Code #108499
Conversation
|
Can the same problem happen in |
| } | ||
|
|
||
| // Before we return -1, check if the remaining source contains any special or non-Ascii characters. | ||
| if (remainingSource.ContainsAnyExcept(s_nonSpecialAsciiChars)) |
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.
Do we really need to go through the whole remainingSource again? It looks expensive.
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.
remainingSource is the remaining part of the source which has not been processed before. So, we are not going through it again. Also, we do that only when there is no match found in the previous part of the source. I expect in common cases remainingSource will be very short as it will have length less than the target length.
I have updated |
|
@jkotas please let me know if you have more comments or if we are good to go. Thanks! |
|
@stephentoub @ericstj is possible to help code reviewing this fix? @jkotas is off and I want to get this soon to 9.0 release and possibly to 8.0 too. |
MihaZupan
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.
That looks correct to me
|
/backport to release/9.0 |
|
Started backporting to release/9.0: https://github.com/dotnet/runtime/actions/runs/11188199484 |
|
/backport to release/8.0-staging |
|
Started backporting to release/8.0-staging: https://github.com/dotnet/runtime/actions/runs/11188206621 |
* Fix IndexOf Optimization Code * small typo * Address the feedback * Exclude hybrid globalization runs on the new added tests
This reverts commit f1de7b0.
Fixes #108424
We have optimization code that attempts to perform the
IndexOfoperation using ASCII characters until the operation completes or encounters a character that requires linguistic processing, at which point it falls back to the slower ICU path. However, there was an issue where, if the source was longer than the value, the optimization would reach the boundary in the source and incorrectly conclude that no match was found. The code wasn't checking the remaining characters in the source to determine if it needed to fall back to the ICU path, which causedIndexOfto return incorrect results.Example
" est".IndexOf("est", System.StringComparison.InvariantCultureIgnoreCase); // -1returns wrong result-1while"est".IndexOf("est", System.StringComparison.InvariantCultureIgnoreCase); // 0returns correct result0.