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 · 85674a8 · 85674a8
1 parent ac2d309
commit 85674a8
Show file tree

Hide file tree

Showing 34 changed files with 89 additions and 53 deletions.
diff --git a/hphp/hack/src/Cargo.lock b/hphp/hack/src/Cargo.lock
diff --git a/hphp/hack/src/elab/Cargo.toml b/hphp/hack/src/elab/Cargo.toml
@@ -11,7 +11,7 @@ license = "MIT"
 path = "elab.rs"
 
 [dependencies]
-bitflags = "1.3"
+bitflags = "2.4"
 bstr = { version = "1.4.0", features = ["serde", "std", "unicode"] }
 core_utils_rust = { version = "0.0.0", path = "../utils/core" }
 elaborate_namespaces_visitor = { version = "0.0.0", path = "../naming/cargo/elaborate_namespaces" }

diff --git a/hphp/hack/src/elab/env.rs b/hphp/hack/src/elab/env.rs
@@ -16,6 +16,7 @@ pub struct ProgramSpecificOptions {
 }
 
 bitflags! {
+    #[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
     struct Flags: u16 {
         const SOFT_AS_LIKE = 1 << 0;
         const HKT_ENABLED = 1 << 1;

diff --git a/hphp/hack/src/elab/passes/elab_const_expr.rs b/hphp/hack/src/elab/passes/elab_const_expr.rs
@@ -33,7 +33,7 @@ pub struct ElabConstExprPass {
 }
 
 bitflags! {
-    #[derive(Default)]
+    #[derive(Default, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
     struct Flags: u8 {
         const IN_ENUM_CLASS = 1 << 0;
         const ENFORCE_CONST_EXPR = 1 << 1;

diff --git a/hphp/hack/src/elab/passes/elab_hint_haccess.rs b/hphp/hack/src/elab/passes/elab_hint_haccess.rs
@@ -20,7 +20,7 @@ pub struct ElabHintHaccessPass {
 }
 
 bitflags! {
-    #[derive(Default)]
+    #[derive(Default, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
     struct Flags: u8 {
         const IN_CONTEXT = 1 << 0;
         const IN_HACCESS = 1 << 1;

diff --git a/hphp/hack/src/elab/passes/elab_hint_this.rs b/hphp/hack/src/elab/passes/elab_hint_this.rs
@@ -38,7 +38,7 @@ enum Context {
 }
 
 bitflags! {
-    #[derive(Default)]
+    #[derive(Default, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
     struct Flags: u8 {
         const FORBID_THIS= 1 << 0;
         const IS_TOP_LEVEL_HACCESS_ROOT= 1 << 1;

diff --git a/hphp/hack/src/elab/passes/validate_coroutine.rs b/hphp/hack/src/elab/passes/validate_coroutine.rs
@@ -24,7 +24,7 @@ pub struct ValidateCoroutinePass {
     flags: Flags,
 }
 bitflags! {
-    #[derive(Default)]
+    #[derive(Default, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
     struct Flags: u8 {
         const IS_SYNC = 1 << 0;
         const IS_GENERATOR = 1 << 1;

diff --git a/hphp/hack/src/elab/passes/validate_hint_habstr.rs b/hphp/hack/src/elab/passes/validate_hint_habstr.rs
@@ -30,7 +30,7 @@ pub struct ValidateHintHabstrPass {
 }
 
 bitflags! {
-    #[derive(Default)]
+    #[derive(Default, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
     struct Flags: u8 {
         const IN_METHOD_OR_FUN = 1 << 0;
         const IN_WHERE_CONSTRAINT = 1 << 1;

diff --git a/hphp/hack/src/hackc/emitter/cargo/emit_unit/Cargo.toml b/hphp/hack/src/hackc/emitter/cargo/emit_unit/Cargo.toml
@@ -12,7 +12,7 @@ path = "../../emit_unit.rs"
 
 [dependencies]
 ast_scope = { version = "0.0.0", path = "../ast_scope" }
-bitflags = "1.3"
+bitflags = "2.4"
 bstr = { version = "1.4.0", features = ["serde", "std", "unicode"] }
 bumpalo = { version = "3.14.0", features = ["allocator_api", "collections"] }
 constant_folder = { version = "0.0.0", path = "../constant_folder" }

diff --git a/hphp/hack/src/hackc/emitter/cargo/env/Cargo.toml b/hphp/hack/src/hackc/emitter/cargo/env/Cargo.toml
@@ -12,7 +12,7 @@ path = "../../env.rs"
 
 [dependencies]
 ast_scope = { version = "0.0.0", path = "../ast_scope" }
-bitflags = "1.3"
+bitflags = "2.4"
 bumpalo = { version = "3.14.0", features = ["allocator_api", "collections"] }
 decl_provider = { version = "0.0.0", path = "../../../decl_provider" }
 ffi = { version = "0.0.0", path = "../../../../utils/ffi" }

diff --git a/hphp/hack/src/hackc/emitter/emit_body.rs b/hphp/hack/src/hackc/emitter/emit_body.rs
@@ -66,6 +66,7 @@ pub struct Args<'a, 'arena> {
 }
 
 bitflags! {
+    #[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
     pub struct Flags: u8 {
         const HAS_COEFFECTS_LOCAL = 1 << 0;
         const SKIP_AWAITABLE = 1 << 1;

diff --git a/hphp/hack/src/hackc/emitter/emit_memoize_method.rs b/hphp/hack/src/hackc/emitter/emit_memoize_method.rs
@@ -577,6 +577,7 @@ struct Args<'r, 'ast, 'arena> {
 }
 
 bitflags! {
+    #[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
     pub struct Flags: u8 {
         const IS_STATIC = 1 << 1;
         const IS_REIFIED = 1 << 2;

diff --git a/hphp/hack/src/hackc/emitter/env.rs b/hphp/hack/src/hackc/emitter/env.rs
@@ -28,7 +28,7 @@ use oxidized::ast;
 use oxidized::namespace_env::Env as NamespaceEnv;
 
 bitflags! {
-    #[derive(Default)]
+    #[derive(Default, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
     pub struct Flags: u8 {
         const NEEDS_LOCAL_THIS =    0b0000_0001;
         const IN_TRY =              0b0000_0010;

diff --git a/hphp/hack/src/hackc/emitter/try_finally_rewriter.rs b/hphp/hack/src/hackc/emitter/try_finally_rewriter.rs
@@ -201,6 +201,7 @@ pub(super) fn emit_return<'a, 'arena, 'decl>(
 }
 
 bitflags! {
+    #[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
     pub(super) struct EmitBreakOrContinueFlags: u8 {
         const IS_BREAK =            0b01;
         const IN_FINALLY_EPILOGUE = 0b10;

diff --git a/hphp/hack/src/hackc/hhbc/cargo/hhbc/Cargo.toml b/hphp/hack/src/hackc/hhbc/cargo/hhbc/Cargo.toml
@@ -11,7 +11,7 @@ license = "MIT"
 path = "../../lib.rs"
 
 [dependencies]
-bitflags = "1.3"
+bitflags = "2.4"
 bstr = { version = "1.4.0", features = ["serde", "std", "unicode"] }
 bumpalo = { version = "3.14.0", features = ["allocator_api", "collections"] }
 emit_opcodes_macro = { version = "0.0.0", path = "../emit_opcodes_macro" }

diff --git a/hphp/hack/src/hackc/hhbc/function.rs b/hphp/hack/src/hackc/hhbc/function.rs
@@ -29,7 +29,7 @@ pub struct Function<'arena> {
 }
 
 bitflags! {
-    #[derive(Default, Serialize)]
+    #[derive(Default, Serialize, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
     #[repr(C)]
     pub struct FunctionFlags: u8 {
         const ASYNC =          1 << 0;

diff --git a/hphp/hack/src/hackc/hhbc/method.rs b/hphp/hack/src/hackc/hhbc/method.rs
@@ -29,7 +29,7 @@ pub struct Method<'arena> {
 }
 
 bitflags! {
-    #[derive(Default, Serialize)]
+    #[derive(Default, Serialize, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
     #[repr(C)]
     pub struct MethodFlags: u16 {
         const IS_ASYNC = 1 << 0;

diff --git a/hphp/hack/src/oxidized/Cargo.toml b/hphp/hack/src/oxidized/Cargo.toml
@@ -14,7 +14,7 @@ path = "lib.rs"
 anyhow = "1.0.75"
 arena_deserializer = { version = "0.0.0", path = "../utils/arena_deserializer" }
 arena_trait = { version = "0.0.0", path = "../arena_trait" }
-bitflags = "1.3"
+bitflags = "2.4"
 bstr = { version = "1.4.0", features = ["serde", "std", "unicode"] }
 bumpalo = { version = "3.14.0", features = ["allocator_api", "collections"] }
 eq_modulo_pos = { version = "0.0.0", path = "../utils/eq_modulo_pos" }

diff --git a/hphp/hack/src/oxidized/manual/method_flags.rs b/hphp/hack/src/oxidized/manual/method_flags.rs
@@ -8,9 +8,10 @@ use eq_modulo_pos::EqModuloPos;
 
 // NB: Keep the values of these flags in sync with shallow_decl_defs.ml.
 
+#[derive(EqModuloPos, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
+pub struct MethodFlags(u8);
 bitflags! {
-    #[derive(EqModuloPos)]
-    pub struct MethodFlags: u8 {
+    impl MethodFlags: u8 {
         const ABSTRACT            = 1 << 0;
         const FINAL               = 1 << 1;
         const OVERRIDE            = 1 << 2;

diff --git a/hphp/hack/src/oxidized/manual/prop_flags.rs b/hphp/hack/src/oxidized/manual/prop_flags.rs
@@ -8,9 +8,10 @@ use eq_modulo_pos::EqModuloPos;
 
 // NB: Keep the values of these flags in sync with shallow_decl_defs.ml.
 
+#[derive(EqModuloPos, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
+pub struct PropFlags(u16);
 bitflags! {
-    #[derive(EqModuloPos)]
-    pub struct PropFlags: u16 {
+    impl PropFlags: u16 {
         const ABSTRACT    = 1 << 0;
         const CONST       = 1 << 1;
         const LATEINIT    = 1 << 2;

diff --git a/hphp/hack/src/oxidized/manual/typing_defs_flags.rs b/hphp/hack/src/oxidized/manual/typing_defs_flags.rs
@@ -11,9 +11,10 @@ use crate::xhp_attribute::XhpAttribute;
 
 // NB: Keep the values of these flags in sync with typing_defs_flags.ml.
 
+#[derive(EqModuloPos, Eq, PartialEq, Ord, PartialOrd, Hash, Debug, Clone, Copy)]
+pub struct FunTypeFlags(u16);
 bitflags! {
-    #[derive(EqModuloPos)]
-    pub struct FunTypeFlags: u16 {
+    impl FunTypeFlags: u16 {
         const RETURN_DISPOSABLE      = 1 << 0;
         const IS_COROUTINE           = 1 << 3;
         const ASYNC                  = 1 << 4;
@@ -28,19 +29,21 @@ bitflags! {
     }
 }
 
+#[derive(EqModuloPos, Eq, PartialEq, Ord, PartialOrd, Hash, Debug, Clone, Copy)]
+pub struct FunParamFlags(u16);
 bitflags! {
-    #[derive(EqModuloPos)]
-    pub struct FunParamFlags: u16 {
+    impl FunParamFlags: u16 {
         const ACCEPT_DISPOSABLE      = 1 << 0;
         const INOUT                  = 1 << 1;
         const HAS_DEFAULT            = 1 << 2;
         const READONLY       = 1 << 8;
     }
 }
 
+#[derive(EqModuloPos, Eq, PartialEq, Ord, PartialOrd, Hash, Debug, Clone, Copy)]
+pub struct ClassEltFlags(u16);
 bitflags! {
-    #[derive(EqModuloPos)]
-    pub struct ClassEltFlags: u16 {
+    impl ClassEltFlags: u16 {
         const ABSTRACT                 = 1 << 0;
         const FINAL                    = 1 << 1;
         const SUPERFLUOUS_OVERRIDE     = 1 << 2;
@@ -61,7 +64,7 @@ bitflags! {
         const NEEDS_INIT               = 1 << 13;
         const SAFE_GLOBAL_VARIABLE     = 1 << 14;
 
-        const XA_FLAGS_MASK = Self::XA_HAS_DEFAULT.bits | Self::XA_TAG_REQUIRED.bits | Self::XA_TAG_LATEINIT.bits;
+        const XA_FLAGS_MASK = Self::XA_HAS_DEFAULT.bits() | Self::XA_TAG_REQUIRED.bits() | Self::XA_TAG_LATEINIT.bits();
     }
 }
 

diff --git a/hphp/hack/src/oxidized_by_ref/Cargo.toml b/hphp/hack/src/oxidized_by_ref/Cargo.toml
@@ -14,7 +14,7 @@ path = "lib.rs"
 arena_collections = { version = "0.0.0", path = "../arena_collections" }
 arena_deserializer = { version = "0.0.0", path = "../utils/arena_deserializer" }
 arena_trait = { version = "0.0.0", path = "../arena_trait" }
-bitflags = "1.3"
+bitflags = "2.4"
 bstr = { version = "1.4.0", features = ["serde", "std", "unicode"] }
 bumpalo = { version = "3.14.0", features = ["allocator_api", "collections"] }
 eq_modulo_pos = { version = "0.0.0", path = "../utils/eq_modulo_pos" }

diff --git a/hphp/hack/src/oxidized_by_ref/manual/method_flags.rs b/hphp/hack/src/oxidized_by_ref/manual/method_flags.rs
@@ -8,9 +8,10 @@ use eq_modulo_pos::EqModuloPos;
 
 // NB: Keep the values of these flags in sync with shallow_decl_defs.ml.
 
+#[derive(EqModuloPos, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
+pub struct MethodFlags(u8);
 bitflags! {
-    #[derive(EqModuloPos)]
-    pub struct MethodFlags: u8 {
+    impl MethodFlags: u8 {
         const ABSTRACT            = 1 << 0;
         const FINAL               = 1 << 1;
         const OVERRIDE            = 1 << 2;

diff --git a/hphp/hack/src/oxidized_by_ref/manual/prop_flags.rs b/hphp/hack/src/oxidized_by_ref/manual/prop_flags.rs
@@ -8,9 +8,10 @@ use eq_modulo_pos::EqModuloPos;
 
 // NB: Keep the values of these flags in sync with shallow_decl_defs.ml.
 
+#[derive(EqModuloPos, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
+pub struct PropFlags(u16);
 bitflags! {
-    #[derive(EqModuloPos)]
-    pub struct PropFlags: u16 {
+    impl PropFlags: u16 {
         const ABSTRACT    = 1 << 0;
         const CONST       = 1 << 1;
         const LATEINIT    = 1 << 2;

diff --git a/hphp/hack/src/parser/cargo/aast_parser/Cargo.toml b/hphp/hack/src/parser/cargo/aast_parser/Cargo.toml
@@ -11,7 +11,7 @@ license = "MIT"
 path = "../../aast_parser_lib.rs"
 
 [dependencies]
-bitflags = "1.3"
+bitflags = "2.4"
 bumpalo = { version = "3.14.0", features = ["allocator_api", "collections"] }
 core_utils_rust = { version = "0.0.0", path = "../../../utils/core" }
 decl_mode_parser = { version = "0.0.0", path = "../../api/cargo/decl_mode_parser" }

diff --git a/hphp/hack/src/parser/cargo/core_types/Cargo.toml b/hphp/hack/src/parser/cargo/core_types/Cargo.toml
@@ -11,7 +11,7 @@ license = "MIT"
 path = "../../parser_core_types_lib.rs"
 
 [dependencies]
-bitflags = "1.3"
+bitflags = "2.4"
 bumpalo = { version = "3.14.0", features = ["allocator_api", "collections"] }
 itertools = "0.11.0"
 line_break_map = { version = "0.0.0", path = "../../../utils/line_break_map" }

diff --git a/hphp/hack/src/parser/coeffects_check.rs b/hphp/hack/src/parser/coeffects_check.rs
@@ -162,6 +162,7 @@ fn has_ignore_coeffect_local_errors_attr(attrs: &[aast::UserAttribute<(), ()>])
 }
 
 bitflags! {
+    #[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
     pub struct ContextFlags: u16 {
         /// Hack, very roughly, has three contexts in which you can see an
         /// expression:

diff --git a/hphp/hack/src/parser/compact_trivia.rs b/hphp/hack/src/parser/compact_trivia.rs
@@ -12,6 +12,7 @@ use crate::trivia_factory::SimpleTriviaFactory;
 use crate::trivia_kind::TriviaKind;
 
 bitflags! {
+    #[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
     pub struct TriviaKinds : u8 {
         const WHITE_SPACE        = 1 << TriviaKind::WhiteSpace as u8;
         const END_OF_LINE        = 1 << TriviaKind::EndOfLine as u8;

diff --git a/hphp/hack/test/hackrs/folded_decls/deferred_init1.php.folded_decls.exp b/hphp/hack/test/hackrs/folded_decls/deferred_init1.php.folded_decls.exp
@@ -5,7 +5,9 @@ FoldedClass {
     ),
     props: {
         "t": FoldedElement {
-            flags: FINAL | NEEDS_INIT,
+            flags: ClassEltFlags(
+                8194,
+            ),
             origin: "\\A",
             visibility: Public,
             deprecated: None,