Auto merge of rust-lang#121557 - RalfJung:const-fn-call-promotion, r=<try>

restrict promotion of `const fn` calls

We only promote them in `const`/`static` initializers, but even that is still unfortunate -- we still cannot add promoteds to required_consts. But we should add them there to make sure it's always okay to evaluate every const we encounter in a MIR body.  That effort of not promoting things that can fail to evaluate is tracked in rust-lang#80619. These `const fn` calls are the last missing piece.

So I propose that we do not promote const-fn calls in const when that may fail without the entire const failing, thereby completing rust-lang#80619. Unfortunately we can't just reject promoting these functions outright due to backwards compatibility. So let's see if we can find a hack that makes crater happy...

For the record, this is the [crater analysis](rust-lang#80243 (comment)) from when I tried to entirely forbid this kind of promotion. It's a tiny amount of breakage and if we had a nice alternative for code like that, we could conceivably push it through... but sadly, inline const expressions are still blocked on t-lang concerns about post-monomorphization errors and we haven't yet figured out an implementation that can resolve those concerns. So we're forced to make progress via other means, such as terrible hacks like this.

Attempt one: only promote calls on the "safe path" at the beginning of a MIR block. This is the path that starts at the start block and continues via gotos and calls, but stops at the first branch. If we had imposed this restriction before stabilizing `if` and `match` in `const`, this would have definitely been sufficient...

EDIT: Turns out that works. :)
**Here's the t-lang [nomination comment](rust-lang#121557 (comment) And here's the [FCP comment](rust-lang#121557 (comment)).

r? `@oli-obk`

Author: bors

compiler/rustc_const_eval/src/const_eval/dummy_machine.rs

@@ -46,6 +46,9 @@ impl<'mir, 'tcx: 'mir> interpret::Machine<'mir, 'tcx> for DummyMachine {
     type MemoryKind = !;
     const PANIC_ON_ALLOC_FAIL: bool = true;
 
+    // We want to just eval random consts in the program, so `eval_mir_const` can fail.
+    const ALL_CONSTS_ARE_PRECHECKED: bool = false;
+
     #[inline(always)]
     fn enforce_alignment(_ecx: &InterpCx<'mir, 'tcx, Self>) -> bool {
         false // no reason to enforce alignment

compiler/rustc_const_eval/src/interpret/eval_context.rs

@@ -822,15 +822,13 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         self.stack_mut().push(frame);
 
         // Make sure all the constants required by this frame evaluate successfully (post-monomorphization check).
-        if M::POST_MONO_CHECKS {
-            for &const_ in &body.required_consts {
-                let c = self
-                    .instantiate_from_current_frame_and_normalize_erasing_regions(const_.const_)?;
-                c.eval(*self.tcx, self.param_env, const_.span).map_err(|err| {
-                    err.emit_note(*self.tcx);
-                    err
-                })?;
-            }
+        for &const_ in &body.required_consts {
+            let c =
+                self.instantiate_from_current_frame_and_normalize_erasing_regions(const_.const_)?;
+            c.eval(*self.tcx, self.param_env, const_.span).map_err(|err| {
+                err.emit_note(*self.tcx);
+                err
+            })?;
         }
 
         // done
@@ -1179,8 +1177,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
     ) -> InterpResult<'tcx, OpTy<'tcx, M::Provenance>> {
         M::eval_mir_constant(self, *val, span, layout, |ecx, val, span, layout| {
             let const_val = val.eval(*ecx.tcx, ecx.param_env, span).map_err(|err| {
-                // FIXME: somehow this is reachable even when POST_MONO_CHECKS is on.
-                // Are we not always populating `required_consts`?
+                if M::ALL_CONSTS_ARE_PRECHECKED && !matches!(err, ErrorHandled::TooGeneric(..)) {
+                    // Looks like the const is not captued by `required_consts`, that's bad.
+                    bug!("interpret const eval failure of {val:?} which is not in required_consts");
+                }
                 err.emit_note(*ecx.tcx);
                 err
             })?;

compiler/rustc_const_eval/src/interpret/machine.rs

@@ -140,8 +140,9 @@ pub trait Machine<'mir, 'tcx: 'mir>: Sized {
     /// Should the machine panic on allocation failures?
     const PANIC_ON_ALLOC_FAIL: bool;
 
-    /// Should post-monomorphization checks be run when a stack frame is pushed?
-    const POST_MONO_CHECKS: bool = true;
+    /// Determines whether `eval_mir_constant` can never fail because all required consts have
+    /// already been checked before.
+    const ALL_CONSTS_ARE_PRECHECKED: bool = true;
 
     /// Whether memory accesses should be alignment-checked.
     fn enforce_alignment(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool;

compiler/rustc_middle/src/mir/consts.rs

@@ -238,6 +238,20 @@ impl<'tcx> Const<'tcx> {
         }
     }
 
+    /// Determines whether we need to add this const to `required_consts`. This is the case if and
+    /// only if evaluating it may error.
+    #[inline]
+    pub fn is_required_const(&self) -> bool {
+        match self {
+            Const::Ty(c) => match c.kind() {
+                ty::ConstKind::Value(_) => false, // already a value, cannot error
+                _ => true,
+            },
+            Const::Val(..) => false, // already a value, cannot error
+            Const::Unevaluated(..) => true,
+        }
+    }
+
     #[inline]
     pub fn try_to_scalar(self) -> Option<Scalar> {
         match self {

compiler/rustc_mir_transform/src/inline.rs

@@ -706,18 +706,12 @@ impl<'tcx> Inliner<'tcx> {
             kind: TerminatorKind::Goto { target: integrator.map_block(START_BLOCK) },
         });
 
-        // Copy only unevaluated constants from the callee_body into the caller_body.
-        // Although we are only pushing `ConstKind::Unevaluated` consts to
-        // `required_consts`, here we may not only have `ConstKind::Unevaluated`
-        // because we are calling `instantiate_and_normalize_erasing_regions`.
-        caller_body.required_consts.extend(callee_body.required_consts.iter().copied().filter(
-            |&ct| match ct.const_ {
-                Const::Ty(_) => {
-                    bug!("should never encounter ty::UnevaluatedConst in `required_consts`")
-                }
-                Const::Val(..) | Const::Unevaluated(..) => true,
-            },
-        ));
+        // Copy required constants from the callee_body into the caller_body. Although we are only
+        // pushing unevaluated consts to `required_consts`, here they may have been evaluated
+        // because we are calling `instantiate_and_normalize_erasing_regions` -- so we filter again.
+        caller_body.required_consts.extend(
+            callee_body.required_consts.into_iter().filter(|ct| ct.const_.is_required_const()),
+        );
         // Now that we incorporated the callee's `required_consts`, we can remove the callee from
         // `mentioned_items` -- but we have to take their `mentioned_items` in return. This does
         // some extra work here to save the monomorphization collector work later. It helps a lot,
@@ -733,8 +727,9 @@ impl<'tcx> Inliner<'tcx> {
             caller_body.mentioned_items.remove(idx);
             caller_body.mentioned_items.extend(callee_body.mentioned_items);
         } else {
-            // If we can't find the callee, there's no point in adding its items.
-            // Probably it already got removed by being inlined elsewhere in the same function.
+            // If we can't find the callee, there's no point in adding its items. Probably it
+            // already got removed by being inlined elsewhere in the same function, so we already
+            // took its items.
         }
     }
 

compiler/rustc_mir_transform/src/lib.rs

@@ -344,6 +344,8 @@ fn mir_promoted(
         body.tainted_by_errors = Some(error_reported);
     }
 
+    // Collect `required_consts` *before* promotion, so if there are any consts being promoted
+    // we still add them to the list in the outer MIR body.
     let mut required_consts = Vec::new();
     let mut required_consts_visitor = RequiredConstsVisitor::new(&mut required_consts);
     for (bb, bb_data) in traversal::reverse_postorder(&body) {

compiler/rustc_mir_transform/src/promote_consts.rs

@@ -13,6 +13,7 @@
 //! move analysis runs after promotion on broken MIR.
 
 use either::{Left, Right};
+use rustc_data_structures::fx::FxHashSet;
 use rustc_hir as hir;
 use rustc_middle::mir;
 use rustc_middle::mir::visit::{MutVisitor, MutatingUseContext, PlaceContext, Visitor};
@@ -175,6 +176,12 @@ fn collect_temps_and_candidates<'tcx>(
 struct Validator<'a, 'tcx> {
     ccx: &'a ConstCx<'a, 'tcx>,
     temps: &'a mut IndexSlice<Local, TempState>,
+    /// For backwards compatibility, we are promoting function calls in `const`/`static`
+    /// initializers. But we want to avoid evaluating code that might panic and that otherwise would
+    /// not have been evaluated, so we only promote such calls in basic blocks that are guaranteed
+    /// to execute. In other words, we only promote such calls in basic blocks that are definitely
+    /// not dead code. Here we cache the result of computing that set of basic blocks.
+    promotion_safe_blocks: Option<FxHashSet<BasicBlock>>,
 }
 
 impl<'a, 'tcx> std::ops::Deref for Validator<'a, 'tcx> {
@@ -260,7 +267,9 @@ impl<'tcx> Validator<'_, 'tcx> {
                     self.validate_rvalue(rhs)
                 }
                 Right(terminator) => match &terminator.kind {
-                    TerminatorKind::Call { func, args, .. } => self.validate_call(func, args),
+                    TerminatorKind::Call { func, args, .. } => {
+                        self.validate_call(func, args, loc.block)
+                    }
                     TerminatorKind::Yield { .. } => Err(Unpromotable),
                     kind => {
                         span_bug!(terminator.source_info.span, "{:?} not promotable", kind);
@@ -587,53 +596,103 @@ impl<'tcx> Validator<'_, 'tcx> {
         Ok(())
     }
 
+    /// Computes the sets of blocks of this MIR that are definitely going to be executed
+    /// if the function returns successfully. That makes it safe to promote calls in them
+    /// that might fail.
+    fn promotion_safe_blocks(body: &mir::Body<'tcx>) -> FxHashSet<BasicBlock> {
+        let mut safe_blocks = FxHashSet::default();
+        let mut safe_block = START_BLOCK;
+        loop {
+            safe_blocks.insert(safe_block);
+            // Let's see if we can find another safe block.
+            safe_block = match body.basic_blocks[safe_block].terminator().kind {
+                TerminatorKind::Goto { target } => target,
+                TerminatorKind::Call { target: Some(target), .. }
+                | TerminatorKind::Drop { target, .. } => {
+                    // This calls a function or the destructor. `target` does not get executed if
+                    // the callee loops or panics. But in both cases the const already fails to
+                    // evaluate, so we are fine considering `target` a safe block for promotion.
+                    target
+                }
+                TerminatorKind::Assert { target, .. } => {
+                    // Similar to above, we only consider successful execution.
+                    target
+                }
+                _ => {
+                    // No next safe block.
+                    break;
+                }
+            };
+        }
+        safe_blocks
+    }
+
+    /// Returns whether the block is "safe" for promotion, which means it cannot be dead code.
+    /// We use this to avoid promoting operations that can fail in dead code.
+    fn is_promotion_safe_block(&mut self, block: BasicBlock) -> bool {
+        let body = self.body;
+        let safe_blocks =
+            self.promotion_safe_blocks.get_or_insert_with(|| Self::promotion_safe_blocks(body));
+        safe_blocks.contains(&block)
+    }
+
     fn validate_call(
         &mut self,
         callee: &Operand<'tcx>,
         args: &[Spanned<Operand<'tcx>>],
+        block: BasicBlock,
     ) -> Result<(), Unpromotable> {
+        // Validate the operands. If they fail, there's no question -- we cannot promote.
+        self.validate_operand(callee)?;
+        for arg in args {
+            self.validate_operand(&arg.node)?;
+        }
+
+        // Functions marked `#[rustc_promotable]` are explicitly allowed to be promoted, so we can
+        // accept them at this point.
         let fn_ty = callee.ty(self.body, self.tcx);
+        if let ty::FnDef(def_id, _) = *fn_ty.kind() {
+            if self.tcx.is_promotable_const_fn(def_id) {
+                return Ok(());
+            }
+        }
 
-        // Inside const/static items, we promote all (eligible) function calls.
-        // Everywhere else, we require `#[rustc_promotable]` on the callee.
-        let promote_all_const_fn = matches!(
+        // Ideally, we'd stop here and reject the rest.
+        // But for backward compatibility, we have to accept some promotion in const/static
+        // initializers. Inline consts are explicitly excluded, they are more recent so we have no
+        // backwards compatibility reason to allow more promotion inside of them.
+        let promote_all_fn = matches!(
             self.const_kind,
             Some(hir::ConstContext::Static(_) | hir::ConstContext::Const { inline: false })
         );
-        if !promote_all_const_fn {
-            if let ty::FnDef(def_id, _) = *fn_ty.kind() {
-                // Never promote runtime `const fn` calls of
-                // functions without `#[rustc_promotable]`.
-                if !self.tcx.is_promotable_const_fn(def_id) {
-                    return Err(Unpromotable);
-                }
-            }
+        if !promote_all_fn {
+            return Err(Unpromotable);
         }
-
+        // Make sure the callee is a `const fn`.
         let is_const_fn = match *fn_ty.kind() {
             ty::FnDef(def_id, _) => self.tcx.is_const_fn_raw(def_id),
             _ => false,
         };
         if !is_const_fn {
             return Err(Unpromotable);
         }
-
-        self.validate_operand(callee)?;
-        for arg in args {
-            self.validate_operand(&arg.node)?;
+        // The problem is, this may promote calls to functions that panic.
+        // We don't want to introduce compilation errors if there's a panic in a call in dead code.
+        // So we ensure that this is not dead code.
+        if !self.is_promotion_safe_block(block) {
+            return Err(Unpromotable);
         }
-
+        // This passed all checks, so let's accept.
         Ok(())
     }
 }
 
-// FIXME(eddyb) remove the differences for promotability in `static`, `const`, `const fn`.
 fn validate_candidates(
     ccx: &ConstCx<'_, '_>,
     temps: &mut IndexSlice<Local, TempState>,
     candidates: &[Candidate],
 ) -> Vec<Candidate> {
-    let mut validator = Validator { ccx, temps };
+    let mut validator = Validator { ccx, temps, promotion_safe_blocks: None };
 
     candidates
         .iter()
@@ -652,6 +711,10 @@ struct Promoter<'a, 'tcx> {
     /// If true, all nested temps are also kept in the
     /// source MIR, not moved to the promoted MIR.
     keep_original: bool,
+
+    /// If true, add the new const (the promoted) to the required_consts of the parent MIR.
+    /// This is initially false and then set by the visitor when it encounters a `Call` terminator.
+    add_to_required: bool,
 }
 
 impl<'a, 'tcx> Promoter<'a, 'tcx> {
@@ -754,6 +817,10 @@ impl<'a, 'tcx> Promoter<'a, 'tcx> {
                 TerminatorKind::Call {
                     mut func, mut args, call_source: desugar, fn_span, ..
                 } => {
+                    // This promoted involves a function call, so it may fail to evaluate.
+                    // Let's make sure it is added to `required_consts` so that that failure cannot get lost.
+                    self.add_to_required = true;
+
                     self.visit_operand(&mut func, loc);
                     for arg in &mut args {
                         self.visit_operand(&mut arg.node, loc);
@@ -788,7 +855,7 @@ impl<'a, 'tcx> Promoter<'a, 'tcx> {
 
     fn promote_candidate(mut self, candidate: Candidate, next_promoted_id: usize) -> Body<'tcx> {
         let def = self.source.source.def_id();
-        let mut rvalue = {
+        let (mut rvalue, promoted_op) = {
             let promoted = &mut self.promoted;
             let promoted_id = Promoted::new(next_promoted_id);
             let tcx = self.tcx;
@@ -798,11 +865,7 @@ impl<'a, 'tcx> Promoter<'a, 'tcx> {
                 let args = tcx.erase_regions(GenericArgs::identity_for_item(tcx, def));
                 let uneval = mir::UnevaluatedConst { def, args, promoted: Some(promoted_id) };
 
-                Operand::Constant(Box::new(ConstOperand {
-                    span,
-                    user_ty: None,
-                    const_: Const::Unevaluated(uneval, ty),
-                }))
+                ConstOperand { span, user_ty: None, const_: Const::Unevaluated(uneval, ty) }
             };
 
             let blocks = self.source.basic_blocks.as_mut();
@@ -835,22 +898,26 @@ impl<'a, 'tcx> Promoter<'a, 'tcx> {
             let promoted_ref = local_decls.push(promoted_ref);
             assert_eq!(self.temps.push(TempState::Unpromotable), promoted_ref);
 
+            let promoted_operand = promoted_operand(ref_ty, span);
             let promoted_ref_statement = Statement {
                 source_info: statement.source_info,
                 kind: StatementKind::Assign(Box::new((
                     Place::from(promoted_ref),
-                    Rvalue::Use(promoted_operand(ref_ty, span)),
+                    Rvalue::Use(Operand::Constant(Box::new(promoted_operand))),
                 ))),
             };
             self.extra_statements.push((loc, promoted_ref_statement));
 
-            Rvalue::Ref(
-                tcx.lifetimes.re_erased,
-                *borrow_kind,
-                Place {
-                    local: mem::replace(&mut place.local, promoted_ref),
-                    projection: List::empty(),
-                },
+            (
+                Rvalue::Ref(
+                    tcx.lifetimes.re_erased,
+                    *borrow_kind,
+                    Place {
+                        local: mem::replace(&mut place.local, promoted_ref),
+                        projection: List::empty(),
+                    },
+                ),
+                promoted_operand,
             )
         };
 
@@ -862,6 +929,12 @@ impl<'a, 'tcx> Promoter<'a, 'tcx> {
 
         let span = self.promoted.span;
         self.assign(RETURN_PLACE, rvalue, span);
+
+        // Now that we did promotion, we know whether we'll want to add this to `required_consts`.
+        if self.add_to_required {
+            self.source.required_consts.push(promoted_op);
+        }
+
         self.promoted
     }
 }
@@ -877,6 +950,14 @@ impl<'a, 'tcx> MutVisitor<'tcx> for Promoter<'a, 'tcx> {
             *local = self.promote_temp(*local);
         }
     }
+
+    fn visit_constant(&mut self, constant: &mut ConstOperand<'tcx>, _location: Location) {
+        if constant.const_.is_required_const() {
+            self.promoted.required_consts.push(*constant);
+        }
+
+        // Skipping `super_constant` as the visitor is otherwise only looking for locals.
+    }
 }
 
 fn promote_candidates<'tcx>(
@@ -930,8 +1011,10 @@ fn promote_candidates<'tcx>(
             temps: &mut temps,
             extra_statements: &mut extra_statements,
             keep_original: false,
+            add_to_required: false,
         };
 
+        // `required_consts` of the promoted itself gets filled while building the MIR body.
         let mut promoted = promoter.promote_candidate(candidate, promotions.len());
         promoted.source.promoted = Some(promotions.next_index());
         promotions.push(promoted);

compiler/rustc_mir_transform/src/required_consts.rs

@@ -1,6 +1,5 @@
 use rustc_middle::mir::visit::Visitor;
-use rustc_middle::mir::{Const, ConstOperand, Location};
-use rustc_middle::ty::ConstKind;
+use rustc_middle::mir::{ConstOperand, Location};
 
 pub struct RequiredConstsVisitor<'a, 'tcx> {
     required_consts: &'a mut Vec<ConstOperand<'tcx>>,
@@ -14,14 +13,8 @@ impl<'a, 'tcx> RequiredConstsVisitor<'a, 'tcx> {
 
 impl<'tcx> Visitor<'tcx> for RequiredConstsVisitor<'_, 'tcx> {
     fn visit_constant(&mut self, constant: &ConstOperand<'tcx>, _: Location) {
-        let const_ = constant.const_;
-        match const_ {
-            Const::Ty(c) => match c.kind() {
-                ConstKind::Param(_) | ConstKind::Error(_) | ConstKind::Value(_) => {}
-                _ => bug!("only ConstKind::Param/Value should be encountered here, got {:#?}", c),
-            },
-            Const::Unevaluated(..) => self.required_consts.push(*constant),
-            Const::Val(..) => {}
+        if constant.const_.is_required_const() {
+            self.required_consts.push(*constant);
         }
     }
 }

tests/ui/consts/const-eval/promoted_errors.noopt.stderr

@@ -51,6 +51,15 @@ const TEST_DROP_NOT_PROMOTE: &String = {
 };
 
 
+// We do not promote function calls in `const` initializers in dead code.
+const fn mk_panic() -> u32 { panic!() }
+const fn mk_false() -> bool { false }
+const Y: () = {
+    if mk_false() {
+        let _x: &'static u32 = &mk_panic(); //~ ERROR temporary value dropped while borrowed
+    }
+};
+
 fn main() {
     // We must not promote things with interior mutability. Not even if we "project it away".
     let _val: &'static _ = &(Cell::new(1), 2).0; //~ ERROR temporary value dropped while borrowed

tests/ui/consts/const-eval/promoted_errors.opt.stderr

@@ -47,6 +47,16 @@ LL |     let x = &String::new();
 LL | };
    | - value is dropped here
 
+error[E0716]: temporary value dropped while borrowed
+  --> $DIR/promote-not.rs:59:33
+   |
+LL |         let _x: &'static u32 = &mk_panic();
+   |                 ------------    ^^^^^^^^^^ creates a temporary value 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[E0716]: temporary value dropped while borrowed
   --> $DIR/promote-not.rs:21:32
    |
@@ -68,7 +78,7 @@ LL | }
    | - temporary value is freed at the end of this statement
 
 error[E0716]: temporary value dropped while borrowed
-  --> $DIR/promote-not.rs:56:29
+  --> $DIR/promote-not.rs:65:29
    |
 LL |     let _val: &'static _ = &(Cell::new(1), 2).0;
    |               ----------    ^^^^^^^^^^^^^^^^^ creates a temporary value which is freed while still in use
@@ -79,7 +89,7 @@ LL | }
    | - temporary value is freed at the end of this statement
 
 error[E0716]: temporary value dropped while borrowed
-  --> $DIR/promote-not.rs:57:29
+  --> $DIR/promote-not.rs:66:29
    |
 LL |     let _val: &'static _ = &(Cell::new(1), 2).1;
    |               ----------    ^^^^^^^^^^^^^^^^^ creates a temporary value which is freed while still in use
@@ -90,7 +100,7 @@ LL | }
    | - temporary value is freed at the end of this statement
 
 error[E0716]: temporary value dropped while borrowed
-  --> $DIR/promote-not.rs:60:29
+  --> $DIR/promote-not.rs:69:29
    |
 LL |     let _val: &'static _ = &(1/0);
    |               ----------    ^^^^^ creates a temporary value which is freed while still in use
@@ -101,7 +111,7 @@ LL | }
    | - temporary value is freed at the end of this statement
 
 error[E0716]: temporary value dropped while borrowed
-  --> $DIR/promote-not.rs:61:29
+  --> $DIR/promote-not.rs:70:29
    |
 LL |     let _val: &'static _ = &(1/(1-1));
    |               ----------    ^^^^^^^^^ creates a temporary value which is freed while still in use
@@ -112,7 +122,7 @@ LL | }
    | - temporary value is freed at the end of this statement
 
 error[E0716]: temporary value dropped while borrowed
-  --> $DIR/promote-not.rs:62:29
+  --> $DIR/promote-not.rs:71:29
    |
 LL |     let _val: &'static _ = &((1+1)/(1-1));
    |               ----------    ^^^^^^^^^^^^^ creates a temporary value which is freed while still in use
@@ -123,7 +133,7 @@ LL | }
    | - temporary value is freed at the end of this statement
 
 error[E0716]: temporary value dropped while borrowed
-  --> $DIR/promote-not.rs:63:29
+  --> $DIR/promote-not.rs:72:29
    |
 LL |     let _val: &'static _ = &(i32::MIN/-1);
    |               ----------    ^^^^^^^^^^^^^ creates a temporary value which is freed while still in use
@@ -134,7 +144,7 @@ LL | }
    | - temporary value is freed at the end of this statement
 
 error[E0716]: temporary value dropped while borrowed
-  --> $DIR/promote-not.rs:64:29
+  --> $DIR/promote-not.rs:73:29
    |
 LL |     let _val: &'static _ = &(i32::MIN/(0-1));
    |               ----------    ^^^^^^^^^^^^^^^^ creates a temporary value which is freed while still in use
@@ -145,7 +155,7 @@ LL | }
    | - temporary value is freed at the end of this statement
 
 error[E0716]: temporary value dropped while borrowed
-  --> $DIR/promote-not.rs:65:29
+  --> $DIR/promote-not.rs:74:29
    |
 LL |     let _val: &'static _ = &(-128i8/-1);
    |               ----------    ^^^^^^^^^^^ creates a temporary value which is freed while still in use
@@ -156,7 +166,7 @@ LL | }
    | - temporary value is freed at the end of this statement
 
 error[E0716]: temporary value dropped while borrowed
-  --> $DIR/promote-not.rs:66:29
+  --> $DIR/promote-not.rs:75:29
    |
 LL |     let _val: &'static _ = &(1%0);
    |               ----------    ^^^^^ creates a temporary value which is freed while still in use
@@ -167,7 +177,7 @@ LL | }
    | - temporary value is freed at the end of this statement
 
 error[E0716]: temporary value dropped while borrowed
-  --> $DIR/promote-not.rs:67:29
+  --> $DIR/promote-not.rs:76:29
    |
 LL |     let _val: &'static _ = &(1%(1-1));
    |               ----------    ^^^^^^^^^ creates a temporary value which is freed while still in use
@@ -178,7 +188,7 @@ LL | }
    | - temporary value is freed at the end of this statement
 
 error[E0716]: temporary value dropped while borrowed
-  --> $DIR/promote-not.rs:68:29
+  --> $DIR/promote-not.rs:77:29
    |
 LL |     let _val: &'static _ = &([1,2,3][4]+1);
    |               ----------    ^^^^^^^^^^^^^^ creates a temporary value which is freed while still in use
@@ -189,7 +199,7 @@ LL | }
    | - temporary value is freed at the end of this statement
 
 error[E0716]: temporary value dropped while borrowed
-  --> $DIR/promote-not.rs:72:29
+  --> $DIR/promote-not.rs:81:29
    |
 LL |     let _val: &'static _ = &TEST_DROP;
    |               ----------    ^^^^^^^^^ creates a temporary value which is freed while still in use
@@ -200,7 +210,7 @@ LL | }
    | - temporary value is freed at the end of this statement
 
 error[E0716]: temporary value dropped while borrowed
-  --> $DIR/promote-not.rs:74:29
+  --> $DIR/promote-not.rs:83:29
    |
 LL |     let _val: &'static _ = &&TEST_DROP;
    |               ----------    ^^^^^^^^^^ creates a temporary value which is freed while still in use
@@ -211,7 +221,7 @@ LL | }
    | - temporary value is freed at the end of this statement
 
 error[E0716]: temporary value dropped while borrowed
-  --> $DIR/promote-not.rs:74:30
+  --> $DIR/promote-not.rs:83:30
    |
 LL |     let _val: &'static _ = &&TEST_DROP;
    |               ----------     ^^^^^^^^^ creates a temporary value which is freed while still in use
@@ -222,7 +232,7 @@ LL | }
    | - temporary value is freed at the end of this statement
 
 error[E0716]: temporary value dropped while borrowed
-  --> $DIR/promote-not.rs:77:29
+  --> $DIR/promote-not.rs:86:29
    |
 LL |     let _val: &'static _ = &(&TEST_DROP,);
    |               ----------    ^^^^^^^^^^^^^ creates a temporary value which is freed while still in use
@@ -233,7 +243,7 @@ LL | }
    | - temporary value is freed at the end of this statement
 
 error[E0716]: temporary value dropped while borrowed
-  --> $DIR/promote-not.rs:77:31
+  --> $DIR/promote-not.rs:86:31
    |
 LL |     let _val: &'static _ = &(&TEST_DROP,);
    |               ----------      ^^^^^^^^^ creates a temporary value which is freed while still in use
@@ -244,7 +254,7 @@ LL | }
    | - temporary value is freed at the end of this statement
 
 error[E0716]: temporary value dropped while borrowed
