improve error messages

Author: Ariel Ben-Yehuda

src/librustc/dep_graph/dep_node.rs

@@ -479,6 +479,7 @@ define_dep_nodes!( <'tcx>
     [] BorrowCheck(DefId),
     [] MirBorrowCheck(DefId),
     [] UnsafetyCheckResult(DefId),
+    [] UnsafeDeriveOnReprPacked(DefId),
 
     [] Reachability,
     [] MirKeys,

src/librustc/ty/maps/mod.rs

@@ -169,6 +169,9 @@ define_maps! { <'tcx>
     /// The result of unsafety-checking this def-id.
     [] fn unsafety_check_result: UnsafetyCheckResult(DefId) -> mir::UnsafetyCheckResult,
 
+    /// HACK: when evaluated, this reports a "unsafe derive on repr(packed)" error
+    [] fn unsafe_derive_on_repr_packed: UnsafeDeriveOnReprPacked(DefId) -> (),
+
     /// The signature of functions and closures.
     [] fn fn_sig: FnSignature(DefId) -> ty::PolyFnSig<'tcx>,
 

src/librustc/ty/maps/plumbing.rs

@@ -768,6 +768,7 @@ pub fn force_from_dep_node<'a, 'gcx, 'lcx>(tcx: TyCtxt<'a, 'gcx, 'lcx>,
         DepKind::BorrowCheck => { force!(borrowck, def_id!()); }
         DepKind::MirBorrowCheck => { force!(mir_borrowck, def_id!()); }
         DepKind::UnsafetyCheckResult => { force!(unsafety_check_result, def_id!()); }
+        DepKind::UnsafeDeriveOnReprPacked => { force!(unsafe_derive_on_repr_packed, def_id!()); }
         DepKind::Reachability => { force!(reachable_set, LOCAL_CRATE); }
         DepKind::MirKeys => { force!(mir_keys, LOCAL_CRATE); }
         DepKind::CrateVariances => { force!(crate_variances, LOCAL_CRATE); }

src/librustc_mir/transform/check_unsafety.rs

@@ -259,6 +259,7 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> {
 pub(crate) fn provide(providers: &mut Providers) {
     *providers = Providers {
         unsafety_check_result,
+        unsafe_derive_on_repr_packed,
         ..*providers
     };
 }
@@ -341,6 +342,27 @@ fn unsafety_check_result<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId)
     }
 }
 
+fn unsafe_derive_on_repr_packed<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) {
+    let lint_node_id = match tcx.hir.as_local_node_id(def_id) {
+        Some(node_id) => node_id,
+        None => bug!("checking unsafety for non-local def id {:?}", def_id)
+    };
+
+    // FIXME: when we make this a hard error, this should have its
+    // own error code.
+    let message = if !tcx.generics_of(def_id).types.is_empty() {
+        format!("#[derive] can't be used on a #[repr(packed)] struct with \
+                 type parameters (error E0133)")
+    } else {
+        format!("#[derive] can't be used on a non-Copy #[repr(packed)] struct \
+                 (error E0133)")
+    };
+    tcx.lint_node(SAFE_PACKED_BORROWS,
+                  lint_node_id,
+                  tcx.def_span(def_id),
+                  &message);
+}
+
 /// Return the NodeId for an enclosing scope that is also `unsafe`
 fn is_enclosed(tcx: TyCtxt,
                used_unsafe: &FxHashSet<ast::NodeId>,
@@ -402,7 +424,6 @@ pub fn check_unsafety<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) {
         unsafe_blocks
     } = tcx.unsafety_check_result(def_id);
 
