Skip to content

Commit

Permalink
Auto merge of rust-lang#127907 - RalfJung:byte_slice_in_packed_struct…
Browse files Browse the repository at this point in the history 
…_with_derive, r=<try>

built-in derive: remove BYTE_SLICE_IN_PACKED_STRUCT_WITH_DERIVE hack and lint

Fixes rust-lang#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 rust-lang#104429, and unicode-org/icu4x#2834 has propagated far enough to let us make this a hard error.

TODO: actually remove the lint.

Cc `@nnethercote` `@Manishearth`
  • Loading branch information
@bors
bors committed Jul 18, 2024
2 parents 52f3c71 + 8c12edc commit 6223d84
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 104 deletions.
54 changes: 6 additions & 48 deletions compiler/rustc_builtin_macros/src/deriving/generic/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<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)]),
);
}
// 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)
})
Expand Down
11 changes: 4 additions & 7 deletions tests/ui/derives/deriving-with-repr-packed.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {}
74 changes: 25 additions & 49 deletions tests/ui/derives/deriving-with-repr-packed.stderr
View file
Original file line number Diff line number Diff line change
@@ -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
|
Expand All @@ -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 <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)
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 <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)
= 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`.

0 comments on commit 6223d84

Please sign in to comment.