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

[byteorder] Remove byteorder feature and crate dep #583

Merged
merged 1 commit into from
Jan 18, 2024

Conversation

joshlf
Copy link
Member

@joshlf joshlf commented 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 force-pushed the byteorder-no-crate-dep branch 2 times, most recently from 5d72b31 to 88161ae Compare December 6, 2023 22:33
src/lib.rs Outdated Show resolved Hide resolved
@joshlf
Copy link
Member Author

joshlf commented Dec 6, 2023

As it stands, this won't compile on our MSRV because it doesn't support const fns with trait bounds (in this case, O: ByteOrder). If we want to keep this as-is, we'll need to land #574 or similar first so that we can have these be conditionally const.

@joshlf joshlf force-pushed the byteorder-no-crate-dep branch 2 times, most recently from a538d86 to da07b83 Compare December 28, 2023 11:51
@joshlf joshlf marked this pull request as ready for review December 28, 2023 11:51
@joshlf joshlf requested a review from jswrenn December 28, 2023 11:51
A,
Usize,
usize,
mem::size_of::<usize>() * 8,
Copy link
Member Author

Choose a reason for hiding this comment

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

The rustdoc CI failures seem to come from the fact that, in define_type!, we use stringify! on this expr in order to generate the doc comment.

I think the simplest thing to do here might just be to introduce a new macro argument that is used in the summary sentence of the doc comment instead of doing stringify!($bits). It would have the text, "A 16-bit unsigned integer" , "A word-sized signed integer", etc. i.e.:

Old

        doc_comment! {
            concat!("A ", stringify!($bits), "-bit ", $number_kind,
            " stored in a given byte order.

New

        doc_comment! {
            concat!($description, " stored in a given byte order.

This would also allow us to get rid of the $number_kind argument.

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 joshlf added this pull request to the merge queue Jan 18, 2024
Merged via the queue into main with commit 7d917bf Jan 18, 2024
127 checks passed
@joshlf joshlf deleted the byteorder-no-crate-dep branch January 18, 2024 23:43
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.

Remove dependency on byteorder crate, make byteorder type methods const
2 participants