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

uv-pep440: DRY up VersionSmall implementation #8834

Merged
merged 1 commit into from
Nov 5, 2024

Conversation

BurntSushi
Copy link
Member

This PR simplifies the VersionSmall implementation a bit by utilizing
more constants. That is, if the bit-level format changes, most of
those changes should be implementable by just changing the constants.
Previously, you would need to audit and tweak the code as well. (The
exception here is push_release. If the release segment bit format is
changed, then that function will need to be tweaked. I didn't think it
was worth over-complicating things to make its implementation more
general.)

@BurntSushi
Copy link
Member Author

BurntSushi commented Nov 5, 2024

With this PR, to add a "local" suffix kind, I believe this is all that's needed (at least, for making changes to the bit-level representation):

$ git diff
diff --git a/crates/uv-pep440/src/version.rs b/crates/uv-pep440/src/version.rs
index 22b9730ed..6e51df393 100644
--- a/crates/uv-pep440/src/version.rs
+++ b/crates/uv-pep440/src/version.rs
@@ -922,22 +922,27 @@ impl VersionSmall {
     const SUFFIX_PRE_BETA: u64 = 3;
     const SUFFIX_PRE_RC: u64 = 4;
     const SUFFIX_NONE: u64 = 5;
-    const SUFFIX_POST: u64 = 6;
-    const SUFFIX_MAX: u64 = 7;
+    const SUFFIX_LOCAL: u64 = 6;
+    const SUFFIX_POST: u64 = 7;
+    const SUFFIX_MAX: u64 = 8;

     // The mask to get only the release segment bits.
     const SUFFIX_RELEASE_MASK: u64 = 0xFFFF_FFFF_FF00_0000;
     // The mask to get the version suffix.
-    const SUFFIX_VERSION_MASK: u64 = 0x001F_FFFF;
+    const SUFFIX_VERSION_MASK: u64 = 0x000F_FFFF;
     // The number of bits used by the version suffix. Shifting the `repr`
     // right by this number of bits should put the suffix kind in the least
     // significant bits.
-    const SUFFIX_VERSION_BIT_LEN: u64 = 21;
+    const SUFFIX_VERSION_BIT_LEN: u64 = 20;
     // The mask to get only the suffix kind, after shifting right by the
     // version bits. If you need to add a bit here, then you'll probably need
     // to take a bit from the suffix version. (Which requires a change to both
     // the mask and the bit length above.)
-    const SUFFIX_KIND_MASK: u64 = 0b111;
+    //
+    // NOTE: If you do change the bit format here, you'll need to bump any
+    // cache versions in uv that use rkyv with `Version` in them. That includes
+    // *at least* the "simple" cache.
+    const SUFFIX_KIND_MASK: u64 = 0b1111;

     #[inline]
     fn new() -> Self {

@charliermarsh
Copy link
Member

Thank you! 🙏

Copy link
Member

@charliermarsh charliermarsh left a comment

Choose a reason for hiding this comment

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

Thanks so helpful.

@BurntSushi BurntSushi force-pushed the ag/small-version-tweaks branch from a01efdc to f53554e Compare November 5, 2024 17:59
This PR simplifies the VersionSmall implementation a bit by utilizing
more constants. That is, if the bit-level format changes, *most* of
those changes should be implementable by just changing the constants.
Previously, you would need to audit and tweak the code as well. (The
exception here is `push_release`. If the release segment bit format is
changed, then that function will need to be tweaked. I didn't think it
was worth over-complicating things to make its implementation more
general.)
@zanieb zanieb added the rustlib Related to our Rust library API label Nov 5, 2024
@BurntSushi BurntSushi force-pushed the ag/small-version-tweaks branch from f53554e to 26c9b4c Compare November 5, 2024 18:09
@BurntSushi BurntSushi merged commit fae03a7 into main Nov 5, 2024
63 checks passed
@BurntSushi BurntSushi deleted the ag/small-version-tweaks branch November 5, 2024 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rustlib Related to our Rust library API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants