Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Update rand crate 0.8.5 -> 0.9.0 #1255

Merged
merged 1 commit into from
Feb 25, 2025
Merged

Conversation

gsson
Copy link

@gsson gsson commented Feb 21, 2025

Also updates the following to align versions and avoid duplicates in transitive dependencies

  • rand_pcg 0.3.1 -> 0.9.0
  • rand_chacha 0.3.1 -> 0.9.0

MSRV for all changes is 1.63, so should hopefully be good

I'm currently trying to reduce the number of duplicate dependencies to reduce some bloat in my projects, and rand is one of them.

Pre-review checklist

  • I have split my patch into logically separate commits.
  • All commit messages clearly explain what they change and why.
  • I added relevant tests for new features and bug fixes.
  • All commits compile, pass static checks and pass test.
  • PR description sums up the changes and reasons why they should be introduced.
  • I have provided docstrings for the public items that I want to introduce.
  • I have adjusted the documentation in ./docs/source/.
  • I added appropriate Fixes: annotations to PR description.

Copy link

github-actions bot commented Feb 21, 2025

cargo semver-checks found no API-breaking changes in this PR! 🎉🥳
Checked commit: a4e5130

@gsson
Copy link
Author

gsson commented Feb 21, 2025

Assume I need to update Cargo.lock.msrv 🤔

I'll get on that shortly!

@muzarski
Copy link
Contributor

Assume I need to update Cargo.lock.msrv 🤔

I'll get on that shortly!

Update procedure is documented here: https://github.com/scylladb/scylla-rust-driver/blob/main/CONTRIBUTING.md#min_rust-workflow-and-cargolockmsrv

@Lorak-mmk
Copy link
Collaborator

Lorak-mmk commented Feb 21, 2025

Clippy failed, but that's most likely not related to your changes, but to the recent release of Rust 1.85 - we will have to fix that.

min_rust failed because you updated dependencies without updating the lock file for MSRV. In contributing.md there are instructions for fixing that: https://github.com/scylladb/scylla-rust-driver/blob/main/CONTRIBUTING.md#min_rust-workflow-and-cargolockmsrv . Could you do that? @muzarski was quicker, I didn't notice the comment, sorry!

One other minor comment: in this case I'd prefer all the changes in the single commit. There are not many of them, so it won't make the commit difficult to read, but it will prevent having commits with deprecation warnings.

@gsson
Copy link
Author

gsson commented Feb 21, 2025

One other minor comment: in this case I'd prefer all the changes in the single commit. There are not many of them, so it won't make the commit difficult to read, but it will prevent having commits with deprecation warnings.

And here I went through all the trouble splitting my original commit 😁

I'll sort that as well

@gsson
Copy link
Author

gsson commented Feb 21, 2025

@Lorak-mmk : I'd probably "fix" the clippy lints like this

impl CqlTimeuuid {
    /// Read 8 most significant bytes of timeuuid from serialized bytes
    fn msb(&self) -> u64 {
        // Scylla and Cassandra use a standard UUID memory layout for MSB:
        // 4 bytes    2 bytes    2 bytes
        // time_low - time_mid - time_hi_and_version
        let bytes = self.0.as_bytes();
        u64::from_be_bytes([
            bytes[6] & 0x0F, bytes[7], bytes[4], bytes[5], bytes[0], bytes[1], bytes[2], bytes[3],
        ])
    }

    fn lsb(&self) -> u64 {
        let bytes = self.0.as_bytes();
        u64::from_be_bytes([
            bytes[8], bytes[9], bytes[10], bytes[11], bytes[12], bytes[13], bytes[14], bytes[15],
        ])
    }

    fn lsb_signed(&self) -> u64 {
        self.lsb() ^ 0x8080808080808080
    }
}

Gets rid of most of the exposed bit twiddling. As far as I've been able to tell where I've done the same elsewhere, the generated code isn't really any worse.

This passes, btw

    #[test]
    fn asdf() {
        let bytes = [1, 2, 3, 4, 5, 6, 7, 8];
        let a = u64::from_be_bytes([
            bytes[6] & 0x0F, bytes[7], bytes[4], bytes[5], bytes[0], bytes[1], bytes[2], bytes[3],
        ]);
        let b = ((bytes[6] & 0x0F) as u64) << 56
            | (bytes[7] as u64) << 48
            | (bytes[4] as u64) << 40
            | (bytes[5] as u64) << 32
            | (bytes[0] as u64) << 24
            | (bytes[1] as u64) << 16
            | (bytes[2] as u64) << 8
            | (bytes[3] as u64);
        assert_eq!(a, b);
    }

@gsson
Copy link
Author

gsson commented Feb 21, 2025

As far as I've been able to tell where I've done the same elsewhere, the generated code isn't really any worse.

Strike that -- the generated code (at least on x86) seems to be significantly better. I think I'll start aggressively change things myself, where it makes sense (in my code bases ofc :) )

https://godbolt.org/z/j78fETzbT

@gsson
Copy link
Author

gsson commented Feb 24, 2025

#1256 should hopefully fix the clippy lints

@Lorak-mmk
Copy link
Collaborator

#1256 should hopefully fix the clippy lints

I've merged it. Please rebase this PR on main.

Also updates the following to align versions and avoid duplicates in transitive dependencies
* `rand_pcg` 0.3.1 -> 0.9.0
* `rand_chacha` 0.3.1 -> 0.9.0
@wprzytula wprzytula requested review from Lorak-mmk, wprzytula and piodul and removed request for wprzytula and piodul February 25, 2025 16:54
@Lorak-mmk Lorak-mmk merged commit c24db77 into scylladb:main Feb 25, 2025
11 checks passed
@Lorak-mmk
Copy link
Collaborator

Thanks!

@gsson gsson deleted the update-rand branch February 25, 2025 16:56
@Lorak-mmk Lorak-mmk mentioned this pull request Mar 4, 2025
8 tasks
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants