Update bitflags to 2.4

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
facebook · Jan 16, 2024 · aa07b28 · aa07b28
1 parent d45c970
commit aa07b28
Show file tree

Hide file tree

Showing 21 changed files with 75 additions and 66 deletions.
diff --git a/eden/mononoke/mercurial/revlog/Cargo.toml b/eden/mononoke/mercurial/revlog/Cargo.toml
@@ -13,7 +13,7 @@ path = "lib.rs"
 [dependencies]
 anyhow = "1.0.75"
 ascii = "1.0"
-bitflags = "1.3"
+bitflags = "2.4"
 bytes = { version = "1.1", features = ["serde"] }
 flate2 = { version = "1.0.26", features = ["rust_backend"], default-features = false }
 futures = "0.1.31"

diff --git a/eden/mononoke/mercurial/revlog/revlog/parser.rs b/eden/mononoke/mercurial/revlog/revlog/parser.rs
@@ -49,6 +49,7 @@ pub mod Badness {
 
 // `Revlog` features
 bitflags! {
+    #[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
     pub struct Features: u16 {
         const INLINE        = 1 << 0;
         const GENERAL_DELTA = 1 << 1;
@@ -57,6 +58,7 @@ bitflags! {
 
 // Per-revision flags
 bitflags! {
+    #[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
     pub struct IdxFlags: u16 {
         const OCTOPUS_MERGE = 1 << 12;
         const EXTSTORED     = 1 << 13;

diff --git a/eden/mononoke/mercurial/types/Cargo.toml b/eden/mononoke/mercurial/types/Cargo.toml
@@ -14,7 +14,7 @@ anyhow = "1.0.75"
 ascii = "1.0"
 async-stream = "0.3"
 async-trait = "0.1.71"
-bitflags = "1.3"
+bitflags = "2.4"
 blobstore = { version = "0.1.0", path = "../../blobstore" }
 bytes = { version = "1.1", features = ["serde"] }
 cloned = { version = "0.1.0", git = "https://github.com/facebookexperimental/rust-shed.git", branch = "main" }

diff --git a/eden/mononoke/mercurial/types/src/flags.rs b/eden/mononoke/mercurial/types/src/flags.rs
@@ -14,6 +14,7 @@ use crate::errors::MononokeHgError;
 
 bitflags! {
     // names are from hg revlog.py
+    #[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
     pub struct RevFlags: u16 {
         const REVIDX_DEFAULT_FLAGS = 0;
         const REVIDX_EXTSTORED = 1 << 13;  // data is stored externally

diff --git a/eden/mononoke/walker/Cargo.toml b/eden/mononoke/walker/Cargo.toml
@@ -15,7 +15,7 @@ async-trait = "0.1.71"
 async_compression = { version = "0.1.0", git = "https://github.com/facebookexperimental/rust-shed.git", branch = "main" }
 auto_impl = "1.1.0"
 basename_suffix_skeleton_manifest = { version = "0.1.0", path = "../derived_data/basename_suffix_skeleton_manifest" }
-bitflags = "1.3"
+bitflags = "2.4"
 blame = { version = "0.1.0", path = "../derived_data/blame" }
 blobrepo = { version = "0.1.0", path = "../blobrepo" }
 blobrepo_hg = { version = "0.1.0", path = "../blobrepo/blobrepo_hg" }

diff --git a/eden/mononoke/walker/src/detail/graph.rs b/eden/mononoke/walker/src/detail/graph.rs
@@ -268,7 +268,7 @@ impl ChangesetKey<HgChangesetId> {
 
 bitflags! {
     /// Some derived data needs unodes as precondition, flags represent what is available in a compact way
-    #[derive(Default)]
+    #[derive(Default, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
     pub struct UnodeFlags: u8 {
         const NONE = 0b00000000;
         const BLAME = 0b00000001;

diff --git a/eden/scm/lib/dag/Cargo.toml b/eden/scm/lib/dag/Cargo.toml
@@ -9,7 +9,7 @@ license = "MIT"
 [dependencies]
 anyhow = "1.0.75"
 async-trait = "0.1.71"
-bitflags = "1.3"
+bitflags = "2.4"
 byteorder = "1.3"
 dag-types = { version = "0.1.0", path = "dag-types", default-features = false }
 drawdag = { version = "0.1.0", path = "../drawdag" }

diff --git a/eden/scm/lib/dag/src/iddagstore/indexedlog_store.rs b/eden/scm/lib/dag/src/iddagstore/indexedlog_store.rs
@@ -938,7 +938,7 @@ mod tests {
             describe_indexedlog_entry(bytes),
             r#"# f0: MAGIC_REWRITE_LAST_FLAT
 # 00 00 00 00 00 00 00 00 05: Previous index Level = 0, Head = 5
-# 01: Flags = HAS_ROOT
+# 01: Flags = SegmentFlags(HAS_ROOT)
 # 00: Level = 0
 # 00 00 00 00 00 00 00 0a: High = 10
 # 0a: Delta = 10 (Low = 0)
@@ -952,7 +952,7 @@ mod tests {
             describe_indexedlog_entry(bytes),
             r#"# f1: MAGIC_REMOVE_SEGMENT
 # 00: Max Level = 0
-# 01: Flags = HAS_ROOT
+# 01: Flags = SegmentFlags(HAS_ROOT)
 # 00: Level = 0
 # 00 00 00 00 00 00 00 0a: High = 10
 # 0a: Delta = 10 (Low = 0)

diff --git a/eden/scm/lib/dag/src/nameset/hints.rs b/eden/scm/lib/dag/src/nameset/hints.rs
@@ -22,6 +22,7 @@ use crate::Id;
 use crate::VerLink;
 
 bitflags! {
+    #[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
     pub struct Flags: u32 {
         /// Full. A full Set & other set X with compatible Dag results in X.
         const FULL = 0x1;

diff --git a/eden/scm/lib/dag/src/nameset/mod.rs b/eden/scm/lib/dag/src/nameset/mod.rs
@@ -1000,35 +1000,35 @@ pub(crate) mod tests {
         assert_eq!(
             hints_ops(&partial, &empty),
             [
-                "- Hints(ID_ASC)",
-                "  Hints(EMPTY | ID_DESC | ID_ASC | TOPO_DESC | ANCESTORS)",
-                "& Hints(EMPTY | ID_DESC | ID_ASC | TOPO_DESC | ANCESTORS)",
-                "  Hints(EMPTY | ID_DESC | ID_ASC | TOPO_DESC | ANCESTORS)",
-                "| Hints(ID_ASC)",
-                "  Hints(ID_ASC)"
+                "- Hints(Flags(ID_ASC))",
+                "  Hints(Flags(EMPTY | ID_DESC | ID_ASC | TOPO_DESC | ANCESTORS))",
+                "& Hints(Flags(EMPTY | ID_DESC | ID_ASC | TOPO_DESC | ANCESTORS))",
+                "  Hints(Flags(EMPTY | ID_DESC | ID_ASC | TOPO_DESC | ANCESTORS))",
+                "| Hints(Flags(ID_ASC))",
+                "  Hints(Flags(ID_ASC))"
             ]
         );
         // Fast paths are not used for "|" because there is no dag associated.
         assert_eq!(
             hints_ops(&partial, &full),
             [
-                "- Hints(ID_ASC)",
-                "  Hints(ID_DESC)",
-                "& Hints(ID_ASC)",
-                "  Hints(ID_ASC)",
-                "| Hints((empty))",
-                "  Hints((empty))"
+                "- Hints(Flags(ID_ASC))",
+                "  Hints(Flags(ID_DESC))",
+                "& Hints(Flags(ID_ASC))",
+                "  Hints(Flags(ID_ASC))",
+                "| Hints(Flags(0x0))",
+                "  Hints(Flags(0x0))"
             ]
         );
         assert_eq!(
             hints_ops(&empty, &full),
             [
-                "- Hints(EMPTY | ID_DESC | ID_ASC | TOPO_DESC | ANCESTORS)",
-                "  Hints(FULL | ID_DESC | ANCESTORS)",
-                "& Hints(EMPTY | ID_DESC | ID_ASC | TOPO_DESC | ANCESTORS)",
-                "  Hints(EMPTY | ID_DESC | ID_ASC | TOPO_DESC | ANCESTORS)",
-                "| Hints(FULL | ID_DESC | ANCESTORS)",
-                "  Hints(FULL | ID_DESC | ANCESTORS)"
+                "- Hints(Flags(EMPTY | ID_DESC | ID_ASC | TOPO_DESC | ANCESTORS))",
+                "  Hints(Flags(FULL | ID_DESC | ANCESTORS))",
+                "& Hints(Flags(EMPTY | ID_DESC | ID_ASC | TOPO_DESC | ANCESTORS))",
+                "  Hints(Flags(EMPTY | ID_DESC | ID_ASC | TOPO_DESC | ANCESTORS))",
+                "| Hints(Flags(FULL | ID_DESC | ANCESTORS))",
+                "  Hints(Flags(FULL | ID_DESC | ANCESTORS))"
             ]
         );
     }
@@ -1111,12 +1111,12 @@ pub(crate) mod tests {
         assert_eq!(
             hints_ops(&bc, &ad),
             [
-                "- Hints(ID_DESC, 20..)",
-                "  Hints(ID_ASC, ..=40)",
-                "& Hints(ID_DESC, 20..=40)",
-                "  Hints(ID_ASC, 20..=40)",
-                "| Hints((empty))",
-                "  Hints((empty))"
+                "- Hints(Flags(ID_DESC), 20..)",
+                "  Hints(Flags(ID_ASC), ..=40)",
+                "& Hints(Flags(ID_DESC), 20..=40)",
+                "  Hints(Flags(ID_ASC), 20..=40)",
+                "| Hints(Flags(0x0))",
+                "  Hints(Flags(0x0))"
             ]
         );
 
@@ -1125,12 +1125,12 @@ pub(crate) mod tests {
         assert_eq!(
             hints_ops(&bc, &ad),
             [
-                "- Hints(ID_DESC, 20..=30)",
-                "  Hints(ID_ASC, 10..=40)",
-                "& Hints(ID_DESC, 20..=30)",
-                "  Hints(ID_ASC, 20..=30)",
-                "| Hints((empty))",
-                "  Hints((empty))"
+                "- Hints(Flags(ID_DESC), 20..=30)",
+                "  Hints(Flags(ID_ASC), 10..=40)",
+                "& Hints(Flags(ID_DESC), 20..=30)",
+                "  Hints(Flags(ID_ASC), 20..=30)",
+                "| Hints(Flags(0x0))",
+                "  Hints(Flags(0x0))"
             ]
         );
     }
@@ -1144,25 +1144,25 @@ pub(crate) mod tests {
         assert_eq!(
             hints_ops(&a, &b),
             [
-                "- Hints((empty))",
-                "  Hints((empty))",
-                "& Hints((empty))",
-                "  Hints((empty))",
-                "| Hints((empty))",
-                "  Hints((empty))"
+                "- Hints(Flags(0x0))",
+                "  Hints(Flags(0x0))",
+                "& Hints(Flags(0x0))",
+                "  Hints(Flags(0x0))",
+                "| Hints(Flags(0x0))",
+                "  Hints(Flags(0x0))"
             ]
         );
 
         b.hints().add_flags(Flags::ANCESTORS);
         assert_eq!(
             hints_ops(&a, &b),
             [
-                "- Hints((empty))",
-                "  Hints((empty))",
-                "& Hints(ANCESTORS)",
-                "  Hints(ANCESTORS)",
-                "| Hints(ANCESTORS)",
-                "  Hints(ANCESTORS)"
+                "- Hints(Flags(0x0))",
+                "  Hints(Flags(0x0))",
+                "& Hints(Flags(ANCESTORS))",
+                "  Hints(Flags(ANCESTORS))",
+                "| Hints(Flags(ANCESTORS))",
+                "  Hints(Flags(ANCESTORS))"
             ]
         );
     }

diff --git a/eden/scm/lib/dag/src/segment.rs b/eden/scm/lib/dag/src/segment.rs
@@ -71,6 +71,7 @@ pub struct IdSegment {
 // only 1 byte overhead.
 
 bitflags! {
+    #[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
     pub struct SegmentFlags: u8 {
         /// This segment has roots (i.e. there is at least one id in
         /// `low..=high`, `parents(id)` is empty).
@@ -327,7 +328,7 @@ mod tests {
         );
         assert_eq!(
             describe_segment_bytes(&seg.0),
-            r#"# 02: Flags = ONLY_HEAD
+            r#"# 02: Flags = SegmentFlags(ONLY_HEAD)
 # 03: Level = 3
 # 00 00 00 00 00 00 00 ca: High = 202
 # 65: Delta = 101 (Low = 101)

diff --git a/eden/scm/lib/mutationstore/Cargo.toml b/eden/scm/lib/mutationstore/Cargo.toml
@@ -7,7 +7,7 @@ edition = "2021"
 
 [dependencies]
 anyhow = "1.0.75"
-bitflags = "1.3"
+bitflags = "2.4"
 dag = { version = "0.1.0", path = "../dag", features = ["render"] }
 futures = { version = "0.3.28", features = ["async-await", "compat"] }
 indexedlog = { version = "0.1.0", path = "../indexedlog" }

diff --git a/eden/scm/lib/mutationstore/src/lib.rs b/eden/scm/lib/mutationstore/src/lib.rs
@@ -62,6 +62,7 @@ pub struct MutationStore {
 }
 
 bitflags! {
+    #[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
     pub struct DagFlags: u8 {
         /// Include successors.
         const SUCCESSORS = 0b1;

diff --git a/eden/scm/lib/pathmatcher/Cargo.toml b/eden/scm/lib/pathmatcher/Cargo.toml
@@ -7,7 +7,7 @@ edition = "2021"
 
 [dependencies]
 anyhow = "1.0.75"
-bitflags = "1.3"
+bitflags = "2.4"
 fancy-regex = "0.10.0"
 fs-err = { version = "2.6.0", features = ["tokio"] }
 glob = "0.3"

diff --git a/eden/scm/lib/pathmatcher/src/tree_matcher.rs b/eden/scm/lib/pathmatcher/src/tree_matcher.rs
@@ -23,6 +23,7 @@ use crate::DirectoryMatch;
 use crate::Matcher;
 
 bitflags! {
+    #[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
     struct RuleFlags: u8 {
         // A negative rule.
         const NEGATIVE = 1;

diff --git a/eden/scm/lib/renderdag/Cargo.toml b/eden/scm/lib/renderdag/Cargo.toml
@@ -7,7 +7,7 @@ edition = "2021"
 license = "MIT"
 
 [dependencies]
-bitflags = "1.3"
+bitflags = "2.4"
 itertools = "0.11.0"
 serde = { version = "1.0.185", features = ["derive", "rc"], optional = true }
 

diff --git a/eden/scm/lib/renderdag/src/render.rs b/eden/scm/lib/renderdag/src/render.rs
@@ -210,8 +210,8 @@ pub enum PadLine {
 
 bitflags! {
     /// A column in a linking row.
-    #[derive(Default)]
     #[cfg_attr(feature = "serialize", derive(Serialize))]
+    #[derive(Default, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
     pub struct LinkLine: u16 {
         /// This cell contains a horizontal line that connects to a parent.
         const HORIZ_PARENT = 0b0_0000_0000_0001;
@@ -258,15 +258,15 @@ bitflags! {
         /// line, and other nodes that are also connected vertically.
         const CHILD = 0b1_0000_0000_0000;
 
-        const HORIZONTAL = Self::HORIZ_PARENT.bits | Self::HORIZ_ANCESTOR.bits;
-        const VERTICAL = Self::VERT_PARENT.bits | Self::VERT_ANCESTOR.bits;
-        const LEFT_FORK = Self::LEFT_FORK_PARENT.bits | Self::LEFT_FORK_ANCESTOR.bits;
-        const RIGHT_FORK = Self::RIGHT_FORK_PARENT.bits | Self::RIGHT_FORK_ANCESTOR.bits;
-        const LEFT_MERGE = Self::LEFT_MERGE_PARENT.bits | Self::LEFT_MERGE_ANCESTOR.bits;
-        const RIGHT_MERGE = Self::RIGHT_MERGE_PARENT.bits | Self::RIGHT_MERGE_ANCESTOR.bits;
-        const ANY_MERGE = Self::LEFT_MERGE.bits | Self::RIGHT_MERGE.bits;
-        const ANY_FORK = Self::LEFT_FORK.bits | Self::RIGHT_FORK.bits;
-        const ANY_FORK_OR_MERGE = Self::ANY_MERGE.bits | Self::ANY_FORK.bits;
+        const HORIZONTAL = Self::HORIZ_PARENT.bits() | Self::HORIZ_ANCESTOR.bits();
+        const VERTICAL = Self::VERT_PARENT.bits() | Self::VERT_ANCESTOR.bits();
+        const LEFT_FORK = Self::LEFT_FORK_PARENT.bits() | Self::LEFT_FORK_ANCESTOR.bits();
+        const RIGHT_FORK = Self::RIGHT_FORK_PARENT.bits() | Self::RIGHT_FORK_ANCESTOR.bits();
+        const LEFT_MERGE = Self::LEFT_MERGE_PARENT.bits() | Self::LEFT_MERGE_ANCESTOR.bits();
+        const RIGHT_MERGE = Self::RIGHT_MERGE_PARENT.bits() | Self::RIGHT_MERGE_ANCESTOR.bits();
+        const ANY_MERGE = Self::LEFT_MERGE.bits() | Self::RIGHT_MERGE.bits();
+        const ANY_FORK = Self::LEFT_FORK.bits() | Self::RIGHT_FORK.bits();
+        const ANY_FORK_OR_MERGE = Self::ANY_MERGE.bits() | Self::ANY_FORK.bits();
     }
 }
 

diff --git a/eden/scm/lib/treestate/Cargo.toml b/eden/scm/lib/treestate/Cargo.toml
@@ -9,7 +9,7 @@ edition = "2021"
 [dependencies]
 anyhow = "1.0.75"
 atomicfile = { version = "0.1.0", path = "../atomicfile" }
-bitflags = "1.3"
+bitflags = "2.4"
 byteorder = "1.3"
 fs-err = { version = "2.6.0", features = ["tokio"] }
 fs2 = "0.4"

diff --git a/eden/scm/lib/treestate/src/filestate.rs b/eden/scm/lib/treestate/src/filestate.rs
@@ -53,6 +53,7 @@ bitflags! {
     /// | untracked | no       | no       | no         | no      |
     /// | ignored   | no       | no       | no         | yes     |
     #[cfg_attr(test, derive(Default))]
+    #[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
     pub struct StateFlags: u16 {
         /// Exist in the first working parent.
         const EXIST_P1 = 1;
@@ -84,7 +85,7 @@ bitflags! {
 
 impl StateFlags {
     pub fn to_bits(self) -> u16 {
-        self.bits
+        self.bits()
     }
 }
 

diff --git a/eden/scm/lib/workingcopy/Cargo.toml b/eden/scm/lib/workingcopy/Cargo.toml
@@ -8,7 +8,7 @@ edition = "2021"
 [dependencies]
 anyhow = "1.0.75"
 async-runtime = { version = "0.1.0", path = "../async-runtime" }
-bitflags = "1.3"
+bitflags = "2.4"
 configloader = { version = "0.1.0", path = "../config/loader" }
 configmodel = { version = "0.1.0", path = "../config/model" }
 crossbeam = "0.8"

diff --git a/eden/scm/lib/workingcopy/src/metadata.rs b/eden/scm/lib/workingcopy/src/metadata.rs
@@ -27,6 +27,7 @@ pub(crate) struct File {
 }
 
 bitflags! {
+    #[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
     pub(crate) struct MetadataFlags: u8 {
         const IS_SYMLINK = 1 << 0;
         const IS_EXEC = 1 << 1;