8000 feat: add rkyv support via derive by lightsing · Pull Request #484 · recmo/uint · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

feat: add rkyv support via derive #484

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

Closed
wants to merge 1 commit into from

Conversation

lightsing
Copy link

see #483

@lightsing lightsing requested a review from prestwich as a code owner June 10, 2025 03:27
@lightsing lightsing mentioned this pull request Jun 10, 2025
3 tasks
@@ -148,6 +149,10 @@ pub mod nightly {
///
/// [std-overflow]: https://doc.rust-lang.org/reference/expressions/operator-expr.html#overflow
#[derive(Clone, Copy, Eq, PartialEq, Hash)]
#[cfg_attr(
feature = "rkyv",
derive(rkyv::Archive, rkyv::Serialize, rkyv::Deserialize),
Copy link
Collaborator

Choose a reason for hiding this comment

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

my understanding is that this would serialize extra bytes when the BITS % 64 != 0 because it will serialize the entire array regardless, yes?

Copy link
Author

Choose a reason for hiding this comment

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

yes, it serialize [u64; LIMBS] regardless of the BITS by the nature of rkyv

Copy link
Collaborator

Choose a reason for hiding this comment

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

#483 behaves this way as well? wouldn't it be desirable to instead cast to a [u8] of the appropriate size to avoid extra 0-bytes?

Copy link
Author

Choose a reason for hiding this comment

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

not sure if it's a good idea.

Copy link
Collaborator

Choose a reason for hiding this comment

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

8000

why is that? seems like a minimal encoding would be desirable?

@prestwich
Copy link
Collaborator

closing this as #483 is preferred

@prestwich prestwich closed this Jun 27, 2025
@lightsing lightsing deleted the feat/rkyv-derive branch July 4, 2025 02:56
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.

2 participants
0