Skip to content

Commit

Permalink
Auto merge of #117511 - gurry:117406-err-packed-structs, r=compiler-e…
Browse files Browse the repository at this point in the history
…rrors

Emit explanatory note for move errors in packed struct derives

Derive expansions for packed structs with non-`Copy` fields cause move errors because they prefer copying over borrowing since borrowing the fields of a packed struct can result in unaligned access.

This underlying cause of the errors, however, is not apparent to the user. This PR adds a diagnostic note to make it clear to the user (the new note is on the second last line):

```
tests/ui/derives/deriving-with-repr-packed-move-errors.rs:13:16
   |
12 | #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Default)]
   |          ----- in this derive macro expansion
13 | struct StructA(String);
   |                ^^^^^^ move occurs because `self.0` has type `String`, 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)
```

Fixes #117406

Partially addresses #110777
  • Loading branch information
bors committed Nov 7, 2023
2 parents 189d6c7 + 4b3ece4 commit 7b97a5c
Show file tree
Hide file tree
Showing 4 changed files with 290 additions and 1 deletion.
20 changes: 19 additions & 1 deletion compiler/rustc_borrowck/src/diagnostics/move_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use rustc_errors::{Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed
use rustc_middle::mir::*;
use rustc_middle::ty::{self, Ty};
use rustc_mir_dataflow::move_paths::{LookupResult, MovePathIndex};
use rustc_span::{BytePos, Span};
use rustc_span::{BytePos, ExpnKind, MacroKind, Span};

use crate::diagnostics::CapturedMessageOpt;
use crate::diagnostics::{DescribePlaceOpt, UseSpans};
Expand Down Expand Up @@ -488,6 +488,8 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
args_span,
}
});

self.add_note_for_packed_struct_derive(err, original_path.local);
}
}
}
Expand Down Expand Up @@ -594,4 +596,20 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
);
}
}

/// Adds an explanatory note if the move error occurs in a derive macro
/// expansion of a packed struct.
/// Such errors happen because derive macro expansions shy away from taking
/// references to the struct's fields since doing so would be undefined behaviour
fn add_note_for_packed_struct_derive(&self, err: &mut Diagnostic, local: Local) {
let local_place: PlaceRef<'tcx> = local.into();
let local_ty = local_place.ty(self.body.local_decls(), self.infcx.tcx).ty.peel_refs();

if let Some(adt) = local_ty.ty_adt_def()
&& adt.repr().packed()
&& let ExpnKind::Macro(MacroKind::Derive, name) = self.body.span.ctxt().outer_expn_data().kind
{
err.note(format!("`#[derive({name})]` triggers a move because taking references to the fields of a packed struct is undefined behaviour"));
}
}
}
96 changes: 96 additions & 0 deletions tests/ui/derives/deriving-with-repr-packed-move-errors.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
// Check that deriving builtin traits for a packed struct with
// non-Copy fields emits move errors along with an additional
// diagnostic note explaining the reason
// See issue #117406

use std::fmt::{Debug, Formatter, Result};
use std::cmp::Ordering;

// Packed + derives: additional diagnostic should be emitted
// for each of Debug, PartialEq and PartialOrd
#[repr(packed)]
#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Default)]
struct StructA(String);
//~^ ERROR: cannot move out of `self` which is behind a shared reference
//~| ERROR: cannot move out of `self` which is behind a shared reference
//~| ERROR: cannot move out of `other` which is behind a shared reference
//~| ERROR: cannot move out of `self` which is behind a shared reference
//~| ERROR: cannot move out of `other` which is behind a shared reference
//~| ERROR: cannot move out of `self` which is behind a shared reference
//~| ERROR: cannot move out of `other` which is behind a shared reference
//~| ERROR: cannot move out of `self` which is behind a shared reference
//~| ERROR: cannot move out of `self` which is behind a shared reference


// Unrelated impl: additinal diagnostic should NOT be emitted
impl StructA {
fn fmt(&self) -> String {
self.0 //~ ERROR: cannot move out of `self` which is behind a shared reference
}
}

// Packed + manual impls: additional diagnostic should NOT be emitted
#[repr(packed)]
struct StructB(String);

impl Debug for StructB {
fn fmt(&self, f: &mut Formatter) -> Result {
let x = &{ self.0 }; //~ ERROR: cannot move out of `self` which is behind a shared reference
write!(f, "{}", x)
}
}

impl PartialEq for StructB {
fn eq(&self, other: &StructB) -> bool {
({ self.0 }) == ({ other.0 })
//~^ ERROR: cannot move out of `self` which is behind a shared reference
//~| ERROR: cannot move out of `other` which is behind a shared reference
}
}

