From 171cbc01efe103f255f83afa2a70768e1d373edb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Sun, 17 Oct 2021 00:00:00 +0000 Subject: [PATCH 1/2] Rename `needs_drop` to `needs_non_const_drop` --- .../src/transform/check_consts/check.rs | 22 +++++++++---------- .../check_consts/post_drop_elaboration.rs | 2 +- .../src/transform/check_consts/qualifs.rs | 4 ++-- compiler/rustc_middle/src/mir/query.rs | 2 +- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_const_eval/src/transform/check_consts/check.rs b/compiler/rustc_const_eval/src/transform/check_consts/check.rs index d704c4335c754..8cd75dd8e2841 100644 --- a/compiler/rustc_const_eval/src/transform/check_consts/check.rs +++ b/compiler/rustc_const_eval/src/transform/check_consts/check.rs @@ -39,7 +39,7 @@ type QualifResults<'mir, 'tcx, Q> = #[derive(Default)] pub struct Qualifs<'mir, 'tcx> { has_mut_interior: Option>, - needs_drop: Option>, + needs_non_const_drop: Option>, indirectly_mutable: Option>, } @@ -70,10 +70,10 @@ impl Qualifs<'mir, 'tcx> { indirectly_mutable.get().contains(local) } - /// Returns `true` if `local` is `NeedsDrop` at the given `Location`. + /// Returns `true` if `local` is `NeedsNonConstDrop` at the given `Location`. /// /// Only updates the cursor if absolutely necessary - pub fn needs_drop( + pub fn needs_non_const_drop( &mut self, ccx: &'mir ConstCx<'mir, 'tcx>, local: Local, @@ -84,7 +84,7 @@ impl Qualifs<'mir, 'tcx> { return false; } - let needs_drop = self.needs_drop.get_or_insert_with(|| { + let needs_non_const_drop = self.needs_non_const_drop.get_or_insert_with(|| { let ConstCx { tcx, body, .. } = *ccx; FlowSensitiveAnalysis::new(NeedsNonConstDrop, ccx) @@ -93,8 +93,8 @@ impl Qualifs<'mir, 'tcx> { .into_results_cursor(&body) }); - needs_drop.seek_before_primary_effect(location); - needs_drop.get().contains(local) || self.indirectly_mutable(ccx, local, location) + needs_non_const_drop.seek_before_primary_effect(location); + needs_non_const_drop.get().contains(local) || self.indirectly_mutable(ccx, local, location) } /// Returns `true` if `local` is `HasMutInterior` at the given `Location`. @@ -172,7 +172,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, error_occured, @@ -999,7 +999,7 @@ impl Visitor<'tcx> for Checker<'mir, 'tcx> { } // Forbid all `Drop` terminators unless the place being dropped is a local with no - // projections that cannot be `NeedsDrop`. + // projections that cannot be `NeedsNonConstDrop`. TerminatorKind::Drop { place: dropped_place, .. } | TerminatorKind::DropAndReplace { place: dropped_place, .. } => { // If we are checking live drops after drop-elaboration, don't emit duplicate @@ -1019,15 +1019,15 @@ impl Visitor<'tcx> for Checker<'mir, 'tcx> { return; } - let needs_drop = if let Some(local) = dropped_place.as_local() { + 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_drop(self.ccx, local, location) + self.qualifs.needs_non_const_drop(self.ccx, local, location) } else { true }; - if needs_drop { + if needs_non_const_drop { self.check_op_spanned( ops::LiveDrop { dropped_at: Some(terminator.source_info.span) }, err_span, diff --git a/compiler/rustc_const_eval/src/transform/check_consts/post_drop_elaboration.rs b/compiler/rustc_const_eval/src/transform/check_consts/post_drop_elaboration.rs index 1a8c8b1c78d08..7a2be3c3bad32 100644 --- a/compiler/rustc_const_eval/src/transform/check_consts/post_drop_elaboration.rs +++ b/compiler/rustc_const_eval/src/transform/check_consts/post_drop_elaboration.rs @@ -97,7 +97,7 @@ impl Visitor<'tcx> for CheckLiveDrops<'mir, 'tcx> { // `src/test/ui/consts/control-flow/drop-pass.rs`; e.g., when an `Option>` 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_drop(self.ccx, dropped_place.local, location) { + 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); diff --git a/compiler/rustc_const_eval/src/transform/check_consts/qualifs.rs b/compiler/rustc_const_eval/src/transform/check_consts/qualifs.rs index 5eb7d7a91cc76..689aa0993711f 100644 --- a/compiler/rustc_const_eval/src/transform/check_consts/qualifs.rs +++ b/compiler/rustc_const_eval/src/transform/check_consts/qualifs.rs @@ -21,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: NeedsNonConstDrop::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, } @@ -108,7 +108,7 @@ impl Qualif for NeedsNonConstDrop { const IS_CLEARED_ON_MOVE: bool = true; fn in_qualifs(qualifs: &ConstQualifs) -> bool { - qualifs.needs_drop + qualifs.needs_non_const_drop } fn in_any_value_of_ty(cx: &ConstCx<'_, 'tcx>, mut ty: Ty<'tcx>) -> bool { diff --git a/compiler/rustc_middle/src/mir/query.rs b/compiler/rustc_middle/src/mir/query.rs index d5541d7890c77..98f116a16e2b2 100644 --- a/compiler/rustc_middle/src/mir/query.rs +++ b/compiler/rustc_middle/src/mir/query.rs @@ -224,7 +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, } From 915a581bcb3d9b7e1e2ef0da4fbfbae6b1a7fa7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Sun, 17 Oct 2021 00:00:00 +0000 Subject: [PATCH 2/2] Do not promote values with const drop that need to be dropped 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. --- .../src/transform/check_consts/check.rs | 31 ++++++++++++++++++- .../src/transform/check_consts/qualifs.rs | 29 +++++++++++++++-- .../src/transform/promote_consts.rs | 2 +- compiler/rustc_middle/src/mir/query.rs | 1 + src/test/ui/consts/promoted-const-drop.rs | 15 +++++++++ src/test/ui/consts/promoted-const-drop.stderr | 24 ++++++++++++++ 6 files changed, 97 insertions(+), 5 deletions(-) create mode 100644 src/test/ui/consts/promoted-const-drop.rs create mode 100644 src/test/ui/consts/promoted-const-drop.stderr diff --git a/compiler/rustc_const_eval/src/transform/check_consts/check.rs b/compiler/rustc_const_eval/src/transform/check_consts/check.rs index 8cd75dd8e2841..03e60deea2783 100644 --- a/compiler/rustc_const_eval/src/transform/check_consts/check.rs +++ b/compiler/rustc_const_eval/src/transform/check_consts/check.rs @@ -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; @@ -39,6 +39,7 @@ type QualifResults<'mir, 'tcx, Q> = #[derive(Default)] pub struct Qualifs<'mir, 'tcx> { has_mut_interior: Option>, + needs_drop: Option>, needs_non_const_drop: Option>, indirectly_mutable: Option>, } @@ -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 @@ -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, diff --git a/compiler/rustc_const_eval/src/transform/check_consts/qualifs.rs b/compiler/rustc_const_eval/src/transform/check_consts/qualifs.rs index 689aa0993711f..dd2980d40ade7 100644 --- a/compiler/rustc_const_eval/src/transform/check_consts/qualifs.rs +++ b/compiler/rustc_const_eval/src/transform/check_consts/qualifs.rs @@ -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, @@ -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 { diff --git a/compiler/rustc_const_eval/src/transform/promote_consts.rs b/compiler/rustc_const_eval/src/transform/promote_consts.rs index 7cfe3d7f809e9..ebcc8213c604b 100644 --- a/compiler/rustc_const_eval/src/transform/promote_consts.rs +++ b/compiler/rustc_const_eval/src/transform/promote_consts.rs @@ -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::(place.local) { + if self.qualif_local::(place.local) { return Err(Unpromotable); } diff --git a/compiler/rustc_middle/src/mir/query.rs b/compiler/rustc_middle/src/mir/query.rs index 98f116a16e2b2..cb3f3850958ec 100644 --- a/compiler/rustc_middle/src/mir/query.rs +++ b/compiler/rustc_middle/src/mir/query.rs @@ -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, diff --git a/src/test/ui/consts/promoted-const-drop.rs b/src/test/ui/consts/promoted-const-drop.rs new file mode 100644 index 0000000000000..c896c011ab66a --- /dev/null +++ b/src/test/ui/consts/promoted-const-drop.rs @@ -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 +} diff --git a/src/test/ui/consts/promoted-const-drop.stderr b/src/test/ui/consts/promoted-const-drop.stderr new file mode 100644 index 0000000000000..184ba0ea3b377 --- /dev/null +++ b/src/test/ui/consts/promoted-const-drop.stderr @@ -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`.