Skip to content

Commit

Permalink
mir validator: don't store mir phase
Browse files Browse the repository at this point in the history
  • Loading branch information
@lcnr
lcnr committed Dec 2, 2024
1 parent cb2bd2b commit c71705d
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 57 deletions.
10 changes: 1 addition & 9 deletions compiler/rustc_mir_transform/src/inline.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_transform/src/pass_manager.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
79 changes: 32 additions & 47 deletions compiler/rustc_mir_transform/src/validate.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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() {
Expand All @@ -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)
Expand All @@ -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(),
Expand All @@ -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);
}

Expand All @@ -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<BasicBlock>,
value_cache: FxHashSet<u128>,
Expand Down Expand Up @@ -294,41 +282,41 @@ 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",
);
}
}
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",
);
}
}
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) {
self.fail(location, format!("explicit `{kind:?}` is forbidden"));
}
}
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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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);
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -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
}
Expand All @@ -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)>,
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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,
Expand All @@ -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;

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:?}"));
}
}
Expand All @@ -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:?}"));
}
}
Expand Down Expand Up @@ -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.

Expand Down Expand Up @@ -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
),
);
}
Expand Down Expand Up @@ -1404,15 +1389,15 @@ 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",
);
}
}
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",
Expand Down Expand Up @@ -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();
Expand All @@ -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) {
Expand Down

0 comments on commit c71705d

Please sign in to comment.