diff --git a/compiler/rustc_hir_analysis/src/coherence/builtin.rs b/compiler/rustc_hir_analysis/src/coherence/builtin.rs index 3b49bc41ffe43..2eea65125b038 100644 --- a/compiler/rustc_hir_analysis/src/coherence/builtin.rs +++ b/compiler/rustc_hir_analysis/src/coherence/builtin.rs @@ -103,7 +103,7 @@ fn visit_implementation_of_copy(checker: &Checker<'_>) -> Result<(), ErrorGuaran } let cause = traits::ObligationCause::misc(DUMMY_SP, impl_did); - match type_allowed_to_implement_copy(tcx, param_env, self_type, cause) { + match type_allowed_to_implement_copy(tcx, param_env, self_type, cause, impl_header.safety) { Ok(()) => Ok(()), Err(CopyImplementationError::InfringingFields(fields)) => { let span = tcx.hir().expect_item(impl_did).expect_impl().self_ty.span; @@ -123,6 +123,12 @@ fn visit_implementation_of_copy(checker: &Checker<'_>) -> Result<(), ErrorGuaran let span = tcx.hir().expect_item(impl_did).expect_impl().self_ty.span; Err(tcx.dcx().emit_err(errors::CopyImplOnTypeWithDtor { span })) } + Err(CopyImplementationError::HasUnsafeFields) => { + let span = tcx.hir().expect_item(impl_did).expect_impl().self_ty.span; + Err(tcx + .dcx() + .span_delayed_bug(span, format!("cannot implement `Copy` for `{}`", self_type))) + } } } diff --git a/compiler/rustc_hir_analysis/src/coherence/unsafety.rs b/compiler/rustc_hir_analysis/src/coherence/unsafety.rs index d66114a50d78b..86839e4033034 100644 --- a/compiler/rustc_hir_analysis/src/coherence/unsafety.rs +++ b/compiler/rustc_hir_analysis/src/coherence/unsafety.rs @@ -3,7 +3,7 @@ use rustc_errors::codes::*; use rustc_errors::struct_span_code_err; -use rustc_hir::Safety; +use rustc_hir::{LangItem, Safety}; use rustc_middle::ty::ImplPolarity::*; use rustc_middle::ty::print::PrintTraitRefExt as _; use rustc_middle::ty::{ImplTraitHeader, TraitDef, TyCtxt}; @@ -20,7 +20,19 @@ pub(super) fn check_item( tcx.generics_of(def_id).own_params.iter().find(|p| p.pure_wrt_drop).map(|_| "may_dangle"); let trait_ref = trait_header.trait_ref.instantiate_identity(); - match (trait_def.safety, unsafe_attr, trait_header.safety, trait_header.polarity) { + let is_copy = tcx.is_lang_item(trait_def.def_id, LangItem::Copy); + let trait_def_safety = if is_copy { + // If `Self` has unsafe fields, `Copy` is unsafe to implement. + if trait_header.trait_ref.skip_binder().self_ty().has_unsafe_fields() { + rustc_hir::Safety::Unsafe + } else { + rustc_hir::Safety::Safe + } + } else { + trait_def.safety + }; + + match (trait_def_safety, unsafe_attr, trait_header.safety, trait_header.polarity) { (Safety::Safe, None, Safety::Unsafe, Positive | Reservation) => { let span = tcx.def_span(def_id); return Err(struct_span_code_err!( @@ -48,12 +60,22 @@ pub(super) fn check_item( "the trait `{}` requires an `unsafe impl` declaration", trait_ref.print_trait_sugared() ) - .with_note(format!( - "the trait `{}` enforces invariants that the compiler can't check. \ - Review the trait documentation and make sure this implementation \ - upholds those invariants before adding the `unsafe` keyword", - trait_ref.print_trait_sugared() - )) + .with_note(if is_copy { + format!( + "the trait `{}` cannot be safely implemented for `{}` \ + because it has unsafe fields. Review the invariants \ + of those fields before adding an `unsafe impl`", + trait_ref.print_trait_sugared(), + trait_ref.self_ty(), + ) + } else { + format!( + "the trait `{}` enforces invariants that the compiler can't check. \ + Review the trait documentation and make sure this implementation \ + upholds those invariants before adding the `unsafe` keyword", + trait_ref.print_trait_sugared() + ) + }) .with_span_suggestion_verbose( span.shrink_to_lo(), "add `unsafe` to this trait implementation", diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index ec08519892203..093cc16fb4c18 100644 --- a/compiler/rustc_lint/src/builtin.rs +++ b/compiler/rustc_lint/src/builtin.rs @@ -625,6 +625,7 @@ impl<'tcx> LateLintPass<'tcx> for MissingCopyImplementations { cx.param_env, ty, traits::ObligationCause::misc(item.span, item.owner_id.def_id), + hir::Safety::Safe, ) .is_ok() { diff --git a/compiler/rustc_middle/src/ty/sty.rs b/compiler/rustc_middle/src/ty/sty.rs index 474062218c97a..3fbc23924f59a 100644 --- a/compiler/rustc_middle/src/ty/sty.rs +++ b/compiler/rustc_middle/src/ty/sty.rs @@ -980,11 +980,7 @@ impl<'tcx> rustc_type_ir::inherent::Ty> for Ty<'tcx> { } fn has_unsafe_fields(self) -> bool { - if let ty::Adt(adt_def, ..) = self.kind() { - adt_def.all_fields().any(|x| x.safety == hir::Safety::Unsafe) - } else { - false - } + Ty::has_unsafe_fields(self) } } diff --git a/compiler/rustc_middle/src/ty/util.rs b/compiler/rustc_middle/src/ty/util.rs index 57054bd1a0b24..b9a45ea3c2c5a 100644 --- a/compiler/rustc_middle/src/ty/util.rs +++ b/compiler/rustc_middle/src/ty/util.rs @@ -1288,6 +1288,15 @@ impl<'tcx> Ty<'tcx> { } } + /// Checks whether this type is an ADT that has unsafe fields. + pub fn has_unsafe_fields(self) -> bool { + if let ty::Adt(adt_def, ..) = self.kind() { + adt_def.all_fields().any(|x| x.safety == hir::Safety::Unsafe) + } else { + false + } + } + /// Get morphology of the async drop glue, needed for types which do not /// use async drop. To get async drop glue morphology for a definition see /// [`TyCtxt::async_drop_glue_morphology`]. Used for `AsyncDestruct::Destructor` diff --git a/compiler/rustc_trait_selection/src/traits/misc.rs b/compiler/rustc_trait_selection/src/traits/misc.rs index 3b17fa6b03285..d216ae7291337 100644 --- a/compiler/rustc_trait_selection/src/traits/misc.rs +++ b/compiler/rustc_trait_selection/src/traits/misc.rs @@ -18,6 +18,7 @@ pub enum CopyImplementationError<'tcx> { InfringingFields(Vec<(&'tcx ty::FieldDef, Ty<'tcx>, InfringingFieldsReason<'tcx>)>), NotAnAdt, HasDestructor, + HasUnsafeFields, } pub enum ConstParamTyImplementationError<'tcx> { @@ -39,11 +40,16 @@ pub enum InfringingFieldsReason<'tcx> { /// /// If it's not an ADT, int ty, `bool`, float ty, `char`, raw pointer, `!`, /// a reference or an array returns `Err(NotAnAdt)`. +/// +/// If the impl is `Safe`, `self_type` must not have unsafe fields. When used to +/// generate suggestions in lints, `Safe` should be supplied so as to not +/// suggest implementing `Copy` for types with unsafe fields. pub fn type_allowed_to_implement_copy<'tcx>( tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>, self_type: Ty<'tcx>, parent_cause: ObligationCause<'tcx>, + impl_safety: hir::Safety, ) -> Result<(), CopyImplementationError<'tcx>> { let (adt, args) = match self_type.kind() { // These types used to have a builtin impl. @@ -78,6 +84,10 @@ pub fn type_allowed_to_implement_copy<'tcx>( return Err(CopyImplementationError::HasDestructor); } + if impl_safety == hir::Safety::Safe && self_type.has_unsafe_fields() { + return Err(CopyImplementationError::HasUnsafeFields); + } + Ok(()) } diff --git a/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs b/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs index 3e2c8467d3220..5e27fd4378976 100644 --- a/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs +++ b/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs @@ -795,8 +795,6 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { | ty::Never | ty::Tuple(_) | ty::CoroutineWitness(..) => { - use rustc_type_ir::inherent::*; - // Only consider auto impls of unsafe traits when there are // no unsafe fields. if self.tcx().trait_is_unsafe(def_id) && self_ty.has_unsafe_fields() { diff --git a/src/tools/clippy/clippy_lints/src/needless_pass_by_value.rs b/src/tools/clippy/clippy_lints/src/needless_pass_by_value.rs index cd90d2f90f77b..6f3f371a68d6b 100644 --- a/src/tools/clippy/clippy_lints/src/needless_pass_by_value.rs +++ b/src/tools/clippy/clippy_lints/src/needless_pass_by_value.rs @@ -200,6 +200,7 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessPassByValue { cx.param_env, ty, traits::ObligationCause::dummy_with_span(span), + rustc_hir::Safety::Safe, ) .is_ok() { diff --git a/tests/ui/unsafe-fields/copy-trait.rs b/tests/ui/unsafe-fields/copy-trait.rs new file mode 100644 index 0000000000000..fb09ed02e3ffe --- /dev/null +++ b/tests/ui/unsafe-fields/copy-trait.rs @@ -0,0 +1,41 @@ +//@ compile-flags: --crate-type=lib + +#![feature(unsafe_fields)] +#![allow(incomplete_features)] +#![deny(missing_copy_implementations)] + +mod good_safe_impl { + enum SafeEnum { + Safe(u8), + } + + impl Copy for SafeEnum {} +} + +mod bad_safe_impl { + enum UnsafeEnum { + Safe(u8), + Unsafe { unsafe field: u8 }, + } + + impl Copy for UnsafeEnum {} + //~^ ERROR the trait `Copy` requires an `unsafe impl` declaration +} + +mod good_unsafe_impl { + enum UnsafeEnum { + Safe(u8), + Unsafe { unsafe field: u8 }, + } + + unsafe impl Copy for UnsafeEnum {} +} + +mod bad_unsafe_impl { + enum SafeEnum { + Safe(u8), + } + + unsafe impl Copy for SafeEnum {} + //~^ ERROR implementing the trait `Copy` is not unsafe +} diff --git a/tests/ui/unsafe-fields/copy-trait.stderr b/tests/ui/unsafe-fields/copy-trait.stderr new file mode 100644 index 0000000000000..5952f8c89c146 --- /dev/null +++ b/tests/ui/unsafe-fields/copy-trait.stderr @@ -0,0 +1,28 @@ +error[E0200]: the trait `Copy` requires an `unsafe impl` declaration + --> $DIR/copy-trait.rs:21:5 + | +LL | impl Copy for UnsafeEnum {} + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: the trait `Copy` cannot be safely implemented for `bad_safe_impl::UnsafeEnum` because it has unsafe fields. Review the invariants of those fields before adding an `unsafe impl` +help: add `unsafe` to this trait implementation + | +LL | unsafe impl Copy for UnsafeEnum {} + | ++++++ + +error[E0199]: implementing the trait `Copy` is not unsafe + --> $DIR/copy-trait.rs:39:5 + | +LL | unsafe impl Copy for SafeEnum {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: remove `unsafe` from this trait implementation + | +LL - unsafe impl Copy for SafeEnum {} +LL + impl Copy for SafeEnum {} + | + +error: aborting due to 2 previous errors + +Some errors have detailed explanations: E0199, E0200. +For more information about an error, try `rustc --explain E0199`.