Skip to content

Commit

Permalink
built-in derive: remove BYTE_SLICE_IN_PACKED_STRUCT_WITH_DERIVE hack …
Browse files Browse the repository at this point in the history
…and lint
  • Loading branch information
RalfJung committed Jul 18, 2024
1 parent 52f3c71 commit ea6e225
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 197 deletions.
54 changes: 6 additions & 48 deletions compiler/rustc_builtin_macros/src/deriving/generic/mod.rs
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
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
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`.
10 changes: 0 additions & 10 deletions tests/ui/deriving/deriving-all-codegen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
63 changes: 0 additions & 63 deletions tests/ui/deriving/deriving-all-codegen.stderr

This file was deleted.

20 changes: 0 additions & 20 deletions tests/ui/deriving/deriving-all-codegen.stdout
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down

0 comments on commit ea6e225

Please sign in to comment.