- 
                Notifications
    You must be signed in to change notification settings 
- Fork 13.9k
          Optimize CharIndices::advance_by
          #137761
        
          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
base: master
Are you sure you want to change the base?
  
    Optimize CharIndices::advance_by
  
  #137761
              
            Conversation
| Benchmarking shows that  Also, whether the  
 | 
| 
 Not this PR, but if that's better than the  | 
| It seems  | 
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.
The change looks good to me, but just to be sure can you add tests either for advance_by or nth in library/alloc/tests/str.rs?
I didn't spot any, and since it doesn't seem to be exercised by a doc-comment either, it'd be good to have a couple of basic things.
21578a9    to
    2c5a5cd      
    Compare
  
    `Chars::advance_by` is already optimized. Delegate to it.
2c5a5cd    to
    a675ddf      
    Compare
  
    | I've added a test for using  | 
| Looks like we overlooked that  | 
| So  I think what you have here is correct, I'd just like to know that if, say, the  | 
| It's been a while since looking at this, so I missed this detail. Sorry for dropping it. | 
Optimize
CharIndices::advance_byby delegating to the optimizedChars::advance_by. Since this can also be used to count chars, add a benchmark for it.(
Interator::nthusesIterator::advance_by.)cc @orlp