about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2023-11-07 00:03:53 +0000
committerbors <bors@rust-lang.org>2023-11-07 00:03:53 +0000
commit7b97a5ca8422d1495a8918106d3249aa405812d4 (patch)
tree02d03506b065bbac102dab8793a370042043d4f6
parent189d6c71f3bb6c52113b5639a80839791974fd22 (diff)
parent4b3ece475d6e77465a30c8b53d7e1e9e861e4b79 (diff)
Auto merge of #117511 - gurry:117406-err-packed-structs, r=compiler-errors
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
-rw-r--r--compiler/rustc_borrowck/src/diagnostics/move_errors.rs20
-rw-r--r--tests/ui/derives/deriving-with-repr-packed-move-errors.rs96
-rw-r--r--tests/ui/derives/deriving-with-repr-packed-move-errors.stderr174
-rw-r--r--tests/ui/derives/deriving-with-repr-packed.stderr1
4 files changed, 290 insertions, 1 deletions
diff --git a/compiler/rustc_borrowck/src/diagnostics/move_errors.rs b/compiler/rustc_borrowck/src/diagnostics/move_errors.rs
index 695ac6980cd..41d6b98d7cf 100644
--- a/compiler/rustc_borrowck/src/diagnostics/move_errors.rs
+++ b/compiler/rustc_borrowck/src/diagnostics/move_errors.rs
@@ -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};
@@ -488,6 +488,8 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
                         args_span,
                     }
                 });
+
+                self.add_note_for_packed_struct_derive(err, original_path.local);
             }
         }
     }
@@ -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"));
+        }
+    }
 }
diff --git a/tests/ui/derives/deriving-with-repr-packed-move-errors.rs b/tests/ui/derives/deriving-with-repr-packed-move-errors.rs
new file mode 100644
index 00000000000..ffeb02d78b8
--- /dev/null
+++ b/tests/ui/derives/deriving-with-repr-packed-move-errors.rs
@@ -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(){
+}
diff --git a/tests/ui/derives/deriving-with-repr-packed-move-errors.stderr b/tests/ui/derives/deriving-with-repr-packed-move-errors.stderr
new file mode 100644
index 00000000000..c538061b365
--- /dev/null
+++ b/tests/ui/derives/deriving-with-repr-packed-move-errors.stderr
@@ -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`.
diff --git a/tests/ui/derives/deriving-with-repr-packed.stderr b/tests/ui/derives/deriving-with-repr-packed.stderr
index 0cfe03869af..bb1fab343a3 100644
--- a/tests/ui/derives/deriving-with-repr-packed.stderr
+++ b/tests/ui/derives/deriving-with-repr-packed.stderr
@@ -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