Rollup merge of #127907 - RalfJung:byte_slice_in_packed_struct_with_derive, r=nnethercote

built-in derive: remove BYTE_SLICE_IN_PACKED_STRUCT_WITH_DERIVE hack and lint

Fixes https://github.com/rust-lang/rust/issues/107457 by turning the lint into a hard error. The lint has been shown in future breakage reports since Rust 1.69 (released in April 2023).

Let's see (via crater) if enough time has passed since https://github.com/rust-lang/rust/pull/104429, and https://github.com/unicode-org/icu4x/pull/2834 has propagated far enough to let us make this a hard error.

Cc ``@nnethercote`` ``@Manishearth``

compiler/rustc_builtin_macros/src/deriving/generic/mod.rs

@@ -181,11 +181,10 @@
 use rustc_ast::ptr::P;
 use rustc_ast::{
     self as ast, BindingMode, ByRef, EnumDef, Expr, GenericArg, GenericParamKind, Generics,
-    Mutability, PatKind, TyKind, VariantData,
+    Mutability, PatKind, VariantData,
 };
 use rustc_attr as attr;
 use rustc_expand::base::{Annotatable, ExtCtxt};
-use rustc_session::lint::builtin::BYTE_SLICE_IN_PACKED_STRUCT_WITH_DERIVE;
 use rustc_span::symbol::{kw, sym, Ident, Symbol};
 use rustc_span::{Span, DUMMY_SP};
 use thin_vec::{thin_vec, ThinVec};
@@ -1599,53 +1598,12 @@ fn create_struct_field_access_fields(
                         ),
                     );
                     if is_packed {
-                        // In general, fields in packed structs are copied via a
+                        // Fields in packed structs are wrapped in a block, e.g. `&{self.0}`,
-                        // block, e.g. `&{self.0}`. The two exceptions are `[u8]`
+                        // causing a copy instead of a (potentially misaligned) reference.
-                        // and `str` fields, which cannot be copied and also never
-                        // cause unaligned references. These exceptions are allowed
-                        // to handle the `FlexZeroSlice` type in the `zerovec`
-                        // crate within `icu4x-0.9.0`.
-                        //
-                        // Once use of `icu4x-0.9.0` has dropped sufficiently, this
-                        // exception should be removed.
-                        let is_simple_path = |ty: &P<ast::Ty>, sym| {
-                            if let TyKind::Path(None, ast::Path { segments, .. }) = &ty.kind
-                                && let [seg] = segments.as_slice()
-                                && seg.ident.name == sym
-                                && seg.args.is_none()
-                            {
-                                true
-                            } else {
-                                false
-                            }
-                        };
-
-                        let exception = if let TyKind::Slice(ty) = &struct_field.ty.kind
-                            && is_simple_path(ty, sym::u8)
-                        {
-                            Some("byte")
-                        } else if is_simple_path(&struct_field.ty, sym::str) {
-                            Some("string")
-                        } else {
-                            None
-                        };
-
-                        if let Some(ty) = exception {
-                            cx.sess.psess.buffer_lint(
-                                BYTE_SLICE_IN_PACKED_STRUCT_WITH_DERIVE,
-                                sp,
-                                ast::CRATE_NODE_ID,
-                                rustc_lint_defs::BuiltinLintDiag::ByteSliceInPackedStructWithDerive {
-                                    ty: ty.to_string(),
-                                },
-                            );
-                        } else {
-                            // Wrap the expression in `{...}`, causing a copy.
                         field_expr = cx.expr_block(
                             cx.block(struct_field.span, thin_vec![cx.stmt_expr(field_expr)]),
                         );
                     }
-                    }
                     cx.expr_addr_of(sp, field_expr)
                 })
                 .collect()

compiler/rustc_lint/src/lib.rs

