rustc_arena: add alloc_str
#118660
rustc_arena: add alloc_str
#118660
Conversation
|
(rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
S-waiting-on-review
This comment was marked as resolved.
This comment was marked as resolved.
Two places called `from_utf8_unchecked` for strings from `alloc_slice`, and one's SAFETY comment said this was for lack of `alloc_str` -- so let's just add that instead!
| #[inline] | ||
| pub fn alloc_str(&self, string: &str) -> &str { | ||
| if string.is_empty() { | ||
| return ""; |
There was a problem hiding this comment.
Maybe remove this case and keep the panic?
No current use requires it.
There was a problem hiding this comment.
We can if you prefer, but I think for perf the branchiness should end up equivalent, just changing whether it jumps to a panic or return. And the empty check here is consistent with its alloc_slice neighbor. A panic still makes sense if we want that treated as a bug -- is that it?
There was a problem hiding this comment.
In symbol.rs that would certainly be a bug, with SymbolName::new I'm less sure, but it looks like the string cannot be empty there either.
I don't have much preference here, let's land as is.
There was a problem hiding this comment.
The symbol.rs case is already using a direct DroplessArena, where it would panic. Only the macro-declared arena has the empty return case, in both alloc_slice and this new alloc_str.
petrochenkov
added
S-waiting-on-author
|
@bors r+ rollup |
bors
added
S-waiting-on-bors
…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#117981 (Remove deprecated `--check-cfg` syntax) - rust-lang#118177 (Suppress warnings in LLVM wrapper when targeting MSVC) - rust-lang#118317 (tip for define macro name after `macro_rules!`) - rust-lang#118504 (Enforce `must_use` on associated types and RPITITs that have a must-use trait in bounds) - rust-lang#118660 (rustc_arena: add `alloc_str`) - rust-lang#118681 (Fix is_foreign_item for StableMIR instance ) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#118660 - cuviper:alloc_str, r=petrochenkov rustc_arena: add `alloc_str` Two places called `from_utf8_unchecked` for strings from `alloc_slice`, and one's SAFETY comment said this was for lack of `alloc_str` -- so let's just add that instead!
Two places called
from_utf8_uncheckedfor strings fromalloc_slice,and one's SAFETY comment said this was for lack of
alloc_str-- solet's just add that instead!