Skip to content
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

Consistently use bytes not byte_slice #5816

Merged
merged 16 commits into from
Nov 15, 2024

Conversation

Manishearth
Copy link
Member

Fixes #5815

@Manishearth
Copy link
Member Author

Reviewer note: can be reviewed individually. Do take a look at non-zerovec code affected by each commit in case something incorrect snuck through.

sffc
sffc previously approved these changes Nov 15, 2024
@@ -367,7 +367,7 @@ impl CaseMapDataULE {
/// 6. The equality invariant is satisfied
unsafe impl ULE for CaseMapDataULE {
fn validate_byte_slice(bytes: &[u8]) -> Result<(), UleError> {
let sixteens = RawBytesULE::<2>::parse_byte_slice(bytes)?;
let sixteens = RawBytesULE::<2>::parse_bytes(bytes)?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thought: could be try_from_bytes

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this being parse() but it's worth discussing. I'd want to do that separately IMO.

@@ -97,7 +97,7 @@ impl<'a, V: VarULE + ?Sized> VarZeroCow<'a, V> {
}

/// Construct from an owned slice. Errors if the slice doesn't represent a valid `V`
pub fn parse_owned_byte_slice(bytes: Box<[u8]>) -> Result<Self, UleError> {
pub fn parse_owned_bytes(bytes: Box<[u8]>) -> Result<Self, UleError> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion (optional): try_from_boxed_bytes or parse_boxed_bytes. I think "boxed bytes" is language used elsewhere

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think for a Cow type "owned" is probably better.

utils/zerovec/src/ule/mod.rs Outdated Show resolved Hide resolved
utils/zerovec/src/ule/mod.rs Outdated Show resolved Hide resolved
@Manishearth Manishearth merged commit 486aa25 into unicode-org:main Nov 15, 2024
28 checks passed
@Manishearth Manishearth deleted the bytes-not-byte-slice branch November 15, 2024 01:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clean up as_byte_slice vs as_bytes (etc)
2 participants