From 20206b325f1eddff6f48f075cfe23976d262e395 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 12 Mar 2024 18:15:58 +0100 Subject: [PATCH] compute required_consts before promotion, and add promoteds that may fail --- compiler/rustc_mir_transform/src/lib.rs | 17 ++++--- .../rustc_mir_transform/src/promote_consts.rs | 49 +++++++++++++------ tests/ui/rfcs/rfc-1623-static/rfc1623-2.rs | 4 -- .../ui/rfcs/rfc-1623-static/rfc1623-2.stderr | 42 +--------------- 4 files changed, 44 insertions(+), 68 deletions(-) diff --git a/compiler/rustc_mir_transform/src/lib.rs b/compiler/rustc_mir_transform/src/lib.rs index b6a1c0423f9d8..ad544359ea7af 100644 --- a/compiler/rustc_mir_transform/src/lib.rs +++ b/compiler/rustc_mir_transform/src/lib.rs @@ -343,6 +343,15 @@ 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) { + 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( @@ -352,14 +361,6 @@ 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 226641a5bc726..de4df7d2c2ef5 100644 --- a/compiler/rustc_mir_transform/src/promote_consts.rs +++ b/compiler/rustc_mir_transform/src/promote_consts.rs @@ -687,7 +687,6 @@ impl<'tcx> Validator<'_, 'tcx> { } } -// FIXME(eddyb) remove the differences for promotability in `static`, `const`, `const fn`. fn validate_candidates( ccx: &ConstCx<'_, '_>, temps: &mut IndexSlice, @@ -712,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> { @@ -814,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); @@ -848,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; @@ -858,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(); @@ -898,22 +901,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, ) }; @@ -925,6 +932,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 } } @@ -984,6 +997,11 @@ fn promote_candidates<'tcx>( None, body.tainted_by_errors, ); + // We keep `required_consts` of the new MIR body empty. All consts mentioned here have + // already been added to the parent MIR's `required_consts` (that is computed before + // promotion), and no matter where this promoted const ends up, our parent MIR must be + // somewhere in the reachable dependency chain so we can rely on its required consts being + // evaluated. promoted.phase = MirPhase::Analysis(AnalysisPhase::Initial); let promoter = Promoter { @@ -993,6 +1011,7 @@ fn promote_candidates<'tcx>( temps: &mut temps, extra_statements: &mut extra_statements, keep_original: false, + add_to_required: false, }; let mut promoted = promoter.promote_candidate(candidate, promotions.len()); diff --git a/tests/ui/rfcs/rfc-1623-static/rfc1623-2.rs b/tests/ui/rfcs/rfc-1623-static/rfc1623-2.rs index 7ac329e698b31..5dac0bcc4c500 100644 --- a/tests/ui/rfcs/rfc-1623-static/rfc1623-2.rs +++ b/tests/ui/rfcs/rfc-1623-static/rfc1623-2.rs @@ -28,10 +28,6 @@ static SOME_STRUCT: &SomeStruct = &SomeStruct { f: &id, //~^ ERROR mismatched types //~| ERROR mismatched types - //~| ERROR mismatched types - //~| ERROR mismatched types - //~| 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 `FnOnce` is not general enough }; diff --git a/tests/ui/rfcs/rfc-1623-static/rfc1623-2.stderr b/tests/ui/rfcs/rfc-1623-static/rfc1623-2.stderr index 559572f47affe..52c700c326e30 100644 --- a/tests/ui/rfcs/rfc-1623-static/rfc1623-2.stderr +++ b/tests/ui/rfcs/rfc-1623-static/rfc1623-2.stderr @@ -35,46 +35,6 @@ LL | f: &id, = note: `fn(&Foo<'2>) -> &Foo<'2> {id::<&Foo<'2>>}` must implement `FnOnce<(&'a Foo<'1>,)>`, for any lifetime `'1`... = note: ...but it actually implements `FnOnce<(&Foo<'2>,)>`, for some specific lifetime `'2` -error[E0308]: mismatched types - --> $DIR/rfc1623-2.rs:28:8 - | -LL | f: &id, - | ^^^ one type is more general than the other - | - = note: expected trait `for<'a, 'b> Fn(&'a Foo<'b>)` - found trait `Fn(&Foo<'_>)` - = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` - -error[E0308]: mismatched types - --> $DIR/rfc1623-2.rs:28:8 - | -LL | f: &id, - | ^^^ one type is more general than the other - | - = note: expected trait `for<'a, 'b> Fn(&'a Foo<'b>)` - found trait `Fn(&Foo<'_>)` - = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` - -error: implementation of `FnOnce` is not general enough - --> $DIR/rfc1623-2.rs:28:8 - | -LL | f: &id, - | ^^^ implementation of `FnOnce` is not general enough - | - = note: `fn(&'2 Foo<'_>) -> &'2 Foo<'_> {id::<&'2 Foo<'_>>}` must implement `FnOnce<(&'1 Foo<'b>,)>`, for any lifetime `'1`... - = note: ...but it actually implements `FnOnce<(&'2 Foo<'_>,)>`, for some specific lifetime `'2` - = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` - -error: implementation of `FnOnce` is not general enough - --> $DIR/rfc1623-2.rs:28:8 - | -LL | f: &id, - | ^^^ implementation of `FnOnce` is not general enough - | - = note: `fn(&Foo<'2>) -> &Foo<'2> {id::<&Foo<'2>>}` must implement `FnOnce<(&'a Foo<'1>,)>`, for any lifetime `'1`... - = note: ...but it actually implements `FnOnce<(&Foo<'2>,)>`, for some specific lifetime `'2` - = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` - -error: aborting due to 8 previous errors +error: aborting due to 4 previous errors For more information about this error, try `rustc --explain E0308`.