Skip to content

Commit

Permalink
Rollup merge of #110826 - cjgillot:place-mention-use, r=JakobDegen,lcnr
Browse files Browse the repository at this point in the history
Make PlaceMention a non-mutating use.

Fixes #110781

r? `@JakobDegen`

I don't agree with your statement in #110781 (comment). I suggest that we start fixing `PlaceContext` to be accurate enough for optimizations to use it. This structure is very convenient to use in visitors, and we perhaps have an opportunity to make it less of a footgun.
  • Loading branch information
matthiaskrgr authored May 4, 2023
2 parents bf72b64 + 4ec76df commit 0ac8ebd
Show file tree
Hide file tree
Showing 8 changed files with 49 additions and 10 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/def_use.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ pub fn categorize(context: PlaceContext) -> Option<DefUse> {

// `PlaceMention` and `AscribeUserType` both evaluate the place, which must not
// contain dangling references.
PlaceContext::NonUse(NonUseContext::PlaceMention) |
PlaceContext::NonMutatingUse(NonMutatingUseContext::PlaceMention) |
PlaceContext::NonUse(NonUseContext::AscribeUserTy) |

PlaceContext::MutatingUse(MutatingUseContext::AddressOf) |
Expand Down
8 changes: 3 additions & 5 deletions compiler/rustc_borrowck/src/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -772,12 +772,10 @@ impl<'a, 'b, 'tcx> TypeVerifier<'a, 'b, 'tcx> {

match context {
PlaceContext::MutatingUse(_) => ty::Invariant,
PlaceContext::NonUse(StorageDead | StorageLive | PlaceMention | VarDebugInfo) => {
ty::Invariant
}
PlaceContext::NonUse(StorageDead | StorageLive | VarDebugInfo) => ty::Invariant,
PlaceContext::NonMutatingUse(
Inspect | Copy | Move | SharedBorrow | ShallowBorrow | UniqueBorrow | AddressOf
| Projection,
Inspect | Copy | Move | PlaceMention | SharedBorrow | ShallowBorrow | UniqueBorrow
| AddressOf | Projection,
) => ty::Covariant,
PlaceContext::NonUse(AscribeUserTy) => ty::Covariant,
}
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_codegen_ssa/src/mir/analyze.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,9 @@ impl<'mir, 'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> Visitor<'tcx>
self.assign(local, DefLocation::Body(location));
}

PlaceContext::NonUse(_) | PlaceContext::MutatingUse(MutatingUseContext::Retag) => {}
PlaceContext::NonUse(_)
| PlaceContext::NonMutatingUse(NonMutatingUseContext::PlaceMention)
| PlaceContext::MutatingUse(MutatingUseContext::Retag) => {}

PlaceContext::NonMutatingUse(
NonMutatingUseContext::Copy | NonMutatingUseContext::Move,
Expand Down
9 changes: 6 additions & 3 deletions compiler/rustc_middle/src/mir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ macro_rules! make_mir_visitor {
StatementKind::PlaceMention(place) => {
self.visit_place(
place,
PlaceContext::NonUse(NonUseContext::PlaceMention),
PlaceContext::NonMutatingUse(NonMutatingUseContext::PlaceMention),
location
);
}
Expand Down Expand Up @@ -1251,6 +1251,11 @@ pub enum NonMutatingUseContext {
UniqueBorrow,
/// AddressOf for *const pointer.
AddressOf,
/// PlaceMention statement.
///
/// This statement is executed as a check that the `Place` is live without reading from it,
/// so it must be considered as a non-mutating use.
PlaceMention,
/// Used as base for another place, e.g., `x` in `x.y`. Will not mutate the place.
/// For example, the projection `x.y` is not marked as a mutation in these cases:
/// ```ignore (illustrative)
Expand Down Expand Up @@ -1301,8 +1306,6 @@ pub enum NonUseContext {
AscribeUserTy,
/// The data of a user variable, for debug info.
VarDebugInfo,
/// PlaceMention statement.
PlaceMention,
}

#[derive(Copy, Clone, Debug, PartialEq, Eq)]
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_mir_dataflow/src/impls/liveness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ impl DefUse {
| NonMutatingUseContext::Copy
| NonMutatingUseContext::Inspect
| NonMutatingUseContext::Move
| NonMutatingUseContext::PlaceMention
| NonMutatingUseContext::ShallowBorrow
| NonMutatingUseContext::SharedBorrow
| NonMutatingUseContext::UniqueBorrow,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_mir_transform/src/const_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -752,6 +752,7 @@ impl Visitor<'_> for CanConstProp {
| NonMutatingUse(NonMutatingUseContext::Move)
| NonMutatingUse(NonMutatingUseContext::Inspect)
| NonMutatingUse(NonMutatingUseContext::Projection)
| NonMutatingUse(NonMutatingUseContext::PlaceMention)
| NonUse(_) => {}

// These could be propagated with a smarter analysis or just some careful thinking about
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
- // MIR for `main` before DeadStoreElimination
+ // MIR for `main` after DeadStoreElimination

fn main() -> () {
let mut _0: (); // return place in scope 0 at $DIR/place_mention.rs:+0:11: +0:11
let mut _1: (&str, &str); // in scope 0 at $DIR/place_mention.rs:+3:18: +3:36
scope 1 {
}

bb0: {
StorageLive(_1); // scope 0 at $DIR/place_mention.rs:+3:18: +3:36
_1 = (const "Hello", const "World"); // scope 0 at $DIR/place_mention.rs:+3:18: +3:36
// mir::Constant
// + span: $DIR/place_mention.rs:8:19: 8:26
// + literal: Const { ty: &str, val: Value(Slice(..)) }
// mir::Constant
// + span: $DIR/place_mention.rs:8:28: 8:35
// + literal: Const { ty: &str, val: Value(Slice(..)) }
PlaceMention(_1); // scope 0 at $DIR/place_mention.rs:+3:18: +3:36
StorageDead(_1); // scope 0 at $DIR/place_mention.rs:+3:36: +3:37
_0 = const (); // scope 0 at $DIR/place_mention.rs:+0:11: +4:2
return; // scope 0 at $DIR/place_mention.rs:+4:2: +4:2
}
}

9 changes: 9 additions & 0 deletions tests/mir-opt/dead-store-elimination/place_mention.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// unit-test: DeadStoreElimination
// compile-flags: -Zmir-keep-place-mention

// EMIT_MIR place_mention.main.DeadStoreElimination.diff
fn main() {
// Verify that we account for the `PlaceMention` statement as a use of the tuple,
// and don't remove it as a dead store.
let (_, _) = ("Hello", "World");
}

0 comments on commit 0ac8ebd

Please sign in to comment.