impl PartialOrd for StructB {
fn partial_cmp(&self, other: &StructB) -> Option<Ordering> {
PartialOrd::partial_cmp(&{ self.0 }, &{ other.0 })
//~^ ERROR: cannot move out of `self` which is behind a shared reference
//~| ERROR: cannot move out of `other` which is behind a shared reference
}
}

// NOT packed + derives: additinal diagnostic should NOT be emitted
#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Default)]
struct StructC(String);

// NOT packed + manual impls: additinal dignostic should NOT be emitted
struct StructD(String);

impl Debug for StructD {
fn fmt(&self, f: &mut Formatter) -> Result {
let x = &{ self.0 }; //~ ERROR: cannot move out of `self` which is behind a shared reference
write!(f, "{}", x)
}
}

impl PartialEq for StructD {
fn eq(&self, other: &StructD) -> bool {
({ self.0 }) == ({ other.0 })
//~^ ERROR: cannot move out of `self` which is behind a shared reference
//~| ERROR: cannot move out of `other` which is behind a shared reference
}
}

impl PartialOrd for StructD {
fn partial_cmp(&self, other: &StructD) -> Option<Ordering> {
PartialOrd::partial_cmp(&{ self.0 }, &{ other.0 })
//~^ ERROR: cannot move out of `self` which is behind a shared reference
//~| ERROR: cannot move out of `other` which is behind a shared reference
}
}

// Packed + derives but the move is outside of a derive
// expansion: additinal diagnostic should NOT be emitted
fn func(arg: &StructA) -> String {
arg.0 //~ ERROR: cannot move out of `arg` which is behind a shared reference
}

