Skip to content

Commit

Permalink
Do not promote values with const drop that need to be dropped
Browse files Browse the repository at this point in the history
Changes from #88558 allowed using `~const Drop` in constants by
introducing a new `NeedsNonConstDrop` qualif.

The new qualif was also used for promotion purposes, and allowed
promotion to happen for values that needs to be dropped but which
do have a const drop impl.

Since for promoted the drop implementation is never executed,
this lead to observable change in behaviour. For example:

```rust

struct Panic();

impl const Drop for Panic {
    fn drop(&mut self) {
        panic!();
    }
}

fn main() {
    let _ = &Panic();
}
```

Restore the use of `NeedsDrop` qualif during promotion to avoid the issue.
  • Loading branch information
tmiasko committed Oct 18, 2021
1 parent 171cbc0 commit 915a581
Show file tree
Hide file tree
Showing 6 changed files with 97 additions and 5 deletions.
31 changes: 30 additions & 1 deletion 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, NeedsNonConstDrop};
use super::qualifs::{self, CustomEq, HasMutInterior, NeedsDrop, NeedsNonConstDrop};
use super::resolver::FlowSensitiveAnalysis;
use super::{is_lang_panic_fn, is_lang_special_const_fn, ConstCx, Qualif};
use crate::const_eval::is_unstable_const_fn;
Expand All @@ -39,6 +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_non_const_drop: Option<QualifResults<'mir, 'tcx, NeedsNonConstDrop>>,
indirectly_mutable: Option<IndirectlyMutableResults<'mir, 'tcx>>,
}
Expand Down Expand Up @@ -70,6 +71,33 @@ impl Qualifs<'mir, 'tcx> {
indirectly_mutable.get().contains(local)
}

/// Returns `true` if `local` is `NeedsDrop` at the given `Location`.
///
/// Only updates the cursor if absolutely necessary
pub fn needs_drop(
&mut self,
ccx: &'mir ConstCx<'mir, 'tcx>,
local: Local,
location: Location,
) -> bool {
let ty = ccx.body.local_decls[local].ty;
if !NeedsDrop::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)
.into_engine(tcx, &body)
.iterate_to_fixpoint()
.into_results_cursor(&body)
});

needs_drop.seek_before_primary_effect(location);
needs_drop.get().contains(local) || self.indirectly_mutable(ccx, local, location)
}

/// Returns `true` if `local` is `NeedsNonConstDrop` at the given `Location`.
///
/// Only updates the cursor if absolutely necessary
Expand Down Expand Up @@ -172,6 +200,7 @@ impl Qualifs<'mir, 'tcx> {
};

ConstQualifs {
needs_drop: self.needs_drop(ccx, RETURN_PLACE, return_loc),
needs_non_const_drop: self.needs_non_const_drop(ccx, RETURN_PLACE, return_loc),
has_mut_interior: self.has_mut_interior(ccx, RETURN_PLACE, return_loc),
custom_eq,
Expand Down
29 changes: 26 additions & 3 deletions compiler/rustc_const_eval/src/transform/check_consts/qualifs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +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_non_const_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 @@ -98,9 +99,31 @@ impl Qualif for HasMutInterior {
}

/// Constant containing an ADT that implements `Drop`.
/// 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.
/// This must be ruled out because implicit promotion would remove side-effects
/// that occur as part of dropping that value. N.B., the implicit promotion has
/// to reject const Drop implementations because even if side-effects are ruled
/// out through other means, the execution of the drop could diverge.
pub struct NeedsDrop;

impl Qualif for NeedsDrop {
const ANALYSIS_NAME: &'static str = "flow_needs_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)
}

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

/// Constant containing an ADT that implements non-const `Drop`.
/// This must be ruled out because we cannot run `Drop` during compile-time.
pub struct NeedsNonConstDrop;

impl Qualif for NeedsNonConstDrop {
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 @@ -230,7 +230,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::NeedsNonConstDrop>(place.local) {
if self.qualif_local::<qualifs::NeedsDrop>(place.local) {
return Err(Unpromotable);
}

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/mir/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@ pub struct BorrowCheckResult<'tcx> {
#[derive(Clone, Copy, Debug, Default, TyEncodable, TyDecodable, HashStable)]
pub struct ConstQualifs {
pub has_mut_interior: bool,
pub needs_drop: bool,
pub needs_non_const_drop: bool,
pub custom_eq: bool,
pub error_occured: Option<ErrorReported>,
Expand Down
15 changes: 15 additions & 0 deletions src/test/ui/consts/promoted-const-drop.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#![feature(const_trait_impl)]
#![feature(const_mut_refs)]

struct A();

impl const Drop for A {
fn drop(&mut self) {}
}

const C: A = A();

fn main() {
let _: &'static A = &A(); //~ ERROR temporary value dropped while borrowed
let _: &'static [A] = &[C]; //~ ERROR temporary value dropped while borrowed
}
24 changes: 24 additions & 0 deletions src/test/ui/consts/promoted-const-drop.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
error[E0716]: temporary value dropped while borrowed
--> $DIR/promoted-const-drop.rs:13:26
|
LL | let _: &'static A = &A();
| ---------- ^^^ creates a temporary which is freed while still in use
| |
| type annotation requires that borrow lasts for `'static`
LL | let _: &'static [A] = &[C];
LL | }
| - temporary value is freed at the end of this statement

error[E0716]: temporary value dropped while borrowed
--> $DIR/promoted-const-drop.rs:14:28
|
LL | let _: &'static [A] = &[C];
| ------------ ^^^ creates a temporary which is freed while still in use
| |
| type annotation requires that borrow lasts for `'static`
LL | }
| - temporary value is freed at the end of this statement

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0716`.

0 comments on commit 915a581

Please sign in to comment.