implement replace_with and replacen_with for unstable feature replace_with by alpaylan · Pull Request #147741 · rust-lang/rust (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation8 Commits2 Checks11 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
This is the PR for the tracking issue #147731 for implementation of replace_with.
Relevant to the library team, which will review and decide on the PR/issue.
label
r? @joboet
rustbot has assigned @joboet.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.
Use r? to explicitly pick a reviewer
Comment on lines 346 to 356
| if let Some(from_byte) = match from.as_utf8_pattern() { |
|---|
| Some(Utf8Pattern::StringPattern([from_byte])) => Some(*from_byte), |
| Some(Utf8Pattern::CharPattern(c)) => c.as_ascii().map(|ascii_char |
| _ => None, |
| } { |
| if let Some(sref) = from_byte.as_ascii().map(|ascii_char |
| if let [to_byte] = sref.as_ref().as_bytes() { |
| return unsafe { replace_ascii(self.as_bytes(), from_byte, *to_byte) }; |
| } |
| } |
| } |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be possible to use let chains here?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated with an if-let chain, did you have something else in mind or is this fine? (f70b5e1)
This comment has been minimized.
It does compile with edition 2021 anyway tho
Command { cmd: ... "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make/alloc-no-oom-handling/rmake_out"
"--edition" "2021"
"-Dwarnings" ... }, already_executed: true }
Well, I guess that's fine to revert this then :)
If this ever will get 2024 edition I will cleanup this accordingly
| from: P, |
|---|
| mut f: impl FnMut(&str) -> S, |
| ) -> String { |
| // Fast path for replacing a single ASCII character with another. |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
f might not be side-effect-free, so this optimisation is incorrect.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, that is correct. I'm assuming I should just remove it, or are there any other potential optimization?
Status: This is awaiting some action (such as code changes or more information) from the author.
and removed S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
labels
Labels
Status: This is awaiting some action (such as code changes or more information) from the author.
Relevant to the library team, which will review and decide on the PR/issue.