Skip to content

Implement ~const Destruct effect goal in the new solver #132329

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 5 commits into from
Nov 23, 2024
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
71 changes: 40 additions & 31 deletions compiler/rustc_const_eval/src/check_consts/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use rustc_span::{Span, Symbol, sym};
use rustc_trait_selection::traits::{
Obligation, ObligationCause, ObligationCauseCode, ObligationCtxt,
};
use tracing::{debug, instrument, trace};
use tracing::{instrument, trace};

use super::ops::{self, NonConstOp, Status};
use super::qualifs::{self, HasMutInterior, NeedsDrop, NeedsNonConstDrop};
Expand All @@ -47,7 +47,7 @@ impl<'mir, 'tcx> Qualifs<'mir, 'tcx> {
/// Returns `true` if `local` is `NeedsDrop` at the given `Location`.
///
/// Only updates the cursor if absolutely necessary
fn needs_drop(
pub(crate) fn needs_drop(
&mut self,
ccx: &'mir ConstCx<'mir, 'tcx>,
local: Local,
Expand Down Expand Up @@ -421,6 +421,43 @@ impl<'mir, 'tcx> Checker<'mir, 'tcx> {

true
}

pub fn check_drop_terminator(
&mut self,
dropped_place: Place<'tcx>,
location: Location,
terminator_span: Span,
) {
let ty_of_dropped_place = dropped_place.ty(self.body, self.tcx).ty;

let needs_drop = if let Some(local) = dropped_place.as_local() {
self.qualifs.needs_drop(self.ccx, local, location)
} else {
qualifs::NeedsDrop::in_any_value_of_ty(self.ccx, ty_of_dropped_place)
};
// If this type doesn't need a drop at all, then there's nothing to enforce.
if !needs_drop {
return;
}

let mut err_span = self.span;
let needs_non_const_drop = if let Some(local) = dropped_place.as_local() {
// Use the span where the local was declared as the span of the drop error.
err_span = self.body.local_decls[local].source_info.span;
self.qualifs.needs_non_const_drop(self.ccx, local, location)
} else {
qualifs::NeedsNonConstDrop::in_any_value_of_ty(self.ccx, ty_of_dropped_place)
};

self.check_op_spanned(
ops::LiveDrop {
dropped_at: terminator_span,
dropped_ty: ty_of_dropped_place,
needs_non_const_drop,
},
err_span,
);
}
}

impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
Expand Down Expand Up @@ -866,35 +903,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
return;
}

let mut err_span = self.span;
let ty_of_dropped_place = dropped_place.ty(self.body, self.tcx).ty;

let ty_needs_non_const_drop =
qualifs::NeedsNonConstDrop::in_any_value_of_ty(self.ccx, ty_of_dropped_place);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like previously, we always checked in_any_value_of_ty before checking the qualifs. Any idea why we did that? Your new logic does not do this any more. Might just be a perf thing?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think maybe perf? I'll check perf anyways, but I expect it to not matter.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I have no idea why this would affect perf.

We call in_any_value_of_ty as soon as we call self.qualifs.needs_drop.


debug!(?ty_of_dropped_place, ?ty_needs_non_const_drop);

if !ty_needs_non_const_drop {
return;
}

let needs_non_const_drop = if let Some(local) = dropped_place.as_local() {
// Use the span where the local was declared as the span of the drop error.
err_span = self.body.local_decls[local].source_info.span;
self.qualifs.needs_non_const_drop(self.ccx, local, location)
} else {
true
};

if needs_non_const_drop {
self.check_op_spanned(
ops::LiveDrop {
dropped_at: Some(terminator.source_info.span),
dropped_ty: ty_of_dropped_place,
},
err_span,
);
}
self.check_drop_terminator(*dropped_place, location, terminator.source_info.span);
}

TerminatorKind::InlineAsm { .. } => self.check_op(ops::InlineAsm),
Expand Down
40 changes: 33 additions & 7 deletions compiler/rustc_const_eval/src/check_consts/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -459,17 +459,43 @@ impl<'tcx> NonConstOp<'tcx> for InlineAsm {

#[derive(Debug)]
pub(crate) struct LiveDrop<'tcx> {
pub dropped_at: Option<Span>,
pub dropped_at: Span,
pub dropped_ty: Ty<'tcx>,
pub needs_non_const_drop: bool,
}
impl<'tcx> NonConstOp<'tcx> for LiveDrop<'tcx> {
fn status_in_item(&self, _ccx: &ConstCx<'_, 'tcx>) -> Status {
if self.needs_non_const_drop {
Status::Forbidden
} else {
Status::Unstable {
gate: sym::const_destruct,
gate_already_checked: false,
safe_to_expose_on_stable: false,
is_function_call: false,
}
}
}

