Skip to content

Commit

Permalink
Rollup merge of rust-lang#130175 - nnethercote:rustc_mir_transform-cl…
Browse files Browse the repository at this point in the history
…eanups-3, r=saethlin

`rustc_mir_transform` cleanups 3

More cleanups in the style of rust-lang#129929.

r? `@saethlin`
  • Loading branch information
matthiaskrgr authored Sep 10, 2024
2 parents a204f87 + 8949b44 commit accd77e
Show file tree
Hide file tree
Showing 39 changed files with 569 additions and 570 deletions.
6 changes: 0 additions & 6 deletions compiler/rustc_mir_transform/src/add_call_guards.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,6 @@ pub(super) use self::AddCallGuards::*;

impl<'tcx> crate::MirPass<'tcx> for AddCallGuards {
fn run_pass(&self, _tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
self.add_call_guards(body);
}
}

impl AddCallGuards {
pub(super) fn add_call_guards(&self, body: &mut Body<'_>) {
let mut pred_count: IndexVec<_, _> =
body.basic_blocks.predecessors().iter().map(|ps| ps.len()).collect();
pred_count[START_BLOCK] += 1;
Expand Down
45 changes: 22 additions & 23 deletions compiler/rustc_mir_transform/src/add_moves_for_packed_drops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,35 +40,34 @@ pub(super) struct AddMovesForPackedDrops;
impl<'tcx> crate::MirPass<'tcx> for AddMovesForPackedDrops {
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
debug!("add_moves_for_packed_drops({:?} @ {:?})", body.source, body.span);
add_moves_for_packed_drops(tcx, body);
}
}

fn add_moves_for_packed_drops<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
let patch = add_moves_for_packed_drops_patch(tcx, body);
patch.apply(body);
}

fn add_moves_for_packed_drops_patch<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>) -> MirPatch<'tcx> {
let def_id = body.source.def_id();
let mut patch = MirPatch::new(body);
let param_env = tcx.param_env(def_id);
let def_id = body.source.def_id();
let mut patch = MirPatch::new(body);
let param_env = tcx.param_env(def_id);

for (bb, data) in body.basic_blocks.iter_enumerated() {
let loc = Location { block: bb, statement_index: data.statements.len() };
let terminator = data.terminator();
for (bb, data) in body.basic_blocks.iter_enumerated() {
let loc = Location { block: bb, statement_index: data.statements.len() };
let terminator = data.terminator();

match terminator.kind {
TerminatorKind::Drop { place, .. }
if util::is_disaligned(tcx, body, param_env, place) =>
{
add_move_for_packed_drop(tcx, body, &mut patch, terminator, loc, data.is_cleanup);
match terminator.kind {
TerminatorKind::Drop { place, .. }
if util::is_disaligned(tcx, body, param_env, place) =>
{
add_move_for_packed_drop(
tcx,
body,
&mut patch,
terminator,
loc,
data.is_cleanup,
);
}
_ => {}
}
_ => {}
}
}

patch
patch.apply(body);
}
}