@@ -543,7 +543,7 @@ macro_rules! add_lint_group {
     );
     store.register_removed(
         "suspicious_auto_trait_impls",
-        "no longer needed, see #93367 \
+        "no longer needed, see issue #93367 \
          <https://github.com/rust-lang/rust/issues/93367> for more information",
     );
     store.register_removed(
@@ -565,6 +565,11 @@ macro_rules! add_lint_group {
         "box_pointers",
         "it does not detect other kinds of allocations, and existed only for historical reasons",
     );
+    store.register_removed(
+        "byte_slice_in_packed_struct_with_derive",
+        "converted into hard error, see issue #107457 \
+         <https://github.com/rust-lang/rust/issues/107457> for more information",
+    )
 }
 
 fn register_internals(store: &mut LintStore) {

compiler/rustc_lint_defs/src/builtin.rs

@@ -26,7 +26,6 @@
         BARE_TRAIT_OBJECTS,
         BINDINGS_WITH_VARIANT_NAME,
         BREAK_WITH_LABEL_AND_LOOP,
-        BYTE_SLICE_IN_PACKED_STRUCT_WITH_DERIVE,
         CENUM_IMPL_DROP_CAST,
         COHERENCE_LEAK_CHECK,
         CONFLICTING_REPR_HINTS,
@@ -4315,39 +4314,6 @@
     report_in_external_macro
 }
 
