Skip to content

Commit

Permalink
Auto merge of rust-lang#88558 - fee1-dead:const-drop, r=oli-obk
Browse files Browse the repository at this point in the history
Const drop

The changes are pretty primitive at this point. But at least it works. ^-^

Problems with the current change that I can think of now:
 - [x] `~const Drop` shouldn't change anything in the non-const world.
 - [x] types that do not have drop glues shouldn't fail to satisfy `~const Drop` in const contexts. `struct S { a: u8, b: u16 }` This might not fail for `needs_non_const_drop`, but it will fail in `rustc_trait_selection`.
 - [x] The current change accepts types that have `const Drop` impls but have non-const `Drop` glue.

Fixes rust-lang#88424.

Significant Changes:

- `~const Drop` is no longer treated as a normal trait bound. In non-const contexts, this bound has no effect, but in const contexts, this restricts the input type and all of its transitive fields to either a) have a `const Drop` impl or b) can be trivially dropped (i.e. no drop glue)
- `T: ~const Drop` will not be linted like `T: Drop`.
- Instead of recursing and iterating through the type in `rustc_mir::transform::check_consts`, we use the trait system to special case `~const Drop`. See [`rustc_trait_selection::...::candidate_assembly#assemble_const_drop_candidates`](https://github.com/fee1-dead/rust/blob/const-drop/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs#L817) and others.

Changes not related to `const Drop`ping and/or changes that are insignificant:

 - `Node.constness_for_typeck` no longer returns `hir::Constness::Const` for type aliases in traits. This was previously used to hack how we determine default bound constness for items. But because we now use an explicit opt-in, it is no longer needed.
 - Removed `is_const_impl_raw` query. We have `impl_constness`, and the only existing use of that query uses `HirId`, which means we can just operate it with hir.
 - `ty::Destructor` now has a field `constness`, which represents the constness of the destructor.

r? `@oli-obk`
  • Loading branch information
bors committed Sep 15, 2021
2 parents c3c0f80 + f749e05 commit cdeba02
Show file tree
Hide file tree
Showing 26 changed files with 552 additions and 106 deletions.
4 changes: 3 additions & 1 deletion compiler/rustc_ast_passes/src/ast_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1669,7 +1669,9 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
walk_list!(self, visit_ty, ty);
}
AssocItemKind::Fn(box FnKind(_, ref sig, ref generics, ref body))
if self.in_const_trait_impl || ctxt == AssocCtxt::Trait =>
if self.in_const_trait_impl
|| ctxt == AssocCtxt::Trait
|| matches!(sig.header.constness, Const::Yes(_)) =>
{
self.visit_vis(&item.vis);
self.visit_ident(item.ident);
Expand Down
32 changes: 10 additions & 22 deletions compiler/rustc_const_eval/src/const_eval/fn_queries.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use rustc_hir as hir;
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_hir::def_id::DefId;
use rustc_middle::hir::map::blocks::FnLikeNode;
use rustc_middle::ty::query::Providers;
use rustc_middle::ty::TyCtxt;
Expand Down Expand Up @@ -34,8 +34,14 @@ pub fn is_unstable_const_fn(tcx: TyCtxt<'_>, def_id: DefId) -> Option<Symbol> {
}

pub fn is_parent_const_impl_raw(tcx: TyCtxt<'_>, hir_id: hir::HirId) -> bool {
let parent_id = tcx.hir().get_parent_did(hir_id);
if !parent_id.is_top_level_module() { is_const_impl_raw(tcx, parent_id) } else { false }
let parent_id = tcx.hir().get_parent_node(hir_id);
matches!(
tcx.hir().get(parent_id),
hir::Node::Item(hir::Item {
kind: hir::ItemKind::Impl(hir::Impl { constness: hir::Constness::Const, .. }),
..
})
)
}

/// Checks whether the function has a `const` modifier or, in case it is an intrinsic, whether
Expand Down Expand Up @@ -70,19 +76,6 @@ fn is_const_fn_raw(tcx: TyCtxt<'_>, def_id: DefId) -> bool {
}
}

