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

Remove dependency on byteorder crate, make byteorder type methods const #438

Closed
1 of 3 tasks
Tracked by #450 ...
joshlf opened this issue Oct 1, 2023 · 3 comments · Fixed by #583
Closed
1 of 3 tasks
Tracked by #450 ...

Remove dependency on byteorder crate, make byteorder type methods const #438

joshlf opened this issue Oct 1, 2023 · 3 comments · Fixed by #583
Labels
blocking-next-release This issue should be resolved before we release on crates.io compatibility-breaking Changes that are (likely to be) breaking

Comments

@joshlf
Copy link
Member

joshlf commented Oct 1, 2023

Summary

Remove our dependency on the byteorder crate and remove the byteorder feature, making the byteorder module available unconditinoally.

Progress

In progress in #583.

  • Update #583 to have const fns no longer be const so that it works with our MSRV
  • Support functions which are conditionally const depending on Rust version (as prototyped in #574)
  • Make byteorder functions conditionally const

Description

Some potential users have expressed concerns over our dependency on the byteorder crate. I have not seen any uses of the types in our byteorder module which mix those types with other uses of the byteorder crate, which implies that we could just define our own ByteOrder trait rather than using the one from the byteorder crate and this wouldn't cause problems for users.

While we're at it, we could use a technique like the following to allow the methods on our types to be const fns:

use core::marker::PhantomData;

pub trait ByteOrder {
    #[doc(hidden)]
    const ORDER: Order;
}

#[doc(hidden)]
pub enum Order {
    BigEndian,
    LittleEndian,
}

#[repr(transparent)]
pub struct U64<O>([u8; core::mem::size_of::<u64>()], PhantomData<O>);

impl<O: ByteOrder> U64<O> {
    pub const fn new(u: u64) -> U64<O> {
        let bytes = match O::ORDER {
            Order::BigEndian => u.to_be_bytes(),
            Order::LittleEndian => u.to_le_bytes(),
        };
        
        U64(bytes, PhantomData)
    }
}
@joshlf joshlf changed the title Make methods on byteorder types const fns Remove dependency on byteorder crate Oct 1, 2023
@joshlf joshlf added compatibility-breaking Changes that are (likely to be) breaking blocking-next-release This issue should be resolved before we release on crates.io labels Oct 1, 2023
@joshlf joshlf changed the title Remove dependency on byteorder crate Remove dependency on byteorder crate, make byteorder type methods const Oct 1, 2023
joshlf added a commit that referenced this issue Nov 1, 2023
Remove the `byteorder` feature and the dependency on the `byteorder`
crate. Replace the `ByteOrder` trait and types which implement it from
the `byteorder` crate with our own native implementations. This allows
us to make some of our functions and methods `const`.

Add `Usize` and `Isize` byte order-aware types.

Closes #438
@joshlf joshlf mentioned this issue Dec 4, 2023
87 tasks
joshlf added a commit that referenced this issue Dec 6, 2023
Remove the `byteorder` feature and the dependency on the `byteorder`
crate. Replace the `ByteOrder` trait and types which implement it from
the `byteorder` crate with our own native implementations. This allows
us to make some of our functions and methods `const`.

Add `Usize` and `Isize` byte order-aware types.

Closes #438
joshlf added a commit that referenced this issue Dec 6, 2023
Remove the `byteorder` feature and the dependency on the `byteorder`
crate. Replace the `ByteOrder` trait and types which implement it from
the `byteorder` crate with our own native implementations. This allows
us to make some of our functions and methods `const`.

Add `Usize` and `Isize` byte order-aware types.

Closes #438
joshlf added a commit that referenced this issue Dec 6, 2023
Remove the `byteorder` feature and the dependency on the `byteorder`
crate. Replace the `ByteOrder` trait and types which implement it from
the `byteorder` crate with our own native implementations. This allows
us to make some of our functions and methods `const`.

Add `Usize` and `Isize` byte order-aware types.

Closes #438
joshlf added a commit that referenced this issue Dec 28, 2023
Remove the `byteorder` feature and the dependency on the `byteorder`
crate. Replace the `ByteOrder` trait and types which implement it from
the `byteorder` crate with our own native implementations.

This prepares us for a future commit in which we will make some of our
functions and methods `const`. This commit doesn't implement this yet
because it doesn't play nicely with our current MSRV; in order to
support this, we'll need to introduce machinery that allows us to
compile functions as conditionally `const` depending on what Rust
toolchain version is used. See #574 for a prototype of this machinery.

Add `Usize` and `Isize` byte order-aware types.

Closes #438
joshlf added a commit that referenced this issue Dec 28, 2023
Remove the `byteorder` feature and the dependency on the `byteorder`
crate. Replace the `ByteOrder` trait and types which implement it from
the `byteorder` crate with our own native implementations.

This prepares us for a future commit in which we will make some of our
functions and methods `const`. This commit doesn't implement this yet
because it doesn't play nicely with our current MSRV; in order to
support this, we'll need to introduce machinery that allows us to
compile functions as conditionally `const` depending on what Rust
toolchain version is used. See #574 for a prototype of this machinery.

Add `Usize` and `Isize` byte order-aware types.

Closes #438
joshlf added a commit that referenced this issue Dec 28, 2023
Remove the `byteorder` feature and the dependency on the `byteorder`
crate. Replace the `ByteOrder` trait and types which implement it from
the `byteorder` crate with our own native implementations.

This prepares us for a future commit in which we will make some of our
functions and methods `const`. This commit doesn't implement this yet
because it doesn't play nicely with our current MSRV; in order to
support this, we'll need to introduce machinery that allows us to
compile functions as conditionally `const` depending on what Rust
toolchain version is used. See #574 for a prototype of this machinery.

Add `Usize` and `Isize` byte order-aware types.

Closes #438
joshlf added a commit that referenced this issue Jan 18, 2024
Remove the `byteorder` feature and the dependency on the `byteorder`
crate. Replace the `ByteOrder` trait and types which implement it from
the `byteorder` crate with our own native implementations.

This prepares us for a future commit in which we will make some of our
functions and methods `const`. This commit doesn't implement this yet
because it doesn't play nicely with our current MSRV; in order to
support this, we'll need to introduce machinery that allows us to
compile functions as conditionally `const` depending on what Rust
toolchain version is used. See #574 for a prototype of this machinery.

Add `Usize` and `Isize` byte order-aware types.

Closes #438
github-merge-queue bot pushed a commit that referenced this issue Jan 18, 2024
Remove the `byteorder` feature and the dependency on the `byteorder`
crate. Replace the `ByteOrder` trait and types which implement it from
the `byteorder` crate with our own native implementations.

This prepares us for a future commit in which we will make some of our
functions and methods `const`. This commit doesn't implement this yet
because it doesn't play nicely with our current MSRV; in order to
support this, we'll need to introduce machinery that allows us to
compile functions as conditionally `const` depending on what Rust
toolchain version is used. See #574 for a prototype of this machinery.

Add `Usize` and `Isize` byte order-aware types.

Closes #438
@digama0
Copy link

digama0 commented Nov 1, 2024

Do you intend to implement the ReadBytesExt and WriteBytesExt traits from byteorder? I was previously using zerocopy and byteorder together for this reason, but it's awkward that I now have to deal with conflicting LittleEndian types.

@joshlf
Copy link
Member Author

joshlf commented Nov 1, 2024

What's your specific use case? Can you provide some example code? There's an existing discussion about supporting readers and writers: #158

@digama0
Copy link

digama0 commented Nov 3, 2024

I managed to work around the issue in my actual use case. You can see the code using both the byteorder LE and the zerocopy LE renamed to ZLE. You can search for other uses of LE and ZLE in the file. This is a binary parser, which is reading a binary format using zerocopy and writing another binary format in streaming fashion, usually one integral value at a time. Endianness is important in both cases, but I can't use the same LE type because zerocopy's LE doesn't implement byteorder's traits and vice versa.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocking-next-release This issue should be resolved before we release on crates.io compatibility-breaking Changes that are (likely to be) breaking
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants