Skip to content

Commit

Permalink
Auto merge of #94901 - fee1-dead:destructable, r=oli-obk
Browse files Browse the repository at this point in the history
Rename `~const Drop` to `~const Destruct`

r? `@oli-obk`

Completely switching to `~const Destructible` would be rather complicated, so it seems best to add it for now and wait for it to be backported to beta in the next release.

The rationale is to prevent complications such as #92149 and #94803 by introducing an entirely new trait. And `~const Destructible` reads a bit better than `~const Drop`. Name Bikesheddable.
  • Loading branch information
bors committed Mar 23, 2022
2 parents c99b42c + fe5b813 commit 9280445
Show file tree
Hide file tree
Showing 32 changed files with 308 additions and 187 deletions.
16 changes: 7 additions & 9 deletions compiler/rustc_const_eval/src/transform/check_consts/qualifs.rs
Expand Up @@ -3,6 +3,7 @@
//! See the `Qualif` trait for more info.

use rustc_errors::ErrorGuaranteed;
use rustc_hir::LangItem;
use rustc_infer::infer::TyCtxtInferExt;
use rustc_infer::traits::TraitEngine;
use rustc_middle::mir::*;
Expand Down Expand Up @@ -152,18 +153,14 @@ impl Qualif for NeedsNonConstDrop {
return false;
}

let Some(drop_trait) = cx.tcx.lang_items().drop_trait() else {
// there is no way to define a type that needs non-const drop
// without having the lang item present.
return false;
};
let destruct = cx.tcx.require_lang_item(LangItem::Destruct, None);

let obligation = Obligation::new(
ObligationCause::dummy(),
cx.param_env,
ty::Binder::dummy(ty::TraitPredicate {
trait_ref: ty::TraitRef {
def_id: drop_trait,
def_id: destruct,
substs: cx.tcx.mk_substs_trait(ty, &[]),
},
constness: ty::BoundConstness::ConstIfConst,
Expand All @@ -174,15 +171,16 @@ impl Qualif for NeedsNonConstDrop {
cx.tcx.infer_ctxt().enter(|infcx| {
let mut selcx = SelectionContext::new(&infcx);
let Some(impl_src) = selcx.select(&obligation).ok().flatten() else {
// If we couldn't select a const drop candidate, then it's bad
// If we couldn't select a const destruct candidate, then it's bad
return true;
};

if !matches!(
impl_src,
ImplSource::ConstDrop(_) | ImplSource::Param(_, ty::BoundConstness::ConstIfConst)
ImplSource::ConstDestruct(_)
| ImplSource::Param(_, ty::BoundConstness::ConstIfConst)
) {
// If our const drop candidate is not ConstDrop or implied by the param env,
// If our const destruct candidate is not ConstDestruct or implied by the param env,
// then it's bad
return true;
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_hir/src/lang_items.rs
Expand Up @@ -216,6 +216,7 @@ language_item_table! {
Freeze, sym::freeze, freeze_trait, Target::Trait, GenericRequirement::Exact(0);

Drop, sym::drop, drop_trait, Target::Trait, GenericRequirement::None;
Destruct, sym::destruct, destruct_trait, Target::Trait, GenericRequirement::None;

CoerceUnsized, sym::coerce_unsized, coerce_unsized_trait, Target::Trait, GenericRequirement::Minimum(1);
DispatchFromDyn, sym::dispatch_from_dyn, dispatch_from_dyn_trait, Target::Trait, GenericRequirement::Minimum(1);
Expand Down
4 changes: 0 additions & 4 deletions compiler/rustc_lint/src/traits.rs
Expand Up @@ -93,10 +93,6 @@ impl<'tcx> LateLintPass<'tcx> for DropTraitConstraints {
let Trait(trait_predicate) = predicate.kind().skip_binder() else {
continue
};
if trait_predicate.is_const_if_const() {
// `~const Drop` definitely have meanings so avoid linting here.
continue;
}
let def_id = trait_predicate.trait_ref.def_id;
if cx.tcx.lang_items().drop_trait() == Some(def_id) {
// Explicitly allow `impl Drop`, a drop-guards-as-Voldemort-type pattern.
Expand Down
16 changes: 9 additions & 7 deletions compiler/rustc_middle/src/traits/mod.rs
Expand Up @@ -577,7 +577,7 @@ pub enum ImplSource<'tcx, N> {
TraitAlias(ImplSourceTraitAliasData<'tcx, N>),

/// ImplSource for a `const Drop` implementation.
ConstDrop(ImplSourceConstDropData<N>),
ConstDestruct(ImplSourceConstDestructData<N>),
}

impl<'tcx, N> ImplSource<'tcx, N> {
Expand All @@ -595,7 +595,7 @@ impl<'tcx, N> ImplSource<'tcx, N> {
| ImplSource::Pointee(ImplSourcePointeeData) => Vec::new(),
ImplSource::TraitAlias(d) => d.nested,
ImplSource::TraitUpcasting(d) => d.nested,
ImplSource::ConstDrop(i) => i.nested,
ImplSource::ConstDestruct(i) => i.nested,
}
}

Expand All @@ -613,7 +613,7 @@ impl<'tcx, N> ImplSource<'tcx, N> {
| ImplSource::Pointee(ImplSourcePointeeData) => &[],
ImplSource::TraitAlias(d) => &d.nested,
ImplSource::TraitUpcasting(d) => &d.nested,
ImplSource::ConstDrop(i) => &i.nested,
ImplSource::ConstDestruct(i) => &i.nested,
}
}

Expand Down Expand Up @@ -672,9 +672,11 @@ impl<'tcx, N> ImplSource<'tcx, N> {
nested: d.nested.into_iter().map(f).collect(),
})
}
ImplSource::ConstDrop(i) => ImplSource::ConstDrop(ImplSourceConstDropData {
nested: i.nested.into_iter().map(f).collect(),
}),
ImplSource::ConstDestruct(i) => {
ImplSource::ConstDestruct(ImplSourceConstDestructData {
nested: i.nested.into_iter().map(f).collect(),
})
}
}
}
}
Expand Down Expand Up @@ -767,7 +769,7 @@ pub struct ImplSourceDiscriminantKindData;
pub struct ImplSourcePointeeData;

#[derive(Clone, PartialEq, Eq, TyEncodable, TyDecodable, HashStable, TypeFoldable, Lift)]
pub struct ImplSourceConstDropData<N> {
pub struct ImplSourceConstDestructData<N> {
pub nested: Vec<N>,
}

Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_middle/src/traits/select.rs
Expand Up @@ -146,8 +146,8 @@ pub enum SelectionCandidate<'tcx> {

BuiltinUnsizeCandidate,

/// Implementation of `const Drop`, optionally from a custom `impl const Drop`.
ConstDropCandidate(Option<DefId>),
/// Implementation of `const Destruct`, optionally from a custom `impl const Drop`.
ConstDestructCandidate(Option<DefId>),
}

/// The result of trait evaluation. The order is important
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_middle/src/traits/structural_impls.rs
Expand Up @@ -33,7 +33,7 @@ impl<'tcx, N: fmt::Debug> fmt::Debug for traits::ImplSource<'tcx, N> {

super::ImplSource::TraitUpcasting(ref d) => write!(f, "{:?}", d),

super::ImplSource::ConstDrop(ref d) => write!(f, "{:?}", d),
super::ImplSource::ConstDestruct(ref d) => write!(f, "{:?}", d),
}
}
}
Expand Down Expand Up @@ -120,9 +120,9 @@ impl<'tcx, N: fmt::Debug> fmt::Debug for traits::ImplSourceTraitAliasData<'tcx,
}
}