fn add_move_for_packed_drop<'tcx>(
Expand Down
10 changes: 6 additions & 4 deletions compiler/rustc_mir_transform/src/add_retag.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,9 @@ impl<'tcx> crate::MirPass<'tcx> for AddRetag {
let basic_blocks = body.basic_blocks.as_mut();
let local_decls = &body.local_decls;
let needs_retag = |place: &Place<'tcx>| {
!place.is_indirect_first_projection() // we're not really interested in stores to "outside" locations, they are hard to keep track of anyway
// We're not really interested in stores to "outside" locations, they are hard to keep
// track of anyway.
!place.is_indirect_first_projection()
&& may_contain_reference(place.ty(&*local_decls, tcx).ty, /*depth*/ 3, tcx)
&& !local_decls[place.local].is_deref_temp()
};
Expand Down Expand Up @@ -129,9 +131,9 @@ impl<'tcx> crate::MirPass<'tcx> for AddRetag {
StatementKind::Assign(box (ref place, ref rvalue)) => {
let add_retag = match rvalue {
// Ptr-creating operations already do their own internal retagging, no
// need to also add a retag statement.
// *Except* if we are deref'ing a Box, because those get desugared to directly working
// with the inner raw pointer! That's relevant for `RawPtr` as Miri otherwise makes it
// need to also add a retag statement. *Except* if we are deref'ing a
// Box, because those get desugared to directly working with the inner
// raw pointer! That's relevant for `RawPtr` as Miri otherwise makes it
// a NOP when the original pointer is already raw.
Rvalue::RawPtr(_mutbl, place) => {
// Using `is_box_global` here is a bit sketchy: if this code is
Expand Down
18 changes: 7 additions & 11 deletions compiler/rustc_mir_transform/src/add_subtyping_projections.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,18 +51,14 @@ impl<'a, 'tcx> MutVisitor<'tcx> for SubTypeChecker<'a, 'tcx> {
// // gets transformed to
// let temp: rval_ty = rval;
// let place: place_ty = temp as place_ty;
fn subtype_finder<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
let patch = MirPatch::new(body);
let mut checker = SubTypeChecker { tcx, patcher: patch, local_decls: &body.local_decls };

for (bb, data) in body.basic_blocks.as_mut_preserves_cfg().iter_enumerated_mut() {
checker.visit_basic_block_data(bb, data);
}
checker.patcher.apply(body);
}

impl<'tcx> crate::MirPass<'tcx> for Subtyper {
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
subtype_finder(tcx, body);
let patch = MirPatch::new(body);
let mut checker = SubTypeChecker { tcx, patcher: patch, local_decls: &body.local_decls };

for (bb, data) in body.basic_blocks.as_mut_preserves_cfg().iter_enumerated_mut() {
checker.visit_basic_block_data(bb, data);
}
checker.patcher.apply(body);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ impl<'tcx> Visitor<'tcx> for ConstMutationChecker<'_, 'tcx> {
self.super_statement(stmt, loc);
self.target_local = None;
}

fn visit_rvalue(&mut self, rvalue: &Rvalue<'tcx>, loc: Location) {
if let Rvalue::Ref(_, BorrowKind::Mut { .. }, place) = rvalue {
let local = place.local;
Expand Down
48 changes: 23 additions & 25 deletions compiler/rustc_mir_transform/src/copy_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,37 +27,34 @@ impl<'tcx> crate::MirPass<'tcx> for CopyProp {
#[instrument(level = "trace", skip(self, tcx, body))]
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
debug!(def_id = ?body.source.def_id());
propagate_ssa(tcx, body);
}
}

fn propagate_ssa<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
let param_env = tcx.param_env_reveal_all_normalized(body.source.def_id());
let ssa = SsaLocals::new(tcx, body, param_env);
let param_env = tcx.param_env_reveal_all_normalized(body.source.def_id());
let ssa = SsaLocals::new(tcx, body, param_env);

let fully_moved = fully_moved_locals(&ssa, body);
debug!(?fully_moved);
let fully_moved = fully_moved_locals(&ssa, body);
debug!(?fully_moved);

let mut storage_to_remove = BitSet::new_empty(fully_moved.domain_size());
for (local, &head) in ssa.copy_classes().iter_enumerated() {
if local != head {
storage_to_remove.insert(head);
let mut storage_to_remove = BitSet::new_empty(fully_moved.domain_size());
for (local, &head) in ssa.copy_classes().iter_enumerated() {
if local != head {
storage_to_remove.insert(head);
}
}
}

let any_replacement = ssa.copy_classes().iter_enumerated().any(|(l, &h)| l != h);
let any_replacement = ssa.copy_classes().iter_enumerated().any(|(l, &h)| l != h);

Replacer {
tcx,
copy_classes: ssa.copy_classes(),
fully_moved,
borrowed_locals: ssa.borrowed_locals(),
storage_to_remove,
}
.visit_body_preserves_cfg(body);
Replacer {
tcx,
copy_classes: ssa.copy_classes(),
fully_moved,
borrowed_locals: ssa.borrowed_locals(),
storage_to_remove,
}
.visit_body_preserves_cfg(body);

if any_replacement {
crate::simplify::remove_unused_definitions(body);
if any_replacement {
crate::simplify::remove_unused_definitions(body);
}
}
}

Expand Down Expand Up @@ -140,7 +137,8 @@ impl<'tcx> MutVisitor<'tcx> for Replacer<'_, 'tcx> {

fn visit_operand(&mut self, operand: &mut Operand<'tcx>, loc: Location) {
if let Operand::Move(place) = *operand
// A move out of a projection of a copy is equivalent to a copy of the original projection.
// A move out of a projection of a copy is equivalent to a copy of the original
// projection.
&& !place.is_indirect_first_projection()
&& !self.fully_moved.contains(place.local)
{
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_mir_transform/src/coverage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,8 @@ fn inject_mcdc_statements<'tcx>(
basic_coverage_blocks: &CoverageGraph,
extracted_mappings: &ExtractedMappings,
) {
// Inject test vector update first because `inject_statement` always insert new statement at head.
// Inject test vector update first because `inject_statement` always insert new statement at
// head.
for &mappings::MCDCDecision {
span: _,
ref end_bcbs,
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_mir_transform/src/dataflow_const_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -647,7 +647,8 @@ fn try_write_constant<'tcx>(
ty::FnDef(..) => {}

// Those are scalars, must be handled above.
ty::Bool | ty::Int(_) | ty::Uint(_) | ty::Float(_) | ty::Char => throw_machine_stop_str!("primitive type with provenance"),
ty::Bool | ty::Int(_) | ty::Uint(_) | ty::Float(_) | ty::Char =>
throw_machine_stop_str!("primitive type with provenance"),

ty::Tuple(elem_tys) => {
for (i, elem) in elem_tys.iter().enumerate() {
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_mir_transform/src/deduce_param_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ impl<'tcx> Visitor<'tcx> for DeduceReadOnly {
}
PlaceContext::NonMutatingUse(NonMutatingUseContext::RawBorrow) => {
// Whether mutating though a `&raw const` is allowed is still undecided, so we
// disable any sketchy `readonly` optimizations for now.
// But we only need to do this if the pointer would point into the argument.
// IOW: for indirect places, like `&raw (*local).field`, this surely cannot mutate `local`.
// disable any sketchy `readonly` optimizations for now. But we only need to do
// this if the pointer would point into the argument. IOW: for indirect places,
// like `&raw (*local).field`, this surely cannot mutate `local`.
!place.is_indirect()
}
PlaceContext::NonMutatingUse(..) | PlaceContext::NonUse(..) => {
Expand Down
7 changes: 4 additions & 3 deletions compiler/rustc_mir_transform/src/deduplicate_blocks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ fn find_duplicates(body: &Body<'_>) -> FxHashMap<BasicBlock, BasicBlock> {
// For example, if bb1, bb2 and bb3 are duplicates, we will first insert bb3 in same_hashes.
// Then we will see that bb2 is a duplicate of bb3,
// and insert bb2 with the replacement bb3 in the duplicates list.
// When we see bb1, we see that it is a duplicate of bb3, and therefore insert it in the duplicates list
// with replacement bb3.
// When we see bb1, we see that it is a duplicate of bb3, and therefore insert it in the
// duplicates list with replacement bb3.
// When the duplicates are removed, we will end up with only bb3.
for (bb, bbd) in body.basic_blocks.iter_enumerated().rev().filter(|(_, bbd)| !bbd.is_cleanup) {
// Basic blocks can get really big, so to avoid checking for duplicates in basic blocks
Expand Down Expand Up @@ -105,7 +105,8 @@ struct BasicBlockHashable<'tcx, 'a> {
impl Hash for BasicBlockHashable<'_, '_> {
fn hash<H: Hasher>(&self, state: &mut H) {
hash_statements(state, self.basic_block_data.statements.iter());
// Note that since we only hash the kind, we lose span information if we deduplicate the blocks
// Note that since we only hash the kind, we lose span information if we deduplicate the
// blocks.
self.basic_block_data.terminator().kind.hash(state);
}
}
Expand Down
14 changes: 7 additions & 7 deletions compiler/rustc_mir_transform/src/dest_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ impl<'tcx> crate::MirPass<'tcx> for DestinationPropagation {
}
round_count += 1;

apply_merges(body, tcx, &merges, &merged_locals);
apply_merges(body, tcx, merges, merged_locals);
}

trace!(round_count);
Expand Down Expand Up @@ -281,20 +281,20 @@ struct Candidates {
fn apply_merges<'tcx>(
body: &mut Body<'tcx>,
tcx: TyCtxt<'tcx>,
merges: &FxIndexMap<Local, Local>,
merged_locals: &BitSet<Local>,
merges: FxIndexMap<Local, Local>,
merged_locals: BitSet<Local>,
) {
let mut merger = Merger { tcx, merges, merged_locals };
merger.visit_body_preserves_cfg(body);
}

struct Merger<'a, 'tcx> {
struct Merger<'tcx> {
tcx: TyCtxt<'tcx>,
merges: &'a FxIndexMap<Local, Local>,
merged_locals: &'a BitSet<Local>,
merges: FxIndexMap<Local, Local>,
merged_locals: BitSet<Local>,
}

impl<'a, 'tcx> MutVisitor<'tcx> for Merger<'a, 'tcx> {
impl<'tcx> MutVisitor<'tcx> for Merger<'tcx> {
fn tcx(&self) -> TyCtxt<'tcx> {
self.tcx
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_mir_transform/src/early_otherwise_branch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -261,8 +261,8 @@ fn evaluate_candidate<'tcx>(
// };
// ```
//
// Hoisting the `discriminant(Q)` out of the `A` arm causes us to compute the discriminant of an
// invalid value, which is UB.
// Hoisting the `discriminant(Q)` out of the `A` arm causes us to compute the discriminant
// of an invalid value, which is UB.
// In order to fix this, **we would either need to show that the discriminant computation of
// `place` is computed in all branches**.
// FIXME(#95162) For the moment, we adopt a conservative approach and
Expand Down
16 changes: 8 additions & 8 deletions compiler/rustc_mir_transform/src/elaborate_drops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ use tracing::{debug, instrument};
use crate::deref_separator::deref_finder;

/// During MIR building, Drop terminators are inserted in every place where a drop may occur.
/// However, in this phase, the presence of these terminators does not guarantee that a destructor will run,
/// as the target of the drop may be uninitialized.
/// However, in this phase, the presence of these terminators does not guarantee that a destructor
/// will run, as the target of the drop may be uninitialized.
/// In general, the compiler cannot determine at compile time whether a destructor will run or not.
///
/// At a high level, this pass refines Drop to only run the destructor if the
Expand All @@ -30,10 +30,10 @@ use crate::deref_separator::deref_finder;
/// Once this is complete, Drop terminators in the MIR correspond to a call to the "drop glue" or
/// "drop shim" for the type of the dropped place.
///
/// This pass relies on dropped places having an associated move path, which is then used to determine
/// the initialization status of the place and its descendants.
/// It's worth noting that a MIR containing a Drop without an associated move path is probably ill formed,
/// as it would allow running a destructor on a place behind a reference:
/// This pass relies on dropped places having an associated move path, which is then used to
/// determine the initialization status of the place and its descendants.
/// It's worth noting that a MIR containing a Drop without an associated move path is probably ill
/// formed, as it would allow running a destructor on a place behind a reference:
///
/// ```text
/// fn drop_term<T>(t: &mut T) {
Expand Down Expand Up @@ -377,8 +377,8 @@ impl<'a, 'tcx> ElaborateDropsCtxt<'a, 'tcx> {
);
}
// A drop and replace behind a pointer/array/whatever.
// The borrow checker requires that these locations are initialized before the assignment,
// so we just leave an unconditional drop.
// The borrow checker requires that these locations are initialized before the
// assignment, so we just leave an unconditional drop.
assert!(!data.is_cleanup);
}
}
Expand Down
5 changes: 3 additions & 2 deletions compiler/rustc_mir_transform/src/ffi_unwind_calls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,9 @@ fn has_ffi_unwind_calls(tcx: TyCtxt<'_>, local_def_id: LocalDefId) -> bool {
let fn_def_id = match ty.kind() {
ty::FnPtr(..) => None,
&ty::FnDef(def_id, _) => {
// Rust calls cannot themselves create foreign unwinds (even if they use a non-Rust ABI).
// So the leak of the foreign unwind into Rust can only be elsewhere, not here.
// Rust calls cannot themselves create foreign unwinds (even if they use a non-Rust
// ABI). So the leak of the foreign unwind into Rust can only be elsewhere, not
// here.
if !tcx.is_foreign_item(def_id) {
continue;
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_mir_transform/src/function_item_references.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,8 @@ impl<'tcx> FunctionItemRefChecker<'_, 'tcx> {
{
let mut span = self.nth_arg_span(args, arg_num);
if span.from_expansion() {
// The operand's ctxt wouldn't display the lint since it's inside a macro so
// we have to use the callsite's ctxt.
// The operand's ctxt wouldn't display the lint since it's
// inside a macro so we have to use the callsite's ctxt.
let callsite_ctxt = span.source_callsite().ctxt();
span = span.with_ctxt(callsite_ctxt);
}
Expand Down
Loading

0 comments on commit accd77e

Please sign in to comment.