diff options
| author | León Orell Valerian Liehr <me@fmease.dev> | 2024-12-10 13:51:10 +0100 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2024-12-10 13:51:10 +0100 |
| commit | 3eaa785daa92be680d8549c87f1b68b811904abd (patch) | |
| tree | a31f332d55a2720c2e215d0cc3a8d158e4b2bae0 /compiler/rustc_hir_analysis | |
| parent | ceaca6be471dffee50c5ded5fe923ec74395446a (diff) | |
| parent | 3ce35a4ec5f06828f908a018da083af5eb54301a (diff) | |
| download | rust-3eaa785daa92be680d8549c87f1b68b811904abd.tar.gz rust-3eaa785daa92be680d8549c87f1b68b811904abd.zip | |
Rollup merge of #134008 - jswrenn:unsafe-fields-copy, r=compiler-errors
Make `Copy` unsafe to implement for ADTs with `unsafe` fields As a rule, the application of `unsafe` to a declaration requires that use-sites of that declaration also entail `unsafe`. For example, a field declared `unsafe` may only be read in the lexical context of an `unsafe` block. For nearly all safe traits, the safety obligations of fields are explicitly discharged when they are mentioned in method definitions. For example, idiomatically implementing `Clone` (a safe trait) for a type with unsafe fields will require `unsafe` to clone those fields. Prior to this commit, `Copy` violated this rule. The trait is marked safe, and although it has no explicit methods, its implementation permits reads of `Self`. This commit resolves this by making `Copy` conditionally safe to implement. It remains safe to implement for ADTs without unsafe fields, but unsafe to implement for ADTs with unsafe fields. Tracking: #132922 r? ```@compiler-errors```
Diffstat (limited to 'compiler/rustc_hir_analysis')
| -rw-r--r-- | compiler/rustc_hir_analysis/src/coherence/builtin.rs | 8 | ||||
| -rw-r--r-- | compiler/rustc_hir_analysis/src/coherence/unsafety.rs | 38 |
2 files changed, 37 insertions, 9 deletions
diff --git a/compiler/rustc_hir_analysis/src/coherence/builtin.rs b/compiler/rustc_hir_analysis/src/coherence/builtin.rs index 3b49bc41ffe..2eea65125b0 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 d66114a50d7..86839e40330 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", |