-  --> $DIR/promote-not.rs:80:29
+  --> $DIR/promote-not.rs:89:29
    |
 LL |     let _val: &'static _ = &[&TEST_DROP; 1];
    |               ----------    ^^^^^^^^^^^^^^^ creates a temporary value which is freed while still in use
@@ -255,7 +265,7 @@ LL | }
    | - temporary value is freed at the end of this statement
 
 error[E0716]: temporary value dropped while borrowed
-  --> $DIR/promote-not.rs:80:31
+  --> $DIR/promote-not.rs:89:31
    |
 LL |     let _val: &'static _ = &[&TEST_DROP; 1];
    |               ----------      ^^^^^^^^^    - temporary value is freed at the end of this statement
@@ -264,7 +274,7 @@ LL |     let _val: &'static _ = &[&TEST_DROP; 1];
    |               type annotation requires that borrow lasts for `'static`
 
 error[E0716]: temporary value dropped while borrowed
-  --> $DIR/promote-not.rs:89:26
+  --> $DIR/promote-not.rs:98:26
    |
 LL |     let x: &'static _ = &UnionWithCell { f1: 0 };
    |            ----------    ^^^^^^^^^^^^^^^^^^^^^^^ creates a temporary value which is freed while still in use
@@ -274,7 +284,7 @@ LL |
 LL | }
    | - temporary value is freed at the end of this statement
 