fn main(){
}
174 changes: 174 additions & 0 deletions tests/ui/derives/deriving-with-repr-packed-move-errors.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,174 @@
error[E0507]: cannot move out of `self` which is behind a shared reference
--> $DIR/deriving-with-repr-packed-move-errors.rs:13:16
|
LL | #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Default)]
| ----- in this derive macro expansion
LL | struct StructA(String);
| ^^^^^^ move occurs because `self.0` has type `String`, 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[E0507]: cannot move out of `self` which is behind a shared reference
--> $DIR/deriving-with-repr-packed-move-errors.rs:13:16
|
LL | #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Default)]
| --------- in this derive macro expansion
LL | struct StructA(String);
| ^^^^^^ move occurs because `self.0` has type `String`, which does not implement the `Copy` trait
|
= note: `#[derive(PartialEq)]` triggers a move because taking references to the fields of a packed struct is undefined behaviour
= note: this error originates in the derive macro `PartialEq` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0507]: cannot move out of `other` which is behind a shared reference
--> $DIR/deriving-with-repr-packed-move-errors.rs:13:16
|
LL | #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Default)]
| --------- in this derive macro expansion
LL | struct StructA(String);
| ^^^^^^ move occurs because `other.0` has type `String`, which does not implement the `Copy` trait
|
= note: `#[derive(PartialEq)]` triggers a move because taking references to the fields of a packed struct is undefined behaviour
= note: this error originates in the derive macro `PartialEq` (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-move-errors.rs:13:16
|
LL | #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Default)]
| ---------- in this derive macro expansion
LL | struct StructA(String);
| ^^^^^^ move occurs because `self.0` has type `String`, which does not implement the `Copy` trait
|
= note: `#[derive(PartialOrd)]` triggers a move because taking references to the fields of a packed struct is undefined behaviour
= note: this error originates in the derive macro `PartialOrd` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0507]: cannot move out of `other` which is behind a shared reference
--> $DIR/deriving-with-repr-packed-move-errors.rs:13:16
|
LL | #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Default)]
| ---------- in this derive macro expansion
LL | struct StructA(String);
| ^^^^^^ move occurs because `other.0` has type `String`, which does not implement the `Copy` trait
|
= note: `#[derive(PartialOrd)]` triggers a move because taking references to the fields of a packed struct is undefined behaviour
= note: this error originates in the derive macro `PartialOrd` (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-move-errors.rs:13:16
|
LL | #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Default)]
| --- in this derive macro expansion
LL | struct StructA(String);
| ^^^^^^ move occurs because `self.0` has type `String`, which does not implement the `Copy` trait
|
= note: `#[derive(Ord)]` triggers a move because taking references to the fields of a packed struct is undefined behaviour
= note: this error originates in the derive macro `Ord` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0507]: cannot move out of `other` which is behind a shared reference
--> $DIR/deriving-with-repr-packed-move-errors.rs:13:16
|
LL | #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Default)]
| --- in this derive macro expansion
LL | struct StructA(String);
| ^^^^^^ move occurs because `other.0` has type `String`, which does not implement the `Copy` trait
|
= note: `#[derive(Ord)]` triggers a move because taking references to the fields of a packed struct is undefined behaviour
= note: this error originates in the derive macro `Ord` (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-move-errors.rs:13:16
|
LL | #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Default)]
| ---- in this derive macro expansion
LL | struct StructA(String);
| ^^^^^^ move occurs because `self.0` has type `String`, which does not implement the `Copy` trait
|
= note: `#[derive(Hash)]` triggers a move because taking references to the fields of a packed struct is undefined behaviour
= note: this error originates in the derive macro `Hash` (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-move-errors.rs:13:16
|
LL | #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Default)]
| ----- in this derive macro expansion
LL | struct StructA(String);
| ^^^^^^ move occurs because `self.0` has type `String`, which does not implement the `Copy` trait
|
= note: `#[derive(Clone)]` triggers a move because taking references to the fields of a packed struct is undefined behaviour
= note: this error originates in the derive macro `Clone` (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-move-errors.rs:28:9
|
LL | self.0
| ^^^^^^ move occurs because `self.0` has type `String`, which does not implement the `Copy` trait

error[E0507]: cannot move out of `self` which is behind a shared reference
--> $DIR/deriving-with-repr-packed-move-errors.rs:38:20
|
LL | let x = &{ self.0 };
| ^^^^^^ move occurs because `self.0` has type `String`, which does not implement the `Copy` trait

error[E0507]: cannot move out of `self` which is behind a shared reference
--> $DIR/deriving-with-repr-packed-move-errors.rs:45:12
|
LL | ({ self.0 }) == ({ other.0 })
| ^^^^^^ move occurs because `self.0` has type `String`, which does not implement the `Copy` trait

error[E0507]: cannot move out of `other` which is behind a shared reference
--> $DIR/deriving-with-repr-packed-move-errors.rs:45:28
|
LL | ({ self.0 }) == ({ other.0 })
| ^^^^^^^ move occurs because `other.0` has type `String`, which does not implement the `Copy` trait

error[E0507]: cannot move out of `self` which is behind a shared reference
--> $DIR/deriving-with-repr-packed-move-errors.rs:53:36
|
LL | PartialOrd::partial_cmp(&{ self.0 }, &{ other.0 })
| ^^^^^^ move occurs because `self.0` has type `String`, which does not implement the `Copy` trait

error[E0507]: cannot move out of `other` which is behind a shared reference
--> $DIR/deriving-with-repr-packed-move-errors.rs:53:49
|
LL | PartialOrd::partial_cmp(&{ self.0 }, &{ other.0 })
| ^^^^^^^ move occurs because `other.0` has type `String`, which does not implement the `Copy` trait

error[E0507]: cannot move out of `self` which is behind a shared reference
--> $DIR/deriving-with-repr-packed-move-errors.rs:68:20
|
LL | let x = &{ self.0 };
| ^^^^^^ move occurs because `self.0` has type `String`, which does not implement the `Copy` trait

error[E0507]: cannot move out of `self` which is behind a shared reference
--> $DIR/deriving-with-repr-packed-move-errors.rs:75:12
|
LL | ({ self.0 }) == ({ other.0 })
| ^^^^^^ move occurs because `self.0` has type `String`, which does not implement the `Copy` trait

error[E0507]: cannot move out of `other` which is behind a shared reference
--> $DIR/deriving-with-repr-packed-move-errors.rs:75:28
|
LL | ({ self.0 }) == ({ other.0 })
| ^^^^^^^ move occurs because `other.0` has type `String`, which does not implement the `Copy` trait

error[E0507]: cannot move out of `self` which is behind a shared reference
--> $DIR/deriving-with-repr-packed-move-errors.rs:83:36
|
LL | PartialOrd::partial_cmp(&{ self.0 }, &{ other.0 })
| ^^^^^^ move occurs because `self.0` has type `String`, which does not implement the `Copy` trait

error[E0507]: cannot move out of `other` which is behind a shared reference
--> $DIR/deriving-with-repr-packed-move-errors.rs:83:49
|
LL | PartialOrd::partial_cmp(&{ self.0 }, &{ other.0 })
| ^^^^^^^ move occurs because `other.0` has type `String`, which does not implement the `Copy` trait

error[E0507]: cannot move out of `arg` which is behind a shared reference
--> $DIR/deriving-with-repr-packed-move-errors.rs:92:5
|
LL | arg.0
| ^^^^^ move occurs because `arg.0` has type `String`, which does not implement the `Copy` trait

error: aborting due to 21 previous errors

For more information about this error, try `rustc --explain E0507`.
1 change: 1 addition & 0 deletions tests/ui/derives/deriving-with-repr-packed.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ LL | #[repr(packed)]
LL | struct X(Y);
| ^ move occurs because `self.0` has type `Y`, 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: aborting due to previous error; 2 warnings emitted
Expand Down

0 comments on commit 7b97a5c

Please sign in to comment.