Skip to content

Commit

Permalink
Update bitflags to 2.4
Browse files Browse the repository at this point in the history
Summary:
## Motivation

Since the latest compiler update, we are getting `clippy::bad_bit_mask` errors at the callsites of `bitflags!` macros where one of the variant is zero.
[Upstream won't address it in the `1.x` branch](bitflags/bitflags#373) and recommends upgrading to the `2.x` branch.

We are very close to reaching **zero clippy lints** in [Mononoke and other servers](https://fburl.com/code/pd76yn5e), which would feel nice.

## Specific categories of changes (in case it helps with the code review)

The change from `1.x` to `2.x` introduces a number  of backward compatibility breakages which I had to workaround in our codebase.
See [the release notes for 2.0](https://github.com/bitflags/bitflags/releases/tag/2.0.0) for the explanation for the manual fixes I had to perform at each call site.
 ---
**Adding traits to derive:**
```
#[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
```
> Generated flags types now derive fewer traits. If you need to maintain backwards compatibility, you can derive the following yourself:
 ---
**Replacing read uses of `.bits` with `.bits()`**
> You can now use the .bits() method instead of the old .bits.
> The representation of generated flags types has changed from a struct with the single field bits to a newtype.
 ---
**Replacing raw setting of `.bits` with `.from_bits_retain()`**
Due to the point above, the representation of the type is not exposed anymore. From [the documentation](https://docs.rs/bitflags/latest/bitflags/example_generated/struct.Flags.html#method.from_bits_retain), `from_bits_retain` "convert from a bits value exactly", which matches the old behaviour
 ---
**Replacing the unsafe `from_bits_unchecked` method with `from_bits_retain`**
> The unsafe from_bits_unchecked method is now a safe from_bits_retain method.
 ---
**Extracting some structs outside of the `bitflags!` macro**
Apart from the derives that `bitflags` knows about, such as `serde`, `bitflags` now can't deal with custom derives in macros with the previous syntax. I followed the recommendation from [their documentation](https://docs.rs/bitflags/latest/bitflags/index.html#custom-derives) to declare the `struct` ahead of of the macro call and only declare the `impl` block inside the macro.
 ---
**Changes to test output**
This does not stand out in the release notes, but as of [this upstream PR](bitflags/bitflags#297), the `Debug` output of generated bitflags has changed. This means any tests that rely on this (and of course, there are a few) needed updating.
In particular, the `vespa` tests rely on that output in a non-obvious way. You might have to trust me (and CI) on these ones...

Reviewed By: dtolnay

Differential Revision: D49742979

fbshipit-source-id: c818c37af45f0964e8fdb7ec6173ad66bb982c00
  • Loading branch information
Pierre Chevalier authored and facebook-github-bot committed Jan 16, 2024
1 parent d3daaeb commit b1516c3
Show file tree
Hide file tree
Showing 11 changed files with 15 additions and 14 deletions.
2 changes: 1 addition & 1 deletion reverie-process/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ license = "BSD-2-Clause"

[dependencies]
bincode = "1.3.3"
bitflags = "1.3"
bitflags = "2.4"
colored = "2.1.0"
futures = { version = "0.3.28", features = ["async-await", "compat"] }
libc = "0.2.139"
Expand Down
2 changes: 1 addition & 1 deletion reverie-process/src/namespace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ bitflags::bitflags! {
/// A namespace that may be unshared with [`Command::unshare`].
///
/// [`Command::unshare`]: super::Command::unshare
#[derive(Deserialize, Serialize)]
#[derive(Deserialize, Serialize, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
pub struct Namespace: i32 {
/// Cgroup namespace.
const CGROUP = libc::CLONE_NEWCGROUP;
Expand Down
2 changes: 1 addition & 1 deletion reverie-process/src/seccomp/bpf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ pub const AUDIT_ARCH_PPC: u32 = EM_PPC;
pub const AUDIT_ARCH_PPC64: u32 = EM_PPC64 | __AUDIT_ARCH_64BIT;

bitflags::bitflags! {
#[derive(Default)]
#[derive(Default, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
struct FilterFlags: u32 {
const TSYNC = 1 << 0;
const LOG = 1 << 1;
Expand Down
2 changes: 1 addition & 1 deletion reverie-syscalls/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ repository = "https://github.com/facebookexperimental/reverie"
license = "BSD-2-Clause"

[dependencies]
bitflags = "1.3"
bitflags = "2.4"
derive_more = "0.99.17"
libc = "0.2.139"
nix = "0.25"
Expand Down
6 changes: 3 additions & 3 deletions reverie-syscalls/src/args/clone.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use crate::MemoryAccess;

bitflags::bitflags! {
/// Flags used with the `clone`, `clone3`, or `unshare` syscalls.
#[derive(Serialize, Deserialize)]
#[derive(Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
pub struct CloneFlags: u64 {
/// set if VM shared between processes
const CLONE_VM = libc::CLONE_VM as u64;
Expand Down Expand Up @@ -91,7 +91,7 @@ impl Displayable for CloneFlags {

impl FromToRaw for CloneFlags {
fn from_raw(raw: usize) -> Self {
unsafe { Self::from_bits_unchecked(raw as u64) }
Self::from_bits_retain(raw as u64)
}

fn into_raw(self) -> usize {
Expand All @@ -101,7 +101,7 @@ impl FromToRaw for CloneFlags {

impl From<nix::sched::CloneFlags> for CloneFlags {
fn from(flags: nix::sched::CloneFlags) -> Self {
unsafe { CloneFlags::from_bits_unchecked(flags.bits() as u64) }
CloneFlags::from_bits_retain(flags.bits() as u64)
}
}

Expand Down
2 changes: 1 addition & 1 deletion reverie-syscalls/src/args/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,7 @@ impl<'a> Displayable for Option<StatxPtr<'a>> {

bitflags::bitflags! {
/// stx_mask from statx, see linux/stat.h
#[derive(Serialize, Deserialize)]
#[derive(Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
pub struct StatxMask: u32 {
/// has stx_type
const STATX_TYPE = 0x1;
Expand Down
6 changes: 3 additions & 3 deletions reverie-syscalls/src/args/poll.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ impl From<libc::pollfd> for PollFd {
fn from(pollfd: libc::pollfd) -> Self {
Self {
fd: pollfd.fd,
events: unsafe { PollFlags::from_bits_unchecked(pollfd.events) },
revents: unsafe { PollFlags::from_bits_unchecked(pollfd.revents) },
events: PollFlags::from_bits_retain(pollfd.events),
revents: PollFlags::from_bits_retain(pollfd.revents),
}
}
}
Expand All @@ -60,7 +60,7 @@ impl Displayable for PollFd {

bitflags::bitflags! {
/// Flags for [`PollFd`].
#[derive(Default, Serialize, Deserialize)]
#[derive(Default, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
pub struct PollFlags: libc::c_short {
/// There is data to read.
const POLLIN = libc::POLLIN;
Expand Down
2 changes: 1 addition & 1 deletion reverie/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ license = "BSD-2-Clause"
addr2line = "0.21"
anyhow = "1.0.75"
async-trait = "0.1.71"
bitflags = "1.3"
bitflags = "2.4"
byteorder = "1.3"
lazy_static = "1.4"
libc = "0.2.139"
Expand Down
2 changes: 1 addition & 1 deletion reverie/src/subscription.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use reverie_syscalls::Sysno;
use syscalls::SysnoSet;

bitflags! {
#[derive(Default)]
#[derive(Default, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
struct Instructions: u32 {
const CPUID = 1;
const RDTSC = 2;
Expand Down
2 changes: 1 addition & 1 deletion safeptrace/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ repository = "https://github.com/facebookexperimental/reverie"
license = "BSD-2-Clause"

[dependencies]
bitflags = "1.3"
bitflags = "2.4"
futures = { version = "0.3.28", features = ["async-await", "compat"] }
lazy_static = "1.4"
libc = "0.2.139"
Expand Down
1 change: 1 addition & 0 deletions safeptrace/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,7 @@ struct ptrace_peeksiginfo_args {

bitflags::bitflags! {
/// Flags for ptrace peeksiginfo
#[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
pub struct PeekSigInfoFlags: u32 {
/// dumping signals from the process-wide signal queue. signals are
/// read from the per-thread queue of the specified thread if this
Expand Down

0 comments on commit b1516c3

Please sign in to comment.