-
Notifications
You must be signed in to change notification settings - Fork 185
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
Implement ZeroMapKV
for Pattern<SinglePlaceholder, str>
#5030
Changes from all commits
814c6f4
fe617b3
bbc3d4f
f7bd27b
faa9ea1
d5ea128
cbe0c8f
31652bc
8e5bf3c
14cd6d1
f26a103
d85f218
642b59f
6780b95
7ca8ea0
e116cd4
e40a784
20e1a65
5b65ffc
c48eee3
0c841b9
80ca2df
2d026d3
2a9fb85
b4d8161
99af9a9
ceb9991
1add0fe
0623700
7b2f08b
f3cb3cb
a4dc312
1abc9fe
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
// This file is part of ICU4X. For terms of use, please see the file | ||
// called LICENSE at the top level of the ICU4X source tree | ||
// (online at: https://github.com/unicode-org/icu4x/blob/main/LICENSE ). | ||
|
||
use crate::{Pattern, SinglePlaceholder, SinglePlaceholderPattern}; | ||
|
||
use alloc::boxed::Box; | ||
use zerovec::{maps::ZeroMapKV, ule::VarULE, VarZeroSlice, VarZeroVec, ZeroVecError}; | ||
|
||
impl<'a> ZeroMapKV<'a> for Pattern<SinglePlaceholder, str> { | ||
type Container = VarZeroVec<'a, Pattern<SinglePlaceholder, str>>; | ||
type Slice = VarZeroSlice<Pattern<SinglePlaceholder, str>>; | ||
type GetType = Pattern<SinglePlaceholder, str>; | ||
type OwnedType = Box<Pattern<SinglePlaceholder, str>>; | ||
} | ||
|
||
/// # Safety | ||
/// | ||
/// Safety checklist for `ULE`: | ||
/// | ||
/// 1. `str` does not include any uninitialized or padding bytes. | ||
/// 2. `str` is aligned to 1 byte. | ||
/// 3. The implementation of `validate_byte_slice()` returns an error | ||
/// if any byte is not valid. | ||
/// 4. The implementation of `validate_byte_slice()` returns an error | ||
/// if the slice cannot be used to build a `Pattern<SinglePlaceholder, str>` | ||
/// in its entirety. | ||
/// 5. The implementation of `from_byte_slice_unchecked()` returns a reference to the same data. | ||
/// 6. `parse_byte_slice()` is equivalent to `validate_byte_slice()` followed by `from_byte_slice_unchecked()`. | ||
/// 7. `str` byte equality is semantic equality. | ||
unsafe impl VarULE for Pattern<SinglePlaceholder, str> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thought: this impl is narrower than required; better would be an impl where Store: VarULE Non blocking because we can generalize later. But up to this point we've done a good job of keeping the impls general. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree, but I am thinking also to add imple to Double Pattern or generalize this later. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
fn validate_byte_slice(bytes: &[u8]) -> Result<(), ZeroVecError> { | ||
SinglePlaceholderPattern::try_from_utf8_store(bytes) | ||
.map_err(|_| ZeroVecError::VarZeroVecFormatError)?; | ||
Ok(()) | ||
} | ||
|
||
unsafe fn from_byte_slice_unchecked(bytes: &[u8]) -> &Self { | ||
robertbastian marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// SAFETY: As `validate_byte_slice` succeeded, `try_from_utf8_store` succeeded, which implies valid UTF-8 | ||
let store = core::str::from_utf8_unchecked(bytes); | ||
|
||
// SAFETY: As `validate_byte_slice` succeeded, `try_from_utf8_store` also succeeded | ||
SinglePlaceholderPattern::from_borrowed_store_unchecked(store) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -55,6 +55,8 @@ mod common; | |
mod double; | ||
mod error; | ||
mod frontend; | ||
#[cfg(all(feature = "zerovec", feature = "alloc"))] | ||
mod implementations; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: make this #[cfg(feature = "zerovec")]
mod zerovec; There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
mod multi_named; | ||
#[cfg(feature = "alloc")] | ||
mod parser; | ||
|
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.
should this be
Pattern<SinglePlaceholder, String>
?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.
Yes if it's easy, but not super important
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.
it's just another transmute
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.
why it should be
Pattern<SinglePlaceholder, String>
, if we are usingPattern<SinglePlaceholder, str>
in the datagen ?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.
the implementation will be : #5084