Skip to content

Commit

Permalink
Emit explanatory note for move errors in packed struct derives
Browse files Browse the repository at this point in the history
Derive expansions for packed structs cause move errors because
they prefer copying over borrowing since borrowing the fields of a
packed struct can result in unaligned access and therefore undefined
behaviour.

This underlying cause of the errors, however, is not apparent
to the user. We add a diagnostic note here to remedy that.
  • Loading branch information
gurry committed Nov 3, 2023
1 parent a395214 commit 87b5e40
Show file tree
Hide file tree
Showing 4 changed files with 293 additions and 1 deletion.
23 changes: 22 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,23 @@ 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() {
if adt.repr().packed() {
if 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 87b5e40

Please sign in to comment.