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 rust-lang#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.