Skip to content

Fix const drop checking #118689

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 1 commit into from
Dec 8, 2023
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
23 changes: 10 additions & 13 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::{ControlFlow, Deref};

use super::ops::{self, NonConstOp, Status};
use super::qualifs::{self, CustomEq, HasMutInterior, NeedsDrop};
use super::qualifs::{self, CustomEq, HasMutInterior, NeedsDrop, NeedsNonConstDrop};
use super::resolver::FlowSensitiveAnalysis;
use super::{ConstCx, Qualif};
use crate::const_eval::is_unstable_const_fn;
Expand All @@ -35,7 +35,7 @@ type QualifResults<'mir, 'tcx, Q> =
pub struct Qualifs<'mir, 'tcx> {
has_mut_interior: Option<QualifResults<'mir, 'tcx, HasMutInterior>>,
needs_drop: Option<QualifResults<'mir, 'tcx, NeedsDrop>>,
// needs_non_const_drop: Option<QualifResults<'mir, 'tcx, NeedsNonConstDrop>>,
needs_non_const_drop: Option<QualifResults<'mir, 'tcx, NeedsNonConstDrop>>,
}

impl<'mir, 'tcx> Qualifs<'mir, 'tcx> {
Expand Down Expand Up @@ -78,27 +78,25 @@ impl<'mir, 'tcx> Qualifs<'mir, 'tcx> {
local: Local,
location: Location,
) -> bool {
// FIXME(effects) replace with `NeedsNonconstDrop` after const traits work again
/*
let ty = ccx.body.local_decls[local].ty;
if !NeedsDrop::in_any_value_of_ty(ccx, ty) {
// Peeking into opaque types causes cycles if the current function declares said opaque
// type. Thus we avoid short circuiting on the type and instead run the more expensive
// analysis that looks at the actual usage within this function
if !ty.has_opaque_types() && !NeedsNonConstDrop::in_any_value_of_ty(ccx, ty) {
return false;
}

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

FlowSensitiveAnalysis::new(NeedsDrop, ccx)
.into_engine(tcx, &body)
FlowSensitiveAnalysis::new(NeedsNonConstDrop, ccx)
.into_engine(tcx, body)
.iterate_to_fixpoint()
.into_results_cursor(&body)
.into_results_cursor(body)
});

needs_non_const_drop.seek_before_primary_effect(location);
needs_non_const_drop.get().contains(local)
*/

self.needs_drop(ccx, local, location)
}

/// Returns `true` if `local` is `HasMutInterior` at the given `Location`.
Expand Down Expand Up @@ -1013,9 +1011,8 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
let mut err_span = self.span;
let ty_of_dropped_place = dropped_place.ty(self.body, self.tcx).ty;

// FIXME(effects) replace with `NeedsNonConstDrop` once we fix const traits
let ty_needs_non_const_drop =
qualifs::NeedsDrop::in_any_value_of_ty(self.ccx, ty_of_dropped_place);
qualifs::NeedsNonConstDrop::in_any_value_of_ty(self.ccx, ty_of_dropped_place);

debug!(?ty_of_dropped_place, ?ty_needs_non_const_drop);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use rustc_span::{symbol::sym, 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 @@ -83,8 +83,7 @@ impl<'tcx> Visitor<'tcx> for CheckLiveDrops<'_, 'tcx> {
mir::TerminatorKind::Drop { place: dropped_place, .. } => {
let dropped_ty = dropped_place.ty(self.body, self.tcx).ty;

// FIXME(effects) use `NeedsNonConstDrop`
if !NeedsDrop::in_any_value_of_ty(self.ccx, dropped_ty) {
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;
Expand Down
22 changes: 18 additions & 4 deletions compiler/rustc_const_eval/src/transform/check_consts/qualifs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@ pub fn in_any_value_of_ty<'tcx>(
ConstQualifs {
has_mut_interior: HasMutInterior::in_any_value_of_ty(cx, ty),
needs_drop: NeedsDrop::in_any_value_of_ty(cx, ty),
// FIXME(effects)
needs_non_const_drop: NeedsDrop::in_any_value_of_ty(cx, ty),
needs_non_const_drop: NeedsNonConstDrop::in_any_value_of_ty(cx, ty),
custom_eq: CustomEq::in_any_value_of_ty(cx, ty),
tainted_by_errors,
}
Expand Down Expand Up @@ -155,12 +154,27 @@ impl Qualif for NeedsNonConstDrop {
return false;
}

// FIXME(effects) constness
// FIXME(effects): If `destruct` is not a `const_trait`,
// or effects are disabled in this crate, then give up.
let destruct_def_id = cx.tcx.require_lang_item(LangItem::Destruct, Some(cx.body.span));
if cx.tcx.generics_of(destruct_def_id).host_effect_index.is_none()
|| !cx.tcx.features().effects
{
return NeedsDrop::in_any_value_of_ty(cx, ty);
}

let obligation = Obligation::new(
cx.tcx,
ObligationCause::dummy_with_span(cx.body.span),
cx.param_env,
ty::TraitRef::from_lang_item(cx.tcx, LangItem::Destruct, cx.body.span, [ty]),
ty::TraitRef::new(
cx.tcx,
destruct_def_id,
[
ty::GenericArg::from(ty),
ty::GenericArg::from(cx.tcx.expected_const_effect_param_for_body(cx.def_id())),
],
),
);

let infcx = cx.tcx.infer_ctxt().build();
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_middle/src/ty/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -781,9 +781,9 @@ impl<'tcx> TyCtxt<'tcx> {
}

pub fn expected_const_effect_param_for_body(self, def_id: LocalDefId) -> ty::Const<'tcx> {
// if the callee does have the param, we need to equate the param to some const
// value no matter whether the effects feature is enabled in the local crate,
// because inference will fail if we don't.
// FIXME(effects): This is suspicious and should probably not be done,
// especially now that we enforce host effects and then properly handle
// effect vars during fallback.
let mut host_always_on =
!self.features().effects || self.sess.opts.unstable_opts.unleash_the_miri_inside_of_you;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -872,9 +872,18 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
) {
// 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.
// FIXME(effects)
if true {
candidates.vec.push(ConstDestructCandidate(None));
let Some(host_effect_index) =
self.tcx().generics_of(obligation.predicate.def_id()).host_effect_index
else {
candidates.vec.push(BuiltinCandidate { has_nested: false });
return;
};
// If the obligation has `host = true`, then the obligation is non-const and it's always
// trivially implemented.
if obligation.predicate.skip_binder().trait_ref.args.const_at(host_effect_index)
== self.tcx().consts.true_
{
candidates.vec.push(BuiltinCandidate { has_nested: false });
return;
}

Expand Down
16 changes: 9 additions & 7 deletions compiler/rustc_trait_selection/src/traits/select/confirmation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1172,11 +1172,13 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
obligation: &PolyTraitObligation<'tcx>,
impl_def_id: Option<DefId>,
) -> Result<Vec<PredicateObligation<'tcx>>, SelectionError<'tcx>> {
// `~const Destruct` in a non-const environment is always trivially true, since our type is `Drop`
// FIXME(effects)
if true {
return Ok(vec![]);
}
let Some(host_effect_index) =
self.tcx().generics_of(obligation.predicate.def_id()).host_effect_index
else {
bug!()
};
let host_effect_param: ty::GenericArg<'tcx> =
obligation.predicate.skip_binder().trait_ref.args.const_at(host_effect_index).into();

let drop_trait = self.tcx().require_lang_item(LangItem::Drop, None);

Expand Down Expand Up @@ -1284,7 +1286,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
self.tcx(),
LangItem::Destruct,
cause.span,
[nested_ty],
[nested_ty.into(), host_effect_param],
),
polarity: ty::ImplPolarity::Positive,
}),
Expand All @@ -1310,7 +1312,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
self.tcx(),
LangItem::Destruct,
cause.span,
[nested_ty],
[nested_ty.into(), host_effect_param],
),
polarity: ty::ImplPolarity::Positive,
});
Expand Down
7 changes: 1 addition & 6 deletions tests/ui/consts/precise-drop-with-promoted.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
// Regression test for issue #89938.
// check-pass
// compile-flags: --crate-type=lib
// known-bug: #103507
// failure-status: 101
// normalize-stderr-test "note: .*\n\n" -> ""
// normalize-stderr-test "thread 'rustc' panicked.*\n.*\n" -> ""
// normalize-stderr-test "(error: internal compiler error: [^:]+):\d+:\d+: " -> "$1:LL:CC: "
// rustc-env:RUST_BACKTRACE=0

#![feature(const_precise_live_drops)]

Expand Down
6 changes: 0 additions & 6 deletions tests/ui/consts/precise-drop-with-promoted.stderr

This file was deleted.

4 changes: 2 additions & 2 deletions tests/ui/rfcs/rfc-2632-const-trait-impl/effects/minicore.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// check-pass

#![crate_type = "lib"]
#![feature(no_core, lang_items, unboxed_closures, auto_traits, intrinsics, rustc_attrs)]
#![feature(fundamental)]
Expand All @@ -6,8 +8,6 @@
#![no_std]
#![no_core]

// known-bug: #110395

#[lang = "sized"]
trait Sized {}
#[lang = "copy"]
Expand Down
20 changes: 0 additions & 20 deletions tests/ui/rfcs/rfc-2632-const-trait-impl/effects/minicore.stderr

This file was deleted.