-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Constify [u8]::is_ascii
(unstably)
#111222
Conversation
UTF-8 checking in `const fn`-stabilized back in 1.63, but apparently somehow ASCII checking was never const-ified, despite being simpler.
(rustbot has picked a reviewer for you, use r? to override) |
Hey! It looks like you've submitted a new PR for the library teams! If this PR contains changes to any Examples of
|
LGTM |
library/core/src/slice/ascii.rs
Outdated
// FIXME: once iterators and closures can be used in `const fn`, | ||
// return s.iter().all(|b| b.is_ascii()); | ||
let mut i = 0; | ||
while i < len { |
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.
Instead of having to maintain an index, you could also use slice patterns to fake iteration:
while let [next, rest @ ..] = slice {
slice = rest;
}
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.
Well, you got me curious, so I tuned the heck out of it.
Looks like going backwards is the best in both LLVM and cg_gcc: https://rust.godbolt.org/z/K1rKs1T4d
Constify `[u8]::is_ascii` (unstably) UTF-8 checking in `const fn`-stabilized back in 1.63 (rust-lang#97367), but apparently somehow ASCII checking was never const-ified, despite being simpler. New constness-tracking issue for `is_ascii`: rust-lang#111090 I noticed this working on `ascii::Char`: rust-lang#110998
9de3d01
to
c8c5a58
Compare
Ok, I think I've fixed up the test. I'll iffy it just in case. |
☀️ Test successful - checks-actions |
Finished benchmarking commit (613a5c9): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 653.81s -> 654.562s (0.12%) |
Hah, I had a precursor in #104126, though this one's much nicer. I'll keep that one open for |
Oh, sorry @coolreader18 -- I didn't even think to look if someone had already done it. |
No worries! I'm glad it's now possible to do it where const_eval_select isn't necessary |
UTF-8 checking in
const fn
-stabilized back in 1.63 (#97367), but apparently somehow ASCII checking was never const-ified, despite being simpler.New constness-tracking issue for
is_ascii
: #111090I noticed this working on
ascii::Char
: #110998