Rollup merge of #89988 - tmiasko:unpromote-const-drop, r=oli-obk

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.

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,7 +39,8 @@ 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, NeedsNonConstDrop>>,
+    needs_drop: Option<QualifResults<'mir, 'tcx, NeedsDrop>>,
+    needs_non_const_drop: Option<QualifResults<'mir, 'tcx, NeedsNonConstDrop>>,
     indirectly_mutable: Option<IndirectlyMutableResults<'mir, 'tcx>>,
 }
 
@@ -80,14 +81,14 @@ impl Qualifs<'mir, 'tcx> {
         location: Location,
     ) -> bool {
         let ty = ccx.body.local_decls[local].ty;
-        if !NeedsNonConstDrop::in_any_value_of_ty(ccx, 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(NeedsNonConstDrop, ccx)
+            FlowSensitiveAnalysis::new(NeedsDrop, ccx)
                 .into_engine(tcx, &body)
                 .iterate_to_fixpoint()
                 .into_results_cursor(&body)
@@ -97,6 +98,33 @@ impl Qualifs<'mir, 'tcx> {
         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
+    pub fn needs_non_const_drop(
+        &mut self,
+        ccx: &'mir ConstCx<'mir, 'tcx>,
+        local: Local,
+        location: Location,
+    ) -> bool {
+        let ty = ccx.body.local_decls[local].ty;
+        if !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(NeedsNonConstDrop, ccx)
+                .into_engine(tcx, &body)
+                .iterate_to_fixpoint()
+                .into_results_cursor(&body)
+        });
+
+        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`.
     ///
     /// Only updates the cursor if absolutely necessary.
@@ -173,6 +201,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 +1028,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 +1048,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,

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<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_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);

compiler/rustc_const_eval/src/transform/check_consts/qualifs.rs

@@ -21,7 +21,8 @@ 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_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,17 +99,39 @@ 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 {
     const ANALYSIS_NAME: &'static str = "flow_needs_nonconst_drop";
     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 {

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

compiler/rustc_middle/src/mir/query.rs

@@ -225,6 +225,7 @@ pub struct BorrowCheckResult<'tcx> {
 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>,
 }

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
+}

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`.