/// Checks whether the given item is an `impl` that has a `const` modifier.
fn is_const_impl_raw(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
let hir_id = tcx.hir().local_def_id_to_hir_id(def_id);
let node = tcx.hir().get(hir_id);
matches!(
node,
hir::Node::Item(hir::Item {
kind: hir::ItemKind::Impl(hir::Impl { constness: hir::Constness::Const, .. }),
..
})
)
}

fn is_promotable_const_fn(tcx: TyCtxt<'_>, def_id: DefId) -> bool {
is_const_fn(tcx, def_id)
&& match tcx.lookup_const_stability(def_id) {
Expand All @@ -103,10 +96,5 @@ fn is_promotable_const_fn(tcx: TyCtxt<'_>, def_id: DefId) -> bool {
}

pub fn provide(providers: &mut Providers) {
*providers = Providers {
is_const_fn_raw,
is_const_impl_raw: |tcx, def_id| is_const_impl_raw(tcx, def_id.expect_local()),
is_promotable_const_fn,
..*providers
};
*providers = Providers { is_const_fn_raw, is_promotable_const_fn, ..*providers };
}
18 changes: 9 additions & 9 deletions compiler/rustc_const_eval/src/transform/check_consts/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use std::mem;
use std::ops::Deref;

use super::ops::{self, NonConstOp, Status};
use super::qualifs::{self, CustomEq, HasMutInterior, NeedsDrop};
use super::qualifs::{self, CustomEq, HasMutInterior, NeedsNonConstDrop};
use super::resolver::FlowSensitiveAnalysis;
use super::{is_lang_panic_fn, ConstCx, Qualif};
use crate::const_eval::is_unstable_const_fn;
Expand All @@ -39,7 +39,7 @@ type QualifResults<'mir, 'tcx, Q> =
#[derive(Default)]
pub struct Qualifs<'mir, 'tcx> {
has_mut_interior: Option<QualifResults<'mir, 'tcx, HasMutInterior>>,
needs_drop: Option<QualifResults<'mir, 'tcx, NeedsDrop>>,
needs_drop: Option<QualifResults<'mir, 'tcx, NeedsNonConstDrop>>,
indirectly_mutable: Option<IndirectlyMutableResults<'mir, 'tcx>>,
}

Expand Down Expand Up @@ -80,14 +80,14 @@ impl Qualifs<'mir, 'tcx> {
location: Location,
) -> bool {
let ty = ccx.body.local_decls[local].ty;
if !NeedsDrop::in_any_value_of_ty(ccx, ty) {
if !NeedsNonConstDrop::in_any_value_of_ty(ccx, ty) {
return false;
}

let needs_drop = self.needs_drop.get_or_insert_with(|| {
let ConstCx { tcx, body, .. } = *ccx;

FlowSensitiveAnalysis::new(NeedsDrop, ccx)
FlowSensitiveAnalysis::new(NeedsNonConstDrop, ccx)
.into_engine(tcx, &body)
.iterate_to_fixpoint()
.into_results_cursor(&body)
Expand Down Expand Up @@ -988,12 +988,12 @@ impl Visitor<'tcx> for Checker<'mir, 'tcx> {

let mut err_span = self.span;

// Check to see if the type of this place can ever have a drop impl. If not, this
// `Drop` terminator is frivolous.
let ty_needs_drop =
dropped_place.ty(self.body, self.tcx).ty.needs_drop(self.tcx, self.param_env);
let ty_needs_non_const_drop = qualifs::NeedsNonConstDrop::in_any_value_of_ty(
self.ccx,
dropped_place.ty(self.body, self.tcx).ty,
);

if !ty_needs_drop {
if !ty_needs_non_const_drop {
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use rustc_span::Span;

use super::check::Qualifs;
use super::ops::{self, NonConstOp};
use super::qualifs::{NeedsDrop, Qualif};
use super::qualifs::{NeedsNonConstDrop, Qualif};
use super::ConstCx;

/// Returns `true` if we should use the more precise live drop checker that runs after drop
Expand Down Expand Up @@ -78,10 +78,10 @@ impl Visitor<'tcx> for CheckLiveDrops<'mir, 'tcx> {
match &terminator.kind {
mir::TerminatorKind::Drop { place: dropped_place, .. } => {
let dropped_ty = dropped_place.ty(self.body, self.tcx).ty;
if !NeedsDrop::in_any_value_of_ty(self.ccx, dropped_ty) {
bug!(
"Drop elaboration left behind a Drop for a type that does not need dropping"
);
if !NeedsNonConstDrop::in_any_value_of_ty(self.ccx, dropped_ty) {
// Instead of throwing a bug, we just return here. This is because we have to
// run custom `const Drop` impls.
return;
}

if dropped_place.is_indirect() {
Expand Down
44 changes: 37 additions & 7 deletions compiler/rustc_const_eval/src/transform/check_consts/qualifs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,14 @@
//! See the `Qualif` trait for more info.

use rustc_errors::ErrorReported;
use rustc_hir as hir;
use rustc_infer::infer::TyCtxtInferExt;
use rustc_middle::mir::*;
use rustc_middle::ty::{self, subst::SubstsRef, AdtDef, Ty};
use rustc_span::DUMMY_SP;
use rustc_trait_selection::traits;
use rustc_trait_selection::traits::{
self, ImplSource, Obligation, ObligationCause, SelectionContext,
};

use super::ConstCx;

Expand All @@ -17,7 +21,7 @@ pub fn in_any_value_of_ty(
) -> ConstQualifs {
ConstQualifs {
has_mut_interior: HasMutInterior::in_any_value_of_ty(cx, ty),
needs_drop: NeedsDrop::in_any_value_of_ty(cx, ty),
needs_drop: NeedsNonConstDrop::in_any_value_of_ty(cx, ty),
custom_eq: CustomEq::in_any_value_of_ty(cx, ty),
error_occured,
}
Expand Down Expand Up @@ -97,22 +101,48 @@ impl Qualif for HasMutInterior {
/// This must be ruled out (a) because we cannot run `Drop` during compile-time
/// as that might not be a `const fn`, and (b) because implicit promotion would
/// remove side-effects that occur as part of dropping that value.
pub struct NeedsDrop;
pub struct NeedsNonConstDrop;

impl Qualif for NeedsDrop {
const ANALYSIS_NAME: &'static str = "flow_needs_drop";
impl Qualif for NeedsNonConstDrop {
const ANALYSIS_NAME: &'static str = "flow_needs_nonconst_drop";
const IS_CLEARED_ON_MOVE: bool = true;

fn in_qualifs(qualifs: &ConstQualifs) -> bool {
qualifs.needs_drop
}

fn in_any_value_of_ty(cx: &ConstCx<'_, 'tcx>, ty: Ty<'tcx>) -> bool {
ty.needs_drop(cx.tcx, cx.param_env)
let drop_trait = if let Some(did) = cx.tcx.lang_items().drop_trait() {
did
} else {
// there is no way to define a type that needs non-const drop
// without having the lang item present.
return false;
};
let trait_ref =
ty::TraitRef { def_id: drop_trait, substs: cx.tcx.mk_substs_trait(ty, &[]) };
let obligation = Obligation::new(
ObligationCause::dummy(),
cx.param_env,
ty::Binder::dummy(ty::TraitPredicate {
trait_ref,
constness: ty::BoundConstness::ConstIfConst,
}),
);

let implsrc = cx.tcx.infer_ctxt().enter(|infcx| {
let mut selcx = SelectionContext::with_constness(&infcx, hir::Constness::Const);
selcx.select(&obligation)
});
match implsrc {
Ok(Some(ImplSource::ConstDrop(_)))
| Ok(Some(ImplSource::Param(_, ty::BoundConstness::ConstIfConst))) => false,
_ => true,
}
}

fn in_adt_inherently(cx: &ConstCx<'_, 'tcx>, adt: &'tcx AdtDef, _: SubstsRef<'tcx>) -> bool {
adt.has_dtor(cx.tcx)
adt.has_non_const_dtor(cx.tcx)
}
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/transform/promote_consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ impl<'tcx> Validator<'_, 'tcx> {

// We cannot promote things that need dropping, since the promoted value
// would not get dropped.
if self.qualif_local::<qualifs::NeedsDrop>(place.local) {
if self.qualif_local::<qualifs::NeedsNonConstDrop>(place.local) {
return Err(Unpromotable);
}

Expand Down
8 changes: 1 addition & 7 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3234,12 +3234,7 @@ impl<'hir> Node<'hir> {
}
}

/// Returns `Constness::Const` when this node is a const fn/impl/item,
///
/// HACK(fee1-dead): or an associated type in a trait. This works because
/// only typeck cares about const trait predicates, so although the predicates
/// query would return const predicates when it does not need to be const,
/// it wouldn't have any effect.
/// Returns `Constness::Const` when this node is a const fn/impl/item.
pub fn constness_for_typeck(&self) -> Constness {
match self {
Node::Item(Item {
Expand All @@ -3258,7 +3253,6 @@ impl<'hir> Node<'hir> {

Node::Item(Item { kind: ItemKind::Const(..), .. })
| Node::TraitItem(TraitItem { kind: TraitItemKind::Const(..), .. })
| Node::TraitItem(TraitItem { kind: TraitItemKind::Type(..), .. })
| Node::ImplItem(ImplItem { kind: ImplItemKind::Const(..), .. }) => Constness::Const,

_ => Constness::NotConst,
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_lint/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ declare_lint_pass!(

impl<'tcx> LateLintPass<'tcx> for DropTraitConstraints {
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'tcx>) {
use rustc_middle::ty;
use rustc_middle::ty::PredicateKind::*;

let predicates = cx.tcx.explicit_predicates_of(item.def_id);
Expand All @@ -94,6 +95,10 @@ impl<'tcx> LateLintPass<'tcx> for DropTraitConstraints {
Trait(trait_predicate) => trait_predicate,
_ => continue,
};
if trait_predicate.constness == ty::BoundConstness::ConstIfConst {
// `~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
8 changes: 0 additions & 8 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -552,14 +552,6 @@ rustc_queries! {
desc { |tcx| "checking if item is const fn: `{}`", tcx.def_path_str(key) }
}

/// Returns `true` if this is a const `impl`. **Do not call this function manually.**
///
/// This query caches the base data for the `is_const_impl` helper function, which also
/// takes into account stability attributes (e.g., `#[rustc_const_unstable]`).
query is_const_impl_raw(key: DefId) -> bool {
desc { |tcx| "checking if item is const impl: `{}`", tcx.def_path_str(key) }
}

query asyncness(key: DefId) -> hir::IsAsync {
desc { |tcx| "checking if the function is async: `{}`", tcx.def_path_str(key) }
}
Expand Down
15 changes: 13 additions & 2 deletions compiler/rustc_middle/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -529,6 +529,9 @@ pub enum ImplSource<'tcx, N> {

/// ImplSource for a trait alias.
TraitAlias(ImplSourceTraitAliasData<'tcx, N>),

/// ImplSource for a `const Drop` implementation.
ConstDrop(ImplSourceConstDropData),
}

impl<'tcx, N> ImplSource<'tcx, N> {
Expand All @@ -543,7 +546,8 @@ impl<'tcx, N> ImplSource<'tcx, N> {
ImplSource::Object(d) => d.nested,
ImplSource::FnPointer(d) => d.nested,
ImplSource::DiscriminantKind(ImplSourceDiscriminantKindData)
| ImplSource::Pointee(ImplSourcePointeeData) => Vec::new(),
| ImplSource::Pointee(ImplSourcePointeeData)
| ImplSource::ConstDrop(ImplSourceConstDropData) => Vec::new(),
ImplSource::TraitAlias(d) => d.nested,
ImplSource::TraitUpcasting(d) => d.nested,
}
Expand All @@ -560,7 +564,8 @@ impl<'tcx, N> ImplSource<'tcx, N> {
ImplSource::Object(d) => &d.nested[..],
ImplSource::FnPointer(d) => &d.nested[..],
ImplSource::DiscriminantKind(ImplSourceDiscriminantKindData)
| ImplSource::Pointee(ImplSourcePointeeData) => &[],
| ImplSource::Pointee(ImplSourcePointeeData)
| ImplSource::ConstDrop(ImplSourceConstDropData) => &[],
ImplSource::TraitAlias(d) => &d.nested[..],
ImplSource::TraitUpcasting(d) => &d.nested[..],
}
Expand Down Expand Up @@ -621,6 +626,9 @@ impl<'tcx, N> ImplSource<'tcx, N> {
nested: d.nested.into_iter().map(f).collect(),
})
}
ImplSource::ConstDrop(ImplSourceConstDropData) => {
ImplSource::ConstDrop(ImplSourceConstDropData)
}
}
}
}
Expand Down Expand Up @@ -712,6 +720,9 @@ pub struct ImplSourceDiscriminantKindData;
#[derive(Clone, Debug, PartialEq, Eq, TyEncodable, TyDecodable, HashStable)]
pub struct ImplSourcePointeeData;

#[derive(Clone, Debug, PartialEq, Eq, TyEncodable, TyDecodable, HashStable)]
pub struct ImplSourceConstDropData;

#[derive(Clone, PartialEq, Eq, TyEncodable, TyDecodable, HashStable, TypeFoldable, Lift)]
pub struct ImplSourceTraitAliasData<'tcx, N> {
pub alias_def_id: DefId,
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_middle/src/traits/select.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,9 @@ pub enum SelectionCandidate<'tcx> {
BuiltinObjectCandidate,

BuiltinUnsizeCandidate,

/// Implementation of `const Drop`.
ConstDropCandidate,
}

/// The result of trait evaluation. The order is important
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_middle/src/traits/structural_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ impl<'tcx, N: fmt::Debug> fmt::Debug for traits::ImplSource<'tcx, N> {
super::ImplSource::TraitAlias(ref d) => write!(f, "{:?}", d),

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

super::ImplSource::ConstDrop(ref d) => write!(f, "{:?}", d),
}
}
}
Expand Down Expand Up @@ -125,4 +127,5 @@ TrivialTypeFoldableAndLiftImpls! {
super::IfExpressionCause,
super::ImplSourceDiscriminantKindData,
super::ImplSourcePointeeData,
super::ImplSourceConstDropData,
}
5 changes: 5 additions & 0 deletions compiler/rustc_middle/src/ty/adt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use rustc_data_structures::fingerprint::Fingerprint;
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
use rustc_errors::ErrorReported;
use rustc_hir as hir;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::def_id::DefId;
use rustc_index::vec::{Idx, IndexVec};
Expand Down Expand Up @@ -288,6 +289,10 @@ impl<'tcx> AdtDef {
self.destructor(tcx).is_some()
}

pub fn has_non_const_dtor(&self, tcx: TyCtxt<'tcx>) -> bool {
matches!(self.destructor(tcx), Some(Destructor { constness: hir::Constness::NotConst, .. }))
}

/// Asserts this is a struct or union and returns its unique variant.
pub fn non_enum_variant(&self) -> &VariantDef {
assert!(self.is_struct() || self.is_union());
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1377,6 +1377,8 @@ where
pub struct Destructor {
/// The `DefId` of the destructor method
pub did: DefId,
/// The constness of the destructor method
pub constness: hir::Constness,
}

bitflags! {
Expand Down
Loading

0 comments on commit cdeba02

Please sign in to comment.