Skip to content

Ensure that negative auto impls are always applicable #137764

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 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,
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
Loading