impl<N: fmt::Debug> fmt::Debug for traits::ImplSourceConstDropData<N> {
impl<N: fmt::Debug> fmt::Debug for traits::ImplSourceConstDestructData<N> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "ImplSourceConstDropData(nested={:?})", self.nested)
write!(f, "ImplSourceConstDestructData(nested={:?})", self.nested)
}
}

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/ty/mod.rs
Expand Up @@ -765,6 +765,7 @@ impl<'tcx> TraitPredicate<'tcx> {
if unlikely!(Some(self.trait_ref.def_id) == tcx.lang_items().drop_trait()) {
// remap without changing constness of this predicate.
// this is because `T: ~const Drop` has a different meaning to `T: Drop`
// FIXME(fee1-dead): remove this logic after beta bump
param_env.remap_constness_with(self.constness)
} else {
*param_env = param_env.with_constness(self.constness.and(param_env.constness()))
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Expand Up @@ -571,6 +571,7 @@ symbols! {
deref_target,
derive,
derive_default_enum,
destruct,
destructuring_assignment,
diagnostic,
direct,
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_trait_selection/src/traits/project.rs
Expand Up @@ -1569,7 +1569,7 @@ fn assemble_candidates_from_impls<'cx, 'tcx>(
super::ImplSource::AutoImpl(..)
| super::ImplSource::Builtin(..)
| super::ImplSource::TraitUpcasting(_)
| super::ImplSource::ConstDrop(_) => {
| super::ImplSource::ConstDestruct(_) => {
// These traits have no associated types.
selcx.tcx().sess.delay_span_bug(
obligation.cause.span,
Expand Down Expand Up @@ -1644,7 +1644,7 @@ fn confirm_select_candidate<'cx, 'tcx>(
| super::ImplSource::Builtin(..)
| super::ImplSource::TraitUpcasting(_)
| super::ImplSource::TraitAlias(..)
| super::ImplSource::ConstDrop(_) => {
| super::ImplSource::ConstDestruct(_) => {
// we don't create Select candidates with this kind of resolution
span_bug!(
obligation.cause.span,
Expand Down
Expand Up @@ -5,6 +5,7 @@
//! candidates. See the [rustc dev guide] for more details.
//!
//! [rustc dev guide]:https://rustc-dev-guide.rust-lang.org/traits/resolution.html#candidate-assembly
use hir::LangItem;
use rustc_hir as hir;
use rustc_hir::def_id::DefId;
use rustc_infer::traits::TraitEngine;
Expand Down Expand Up @@ -307,7 +308,16 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
} else if lang_items.drop_trait() == Some(def_id)
&& obligation.predicate.is_const_if_const()
{
self.assemble_const_drop_candidates(obligation, &mut candidates);
// holds to make it easier to transition
// FIXME(fee1-dead): add a note for selection error of `~const Drop`
// when beta is bumped
// FIXME: remove this when beta is bumped
#[cfg(bootstrap)]
{}

candidates.vec.push(SelectionCandidate::ConstDestructCandidate(None))
} else if lang_items.destruct_trait() == Some(def_id) {
self.assemble_const_destruct_candidates(obligation, &mut candidates);
} else {
if lang_items.clone_trait() == Some(def_id) {
// Same builtin conditions as `Copy`, i.e., every type which has builtin support
Expand Down Expand Up @@ -906,15 +916,15 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
}
}

fn assemble_const_drop_candidates(
fn assemble_const_destruct_candidates(
&mut self,
obligation: &TraitObligation<'tcx>,
candidates: &mut SelectionCandidateSet<'tcx>,
) {
// If the predicate is `~const Drop` in a non-const environment, we don't actually need
// If the predicate is `~const Destruct` in a non-const environment, we don't actually need
// to check anything. We'll short-circuit checking any obligations in confirmation, too.
if obligation.param_env.constness() == hir::Constness::NotConst {
candidates.vec.push(ConstDropCandidate(None));
if !obligation.is_const() {
candidates.vec.push(ConstDestructCandidate(None));
return;
}

Expand All @@ -927,7 +937,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
| ty::Param(_)
| ty::Placeholder(_)
| ty::Projection(_) => {
// We don't know if these are `~const Drop`, at least
// We don't know if these are `~const Destruct`, at least
// not structurally... so don't push a candidate.
}

Expand All @@ -951,26 +961,26 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
| ty::Generator(..)
| ty::Tuple(_)
| ty::GeneratorWitness(_) => {
// These are built-in, and cannot have a custom `impl const Drop`.
candidates.vec.push(ConstDropCandidate(None));
// These are built-in, and cannot have a custom `impl const Destruct`.
candidates.vec.push(ConstDestructCandidate(None));
}

ty::Adt(..) => {
// Find a custom `impl Drop` impl, if it exists
let relevant_impl = self.tcx().find_map_relevant_impl(
obligation.predicate.def_id(),
self.tcx().require_lang_item(LangItem::Drop, None),
obligation.predicate.skip_binder().trait_ref.self_ty(),
Some,
);

if let Some(impl_def_id) = relevant_impl {
// Check that `impl Drop` is actually const, if there is a custom impl
if self.tcx().impl_constness(impl_def_id) == hir::Constness::Const {
candidates.vec.push(ConstDropCandidate(Some(impl_def_id)));
candidates.vec.push(ConstDestructCandidate(Some(impl_def_id)));
}
} else {
// Otherwise check the ADT like a built-in type (structurally)
candidates.vec.push(ConstDropCandidate(None));
candidates.vec.push(ConstDestructCandidate(None));
}
}

Expand Down
64 changes: 47 additions & 17 deletions compiler/rustc_trait_selection/src/traits/select/confirmation.rs
Expand Up @@ -8,7 +8,6 @@
//! https://rustc-dev-guide.rust-lang.org/traits/resolution.html#confirmation
use rustc_data_structures::stack::ensure_sufficient_stack;
use rustc_hir::lang_items::LangItem;
use rustc_hir::Constness;
use rustc_index::bit_set::GrowableBitSet;
use rustc_infer::infer::InferOk;
use rustc_infer::infer::LateBoundRegionConversionTime::HigherRankedType;
Expand All @@ -29,9 +28,9 @@ use crate::traits::TraitNotObjectSafe;
use crate::traits::VtblSegment;
use crate::traits::{BuiltinDerivedObligation, ImplDerivedObligation};
use crate::traits::{
ImplSourceAutoImplData, ImplSourceBuiltinData, ImplSourceClosureData, ImplSourceConstDropData,
ImplSourceDiscriminantKindData, ImplSourceFnPointerData, ImplSourceGeneratorData,
ImplSourceObjectData, ImplSourcePointeeData, ImplSourceTraitAliasData,
ImplSourceAutoImplData, ImplSourceBuiltinData, ImplSourceClosureData,
ImplSourceConstDestructData, ImplSourceDiscriminantKindData, ImplSourceFnPointerData,
ImplSourceGeneratorData, ImplSourceObjectData, ImplSourcePointeeData, ImplSourceTraitAliasData,
ImplSourceTraitUpcastingData, ImplSourceUserDefinedData,
};
use crate::traits::{ObjectCastObligation, PredicateObligation, TraitObligation};
Expand Down Expand Up @@ -156,9 +155,9 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
Ok(ImplSource::TraitUpcasting(data))
}

ConstDropCandidate(def_id) => {
let data = self.confirm_const_drop_candidate(obligation, def_id)?;
Ok(ImplSource::ConstDrop(data))
ConstDestructCandidate(def_id) => {
let data = self.confirm_const_destruct_candidate(obligation, def_id)?;
Ok(ImplSource::ConstDestruct(data))
}
}
}
Expand Down Expand Up @@ -1037,14 +1036,23 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
Ok(ImplSourceBuiltinData { nested })
}

fn confirm_const_drop_candidate(
fn confirm_const_destruct_candidate(
&mut self,
obligation: &TraitObligation<'tcx>,
impl_def_id: Option<DefId>,
) -> Result<ImplSourceConstDropData<PredicateObligation<'tcx>>, SelectionError<'tcx>> {
// `~const Drop` in a non-const environment is always trivially true, since our type is `Drop`
if obligation.param_env.constness() == Constness::NotConst {
return Ok(ImplSourceConstDropData { nested: vec![] });
) -> Result<ImplSourceConstDestructData<PredicateObligation<'tcx>>, SelectionError<'tcx>> {
// `~const Destruct` in a non-const environment is always trivially true, since our type is `Drop`
if !obligation.is_const() {
return Ok(ImplSourceConstDestructData { nested: vec![] });
}

let drop_trait = self.tcx().require_lang_item(LangItem::Drop, None);
// FIXME: remove if statement below when beta is bumped
#[cfg(bootstrap)]
{}

if obligation.predicate.skip_binder().def_id() == drop_trait {
return Ok(ImplSourceConstDestructData { nested: vec![] });
}

let tcx = self.tcx();
Expand All @@ -1054,9 +1062,29 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
let cause = obligation.derived_cause(BuiltinDerivedObligation);

// If we have a custom `impl const Drop`, then
// first check it like a regular impl candidate
// first check it like a regular impl candidate.
// This is copied from confirm_impl_candidate but remaps the predicate to `~const Drop` beforehand.
if let Some(impl_def_id) = impl_def_id {
nested.extend(self.confirm_impl_candidate(obligation, impl_def_id).nested);
let obligations = self.infcx.commit_unconditionally(|_| {
let mut new_obligation = obligation.clone();
new_obligation.predicate = new_obligation.predicate.map_bound(|mut trait_pred| {
trait_pred.trait_ref.def_id = drop_trait;
trait_pred
});
let substs = self.rematch_impl(impl_def_id, &new_obligation);
debug!(?substs, "impl substs");
let cause = obligation.derived_cause(ImplDerivedObligation);
ensure_sufficient_stack(|| {
self.vtable_impl(
impl_def_id,
substs,
cause,
new_obligation.recursion_depth + 1,
new_obligation.param_env,
)
})
});
nested.extend(obligations.nested);
}

// We want to confirm the ADT's fields if we have an ADT
Expand Down Expand Up @@ -1114,7 +1142,9 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
self_ty
.rebind(ty::TraitPredicate {
trait_ref: ty::TraitRef {
def_id: self.tcx().require_lang_item(LangItem::Drop, None),
def_id: self
.tcx()
.require_lang_item(LangItem::Destruct, None),
substs: self.tcx().mk_substs_trait(nested_ty, &[]),
},
constness: ty::BoundConstness::ConstIfConst,
Expand All @@ -1140,7 +1170,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
let predicate = self_ty
.rebind(ty::TraitPredicate {
trait_ref: ty::TraitRef {
def_id: self.tcx().require_lang_item(LangItem::Drop, None),
def_id: self.tcx().require_lang_item(LangItem::Destruct, None),
substs: self.tcx().mk_substs_trait(nested_ty, &[]),
},
constness: ty::BoundConstness::ConstIfConst,
Expand All @@ -1158,6 +1188,6 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
}
}

Ok(ImplSourceConstDropData { nested })
Ok(ImplSourceConstDestructData { nested })
}
}

0 comments on commit 9280445

Please sign in to comment.