-declare_lint! {
-    /// The `byte_slice_in_packed_struct_with_derive` lint detects cases where a byte slice field
-    /// (`[u8]`) or string slice field (`str`) is used in a `packed` struct that derives one or
-    /// more built-in traits.
-    ///
-    /// ### Example
-    ///
-    /// ```rust
-    /// #[repr(packed)]
-    /// #[derive(Hash)]
-    /// struct FlexZeroSlice {
-    ///     width: u8,
-    ///     data: [u8],
-    /// }
-    /// ```
-    ///
-    /// {{produces}}
-    ///
-    /// ### Explanation
-    ///
-    /// This was previously accepted but is being phased out, because fields in packed structs are
-    /// now required to implement `Copy` for `derive` to work. Byte slices and string slices are a
-    /// temporary exception because certain crates depended on them.
-    pub BYTE_SLICE_IN_PACKED_STRUCT_WITH_DERIVE,
-    Warn,
-    "`[u8]` or `str` used in a packed struct with `derive`",
-    @future_incompatible = FutureIncompatibleInfo {
-        reason: FutureIncompatibilityReason::FutureReleaseErrorReportInDeps,
-        reference: "issue #107457 <https://github.com/rust-lang/rust/issues/107457>",
-    };
-    report_in_external_macro
-}
-
 declare_lint! {
     /// The `invalid_macro_export_arguments` lint detects cases where `#[macro_export]` is being used with invalid arguments.
     ///

tests/ui/derives/deriving-with-repr-packed.rs

@@ -22,25 +22,22 @@
 struct X(Y);
 //~^ ERROR cannot move out of `self` which is behind a shared reference
 
-// This is currently allowed, but will be phased out at some point. From
-// `zerovec` within icu4x-0.9.0.
 #[derive(Debug)]
 #[repr(packed)]
 struct FlexZeroSlice {
     width: u8,
     data: [u8],
-    //~^ WARNING byte slice in a packed struct that derives a built-in trait
+    //~^ ERROR cannot move
-    //~^^ this was previously accepted
+    //~| ERROR cannot move
 }
 
-// Again, currently allowed, but will be phased out.
 #[derive(Debug)]
 #[repr(packed)]
 struct WithStr {
     width: u8,
     data: str,
-    //~^ WARNING string slice in a packed struct that derives a built-in trait
+    //~^ ERROR cannot move
-    //~^^ this was previously accepted
+    //~| ERROR cannot move
 }
 
 fn main() {}

tests/ui/derives/deriving-with-repr-packed.stderr

@@ -1,32 +1,3 @@
-warning: byte slice in a packed struct that derives a built-in trait
-  --> $DIR/deriving-with-repr-packed.rs:31:5
-   |
-LL | #[derive(Debug)]
-   |          ----- in this derive macro expansion
-...
-LL |     data: [u8],
-   |     ^^^^^^^^^^
-   |
-   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
-   = note: for more information, see issue #107457 <https://github.com/rust-lang/rust/issues/107457>
-   = help: consider implementing the trait by hand, or remove the `packed` attribute
-   = note: `#[warn(byte_slice_in_packed_struct_with_derive)]` on by default
-   = note: this warning originates in the derive macro `Debug` (in Nightly builds, run with -Z macro-backtrace for more info)
-
-warning: string slice in a packed struct that derives a built-in trait
-  --> $DIR/deriving-with-repr-packed.rs:41:5
-   |
-LL | #[derive(Debug)]
-   |          ----- in this derive macro expansion
-...
-LL |     data: str,
-   |     ^^^^^^^^^
-   |
-   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
-   = note: for more information, see issue #107457 <https://github.com/rust-lang/rust/issues/107457>
-   = help: consider implementing the trait by hand, or remove the `packed` attribute
-   = note: this warning originates in the derive macro `Debug` (in Nightly builds, run with -Z macro-backtrace for more info)
-
 error[E0507]: cannot move out of `self` which is behind a shared reference
   --> $DIR/deriving-with-repr-packed.rs:22:10
    |
@@ -47,38 +18,43 @@ LL | struct X(Y);
    = note: `#[derive(Debug)]` triggers a move because taking references to the fields of a packed struct is undefined behaviour
    = note: this error originates in the derive macro `Debug` (in Nightly builds, run with -Z macro-backtrace for more info)
 
-error: aborting due to 1 previous error; 2 warnings emitted
+error[E0161]: cannot move a value of type `[u8]`
+  --> $DIR/deriving-with-repr-packed.rs:29:5
+   |
+LL |     data: [u8],
+   |     ^^^^^^^^^^ the size of `[u8]` cannot be statically determined
 
-For more information about this error, try `rustc --explain E0507`.
+error[E0507]: cannot move out of `self.data` which is behind a shared reference
-Future incompatibility report: Future breakage diagnostic:
+  --> $DIR/deriving-with-repr-packed.rs:29:5
-warning: byte slice in a packed struct that derives a built-in trait
-  --> $DIR/deriving-with-repr-packed.rs:31:5
    |
 LL | #[derive(Debug)]
    |          ----- in this derive macro expansion
 ...
 LL |     data: [u8],
-   |     ^^^^^^^^^^
+   |     ^^^^^^^^^^ move occurs because `self.data` has type `[u8]`, which does not implement the `Copy` trait
    |
-   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
+   = note: `#[derive(Debug)]` triggers a move because taking references to the fields of a packed struct is undefined behaviour
-   = note: for more information, see issue #107457 <https://github.com/rust-lang/rust/issues/107457>
+   = note: this error originates in the derive macro `Debug` (in Nightly builds, run with -Z macro-backtrace for more info)
-   = help: consider implementing the trait by hand, or remove the `packed` attribute
-   = note: `#[warn(byte_slice_in_packed_struct_with_derive)]` on by default
-   = note: this warning originates in the derive macro `Debug` (in Nightly builds, run with -Z macro-backtrace for more info)
 
-Future breakage diagnostic:
+error[E0161]: cannot move a value of type `str`
-warning: string slice in a packed struct that derives a built-in trait
+  --> $DIR/deriving-with-repr-packed.rs:38:5
-  --> $DIR/deriving-with-repr-packed.rs:41:5
+   |
+LL |     data: str,
+   |     ^^^^^^^^^ the size of `str` cannot be statically determined
+
+error[E0507]: cannot move out of `self.data` which is behind a shared reference
+  --> $DIR/deriving-with-repr-packed.rs:38:5
    |
 LL | #[derive(Debug)]
    |          ----- in this derive macro expansion
 ...
 LL |     data: str,
-   |     ^^^^^^^^^
+   |     ^^^^^^^^^ move occurs because `self.data` has type `str`, which does not implement the `Copy` trait
    |
-   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
+   = note: `#[derive(Debug)]` triggers a move because taking references to the fields of a packed struct is undefined behaviour
-   = note: for more information, see issue #107457 <https://github.com/rust-lang/rust/issues/107457>
+   = note: this error originates in the derive macro `Debug` (in Nightly builds, run with -Z macro-backtrace for more info)
-   = help: consider implementing the trait by hand, or remove the `packed` attribute
-   = note: `#[warn(byte_slice_in_packed_struct_with_derive)]` on by default
-   = note: this warning originates in the derive macro `Debug` (in Nightly builds, run with -Z macro-backtrace for more info)
 
+error: aborting due to 5 previous errors
+
+Some errors have detailed explanations: E0161, E0507.
+For more information about an error, try `rustc --explain E0161`.

tests/ui/deriving/deriving-all-codegen.rs

@@ -73,16 +73,6 @@ impl Copy for PackedManualCopy {}
 #[derive(Debug, Hash, PartialEq, Eq, PartialOrd, Ord)]
 struct Unsized([u32]);
 
-// A packed struct with an unsized `[u8]` field. This is currently allowed, but
-// causes a warning and will be phased out at some point.
-#[derive(Debug, Hash)]
-#[repr(packed)]
-struct PackedUnsizedU8([u8]);
-//~^ WARNING byte slice in a packed struct that derives a built-in trait
-//~^^ WARNING byte slice in a packed struct that derives a built-in trait
-//~^^^ this was previously accepted
-//~^^^^ this was previously accepted
-
 trait Trait {
     type A;
 }

tests/ui/deriving/deriving-all-codegen.stderr

@@ -1,63 +0,0 @@
-warning: byte slice in a packed struct that derives a built-in trait
-  --> $DIR/deriving-all-codegen.rs:80:24
-   |
-LL | #[derive(Debug, Hash)]
-   |          ----- in this derive macro expansion
-LL | #[repr(packed)]
-LL | struct PackedUnsizedU8([u8]);
-   |                        ^^^^
-   |
-   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
-   = note: for more information, see issue #107457 <https://github.com/rust-lang/rust/issues/107457>
-   = help: consider implementing the trait by hand, or remove the `packed` attribute
-   = note: `#[warn(byte_slice_in_packed_struct_with_derive)]` on by default
-   = note: this warning originates in the derive macro `Debug` (in Nightly builds, run with -Z macro-backtrace for more info)
-
-warning: byte slice in a packed struct that derives a built-in trait
-  --> $DIR/deriving-all-codegen.rs:80:24
-   |
-LL | #[derive(Debug, Hash)]
-   |                 ---- in this derive macro expansion
-LL | #[repr(packed)]
-LL | struct PackedUnsizedU8([u8]);
-   |                        ^^^^
-   |
-   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
-   = note: for more information, see issue #107457 <https://github.com/rust-lang/rust/issues/107457>
-   = help: consider implementing the trait by hand, or remove the `packed` attribute
-   = note: this warning originates in the derive macro `Hash` (in Nightly builds, run with -Z macro-backtrace for more info)
-
-warning: 2 warnings emitted
-
-Future incompatibility report: Future breakage diagnostic:
-warning: byte slice in a packed struct that derives a built-in trait
-  --> $DIR/deriving-all-codegen.rs:80:24
-   |
-LL | #[derive(Debug, Hash)]
-   |          ----- in this derive macro expansion
-LL | #[repr(packed)]
-LL | struct PackedUnsizedU8([u8]);
-   |                        ^^^^
-   |
-   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
-   = note: for more information, see issue #107457 <https://github.com/rust-lang/rust/issues/107457>
-   = help: consider implementing the trait by hand, or remove the `packed` attribute
-   = note: `#[warn(byte_slice_in_packed_struct_with_derive)]` on by default
-   = note: this warning originates in the derive macro `Debug` (in Nightly builds, run with -Z macro-backtrace for more info)
-
-Future breakage diagnostic:
-warning: byte slice in a packed struct that derives a built-in trait
-  --> $DIR/deriving-all-codegen.rs:80:24
-   |
-LL | #[derive(Debug, Hash)]
-   |                 ---- in this derive macro expansion
-LL | #[repr(packed)]
-LL | struct PackedUnsizedU8([u8]);
-   |                        ^^^^
-   |
-   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
-   = note: for more information, see issue #107457 <https://github.com/rust-lang/rust/issues/107457>
-   = help: consider implementing the trait by hand, or remove the `packed` attribute
-   = note: `#[warn(byte_slice_in_packed_struct_with_derive)]` on by default
-   = note: this warning originates in the derive macro `Hash` (in Nightly builds, run with -Z macro-backtrace for more info)
-

tests/ui/deriving/deriving-all-codegen.stdout

@@ -516,26 +516,6 @@ impl ::core::cmp::Ord for Unsized {
     }
 }
 
-// A packed struct with an unsized `[u8]` field. This is currently allowed, but
-// causes a warning and will be phased out at some point.
-#[repr(packed)]
-struct PackedUnsizedU8([u8]);
-#[automatically_derived]
-impl ::core::fmt::Debug for PackedUnsizedU8 {
-    #[inline]
-    fn fmt(&self, f: &mut ::core::fmt::Formatter) -> ::core::fmt::Result {
-        ::core::fmt::Formatter::debug_tuple_field1_finish(f,
-            "PackedUnsizedU8", &&self.0)
-    }
-}
-#[automatically_derived]
-impl ::core::hash::Hash for PackedUnsizedU8 {
-    #[inline]
-    fn hash<__H: ::core::hash::Hasher>(&self, state: &mut __H) -> () {
-        ::core::hash::Hash::hash(&self.0, state)
-    }
-}
-
 trait Trait {
     type A;
 }