From a70d03b62454f57672e14b48715ae10c01e5fd34 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 6 Feb 2023 12:58:39 +1100 Subject: [PATCH] Extend `BYTE_SLICE_IN_PACKED_STRUCT_WITH_DERIVE`. To temporarily allow a `str` field in a packed struct using `derive`, along with `[u8]`. --- .../src/deriving/generic/mod.rs | 55 ++++++++++++------- compiler/rustc_lint_defs/src/builtin.rs | 9 +-- tests/ui/derives/deriving-with-repr-packed.rs | 10 ++++ .../derives/deriving-with-repr-packed.stderr | 32 ++++++++++- 4 files changed, 81 insertions(+), 25 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs b/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs index a8c0aeb7870..970b9115d8d 100644 --- a/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs +++ b/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs @@ -1557,31 +1557,46 @@ fn create_struct_field_access_fields( }), ), ); - // In general, fields in packed structs are copied via a - // block, e.g. `&{self.0}`. The one exception is `[u8]` - // fields, which cannot be copied and also never cause - // unaligned references. This exception is 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_u8_slice = if let TyKind::Slice(ty) = &struct_field.ty.kind && - let TyKind::Path(None, rustc_ast::Path { segments, .. }) = &ty.kind && - let [seg] = segments.as_slice() && - seg.ident.name == sym::u8 && seg.args.is_none() - { - true - } else { - false - }; if is_packed { - if is_u8_slice { + // 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.parse_sess.buffer_lint_with_diagnostic( BYTE_SLICE_IN_PACKED_STRUCT_WITH_DERIVE, sp, ast::CRATE_NODE_ID, - "byte slice in a packed struct that derives a built-in trait", + &format!( + "{} slice in a packed struct that derives a built-in trait", + ty + ), rustc_lint_defs::BuiltinLintDiagnostics::ByteSliceInPackedStructWithDerive ); } else { diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index 7e9ba4cd22b..9d8ad9d9ed9 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -4073,7 +4073,8 @@ declare_lint! { /// The `byte_slice_in_packed_struct_with_derive` lint detects cases where a byte slice field - /// (`[u8]`) is used in a `packed` struct that derives one or more built-in traits. + /// (`[u8]`) or string slice field (`str`) is used in a `packed` struct that derives one or + /// more built-in traits. /// /// ### Example /// @@ -4091,11 +4092,11 @@ /// ### 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 are a temporary - /// exception because certain crates depended on them. + /// 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]` slice used in a packed struct with `derive`", + "`[u8]` or `str` used in a packed struct with `derive`", @future_incompatible = FutureIncompatibleInfo { reference: "issue #107457 ", reason: FutureIncompatibilityReason::FutureReleaseErrorReportNow, diff --git a/tests/ui/derives/deriving-with-repr-packed.rs b/tests/ui/derives/deriving-with-repr-packed.rs index afa91da133d..58be4519720 100644 --- a/tests/ui/derives/deriving-with-repr-packed.rs +++ b/tests/ui/derives/deriving-with-repr-packed.rs @@ -33,4 +33,14 @@ struct FlexZeroSlice { //~^^ this was previously accepted } +// 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 +} + fn main() {} diff --git a/tests/ui/derives/deriving-with-repr-packed.stderr b/tests/ui/derives/deriving-with-repr-packed.stderr index 7ed84af91bd..0cfe03869af 100644 --- a/tests/ui/derives/deriving-with-repr-packed.stderr +++ b/tests/ui/derives/deriving-with-repr-packed.stderr @@ -13,6 +13,20 @@ LL | data: [u8], = 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 | @@ -24,7 +38,7 @@ LL | struct X(Y); | = note: this error originates in the derive macro `Debug` (in Nightly builds, run with -Z macro-backtrace for more info) -error: aborting due to previous error; 1 warning emitted +error: aborting due to previous error; 2 warnings emitted For more information about this error, try `rustc --explain E0507`. Future incompatibility report: Future breakage diagnostic: @@ -43,3 +57,19 @@ LL | data: [u8], = 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: 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: `#[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) + -- 2.44.0