bjoernager/rust
bjoernager/rust
1
Fork 0
Code Issues Pull requests Activity

Rollup merge of #134008 - jswrenn:unsafe-fields-copy, r=compiler-errors

Browse source 
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```
This commit is contained in:
León Orell Valerian Liehr 2024-12-10 13:51:10 +01:00 • committed by GitHub
commit 3eaa785daa
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 164 additions and 52 deletions
Download patch file Download diff file Expand all files Collapse all files

8
compiler/rustc_hir_analysis/src/coherence/builtin.rs
View file

@ -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)))
}
}
}

38
compiler/rustc_hir_analysis/src/coherence/unsafety.rs
View file

@ -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",