-error: aborting due to 26 previous errors
+error: aborting due to 27 previous errors
 
 Some errors have detailed explanations: E0493, E0716.
 For more information about an error, try `rustc --explain E0493`.

tests/ui/consts/const-eval/promoted_errors.opt_with_overflow_checks.stderr

@@ -5,28 +5,30 @@
 
 //@ build-pass
 
+#![allow(arithmetic_overflow)]
+
+use std::mem;
+
 const fn assert_static<T>(_: &'static T) {}
 
-#[allow(unconditional_panic)]
-const fn fail() -> i32 {
-    1/0
-}
-const C: i32 = {
-    // Promoted that fails to evaluate in dead code -- this must work
-    // (for backwards compatibility reasons).
-    if false {
-        assert_static(&fail());
-    }
+// Function calls in const on the "main path" (not inside conditionals)
+// do get promoted.
+const fn make_thing() -> i32 {
     42
+}
+const C: () = {
+    assert_static(&make_thing());
+    // Make sure this works even when there's other stuff (like function calls) above the relevant
+    // call in the const initializer.
+    assert_static(&make_thing());
 };
 
 fn main() {
     assert_static(&["a", "b", "c"]);
     assert_static(&["d", "e", "f"]);
-    assert_eq!(C, 42);
 
     // make sure that this does not cause trouble despite overflowing
-    assert_static(&(0-1));
+    assert_static(&(0u32 - 1));
 
     // div-by-non-0 (and also not MIN/-1) is okay
     assert_static(&(1/1));
@@ -36,12 +38,16 @@ fn main() {
     assert_static(&(1%1));
 
     // in-bounds array access is okay
-    assert_static(&([1,2,3][0] + 1));
-    assert_static(&[[1,2][1]]);
+    assert_static(&([1, 2, 3][0] + 1));
+    assert_static(&[[1, 2][1]]);
 
     // Top-level projections are not part of the promoted, so no error here.
     if false {
         #[allow(unconditional_panic)]
-        assert_static(&[1,2,3][4]);
+        assert_static(&[1, 2, 3][4]);
     }
+
+    // More complicated case involving control flow and a `#[rustc_promotable]` function
+    let decision = std::hint::black_box(true);
+    let x: &'static usize = if decision { &mem::size_of::<usize>() } else { &0 };
 }

tests/ui/consts/const-eval/promoted_errors.rs

@@ -0,0 +1,37 @@
+error[E0080]: evaluation of `Fail::<i32>::C` failed
+  --> $DIR/collect-in-promoted-const.rs:9:19
+   |
+LL |     const C: () = panic!();
+   |                   ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-promoted-const.rs:9:19
+   |
+   = note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+note: erroneous constant encountered
+  --> $DIR/collect-in-promoted-const.rs:20:21
+   |
+LL |         let _val = &Fail::<T>::C;
+   |                     ^^^^^^^^^^^^
+
+note: erroneous constant encountered
+  --> $DIR/collect-in-promoted-const.rs:20:20
+   |
+LL |         let _val = &Fail::<T>::C;
+   |                    ^^^^^^^^^^^^^
+
+note: erroneous constant encountered
+  --> $DIR/collect-in-promoted-const.rs:20:21
+   |
+LL |         let _val = &Fail::<T>::C;
+   |                     ^^^^^^^^^^^^
+   |
+   = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
+
+note: the above error was encountered while instantiating `fn f::<i32>`
+  --> $DIR/collect-in-promoted-const.rs:25:5
+   |
+LL |     f::<i32>();
+   |     ^^^^^^^^^^
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0080`.

tests/ui/consts/promote-not.rs

@@ -0,0 +1,53 @@
+error[E0080]: evaluation of `Fail::<T>::C` failed
+  --> $DIR/collect-in-promoted-const.rs:9:19
+   |
+LL |     const C: () = panic!();
+   |                   ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-promoted-const.rs:9:19
+   |
+   = note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+note: erroneous constant encountered
+  --> $DIR/collect-in-promoted-const.rs:20:21
+   |
+LL |         let _val = &Fail::<T>::C;
+   |                     ^^^^^^^^^^^^
+
+error[E0080]: evaluation of `Fail::<i32>::C` failed
+  --> $DIR/collect-in-promoted-const.rs:9:19
+   |
+LL |     const C: () = panic!();
+   |                   ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-promoted-const.rs:9:19
+   |
+   = note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+note: erroneous constant encountered
+  --> $DIR/collect-in-promoted-const.rs:20:21
+   |
+LL |         let _val = &Fail::<T>::C;
+   |                     ^^^^^^^^^^^^
+   |
+   = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
+
+note: erroneous constant encountered
+  --> $DIR/collect-in-promoted-const.rs:20:20
+   |
+LL |         let _val = &Fail::<T>::C;
+   |                    ^^^^^^^^^^^^^
+
+note: erroneous constant encountered
+  --> $DIR/collect-in-promoted-const.rs:20:21
+   |
+LL |         let _val = &Fail::<T>::C;
+   |                     ^^^^^^^^^^^^
+   |
+   = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
+
+note: the above error was encountered while instantiating `fn f::<i32>`
+  --> $DIR/collect-in-promoted-const.rs:25:5
+   |
+LL |     f::<i32>();
+   |     ^^^^^^^^^^
+
+error: aborting due to 2 previous errors
+
+For more information about this error, try `rustc --explain E0080`.

tests/ui/consts/promote-not.stderr

@@ -0,0 +1,26 @@
+//@revisions: noopt opt
+//@ build-fail
+//@[noopt] compile-flags: -Copt-level=0
+//@[opt] compile-flags: -O
+//! Make sure we error on erroneous consts even if they get promoted.
+
+struct Fail<T>(T);
+impl<T> Fail<T> {
+    const C: () = panic!(); //~ERROR evaluation of `Fail::<i32>::C` failed
+    //[opt]~^ ERROR evaluation of `Fail::<T>::C` failed
+    // (Not sure why optimizations lead to this being emitted twice, but as long as compilation
+    // fails either way it's fine.)
+}
+
+#[inline(never)]
+fn f<T>() {
+    if false {
+        // If promotion moved `C` from our required_consts to its own, without adding itself to
+        // our required_consts, then we'd miss the const-eval failure here.
+        let _val = &Fail::<T>::C;
+    }
+}
+
+fn main() {
+    f::<i32>();
+}

tests/ui/consts/promotion.rs

@@ -1,7 +1,7 @@
 //@revisions: noopt opt
 //@[noopt] compile-flags: -Copt-level=0
 //@[opt] compile-flags: -O
-//! Make sure we error on erroneous consts even if they are unused.
+//! Make sure we evaluate const fn calls even if they get promoted and their result ignored.
 
 const unsafe fn ub() {
     std::hint::unreachable_unchecked();

tests/ui/consts/required-consts/collect-in-promoted-const.noopt.stderr

@@ -4,7 +4,7 @@ fn non_elidable<'a, 'b>(a: &'a u8, b: &'b u8) -> &'a u8 {
     a
 }
 
-// The incorrect case without `for<'a>` is tested for in `rfc1623-2.rs`
+// The incorrect case without `for<'a>` is tested for in `rfc1623-3.rs`
 static NON_ELIDABLE_FN: &for<'a> fn(&'a u8, &'a u8) -> &'a u8 =
     &(non_elidable as for<'a> fn(&'a u8, &'a u8) -> &'a u8);
 
@@ -26,10 +26,10 @@ static SOME_STRUCT: &SomeStruct = &SomeStruct {
     foo: &Foo { bools: &[false, true] },
     bar: &Bar { bools: &[true, true] },
     f: &id,
-    //~^ ERROR implementation of `Fn` is not general enough
-    //~| ERROR implementation of `Fn` is not general enough
-    //~| ERROR implementation of `FnOnce` is not general enough
+    //~^ ERROR implementation of `FnOnce` is not general enough
     //~| ERROR implementation of `FnOnce` is not general enough
+    //~| ERROR implementation of `Fn` is not general enough
+    //~| ERROR implementation of `Fn` is not general enough
 };
 
 // very simple test for a 'static static with default lifetime