Allocation-free hex escape parsing in UnicodeSet parsing #3725
Conversation
|
🎉 All dependencies have been resolved ! |
skius
force-pushed
the
unicodeset-escape-no-alloc
branch
from
0190186 to
090eb79
Compare
| let first_offset = self.must_peek_index()?; | ||
| let end_offset = self.validate_hex_digits(min, max)?; | ||
|
|
||
| // safety: validate_hex_digits ensures that chars (including the last one) are ascii hex digits, |
There was a problem hiding this comment.
nit: this isn't about "safety" but about not panicking
There was a problem hiding this comment.
thanks, removed all other mentions in this file as well
| // safety: validate_hex_digits ensures that chars (including the last one) are ascii hex digits, | ||
| // which are all exactly one UTF-8 byte long, so slicing on these offsets always respects char boundaries | ||
| #[allow(clippy::indexing_slicing)] | ||
| let hex_source = &self.source[first_offset..=end_offset]; |
There was a problem hiding this comment.
thought: if you make max a const-generic, you can keep the digits you've seen on the stack and don't need to hold on to source. Alternatively, you can merge this with validate_hex_digit and calculate the value while iterating with something like curr = curr * 16 + c.to_digit(16)?
There was a problem hiding this comment.
How would the version that keeps digits on the stack work? IIUC, if I want to use a std parsing function I need a UTF-8 slice, but my iterator only gives me chars.
Do you think holding onto source is a big downside? If I understand correctly, even in the #3550 - polished version of this API the user will never have to deal with a struct that holds onto source (USParser::parse takes source and returns the CPILASL)
In any case, if we want to avoid the two-pass approach, I think manually implementing the hex parsing and doing it on the fly as you suggested is ok
There was a problem hiding this comment.
Yeah the stack based approach is not great, you'd have a [u8; MAX] buffer that you'd unsafely interpret as a str. Holding on to source seems fine
Avoids unnecessary allocations when parsing hex digits, instead creates a subslice from the source &str.
Part of #3684
Depends on #3670
(cc @younies)