diff --git a/src/librustc_mir/transform/check_unsafety.rs b/src/librustc_mir/transform/check_unsafety.rs index 5fe3ad86d4f..74e180ce6df 100644 --- a/src/librustc_mir/transform/check_unsafety.rs +++ b/src/librustc_mir/transform/check_unsafety.rs @@ -373,6 +373,22 @@ fn report_unused_unsafe(tcx: TyCtxt, used_unsafe: &FxHashSet, id: a db.emit(); } +fn builtin_derive_def_id<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> Option { + debug!("builtin_derive_def_id({:?})", def_id); + if let Some(impl_def_id) = tcx.impl_of_method(def_id) { + if tcx.has_attr(impl_def_id, "automatically_derived") { + debug!("builtin_derive_def_id({:?}) - is {:?}", def_id, impl_def_id); + Some(impl_def_id) + } else { + debug!("builtin_derive_def_id({:?}) - not automatically derived", def_id); + None + } + } else { + debug!("builtin_derive_def_id({:?}) - not a method", def_id); + None + } +} + pub fn check_unsafety<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) { debug!("check_unsafety({:?})", def_id); @@ -386,6 +402,7 @@ 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() { @@ -406,11 +423,29 @@ 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)") + } + } else { + format!("{} requires unsafe function or \ + block (error E0133)", description) + }; tcx.lint_node(SAFE_PACKED_BORROWS, lint_node_id, source_info.span, - &format!("{} requires unsafe function or \ - block (error E0133)", description)); + &message); } } } diff --git a/src/libsyntax/ext/derive.rs b/src/libsyntax/ext/derive.rs index 2e70962cad6..c7fa0331c1b 100644 --- a/src/libsyntax/ext/derive.rs +++ b/src/libsyntax/ext/derive.rs @@ -74,7 +74,7 @@ pub fn add_derived_markers(cx: &mut ExtCtxt, span: Span, traits: &[ast::Path] let meta = cx.meta_word(span, Symbol::intern("structural_match")); attrs.push(cx.attribute(span, meta)); } - if names.contains(&Symbol::intern("Copy")) && names.contains(&Symbol::intern("Clone")) { + if names.contains(&Symbol::intern("Copy")) { let meta = cx.meta_word(span, Symbol::intern("rustc_copy_clone_marker")); attrs.push(cx.attribute(span, meta)); } diff --git a/src/libsyntax_ext/deriving/generic/mod.rs b/src/libsyntax_ext/deriving/generic/mod.rs index 5abf524313a..2b565ca51e9 100644 --- a/src/libsyntax_ext/deriving/generic/mod.rs +++ b/src/libsyntax_ext/deriving/generic/mod.rs @@ -413,7 +413,24 @@ impl<'a> TraitDef<'a> { attr::find_repr_attrs(&cx.parse_sess.span_diagnostic, attr) .contains(&attr::ReprPacked) }); - let use_temporaries = is_packed; + let has_no_type_params = match item.node { + ast::ItemKind::Struct(_, ref generics) | + ast::ItemKind::Enum(_, ref generics) | + ast::ItemKind::Union(_, ref generics) => { + generics.ty_params.is_empty() + } + _ => { + // Non-ADT derive is an error, but it should have been + // set earlier; see + // libsyntax/ext/expand.rs:MacroExpander::expand() + return; + } + }; + let is_always_copy = + attr::contains_name(&item.attrs, "rustc_copy_clone_marker") && + has_no_type_params; + let use_temporaries = is_packed && is_always_copy; + let newitem = match item.node { ast::ItemKind::Struct(ref struct_def, ref generics) => { self.expand_struct_def(cx, &struct_def, item.ident, generics, from_scratch, @@ -440,12 +457,7 @@ impl<'a> TraitDef<'a> { return; } } - _ => { - // Non-ADT derive is an error, but it should have been - // set earlier; see - // libsyntax/ext/expand.rs:MacroExpander::expand() - return; - } + _ => unreachable!(), }; // Keep the lint attributes of the previous item to control how the // generated implementations are linted diff --git a/src/test/compile-fail/deriving-with-repr-packed-not-copy.rs b/src/test/compile-fail/deriving-with-repr-packed-not-copy.rs index 5ab916b91fa..55dc3380209 100644 --- a/src/test/compile-fail/deriving-with-repr-packed-not-copy.rs +++ b/src/test/compile-fail/deriving-with-repr-packed-not-copy.rs @@ -8,6 +8,8 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +#![deny(safe_packed_borrows)] + // 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 @@ -17,12 +19,10 @@ struct Y(usize); #[derive(PartialEq)] -//~^ ERROR cannot move out of borrowed -//~| ERROR cannot move out of borrowed -//~| ERROR cannot move out of borrowed -//~| ERROR cannot move out of borrowed #[repr(packed)] struct X(Y); +//~^ ERROR #[derive] can't be used on a non-Copy #[repr(packed)] +//~| hard error fn main() { } diff --git a/src/test/run-pass/deriving-with-repr-packed.rs b/src/test/run-pass/deriving-with-repr-packed.rs index fcc31b462f8..f5130908c0b 100644 --- a/src/test/run-pass/deriving-with-repr-packed.rs +++ b/src/test/run-pass/deriving-with-repr-packed.rs @@ -31,7 +31,7 @@ impl PartialEq for Aligned { } #[repr(packed)] -#[derive(PartialEq)] +#[derive(Copy, Clone, PartialEq)] struct Packed(Aligned, Aligned); #[derive(PartialEq)] diff --git a/src/test/ui/deriving-with-repr-packed.rs b/src/test/ui/deriving-with-repr-packed.rs new file mode 100644 index 00000000000..85e0c3ba14e --- /dev/null +++ b/src/test/ui/deriving-with-repr-packed.rs @@ -0,0 +1,27 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![deny(safe_packed_borrows)] + +// check that deriving a non-Copy packed struct is an error. +#[derive(Copy, Clone, PartialEq, Eq)] +#[repr(packed)] +pub struct Foo(T, T, T); +//~^ ERROR #[derive] can't be used +//~| hard error +//~^^^ ERROR #[derive] can't be used +//~| hard error +#[derive(PartialEq, Eq)] +#[repr(packed)] +pub struct Bar(u32, u32, u32); +//~^ ERROR #[derive] can't be used +//~| hard error + +fn main() {} diff --git a/src/test/ui/deriving-with-repr-packed.stderr b/src/test/ui/deriving-with-repr-packed.stderr new file mode 100644 index 00000000000..24ce8a1e59f --- /dev/null +++ b/src/test/ui/deriving-with-repr-packed.stderr @@ -0,0 +1,34 @@ +error: #[derive] can't be used on a #[repr(packed)] struct with type parameters (error E0133) + --> $DIR/deriving-with-repr-packed.rs:16:19 + | +16 | pub struct Foo(T, T, T); + | ^^ + | +note: lint level defined here + --> $DIR/deriving-with-repr-packed.rs:11:9 + | +11 | #![deny(safe_packed_borrows)] + | ^^^^^^^^^^^^^^^^^^^ + = 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 + +error: #[derive] can't be used on a #[repr(packed)] struct with type parameters (error E0133) + --> $DIR/deriving-with-repr-packed.rs:16:19 + | +16 | pub struct Foo(T, T, T); + | ^^ + | + = 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 + +error: #[derive] can't be used on a non-Copy #[repr(packed)] struct (error E0133) + --> $DIR/deriving-with-repr-packed.rs:23:16 + | +23 | pub struct Bar(u32, u32, u32); + | ^^^^ + | + = 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 + +error: aborting due to 5 previous errors +