Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

built-in derive: remove BYTE_SLICE_IN_PACKED_STRUCT_WITH_DERIVE hack and lint #127907

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
7 changes: 6 additions & 1 deletion compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
<https://github.com/rust-lang/rust/issues/93367> for more information",
);
store.register_removed(
Expand All @@ -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 \
<https://github.com/rust-lang/rust/issues/107457> for more information",
)
}

fn register_internals(store: &mut LintStore) {
Expand Down
34 changes: 0 additions & 34 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 <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.
///
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
Loading