rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
97.67k stars 12.63k forks source link

Making `unaligned_references` a hard error broke derives on packed structs without warning #110645

Closed LegionMammal978 closed 2 months ago

LegionMammal978 commented 1 year ago

I tried this code:

#[derive(Clone)]
pub struct Foo;

#[derive(Clone)]
#[repr(packed)]
pub struct Bar(Foo);

I expected to see this happen: A deny-by-default unaligned_references lint (or similar) in Rust 1.68.2, and a hard error in Rust 1.69.0.

Instead, this happened: No lint in Rust 1.68.2, and a hard error in Rust 1.69.0:

error[E0507]: cannot move out of `self` which is behind a shared reference
 --> src/lib.rs:6:16
  |
4 | #[derive(Clone)]
  |          ----- in this derive macro expansion
5 | #[repr(packed)]
6 | pub struct Bar(Foo);
  |                ^^^ move occurs because `self.0` has type `Foo`, which does not implement the `Copy` trait
  |
  = note: this error originates in the derive macro `Clone` (in Nightly builds, run with -Z macro-backtrace for more info)

(This issue was originally reported by @UncombedCoconut, who found that the bbchallenge decider-finite-automata-reduction crate no longer compiles in 1.69.0, due to the use of derive macros on a packed struct.)

Meta

rustc --version --verbose:

rustc 1.69.0 (84c898d65 2023-04-16)
binary: rustc
commit-hash: 84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc
commit-date: 2023-04-16
host: x86_64-unknown-linux-gnu
release: 1.69.0
LLVM version: 15.0.7
LegionMammal978 commented 1 year ago

Ah, it appears that this is #109744. However, the 1.69.0 relnotes do not include this breaking change: they only give the PR title of "Deriving built-in traits on packed structs works with Copy fields", and the compatibility notes only talk about the unaligned_references case which was already deny-by-default. It might be helpful to add a notice.

apiraino commented 1 year ago

This is something that could be handled in the release notes (Zulip discussion).

@rustbot label -I-prioritize

Enselic commented 2 months ago

Triage: It does not seem like this was added to release notes. But at this point, no one is going to read such old release notes anyway. Let's close as won't fix. Keeping this issue open won't do any good.