fn build_error(&self, ccx: &ConstCx<'_, 'tcx>, span: Span) -> Diag<'tcx> {
ccx.dcx().create_err(errors::LiveDrop {
span,
dropped_ty: self.dropped_ty,
kind: ccx.const_kind(),
dropped_at: self.dropped_at,
})
if self.needs_non_const_drop {
ccx.dcx().create_err(errors::LiveDrop {
span,
dropped_ty: self.dropped_ty,
kind: ccx.const_kind(),
dropped_at: self.dropped_at,
})
} else {
ccx.tcx.sess.create_feature_err(
errors::LiveDrop {
span,
dropped_ty: self.dropped_ty,
kind: ccx.const_kind(),
dropped_at: self.dropped_at,
},
sym::const_destruct,
)
}
}
}

Expand Down
57 changes: 12 additions & 45 deletions compiler/rustc_const_eval/src/check_consts/post_drop_elaboration.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,11 @@
use rustc_middle::mir::visit::Visitor;
use rustc_middle::mir::{self, BasicBlock, Location};
use rustc_middle::ty::{Ty, TyCtxt};
use rustc_span::Span;
use rustc_middle::ty::TyCtxt;
use rustc_span::symbol::sym;
use tracing::trace;

use super::ConstCx;
use super::check::Qualifs;
use super::ops::{self, NonConstOp};
use super::qualifs::{NeedsNonConstDrop, Qualif};
use crate::check_consts::check::Checker;
use crate::check_consts::rustc_allow_const_fn_unstable;

/// Returns `true` if we should use the more precise live drop checker that runs after drop
Expand Down Expand Up @@ -45,29 +42,16 @@ pub fn check_live_drops<'tcx>(tcx: TyCtxt<'tcx>, body: &mir::Body<'tcx>) {
return;
}

let mut visitor = CheckLiveDrops { ccx: &ccx, qualifs: Qualifs::default() };
// I know it's not great to be creating a new const checker, but I'd
// rather use it so we can deduplicate the error emitting logic that
// it contains.
let mut visitor = CheckLiveDrops { checker: Checker::new(&ccx) };

visitor.visit_body(body);
}

struct CheckLiveDrops<'mir, 'tcx> {
ccx: &'mir ConstCx<'mir, 'tcx>,
qualifs: Qualifs<'mir, 'tcx>,
}

// So we can access `body` and `tcx`.
impl<'mir, 'tcx> std::ops::Deref for CheckLiveDrops<'mir, 'tcx> {
type Target = ConstCx<'mir, 'tcx>;

fn deref(&self) -> &Self::Target {
self.ccx
}
}

impl<'tcx> CheckLiveDrops<'_, 'tcx> {
fn check_live_drop(&self, span: Span, dropped_ty: Ty<'tcx>) {
ops::LiveDrop { dropped_at: None, dropped_ty }.build_error(self.ccx, span).emit();
}
checker: Checker<'mir, 'tcx>,
}

impl<'tcx> Visitor<'tcx> for CheckLiveDrops<'_, 'tcx> {
Expand All @@ -87,28 +71,11 @@ impl<'tcx> Visitor<'tcx> for CheckLiveDrops<'_, 'tcx> {

match &terminator.kind {
mir::TerminatorKind::Drop { place: dropped_place, .. } => {
let dropped_ty = dropped_place.ty(self.body, self.tcx).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;
}

if dropped_place.is_indirect() {
self.check_live_drop(terminator.source_info.span, dropped_ty);
return;
}

// Drop elaboration is not precise enough to accept code like
// `tests/ui/consts/control-flow/drop-pass.rs`; e.g., when an `Option<Vec<T>>` is
// initialized with `None` and never changed, it still emits drop glue.
// Hence we additionally check the qualifs here to allow more code to pass.
if self.qualifs.needs_non_const_drop(self.ccx, dropped_place.local, location) {
// Use the span where the dropped local was declared for the error.
let span = self.body.local_decls[dropped_place.local].source_info.span;
self.check_live_drop(span, dropped_ty);
}
self.checker.check_drop_terminator(
*dropped_place,
location,
terminator.source_info.span,
);
}

mir::TerminatorKind::UnwindTerminate(_)
Expand Down
Loading
Loading