Stabilise inherent_ascii_escape (FCP in #77174) #93886
Conversation
|
(rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
library/core/src/num/mod.rs
Outdated
| @@ -804,7 +803,7 @@ impl u8 { | |||
| /// ``` | |||
| #[must_use = "this returns the escaped byte as an iterator, \ | |||
| without modifying the original"] | |||
| #[unstable(feature = "inherent_ascii_escape", issue = "77174")] | |||
| #[stable(feature = "inherent_ascii_escape", since = "1.60.0")] | |||
| #[inline] | |||
| pub fn escape_ascii(&self) -> ascii::EscapeDefault { | |||
There was a problem hiding this comment.
This doesn't really matter -- particularly with the inline annotation -- but it feels like this should take u8 rather than &u8.
I guess we've already FCP'd with this signature and changing the signature might cause some use sites to break at this point, but seems unfortunate.
There was a problem hiding this comment.
This is an unfortunate side-effect of all the AsciiExt methods being moved to inherent versions while retaining the references. :(
Sadly, it's the most consistent with the other ASCII methods.
There was a problem hiding this comment.
The tracking issue FCP is actually for fn escape_ascii(self) -> ascii::EscapeDefault, and it looks like the original PR introducing these had a similar comment to mine requesting we use self (#73111 (comment))
I think we should update this to be self rather than &self prior to stabilizing it; consistency with other methods isn't really important.
There was a problem hiding this comment.
Oh wow, that's 100% my bad. I'll make another PR to merge that change then rebase this on that. Not sure whether you'd wanna do a second FCP or just merge it.
There was a problem hiding this comment.
(rebase done, pending tests passing)
There was a problem hiding this comment.
I don't think we need a second FCP, I'm happy to r+ this PR directly.
There was a problem hiding this comment.
Sounds good to me! Figured I'd be safe just in case you wanted to wait a bit between the change and stabilisation.
There was a problem hiding this comment.
FWIW, self vs &self matters quite a bit when the method in question is passed as a function pointer, eg. to an iterator combinator. Having to eta expand Foo::bar to |x| x.bar() is a papercut, particularly if it's inconsistent.
clarfonthey
force-pushed
the
stable_ascii_escape
branch
from
d33ad9a
to
de6e973
Compare
|
@bors r+ rollup |
|
📌 Commit de6e973 has been approved by |
bors
added
S-waiting-on-bors
…askrgr Rollup of 9 pull requests Successful merges: - rust-lang#89926 (make `Instant::{duration_since, elapsed, sub}` saturating and remove workarounds) - rust-lang#90532 (More informative error message for E0015) - rust-lang#93810 (Improve chalk integration) - rust-lang#93851 (More practical examples for `Option::and_then` & `Result::and_then`) - rust-lang#93885 (bootstrap.py: Suggest disabling download-ci-llvm option if url fails to download) - rust-lang#93886 (Stabilise inherent_ascii_escape (FCP in rust-lang#77174)) - rust-lang#93930 (add link to format_args! when mention it in docs) - rust-lang#93936 (Couple of driver cleanups) - rust-lang#93944 (Don't relabel to a team if there is already a team label) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Implements #77174, which completed its FCP.
This does not deprecate any existing methods or structs, as that is tracked in #93887. That stated, people should prefer using
u8::escape_asciitostd::ascii::escape_default.