From 8b90e70e0634a3392b77f514c9755058b1fb85f7 Mon Sep 17 00:00:00 2001 From: lcnr Date: Mon, 2 Dec 2024 13:23:16 +0100 Subject: [PATCH] mir validator: don't store mir phase --- compiler/rustc_mir_transform/src/inline.rs | 10 +-- .../rustc_mir_transform/src/pass_manager.rs | 2 +- compiler/rustc_mir_transform/src/validate.rs | 79 ++++++++----------- 3 files changed, 34 insertions(+), 57 deletions(-) diff --git a/compiler/rustc_mir_transform/src/inline.rs b/compiler/rustc_mir_transform/src/inline.rs index 0878fa26a92c6..79c38b50459c4 100644 --- a/compiler/rustc_mir_transform/src/inline.rs +++ b/compiler/rustc_mir_transform/src/inline.rs @@ -220,15 +220,7 @@ impl<'tcx> Inliner<'tcx> { // Normally, this shouldn't be required, but trait normalization failure can create a // validation ICE. - if !validate_types( - self.tcx, - MirPhase::Runtime(RuntimePhase::Optimized), - self.typing_env, - &callee_body, - &caller_body, - ) - .is_empty() - { + if !validate_types(self.tcx, self.typing_env, &callee_body, &caller_body).is_empty() { return Err("failed to validate callee body"); } diff --git a/compiler/rustc_mir_transform/src/pass_manager.rs b/compiler/rustc_mir_transform/src/pass_manager.rs index 779e7f22101c2..8a45ce0762d59 100644 --- a/compiler/rustc_mir_transform/src/pass_manager.rs +++ b/compiler/rustc_mir_transform/src/pass_manager.rs @@ -292,7 +292,7 @@ fn run_passes_inner<'tcx>( } pub(super) fn validate_body<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>, when: String) { - validate::Validator { when, mir_phase: body.phase }.run_pass(tcx, body); + validate::Validator { when }.run_pass(tcx, body); } fn dump_mir_for_pass<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>, pass_name: &str, is_after: bool) { diff --git a/compiler/rustc_mir_transform/src/validate.rs b/compiler/rustc_mir_transform/src/validate.rs index 51e5c49cea104..936d88e9ac034 100644 --- a/compiler/rustc_mir_transform/src/validate.rs +++ b/compiler/rustc_mir_transform/src/validate.rs @@ -29,12 +29,6 @@ enum EdgeKind { pub(super) struct Validator { /// Describes at which point in the pipeline this validation is happening. pub when: String, - /// The phase for which we are upholding the dialect. If the given phase forbids a specific - /// element, this validator will now emit errors if that specific element is encountered. - /// Note that phases that change the dialect cause all *following* phases to check the - /// invariants of the new dialect. A phase that changes dialects never checks the new invariants - /// itself. - pub mir_phase: MirPhase, } impl<'tcx> crate::MirPass<'tcx> for Validator { @@ -46,11 +40,9 @@ impl<'tcx> crate::MirPass<'tcx> for Validator { if matches!(body.source.instance, InstanceKind::Intrinsic(..) | InstanceKind::Virtual(..)) { return; } - debug_assert_eq!(self.mir_phase, body.phase); let def_id = body.source.def_id(); - let mir_phase = body.phase; let typing_env = body.typing_env(tcx); - let can_unwind = if mir_phase <= MirPhase::Runtime(RuntimePhase::Initial) { + let can_unwind = if body.phase <= MirPhase::Runtime(RuntimePhase::Initial) { // In this case `AbortUnwindingCalls` haven't yet been executed. true } else if !tcx.def_kind(def_id).is_fn_like() { @@ -64,9 +56,7 @@ impl<'tcx> crate::MirPass<'tcx> for Validator { ty::Coroutine(..) => ExternAbi::Rust, // No need to do MIR validation on error bodies ty::Error(_) => return, - _ => { - span_bug!(body.span, "unexpected body ty: {:?} phase {:?}", body_ty, mir_phase) - } + _ => span_bug!(body.span, "unexpected body ty: {body_ty:?}"), }; ty::layout::fn_can_unwind(tcx, Some(def_id), body_abi) @@ -76,7 +66,6 @@ impl<'tcx> crate::MirPass<'tcx> for Validator { when: &self.when, body, tcx, - mir_phase, unwind_edge_count: 0, reachable_blocks: traversal::reachable_as_bitset(body), value_cache: FxHashSet::default(), @@ -86,7 +75,7 @@ impl<'tcx> crate::MirPass<'tcx> for Validator { cfg_checker.check_cleanup_control_flow(); // Also run the TypeChecker. - for (location, msg) in validate_types(tcx, self.mir_phase, typing_env, body, body) { + for (location, msg) in validate_types(tcx, typing_env, body, body) { cfg_checker.fail(location, msg); } @@ -107,7 +96,6 @@ struct CfgChecker<'a, 'tcx> { when: &'a str, body: &'a Body<'tcx>, tcx: TyCtxt<'tcx>, - mir_phase: MirPhase, unwind_edge_count: usize, reachable_blocks: BitSet, value_cache: FxHashSet, @@ -294,7 +282,7 @@ impl<'a, 'tcx> Visitor<'tcx> for CfgChecker<'a, 'tcx> { fn visit_statement(&mut self, statement: &Statement<'tcx>, location: Location) { match &statement.kind { StatementKind::AscribeUserType(..) => { - if self.mir_phase >= MirPhase::Runtime(RuntimePhase::Initial) { + if self.body.phase >= MirPhase::Runtime(RuntimePhase::Initial) { self.fail( location, "`AscribeUserType` should have been removed after drop lowering phase", @@ -302,7 +290,7 @@ impl<'a, 'tcx> Visitor<'tcx> for CfgChecker<'a, 'tcx> { } } StatementKind::FakeRead(..) => { - if self.mir_phase >= MirPhase::Runtime(RuntimePhase::Initial) { + if self.body.phase >= MirPhase::Runtime(RuntimePhase::Initial) { self.fail( location, "`FakeRead` should have been removed after drop lowering phase", @@ -310,17 +298,17 @@ impl<'a, 'tcx> Visitor<'tcx> for CfgChecker<'a, 'tcx> { } } StatementKind::SetDiscriminant { .. } => { - if self.mir_phase < MirPhase::Runtime(RuntimePhase::Initial) { + if self.body.phase < MirPhase::Runtime(RuntimePhase::Initial) { self.fail(location, "`SetDiscriminant`is not allowed until deaggregation"); } } StatementKind::Deinit(..) => { - if self.mir_phase < MirPhase::Runtime(RuntimePhase::Initial) { + if self.body.phase < MirPhase::Runtime(RuntimePhase::Initial) { self.fail(location, "`Deinit`is not allowed until deaggregation"); } } StatementKind::Retag(kind, _) => { - // FIXME(JakobDegen) The validator should check that `self.mir_phase < + // FIXME(JakobDegen) The validator should check that `self.body.phase < // DropsLowered`. However, this causes ICEs with generation of drop shims, which // seem to fail to set their `MirPhase` correctly. if matches!(kind, RetagKind::TwoPhase) { @@ -328,7 +316,7 @@ impl<'a, 'tcx> Visitor<'tcx> for CfgChecker<'a, 'tcx> { } } StatementKind::Coverage(kind) => { - if self.mir_phase >= MirPhase::Analysis(AnalysisPhase::PostCleanup) + if self.body.phase >= MirPhase::Analysis(AnalysisPhase::PostCleanup) && let CoverageKind::BlockMarker { .. } | CoverageKind::SpanMarker { .. } = kind { self.fail( @@ -391,7 +379,7 @@ impl<'a, 'tcx> Visitor<'tcx> for CfgChecker<'a, 'tcx> { // the return edge from the call. FIXME(tmiasko): Since this is a strictly code // generation concern, the code generation should be responsible for handling // it. - if self.mir_phase >= MirPhase::Runtime(RuntimePhase::Optimized) + if self.body.phase >= MirPhase::Runtime(RuntimePhase::Optimized) && self.is_critical_call_edge(target, unwind) { self.fail( @@ -440,7 +428,7 @@ impl<'a, 'tcx> Visitor<'tcx> for CfgChecker<'a, 'tcx> { if self.body.coroutine.is_none() { self.fail(location, "`Yield` cannot appear outside coroutine bodies"); } - if self.mir_phase >= MirPhase::Runtime(RuntimePhase::Initial) { + if self.body.phase >= MirPhase::Runtime(RuntimePhase::Initial) { self.fail(location, "`Yield` should have been replaced by coroutine lowering"); } self.check_edge(location, *resume, EdgeKind::Normal); @@ -449,7 +437,7 @@ impl<'a, 'tcx> Visitor<'tcx> for CfgChecker<'a, 'tcx> { } } TerminatorKind::FalseEdge { real_target, imaginary_target } => { - if self.mir_phase >= MirPhase::Runtime(RuntimePhase::Initial) { + if self.body.phase >= MirPhase::Runtime(RuntimePhase::Initial) { self.fail( location, "`FalseEdge` should have been removed after drop elaboration", @@ -459,7 +447,7 @@ impl<'a, 'tcx> Visitor<'tcx> for CfgChecker<'a, 'tcx> { self.check_edge(location, *imaginary_target, EdgeKind::Normal); } TerminatorKind::FalseUnwind { real_target, unwind } => { - if self.mir_phase >= MirPhase::Runtime(RuntimePhase::Initial) { + if self.body.phase >= MirPhase::Runtime(RuntimePhase::Initial) { self.fail( location, "`FalseUnwind` should have been removed after drop elaboration", @@ -478,7 +466,7 @@ impl<'a, 'tcx> Visitor<'tcx> for CfgChecker<'a, 'tcx> { if self.body.coroutine.is_none() { self.fail(location, "`CoroutineDrop` cannot appear outside coroutine bodies"); } - if self.mir_phase >= MirPhase::Runtime(RuntimePhase::Initial) { + if self.body.phase >= MirPhase::Runtime(RuntimePhase::Initial) { self.fail( location, "`CoroutineDrop` should have been replaced by coroutine lowering", @@ -532,13 +520,11 @@ impl<'a, 'tcx> Visitor<'tcx> for CfgChecker<'a, 'tcx> { /// `optimized_mir` is available. pub(super) fn validate_types<'tcx>( tcx: TyCtxt<'tcx>, - mir_phase: MirPhase, typing_env: ty::TypingEnv<'tcx>, body: &Body<'tcx>, caller_body: &Body<'tcx>, ) -> Vec<(Location, String)> { - let mut type_checker = - TypeChecker { body, caller_body, tcx, typing_env, mir_phase, failures: Vec::new() }; + let mut type_checker = TypeChecker { body, caller_body, tcx, typing_env, failures: Vec::new() }; type_checker.visit_body(body); type_checker.failures } @@ -548,7 +534,6 @@ struct TypeChecker<'a, 'tcx> { caller_body: &'a Body<'tcx>, tcx: TyCtxt<'tcx>, typing_env: ty::TypingEnv<'tcx>, - mir_phase: MirPhase, failures: Vec<(Location, String)>, } @@ -577,7 +562,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { // After borrowck subtyping should be fully explicit via // `Subtype` projections. - let variance = if self.mir_phase >= MirPhase::Runtime(RuntimePhase::Initial) { + let variance = if self.body.phase >= MirPhase::Runtime(RuntimePhase::Initial) { Variance::Invariant } else { Variance::Covariant @@ -618,7 +603,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { fn visit_operand(&mut self, operand: &Operand<'tcx>, location: Location) { // This check is somewhat expensive, so only run it when -Zvalidate-mir is passed. if self.tcx.sess.opts.unstable_opts.validate_mir - && self.mir_phase < MirPhase::Runtime(RuntimePhase::Initial) + && self.body.phase < MirPhase::Runtime(RuntimePhase::Initial) { // `Operand::Copy` is only supposed to be used with `Copy` types. if let Operand::Copy(place) = operand { @@ -642,7 +627,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { ) { match elem { ProjectionElem::OpaqueCast(ty) - if self.mir_phase >= MirPhase::Runtime(RuntimePhase::Initial) => + if self.body.phase >= MirPhase::Runtime(RuntimePhase::Initial) => { self.fail( location, @@ -656,7 +641,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { } } ProjectionElem::Deref - if self.mir_phase >= MirPhase::Runtime(RuntimePhase::PostCleanup) => + if self.body.phase >= MirPhase::Runtime(RuntimePhase::PostCleanup) => { let base_ty = place_ref.ty(&self.body.local_decls, self.tcx).ty; @@ -856,7 +841,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { // Set off any `bug!`s in the type computation code let _ = place.ty(&self.body.local_decls, self.tcx); - if self.mir_phase >= MirPhase::Runtime(RuntimePhase::Initial) + if self.body.phase >= MirPhase::Runtime(RuntimePhase::Initial) && place.projection.len() > 1 && cntxt != PlaceContext::NonUse(NonUseContext::VarDebugInfo) && place.projection[1..].contains(&ProjectionElem::Deref) @@ -974,7 +959,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { } } AggregateKind::RawPtr(pointee_ty, mutability) => { - if !matches!(self.mir_phase, MirPhase::Runtime(_)) { + if !matches!(self.body.phase, MirPhase::Runtime(_)) { // It would probably be fine to support this in earlier phases, but at the // time of writing it's only ever introduced from intrinsic lowering, so // earlier things just `bug!` on it. @@ -1016,7 +1001,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { } }, Rvalue::Ref(_, BorrowKind::Fake(_), _) => { - if self.mir_phase >= MirPhase::Runtime(RuntimePhase::Initial) { + if self.body.phase >= MirPhase::Runtime(RuntimePhase::Initial) { self.fail( location, "`Assign` statement with a `Fake` borrow should have been removed in runtime MIR", @@ -1130,7 +1115,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { ); } UnOp::PtrMetadata => { - if !matches!(self.mir_phase, MirPhase::Runtime(_)) { + if !matches!(self.body.phase, MirPhase::Runtime(_)) { // It would probably be fine to support this in earlier phases, but at // the time of writing it's only ever introduced from intrinsic // lowering or other runtime-phase optimization passes, so earlier @@ -1206,7 +1191,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { "CastKind::{kind:?} output must be a raw const pointer, not {:?}", ty::RawPtr(_, Mutability::Not) ); - if self.mir_phase >= MirPhase::Analysis(AnalysisPhase::PostCleanup) { + if self.body.phase >= MirPhase::Analysis(AnalysisPhase::PostCleanup) { self.fail(location, format!("After borrowck, MIR disallows {kind:?}")); } } @@ -1222,7 +1207,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { "CastKind::{kind:?} output must be a raw pointer, not {:?}", ty::RawPtr(..) ); - if self.mir_phase >= MirPhase::Analysis(AnalysisPhase::PostCleanup) { + if self.body.phase >= MirPhase::Analysis(AnalysisPhase::PostCleanup) { self.fail(location, format!("After borrowck, MIR disallows {kind:?}")); } } @@ -1288,7 +1273,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { } } CastKind::Transmute => { - if let MirPhase::Runtime(..) = self.mir_phase { + if let MirPhase::Runtime(..) = self.body.phase { // Unlike `mem::transmute`, a MIR `Transmute` is well-formed // for any two `Sized` types, just potentially UB to run. @@ -1317,7 +1302,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { location, format!( "Transmute is not supported in non-runtime phase {:?}.", - self.mir_phase + self.body.phase ), ); } @@ -1404,7 +1389,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { } } StatementKind::AscribeUserType(..) => { - if self.mir_phase >= MirPhase::Runtime(RuntimePhase::Initial) { + if self.body.phase >= MirPhase::Runtime(RuntimePhase::Initial) { self.fail( location, "`AscribeUserType` should have been removed after drop lowering phase", @@ -1412,7 +1397,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { } } StatementKind::FakeRead(..) => { - if self.mir_phase >= MirPhase::Runtime(RuntimePhase::Initial) { + if self.body.phase >= MirPhase::Runtime(RuntimePhase::Initial) { self.fail( location, "`FakeRead` should have been removed after drop lowering phase", @@ -1463,7 +1448,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { } } StatementKind::SetDiscriminant { place, .. } => { - if self.mir_phase < MirPhase::Runtime(RuntimePhase::Initial) { + if self.body.phase < MirPhase::Runtime(RuntimePhase::Initial) { self.fail(location, "`SetDiscriminant`is not allowed until deaggregation"); } let pty = place.ty(&self.body.local_decls, self.tcx).ty.kind(); @@ -1477,12 +1462,12 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { } } StatementKind::Deinit(..) => { - if self.mir_phase < MirPhase::Runtime(RuntimePhase::Initial) { + if self.body.phase < MirPhase::Runtime(RuntimePhase::Initial) { self.fail(location, "`Deinit`is not allowed until deaggregation"); } } StatementKind::Retag(kind, _) => { - // FIXME(JakobDegen) The validator should check that `self.mir_phase < + // FIXME(JakobDegen) The validator should check that `self.body.phase < // DropsLowered`. However, this causes ICEs with generation of drop shims, which // seem to fail to set their `MirPhase` correctly. if matches!(kind, RetagKind::TwoPhase) {