-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Add String::extend_from_within
#85801
Conversation
This patch adds `String::extend_from_within` function under the `string_extend_from_within` feature gate similar to the `Vec::extend_from_within` function.
(rust-highfive has picked a reviewer for you, use r? to override) |
assert!(self.is_char_boundary(start)); | ||
assert!(self.is_char_boundary(end)); |
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.
If panic happens, how does one know if it panics because of start or end?
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.
I think it should be possible by checking the line number in the panic message. It isn't convenient, but it is possible.
Also, note that the same pattern appears multiple times in this file:
rust/library/alloc/src/string.rs
Lines 1643 to 1644 in 23f9b92
assert!(self.is_char_boundary(start)); | |
assert!(self.is_char_boundary(end)); |
rust/library/alloc/src/string.rs
Lines 1688 to 1700 in 23f9b92
let start = range.start_bound(); | |
match start { | |
Included(&n) => assert!(self.is_char_boundary(n)), | |
Excluded(&n) => assert!(self.is_char_boundary(n + 1)), | |
Unbounded => {} | |
}; | |
// WARNING: Inlining this variable would be unsound (#81138) | |
let end = range.end_bound(); | |
match end { | |
Included(&n) => assert!(self.is_char_boundary(n + 1)), | |
Excluded(&n) => assert!(self.is_char_boundary(n)), | |
Unbounded => {} | |
}; |
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.
Maybe we should have a separate issue for this.
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.
Wait, it's not necessary. The current panic error message is good enough. (but still without the function name, like drain
)
thread 'main' panicked at 'assertion failed: self.is_char_boundary(start)', /rustc/9bc8c42bb2f19e745a63f3445f1ac248fb015e53/library/alloc/src/string.rs:1571:9
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.
Ah, so when called without a message, assert!
prints the passed expression. That's nice.
Seems reasonable, and I appreciate that it calls @bors r+ |
📌 Commit 23f9b92 has been approved by |
…laumeGomez Rollup of 8 pull requests Successful merges: - rust-lang#85285 (Add eslint checks to CI) - rust-lang#85709 (Use correct edition when parsing `:pat` matchers) - rust-lang#85762 (Do not try to build LLVM with Zlib on Windows) - rust-lang#85770 (Remove `--print unversioned-files` from rustdoc ) - rust-lang#85781 (Add documentation for aarch64-apple-ios-sim target) - rust-lang#85801 (Add `String::extend_from_within`) - rust-lang#85817 (Fix a typo) - rust-lang#85818 (Don't drop `PResult` without handling the error) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
This PR adds
String::extend_from_within
function under thestring_extend_from_within
feature gate similar to theVec::extend_from_within
function.