Skip to content

Commit

Permalink
Ensure that negative auto impls are always applicable
Browse files Browse the repository at this point in the history
  • Loading branch information
compiler-errors committed Feb 28, 2025
1 parent f45d4ac commit 2688777
Show file tree
Hide file tree
Showing 19 changed files with 275 additions and 107 deletions.
10 changes: 6 additions & 4 deletions compiler/rustc_data_structures/src/marker.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::alloc::Allocator;

#[rustc_on_unimplemented(message = "`{Self}` doesn't implement `DynSend`. \
Add it to `rustc_data_structures::marker` or use `IntoDynSyncSend` if it's already `Send`")]
// This is an auto trait for types which can be sent across threads if `sync::is_dyn_thread_safe()`
Expand Down Expand Up @@ -28,8 +30,8 @@ impls_dyn_send_neg!(
[*const T where T: ?Sized]
[*mut T where T: ?Sized]
[std::ptr::NonNull<T> where T: ?Sized]
[std::rc::Rc<T> where T: ?Sized]
[std::rc::Weak<T> where T: ?Sized]
[std::rc::Rc<T, A> where T: ?Sized, A: Allocator]
[std::rc::Weak<T, A> where T: ?Sized, A: Allocator]
[std::sync::MutexGuard<'_, T> where T: ?Sized]
[std::sync::RwLockReadGuard<'_, T> where T: ?Sized]
[std::sync::RwLockWriteGuard<'_, T> where T: ?Sized]
Expand Down Expand Up @@ -96,8 +98,8 @@ impls_dyn_sync_neg!(
[std::cell::RefCell<T> where T: ?Sized]
[std::cell::UnsafeCell<T> where T: ?Sized]
[std::ptr::NonNull<T> where T: ?Sized]
[std::rc::Rc<T> where T: ?Sized]
[std::rc::Weak<T> where T: ?Sized]
[std::rc::Rc<T, A> where T: ?Sized, A: Allocator]
[std::rc::Weak<T, A> where T: ?Sized, A: Allocator]
[std::cell::OnceCell<T> where T]
[std::sync::mpsc::Receiver<T> where T]
[std::sync::mpsc::Sender<T> where T]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,15 @@
//! This module contains methods that assist in checking that impls are general
//! enough, i.e. that they always apply to every valid instantaiton of the ADT
//! they're implemented for.
//!
//! This is necessary for `Drop` and negative impls to be well-formed.
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::codes::*;
use rustc_errors::{ErrorGuaranteed, struct_span_code_err};
use rustc_infer::infer::{RegionResolutionError, TyCtxtInferExt};
use rustc_infer::traits::{ObligationCause, ObligationCauseCode};
use rustc_middle::span_bug;
use rustc_middle::ty::util::CheckRegions;
use rustc_middle::ty::{self, GenericArgsRef, Ty, TyCtxt, TypingMode};
use rustc_trait_selection::regions::InferCtxtRegionExt;
Expand All @@ -27,11 +34,12 @@ use crate::hir::def_id::{DefId, LocalDefId};
/// 3. Any bounds on the generic parameters must be reflected in the
/// struct/enum definition for the nominal type itself (i.e.
/// cannot do `struct S<T>; impl<T:Clone> Drop for S<T> { ... }`).
///
pub(crate) fn check_drop_impl(
tcx: TyCtxt<'_>,
drop_impl_did: DefId,
) -> Result<(), ErrorGuaranteed> {
let drop_impl_did = drop_impl_did.expect_local();

match tcx.impl_polarity(drop_impl_did) {
ty::ImplPolarity::Positive => {}
ty::ImplPolarity::Negative => {
Expand All @@ -45,55 +53,107 @@ pub(crate) fn check_drop_impl(
}));
}
}
let dtor_self_type = tcx.type_of(drop_impl_did).instantiate_identity();
match dtor_self_type.kind() {

tcx.ensure_ok().orphan_check_impl(drop_impl_did)?;

let dtor_impl_trait_ref = tcx.impl_trait_ref(drop_impl_did).unwrap().instantiate_identity();

match dtor_impl_trait_ref.self_ty().kind() {
ty::Adt(adt_def, adt_to_impl_args) => {
ensure_drop_params_and_item_params_correspond(
ensure_impl_params_and_item_params_correspond(
tcx,
drop_impl_did.expect_local(),
drop_impl_did,
adt_def.did(),
adt_to_impl_args,
)?;

ensure_drop_predicates_are_implied_by_item_defn(
ensure_impl_predicates_are_implied_by_item_defn(
tcx,
drop_impl_did.expect_local(),
adt_def.did().expect_local(),
drop_impl_did,
adt_def.did(),
adt_to_impl_args,
)
}
_ => {
// Destructors only work on nominal types. This was
// already checked by coherence, but compilation may
// not have been terminated.
let span = tcx.def_span(drop_impl_did);
let reported = tcx.dcx().span_delayed_bug(
span,
format!("should have been rejected by coherence check: {dtor_self_type}"),
);
Err(reported)
span_bug!(tcx.def_span(drop_impl_did), "incoherent impl of Drop");
}
}
}

fn ensure_drop_params_and_item_params_correspond<'tcx>(
pub(crate) fn check_negative_auto_trait_impl<'tcx>(
tcx: TyCtxt<'tcx>,
drop_impl_did: LocalDefId,
self_type_did: DefId,
impl_def_id: LocalDefId,
impl_trait_ref: ty::TraitRef<'tcx>,
polarity: ty::ImplPolarity,
) -> Result<(), ErrorGuaranteed> {
let ty::ImplPolarity::Negative = polarity else {
return Ok(());
};

if !tcx.trait_is_auto(impl_trait_ref.def_id) {
return Ok(());
}

if tcx.defaultness(impl_def_id).is_default() {
tcx.dcx().span_delayed_bug(tcx.def_span(impl_def_id), "default impl cannot be negative");
}

tcx.ensure_ok().orphan_check_impl(impl_def_id)?;

match impl_trait_ref.self_ty().kind() {
ty::Adt(adt_def, adt_to_impl_args) => {
ensure_impl_params_and_item_params_correspond(
tcx,
impl_def_id,
adt_def.did(),
adt_to_impl_args,
)?;

ensure_impl_predicates_are_implied_by_item_defn(
tcx,
impl_def_id,
adt_def.did(),
adt_to_impl_args,
)
}
_ => {
if tcx.features().auto_traits() {
// NOTE: We ignore the applicability check for negative auto impls
// defined in libcore. In the (almost impossible) future where we
// stabilize auto impls, then the proper applicability check MUST
// be implemented here to handle non-ADT rigid types.
Ok(())
} else {
span_bug!(tcx.def_span(impl_def_id), "incoherent impl of negative auto trait");
}
}
}
}

fn ensure_impl_params_and_item_params_correspond<'tcx>(
tcx: TyCtxt<'tcx>,
impl_def_id: LocalDefId,
adt_def_id: DefId,
adt_to_impl_args: GenericArgsRef<'tcx>,
) -> Result<(), ErrorGuaranteed> {
let Err(arg) = tcx.uses_unique_generic_params(adt_to_impl_args, CheckRegions::OnlyParam) else {
return Ok(());
};

let drop_impl_span = tcx.def_span(drop_impl_did);
let item_span = tcx.def_span(self_type_did);
let self_descr = tcx.def_descr(self_type_did);
let drop_impl_span = tcx.def_span(impl_def_id);
let item_span = tcx.def_span(adt_def_id);
let self_descr = tcx.def_descr(adt_def_id);
let polarity = match tcx.impl_polarity(impl_def_id) {
ty::ImplPolarity::Positive | ty::ImplPolarity::Reservation => "",
ty::ImplPolarity::Negative => "!",
};
let trait_name = tcx
.item_name(tcx.trait_id_of_impl(impl_def_id.to_def_id()).expect("expected impl of trait"));
let mut err = struct_span_code_err!(
tcx.dcx(),
drop_impl_span,
E0366,
"`Drop` impls cannot be specialized"
"`{polarity}{trait_name}` impls cannot be specialized",
);
match arg {
ty::util::NotUniqueParam::DuplicateParam(arg) => {
Expand All @@ -116,17 +176,22 @@ fn ensure_drop_params_and_item_params_correspond<'tcx>(
/// Confirms that all predicates defined on the `Drop` impl (`drop_impl_def_id`) are able to be
/// proven from within `adt_def_id`'s environment. I.e. all the predicates on the impl are
/// implied by the ADT being well formed.
fn ensure_drop_predicates_are_implied_by_item_defn<'tcx>(
fn ensure_impl_predicates_are_implied_by_item_defn<'tcx>(
tcx: TyCtxt<'tcx>,
drop_impl_def_id: LocalDefId,
adt_def_id: LocalDefId,
impl_def_id: LocalDefId,
adt_def_id: DefId,
adt_to_impl_args: GenericArgsRef<'tcx>,
) -> Result<(), ErrorGuaranteed> {
let infcx = tcx.infer_ctxt().build(TypingMode::non_body_analysis());
let ocx = ObligationCtxt::new_with_diagnostics(&infcx);

let impl_span = tcx.def_span(drop_impl_def_id.to_def_id());

let impl_span = tcx.def_span(impl_def_id.to_def_id());
let trait_name = tcx
.item_name(tcx.trait_id_of_impl(impl_def_id.to_def_id()).expect("expected impl of trait"));
let polarity = match tcx.impl_polarity(impl_def_id) {
ty::ImplPolarity::Positive | ty::ImplPolarity::Reservation => "",
ty::ImplPolarity::Negative => "!",
};
// Take the param-env of the adt and instantiate the args that show up in
// the implementation's self type. This gives us the assumptions that the
// self ty of the implementation is allowed to know just from it being a
Expand All @@ -145,17 +210,21 @@ fn ensure_drop_predicates_are_implied_by_item_defn<'tcx>(
let adt_env =
ty::EarlyBinder::bind(tcx.param_env(adt_def_id)).instantiate(tcx, adt_to_impl_args);

let fresh_impl_args = infcx.fresh_args_for_item(impl_span, drop_impl_def_id.to_def_id());
let fresh_impl_args = infcx.fresh_args_for_item(impl_span, impl_def_id.to_def_id());
let fresh_adt_ty =
tcx.impl_trait_ref(drop_impl_def_id).unwrap().instantiate(tcx, fresh_impl_args).self_ty();
tcx.impl_trait_ref(impl_def_id).unwrap().instantiate(tcx, fresh_impl_args).self_ty();

ocx.eq(&ObligationCause::dummy_with_span(impl_span), adt_env, fresh_adt_ty, impl_adt_ty)
.unwrap();
.expect("equating fully generic trait ref should never fail");

for (clause, span) in tcx.predicates_of(drop_impl_def_id).instantiate(tcx, fresh_impl_args) {
let normalize_cause = traits::ObligationCause::misc(span, adt_def_id);
for (clause, span) in tcx.predicates_of(impl_def_id).instantiate(tcx, fresh_impl_args) {
let normalize_cause = traits::ObligationCause::misc(span, impl_def_id);
let pred = ocx.normalize(&normalize_cause, adt_env, clause);
let cause = traits::ObligationCause::new(span, adt_def_id, ObligationCauseCode::DropImpl);
let cause = traits::ObligationCause::new(
span,
impl_def_id,
ObligationCauseCode::AlwaysApplicableImpl,
);
ocx.register_obligation(traits::Obligation::new(tcx, cause, adt_env, pred));
}

Expand All @@ -173,13 +242,13 @@ fn ensure_drop_predicates_are_implied_by_item_defn<'tcx>(
let root_predicate = error.root_obligation.predicate;
if root_predicates.insert(root_predicate) {
let item_span = tcx.def_span(adt_def_id);
let self_descr = tcx.def_descr(adt_def_id.to_def_id());
let self_descr = tcx.def_descr(adt_def_id);
guar = Some(
struct_span_code_err!(
tcx.dcx(),
error.root_obligation.cause.span,
E0367,
"`Drop` impl requires `{root_predicate}` \
"`{polarity}{trait_name}` impl requires `{root_predicate}` \
but the {self_descr} it is implemented for does not",
)
.with_span_note(item_span, "the implementor must specify the same requirement")
Expand All @@ -190,12 +259,12 @@ fn ensure_drop_predicates_are_implied_by_item_defn<'tcx>(
return Err(guar.unwrap());
}

let errors = ocx.infcx.resolve_regions(adt_def_id, adt_env, []);
let errors = ocx.infcx.resolve_regions(impl_def_id, adt_env, []);
if !errors.is_empty() {
let mut guar = None;
for error in errors {
let item_span = tcx.def_span(adt_def_id);
let self_descr = tcx.def_descr(adt_def_id.to_def_id());
let self_descr = tcx.def_descr(adt_def_id);
let outlives = match error {
RegionResolutionError::ConcreteFailure(_, a, b) => format!("{b}: {a}"),
RegionResolutionError::GenericBoundFailure(_, generic, r) => {
Expand All @@ -212,7 +281,7 @@ fn ensure_drop_predicates_are_implied_by_item_defn<'tcx>(
tcx.dcx(),
error.origin().span(),
E0367,
"`Drop` impl requires `{outlives}` \
"`{polarity}{trait_name}` impl requires `{outlives}` \
but the {self_descr} it is implemented for does not",
)
.with_span_note(item_span, "the implementor must specify the same requirement")
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_hir_analysis/src/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,9 @@ a type parameter).
*/

pub mod always_applicable;
mod check;
mod compare_impl_item;
pub mod dropck;
mod entry;
pub mod intrinsic;
pub mod intrinsicck;
Expand Down Expand Up @@ -113,11 +113,11 @@ pub fn provide(providers: &mut Providers) {
}

fn adt_destructor(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Option<ty::Destructor> {
tcx.calculate_dtor(def_id.to_def_id(), dropck::check_drop_impl)
tcx.calculate_dtor(def_id.to_def_id(), always_applicable::check_drop_impl)
}

fn adt_async_destructor(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Option<ty::AsyncDestructor> {
tcx.calculate_async_dtor(def_id.to_def_id(), dropck::check_drop_impl)
tcx.calculate_async_dtor(def_id.to_def_id(), always_applicable::check_drop_impl)
}

/// Given a `DefId` for an opaque type in return position, find its parent item's return
Expand Down
26 changes: 17 additions & 9 deletions compiler/rustc_hir_analysis/src/coherence/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use rustc_span::{ErrorGuaranteed, sym};
use rustc_type_ir::elaborate;
use tracing::debug;

use crate::check::always_applicable;
use crate::errors;

mod builtin;
Expand All @@ -24,11 +25,12 @@ mod inherent_impls_overlap;
mod orphan;
mod unsafety;

fn check_impl(
tcx: TyCtxt<'_>,
fn check_impl<'tcx>(
tcx: TyCtxt<'tcx>,
impl_def_id: LocalDefId,
trait_ref: ty::TraitRef<'_>,
trait_def: &ty::TraitDef,
trait_ref: ty::TraitRef<'tcx>,
trait_def: &'tcx ty::TraitDef,
polarity: ty::ImplPolarity,
) -> Result<(), ErrorGuaranteed> {
debug!(
"(checking implementation) adding impl for trait '{:?}', item '{}'",
Expand All @@ -44,6 +46,12 @@ fn check_impl(

enforce_trait_manually_implementable(tcx, impl_def_id, trait_ref.def_id, trait_def)
.and(enforce_empty_impls_for_marker_traits(tcx, impl_def_id, trait_ref.def_id, trait_def))
.and(always_applicable::check_negative_auto_trait_impl(
tcx,
impl_def_id,
trait_ref,
polarity,
))
}

fn enforce_trait_manually_implementable(
Expand Down Expand Up @@ -154,16 +162,16 @@ fn coherent_trait(tcx: TyCtxt<'_>, def_id: DefId) -> Result<(), ErrorGuaranteed>
let mut res = tcx.ensure_ok().specialization_graph_of(def_id);

for &impl_def_id in impls {
let trait_header = tcx.impl_trait_header(impl_def_id).unwrap();
let trait_ref = trait_header.trait_ref.instantiate_identity();
let impl_header = tcx.impl_trait_header(impl_def_id).unwrap();
let trait_ref = impl_header.trait_ref.instantiate_identity();
let trait_def = tcx.trait_def(trait_ref.def_id);

res = res
.and(check_impl(tcx, impl_def_id, trait_ref, trait_def))
.and(check_impl(tcx, impl_def_id, trait_ref, trait_def, impl_header.polarity))
.and(check_object_overlap(tcx, impl_def_id, trait_ref))
.and(unsafety::check_item(tcx, impl_def_id, trait_header, trait_def))
.and(unsafety::check_item(tcx, impl_def_id, impl_header, trait_def))
.and(tcx.ensure_ok().orphan_check_impl(impl_def_id))
.and(builtin::check_trait(tcx, def_id, impl_def_id, trait_header));
.and(builtin::check_trait(tcx, def_id, impl_def_id, impl_header));
}

res
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_middle/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -397,9 +397,9 @@ pub enum ObligationCauseCode<'tcx> {

RustCall,

/// Obligations to prove that a `std::ops::Drop` impl is not stronger than
/// Obligations to prove that a `Drop` or negative auto trait impl is not stronger than
/// the ADT it's being implemented for.
DropImpl,
AlwaysApplicableImpl,

/// Requirement for a `const N: Ty` to implement `Ty: ConstParamTy`
ConstParam(Ty<'tcx>),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2695,7 +2695,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
| ObligationCauseCode::LetElse
| ObligationCauseCode::BinOp { .. }
| ObligationCauseCode::AscribeUserTypeProvePredicate(..)
| ObligationCauseCode::DropImpl
| ObligationCauseCode::AlwaysApplicableImpl
| ObligationCauseCode::ConstParam(_)
| ObligationCauseCode::ReferenceOutlivesReferent(..)
| ObligationCauseCode::ObjectTypeBound(..) => {}
Expand Down
1 change: 0 additions & 1 deletion library/core/src/convert/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -778,7 +778,6 @@ impl<T> From<T> for T {
///
/// [#64715]: https://github.com/rust-lang/rust/issues/64715
#[stable(feature = "convert_infallible", since = "1.34.0")]
#[allow(unused_attributes)] // FIXME(#58633): do a principled fix instead.
#[rustc_reservation_impl = "permitting this impl would forbid us from adding \
`impl<T> From<!> for T` later; see rust-lang/rust#64715 for details"]
impl<T> From<!> for T {
Expand Down
Loading

0 comments on commit 2688777

Please sign in to comment.