-    let mut emitted_derive_error = false;
     for &UnsafetyViolation {
         source_info, description, kind
     } in violations.iter() {
@@ -423,29 +444,15 @@ pub fn check_unsafety<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) {
                                         block (error E0133)", description));
             }
             UnsafetyViolationKind::BorrowPacked(lint_node_id) => {
-                if emitted_derive_error {
-                    continue
-                }
-
-                let message = if let Some(impl_def_id) = builtin_derive_def_id(tcx, def_id) {
-                    emitted_derive_error = true;
-                    // FIXME: when we make this a hard error, this should have its
-                    // own error code.
-                    if !tcx.generics_of(impl_def_id).types.is_empty() {
-                        format!("#[derive] can't be used on a #[repr(packed)] struct with \
-                                 type parameters (error E0133)")
-                    } else {
-                        format!("#[derive] can't be used on a non-Copy #[repr(packed)] struct \
-                                (error E0133)")
-                    }
+                if let Some(impl_def_id) = builtin_derive_def_id(tcx, def_id) {
+                    tcx.unsafe_derive_on_repr_packed(impl_def_id);
                 } else {
-                   format!("{} requires unsafe function or \
-                            block (error E0133)", description)
-                };
-                tcx.lint_node(SAFE_PACKED_BORROWS,
-                              lint_node_id,
-                              source_info.span,
-                              &message);
+                    tcx.lint_node(SAFE_PACKED_BORROWS,
+                                  lint_node_id,
+                                  source_info.span,
+                                  &format!("{} requires unsafe function or \
+                                            block (error E0133)", description));
+                }
             }
         }
     }

src/test/compile-fail/deriving-with-repr-packed-not-copy.rs

@@ -10,18 +10,32 @@
 
 #![deny(safe_packed_borrows)]
 
-// check that deriving a non-Copy packed struct is an error.
+// check that derive on a packed struct with non-Copy fields
+// correctly. This can't be made to work perfectly because
+// we can't just use the field from the struct as it might
+// not be aligned.
+
 #[derive(Copy, Clone, PartialEq, Eq)]
-#[repr(packed)]
-pub struct Foo<T>(T, T, T);
 //~^ ERROR #[derive] can't be used
 //~| hard error
 //~^^^ ERROR #[derive] can't be used
 //~| hard error
+#[repr(packed)]
+pub struct Foo<T>(T, T, T);
+
 #[derive(PartialEq, Eq)]
+//~^ ERROR #[derive] can't be used
+//~| hard error
 #[repr(packed)]
 pub struct Bar(u32, u32, u32);
-//~^ ERROR #[derive] can't be used
+
+#[derive(PartialEq)]
+struct Y(usize);
+
+#[derive(PartialEq)]
+//~^ ERROR #[derive] can't be used on a non-Copy #[repr(packed)]
 //~| hard error
+#[repr(packed)]
+struct X(Y);
 
 fn main() {}

src/test/ui/deriving-with-repr-packed.rs

@@ -1,8 +1,8 @@
 error: #[derive] can't be used on a #[repr(packed)] struct with type parameters (error E0133)
-  --> $DIR/deriving-with-repr-packed.rs:16:19
+  --> $DIR/deriving-with-repr-packed.rs:18:16
    |
-16 | pub struct Foo<T>(T, T, T);
-   |                   ^^
+18 | #[derive(Copy, Clone, PartialEq, Eq)]
+   |                ^^^^^
    |
 note: lint level defined here
   --> $DIR/deriving-with-repr-packed.rs:11:9
@@ -13,22 +13,31 @@ note: lint level defined here
    = note: for more information, see issue #46043 <https://github.com/rust-lang/rust/issues/46043>
 
 error: #[derive] can't be used on a #[repr(packed)] struct with type parameters (error E0133)
-  --> $DIR/deriving-with-repr-packed.rs:16:19
+  --> $DIR/deriving-with-repr-packed.rs:18:23
    |
-16 | pub struct Foo<T>(T, T, T);
-   |                   ^^
+18 | #[derive(Copy, Clone, PartialEq, Eq)]
+   |                       ^^^^^^^^^
    |
    = 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 #46043 <https://github.com/rust-lang/rust/issues/46043>
 
 error: #[derive] can't be used on a non-Copy #[repr(packed)] struct (error E0133)
-  --> $DIR/deriving-with-repr-packed.rs:23:16
+  --> $DIR/deriving-with-repr-packed.rs:26:10
    |
-23 | pub struct Bar(u32, u32, u32);
-   |                ^^^^
+26 | #[derive(PartialEq, Eq)]
+   |          ^^^^^^^^^
    |
    = 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 #46043 <https://github.com/rust-lang/rust/issues/46043>
 
-error: aborting due to 5 previous errors
+error: #[derive] can't be used on a non-Copy #[repr(packed)] struct (error E0133)
+  --> $DIR/deriving-with-repr-packed.rs:35:10
+   |
+35 | #[derive(PartialEq)]
+   |          ^^^^^^^^^
+   |
+   = 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 #46043 <https://github.com/rust-lang/rust/issues/46043>
+
+error: aborting due to 4 previous errors