From bda31d14f47e49b0aa69173bf8793a9a6e0bead7 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 18 Jul 2024 12:26:19 +0200 Subject: [PATCH] built-in derive: remove BYTE_SLICE_IN_PACKED_STRUCT_WITH_DERIVE hack and lint --- .../src/deriving/generic/mod.rs | 54 ++------------ compiler/rustc_lint/src/lib.rs | 7 +- compiler/rustc_lint_defs/src/builtin.rs | 34 --------- tests/ui/derives/deriving-with-repr-packed.rs | 11 +-- .../derives/deriving-with-repr-packed.stderr | 74 +++++++------------ tests/ui/deriving/deriving-all-codegen.rs | 10 --- tests/ui/deriving/deriving-all-codegen.stderr | 63 ---------------- tests/ui/deriving/deriving-all-codegen.stdout | 20 ----- 8 files changed, 41 insertions(+), 232 deletions(-) delete mode 100644 tests/ui/deriving/deriving-all-codegen.stderr diff --git a/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs b/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs index ba289f9552e22..fc1cf71a130a2 100644 --- a/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs +++ b/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs @@ -181,11 +181,10 @@ use crate::{deriving, errors}; 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 std::cell::RefCell; @@ -1599,52 +1598,11 @@ impl<'a> TraitDef<'a> { ), ); if is_packed { - // In general, fields in packed structs are copied via a - // block, e.g. `&{self.0}`. The two exceptions are `[u8]` - // 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, 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)]), - ); - } + // Fields in packed structs are wrapped in a block, e.g. `&{self.0}`, + // causing a copy instead of a (potentially misaligned) reference. + field_expr = cx.expr_block( + cx.block(struct_field.span, thin_vec![cx.stmt_expr(field_expr)]), + ); } cx.expr_addr_of(sp, field_expr) }) diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index 4e83ef0c6291b..7f4904bb48a41 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -537,7 +537,7 @@ fn register_builtins(store: &mut LintStore) { ); store.register_removed( "suspicious_auto_trait_impls", - "no longer needed, see #93367 \ + "no longer needed, see issue #93367 \ for more information", ); store.register_removed( @@ -559,6 +559,11 @@ fn register_builtins(store: &mut LintStore) { "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 \ + for more information", + ) } fn register_internals(store: &mut LintStore) { diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index aa7844f40121b..f37d3d402df02 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -25,7 +25,6 @@ declare_lint_pass! { 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, @@ -4246,39 +4245,6 @@ declare_lint! { 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 ", - }; - report_in_external_macro -} - declare_lint! { /// The `invalid_macro_export_arguments` lint detects cases where `#[macro_export]` is being used with invalid arguments. /// diff --git a/tests/ui/derives/deriving-with-repr-packed.rs b/tests/ui/derives/deriving-with-repr-packed.rs index 58be451972017..d17b52842cefd 100644 --- a/tests/ui/derives/deriving-with-repr-packed.rs +++ b/tests/ui/derives/deriving-with-repr-packed.rs @@ -22,25 +22,22 @@ struct Y(usize); 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 - //~^^ this was previously accepted + //~^ ERROR cannot move + //~| 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 - //~^^ this was previously accepted + //~^ ERROR cannot move + //~| ERROR cannot move } fn main() {} diff --git a/tests/ui/derives/deriving-with-repr-packed.stderr b/tests/ui/derives/deriving-with-repr-packed.stderr index a8523d25cab9b..321ffb27eeb11 100644 --- a/tests/ui/derives/deriving-with-repr-packed.stderr +++ b/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 - = 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 - = 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`. -Future incompatibility report: Future breakage diagnostic: -warning: byte slice in a packed struct that derives a built-in trait - --> $DIR/deriving-with-repr-packed.rs:31:5 +error[E0507]: cannot move out of `self.data` which is behind a shared reference + --> $DIR/deriving-with-repr-packed.rs:29: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 + | + = 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[E0161]: cannot move a value of type `str` + --> $DIR/deriving-with-repr-packed.rs:38:5 | - = 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 - = 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) +LL | data: str, + | ^^^^^^^^^ the size of `str` cannot be statically determined -Future breakage diagnostic: -warning: string slice in a packed struct that derives a built-in trait - --> $DIR/deriving-with-repr-packed.rs:41:5 +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: for more information, see issue #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) + = 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 5 previous errors +Some errors have detailed explanations: E0161, E0507. +For more information about an error, try `rustc --explain E0161`. diff --git a/tests/ui/deriving/deriving-all-codegen.rs b/tests/ui/deriving/deriving-all-codegen.rs index 6fa4f74f2a508..eab2b4f1f5335 100644 --- a/tests/ui/deriving/deriving-all-codegen.rs +++ b/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; } diff --git a/tests/ui/deriving/deriving-all-codegen.stderr b/tests/ui/deriving/deriving-all-codegen.stderr deleted file mode 100644 index 503f0cae73b69..0000000000000 --- a/tests/ui/deriving/deriving-all-codegen.stderr +++ /dev/null @@ -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 - = 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 - = 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 - = 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 - = 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) - diff --git a/tests/ui/deriving/deriving-all-codegen.stdout b/tests/ui/deriving/deriving-all-codegen.stdout index 6b69b57c51619..6503c87099040 100644 --- a/tests/ui/deriving/deriving-all-codegen.stdout +++ b/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; }