diff --git a/compiler/rustc_mir_transform/src/lib.rs b/compiler/rustc_mir_transform/src/lib.rs index 945c3c662a604..f0cced5abcbf6 100644 --- a/compiler/rustc_mir_transform/src/lib.rs +++ b/compiler/rustc_mir_transform/src/lib.rs @@ -343,13 +343,6 @@ fn mir_promoted( body.tainted_by_errors = Some(error_reported); } - 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) { - required_consts_visitor.visit_basic_block_data(bb, bb_data); - } - body.required_consts = required_consts; - // What we need to run borrowck etc. let promote_pass = promote_consts::PromoteTemps::default(); pm::run_passes( @@ -359,6 +352,14 @@ fn mir_promoted( Some(MirPhase::Analysis(AnalysisPhase::Initial)), ); + // Promotion generates new consts; we run this after promotion to ensure they are accounted for. + 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) { + required_consts_visitor.visit_basic_block_data(bb, bb_data); + } + body.required_consts = required_consts; + let promoted = promote_pass.promoted_fragments.into_inner(); (tcx.alloc_steal_mir(body), tcx.alloc_steal_promoted(promoted)) } diff --git a/compiler/rustc_mir_transform/src/promote_consts.rs b/compiler/rustc_mir_transform/src/promote_consts.rs index 577b8f2080fcd..17e9ea2b97d7d 100644 --- a/compiler/rustc_mir_transform/src/promote_consts.rs +++ b/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,11 @@ fn collect_temps_and_candidates<'tcx>( struct Validator<'a, 'tcx> { ccx: &'a ConstCx<'a, 'tcx>, temps: &'a mut IndexSlice, + /// For backwards compatibility, we are promoting function calls in `const`/`static` + /// initializers. But we want to avoid evaluating code that otherwise would not have been + /// evaluated, so we only do thos in basic blocks that are guaranteed to evaluate. Here we cache + /// the result of computing that set of basic blocks. + const_fn_safe_blocks: Option>, } impl<'a, 'tcx> std::ops::Deref for Validator<'a, 'tcx> { @@ -260,7 +266,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); @@ -564,20 +572,59 @@ 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 const_fn_safe_blocks(body: &mir::Body<'tcx>) -> FxHashSet { + 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 + } + + fn is_const_fn_safe_block(&mut self, block: BasicBlock) -> bool { + let body = self.body; + let safe_blocks = + self.const_fn_safe_blocks.get_or_insert_with(|| Self::const_fn_safe_blocks(body)); + safe_blocks.contains(&block) + } + fn validate_call( &mut self, callee: &Operand<'tcx>, args: &[Spanned>], + block: BasicBlock, ) -> Result<(), Unpromotable> { let fn_ty = callee.ty(self.body, self.tcx); - // Inside const/static items, we promote all (eligible) function calls. + // Inside const/static items, we may promote (eligible) function calls. // Everywhere else, we require `#[rustc_promotable]` on the callee. - let promote_all_const_fn = matches!( + let promote_const_fn = matches!( self.const_kind, Some(hir::ConstContext::Static(_) | hir::ConstContext::Const { inline: false }) ); - if !promote_all_const_fn { + if !promote_const_fn { if let ty::FnDef(def_id, _) = *fn_ty.kind() { // Never promote runtime `const fn` calls of // functions without `#[rustc_promotable]`. @@ -595,6 +642,11 @@ impl<'tcx> Validator<'_, 'tcx> { return Err(Unpromotable); } + if !self.is_const_fn_safe_block(block) { + // This function may error, and it may not even be executed as part of this `const`, so don't promote. + return Err(Unpromotable); + } + self.validate_operand(callee)?; for arg in args { self.validate_operand(&arg.node)?; @@ -610,7 +662,7 @@ fn validate_candidates( temps: &mut IndexSlice, candidates: &[Candidate], ) -> Vec { - let mut validator = Validator { ccx, temps }; + let mut validator = Validator { ccx, temps, const_fn_safe_blocks: None }; candidates .iter() diff --git a/tests/ui/consts/const-eval/promoted_errors.noopt.stderr b/tests/ui/consts/const-eval/promoted_errors.noopt.stderr deleted file mode 100644 index 2a254bfde8204..0000000000000 --- a/tests/ui/consts/const-eval/promoted_errors.noopt.stderr +++ /dev/null @@ -1,44 +0,0 @@ -warning: this arithmetic operation will overflow - --> $DIR/promoted_errors.rs:15:5 - | -LL | 0 - 1 - | ^^^^^ attempt to compute `0_u32 - 1_u32`, which would overflow - | -note: the lint level is defined here - --> $DIR/promoted_errors.rs:11:9 - | -LL | #![warn(arithmetic_overflow, unconditional_panic)] - | ^^^^^^^^^^^^^^^^^^^ - -warning: this operation will panic at runtime - --> $DIR/promoted_errors.rs:19:5 - | -LL | 1 / 0 - | ^^^^^ attempt to divide `1_i32` by zero - | -note: the lint level is defined here - --> $DIR/promoted_errors.rs:11:30 - | -LL | #![warn(arithmetic_overflow, unconditional_panic)] - | ^^^^^^^^^^^^^^^^^^^ - -warning: this operation will panic at runtime - --> $DIR/promoted_errors.rs:23:5 - | -LL | 1 / (1 - 1) - | ^^^^^^^^^^^ attempt to divide `1_i32` by zero - -warning: this operation will panic at runtime - --> $DIR/promoted_errors.rs:27:5 - | -LL | 1 / (false as i32) - | ^^^^^^^^^^^^^^^^^^ attempt to divide `1_i32` by zero - -warning: this operation will panic at runtime - --> $DIR/promoted_errors.rs:31:5 - | -LL | [1, 2, 3][4] - | ^^^^^^^^^^^^ index out of bounds: the length is 3 but the index is 4 - -warning: 5 warnings emitted - diff --git a/tests/ui/consts/const-eval/promoted_errors.opt.stderr b/tests/ui/consts/const-eval/promoted_errors.opt.stderr deleted file mode 100644 index 2a254bfde8204..0000000000000 --- a/tests/ui/consts/const-eval/promoted_errors.opt.stderr +++ /dev/null @@ -1,44 +0,0 @@ -warning: this arithmetic operation will overflow - --> $DIR/promoted_errors.rs:15:5 - | -LL | 0 - 1 - | ^^^^^ attempt to compute `0_u32 - 1_u32`, which would overflow - | -note: the lint level is defined here - --> $DIR/promoted_errors.rs:11:9 - | -LL | #![warn(arithmetic_overflow, unconditional_panic)] - | ^^^^^^^^^^^^^^^^^^^ - -warning: this operation will panic at runtime - --> $DIR/promoted_errors.rs:19:5 - | -LL | 1 / 0 - | ^^^^^ attempt to divide `1_i32` by zero - | -note: the lint level is defined here - --> $DIR/promoted_errors.rs:11:30 - | -LL | #![warn(arithmetic_overflow, unconditional_panic)] - | ^^^^^^^^^^^^^^^^^^^ - -warning: this operation will panic at runtime - --> $DIR/promoted_errors.rs:23:5 - | -LL | 1 / (1 - 1) - | ^^^^^^^^^^^ attempt to divide `1_i32` by zero - -warning: this operation will panic at runtime - --> $DIR/promoted_errors.rs:27:5 - | -LL | 1 / (false as i32) - | ^^^^^^^^^^^^^^^^^^ attempt to divide `1_i32` by zero - -warning: this operation will panic at runtime - --> $DIR/promoted_errors.rs:31:5 - | -LL | [1, 2, 3][4] - | ^^^^^^^^^^^^ index out of bounds: the length is 3 but the index is 4 - -warning: 5 warnings emitted - diff --git a/tests/ui/consts/const-eval/promoted_errors.opt_with_overflow_checks.stderr b/tests/ui/consts/const-eval/promoted_errors.opt_with_overflow_checks.stderr deleted file mode 100644 index 2a254bfde8204..0000000000000 --- a/tests/ui/consts/const-eval/promoted_errors.opt_with_overflow_checks.stderr +++ /dev/null @@ -1,44 +0,0 @@ -warning: this arithmetic operation will overflow - --> $DIR/promoted_errors.rs:15:5 - | -LL | 0 - 1 - | ^^^^^ attempt to compute `0_u32 - 1_u32`, which would overflow - | -note: the lint level is defined here - --> $DIR/promoted_errors.rs:11:9 - | -LL | #![warn(arithmetic_overflow, unconditional_panic)] - | ^^^^^^^^^^^^^^^^^^^ - -warning: this operation will panic at runtime - --> $DIR/promoted_errors.rs:19:5 - | -LL | 1 / 0 - | ^^^^^ attempt to divide `1_i32` by zero - | -note: the lint level is defined here - --> $DIR/promoted_errors.rs:11:30 - | -LL | #![warn(arithmetic_overflow, unconditional_panic)] - | ^^^^^^^^^^^^^^^^^^^ - -warning: this operation will panic at runtime - --> $DIR/promoted_errors.rs:23:5 - | -LL | 1 / (1 - 1) - | ^^^^^^^^^^^ attempt to divide `1_i32` by zero - -warning: this operation will panic at runtime - --> $DIR/promoted_errors.rs:27:5 - | -LL | 1 / (false as i32) - | ^^^^^^^^^^^^^^^^^^ attempt to divide `1_i32` by zero - -warning: this operation will panic at runtime - --> $DIR/promoted_errors.rs:31:5 - | -LL | [1, 2, 3][4] - | ^^^^^^^^^^^^ index out of bounds: the length is 3 but the index is 4 - -warning: 5 warnings emitted - diff --git a/tests/ui/consts/const-eval/promoted_errors.rs b/tests/ui/consts/const-eval/promoted_errors.rs deleted file mode 100644 index e806d4a32468d..0000000000000 --- a/tests/ui/consts/const-eval/promoted_errors.rs +++ /dev/null @@ -1,52 +0,0 @@ -//@ revisions: noopt opt opt_with_overflow_checks -//@[noopt]compile-flags: -C opt-level=0 -//@[opt]compile-flags: -O -//@[opt_with_overflow_checks]compile-flags: -C overflow-checks=on -O - -//@ build-pass -//@ ignore-pass (test emits codegen-time warnings and verifies that they are not errors) - -//! This test ensures that when we promote code that fails to evaluate, the build still succeeds. - -#![warn(arithmetic_overflow, unconditional_panic)] - -// The only way to have promoteds that fail is in `const fn` called from `const`/`static`. -const fn overflow() -> u32 { - 0 - 1 - //~^ WARN this arithmetic operation will overflow -} -const fn div_by_zero1() -> i32 { - 1 / 0 - //~^ WARN this operation will panic at runtime -} -const fn div_by_zero2() -> i32 { - 1 / (1 - 1) - //~^ WARN this operation will panic at runtime -} -const fn div_by_zero3() -> i32 { - 1 / (false as i32) - //~^ WARN this operation will panic at runtime -} -const fn oob() -> i32 { - [1, 2, 3][4] - //~^ WARN this operation will panic at runtime -} - -const fn mk_false() -> bool { false } - -// An actually used constant referencing failing promoteds in dead code. -// This needs to always work. -const Y: () = { - if mk_false() { - let _x: &'static u32 = &overflow(); - let _x: &'static i32 = &div_by_zero1(); - let _x: &'static i32 = &div_by_zero2(); - let _x: &'static i32 = &div_by_zero3(); - let _x: &'static i32 = &oob(); - } - () -}; - -fn main() { - Y; -} diff --git a/tests/ui/consts/promote-not.rs b/tests/ui/consts/promote-not.rs index 907617052f119..f32d8371f3ca8 100644 --- a/tests/ui/consts/promote-not.rs +++ b/tests/ui/consts/promote-not.rs @@ -41,6 +41,15 @@ const TEST_INTERIOR_MUT: () = { const TEST_DROP: String = String::new(); +// 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 diff --git a/tests/ui/consts/promote-not.stderr b/tests/ui/consts/promote-not.stderr index 524d69817217d..da27290fa9281 100644 --- a/tests/ui/consts/promote-not.stderr +++ b/tests/ui/consts/promote-not.stderr @@ -38,6 +38,16 @@ LL | let _val: &'static _ = &(Cell::new(1), 2).1; LL | }; | - temporary value is freed at the end of this statement +error[E0716]: temporary value dropped while borrowed + --> $DIR/promote-not.rs:49: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:20:32 | @@ -59,7 +69,7 @@ LL | } | - temporary value is freed at the end of this statement error[E0716]: temporary value dropped while borrowed - --> $DIR/promote-not.rs:46:29 + --> $DIR/promote-not.rs:55:29 | LL | let _val: &'static _ = &(Cell::new(1), 2).0; | ---------- ^^^^^^^^^^^^^^^^^ creates a temporary value which is freed while still in use @@ -70,7 +80,7 @@ LL | } | - temporary value is freed at the end of this statement error[E0716]: temporary value dropped while borrowed - --> $DIR/promote-not.rs:47:29 + --> $DIR/promote-not.rs:56:29 | LL | let _val: &'static _ = &(Cell::new(1), 2).1; | ---------- ^^^^^^^^^^^^^^^^^ creates a temporary value which is freed while still in use @@ -81,7 +91,7 @@ LL | } | - temporary value is freed at the end of this statement error[E0716]: temporary value dropped while borrowed - --> $DIR/promote-not.rs:50:29 + --> $DIR/promote-not.rs:59:29 | LL | let _val: &'static _ = &(1/0); | ---------- ^^^^^ creates a temporary value which is freed while still in use @@ -92,7 +102,7 @@ LL | } | - temporary value is freed at the end of this statement error[E0716]: temporary value dropped while borrowed - --> $DIR/promote-not.rs:51:29 + --> $DIR/promote-not.rs:60:29 | LL | let _val: &'static _ = &(1/(1-1)); | ---------- ^^^^^^^^^ creates a temporary value which is freed while still in use @@ -103,7 +113,7 @@ LL | } | - temporary value is freed at the end of this statement error[E0716]: temporary value dropped while borrowed - --> $DIR/promote-not.rs:52:29 + --> $DIR/promote-not.rs:61:29 | LL | let _val: &'static _ = &(1%0); | ---------- ^^^^^ creates a temporary value which is freed while still in use @@ -114,7 +124,7 @@ LL | } | - temporary value is freed at the end of this statement error[E0716]: temporary value dropped while borrowed - --> $DIR/promote-not.rs:53:29 + --> $DIR/promote-not.rs:62:29 | LL | let _val: &'static _ = &(1%(1-1)); | ---------- ^^^^^^^^^ creates a temporary value which is freed while still in use @@ -125,7 +135,7 @@ LL | } | - temporary value is freed at the end of this statement error[E0716]: temporary value dropped while borrowed - --> $DIR/promote-not.rs:54:29 + --> $DIR/promote-not.rs:63:29 | LL | let _val: &'static _ = &([1,2,3][4]+1); | ---------- ^^^^^^^^^^^^^^ creates a temporary value which is freed while still in use @@ -136,7 +146,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 _ = &TEST_DROP; | ---------- ^^^^^^^^^ creates a temporary value which is freed while still in use @@ -147,7 +157,7 @@ LL | } | - temporary value is freed at the end of this statement error[E0716]: temporary value dropped while borrowed - --> $DIR/promote-not.rs:59:29 + --> $DIR/promote-not.rs:68:29 | LL | let _val: &'static _ = &&TEST_DROP; | ---------- ^^^^^^^^^^ creates a temporary value which is freed while still in use @@ -158,7 +168,7 @@ LL | } | - temporary value is freed at the end of this statement error[E0716]: temporary value dropped while borrowed - --> $DIR/promote-not.rs:59:30 + --> $DIR/promote-not.rs:68:30 | LL | let _val: &'static _ = &&TEST_DROP; | ---------- ^^^^^^^^^ creates a temporary value which is freed while still in use @@ -169,7 +179,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 _ = &(&TEST_DROP,); | ---------- ^^^^^^^^^^^^^ creates a temporary value which is freed while still in use @@ -180,7 +190,7 @@ LL | } | - temporary value is freed at the end of this statement error[E0716]: temporary value dropped while borrowed - --> $DIR/promote-not.rs:62:31 + --> $DIR/promote-not.rs:71:31 | LL | let _val: &'static _ = &(&TEST_DROP,); | ---------- ^^^^^^^^^ creates a temporary value which is freed while still in use @@ -191,7 +201,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 _ = &[&TEST_DROP; 1]; | ---------- ^^^^^^^^^^^^^^^ creates a temporary value which is freed while still in use @@ -202,7 +212,7 @@ LL | } | - temporary value is freed at the end of this statement error[E0716]: temporary value dropped while borrowed - --> $DIR/promote-not.rs:65:31 + --> $DIR/promote-not.rs:74:31 | LL | let _val: &'static _ = &[&TEST_DROP; 1]; | ---------- ^^^^^^^^^ - temporary value is freed at the end of this statement @@ -210,6 +220,6 @@ LL | let _val: &'static _ = &[&TEST_DROP; 1]; | | creates a temporary value which is freed while still in use | type annotation requires that borrow lasts for `'static` -error: aborting due to 20 previous errors +error: aborting due to 21 previous errors For more information about this error, try `rustc --explain E0716`. diff --git a/tests/ui/consts/promotion.rs b/tests/ui/consts/promotion.rs index 211dcf8a4e8f2..c46d88ec13ee6 100644 --- a/tests/ui/consts/promotion.rs +++ b/tests/ui/consts/promotion.rs @@ -5,28 +5,23 @@ //@ build-pass +#![allow(arithmetic_overflow)] + const fn assert_static(_: &'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()); - } - 42 +// 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()); }; 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 is okay assert_static(&(1/1));