Skip to content

Ignore fake borrows for packed field check #137257

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 22, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions compiler/rustc_middle/src/mir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1364,13 +1364,13 @@ impl PlaceContext {
matches!(self, PlaceContext::MutatingUse(MutatingUseContext::Drop))
}

/// Returns `true` if this place context represents a borrow.
/// Returns `true` if this place context represents a borrow, excluding fake borrows
/// (which are an artifact of borrowck and not actually borrows in runtime MIR).
pub fn is_borrow(self) -> bool {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be worth updating the doc comment to explicitly say that only "real" borrows (that exist operationally) are considered here?

Another option might be to move the packed-place-borrow-checker to after SharedBorrow deletion, but that might be a bit too late.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I presumed that the packed checker was early on so as to avoid any optimizations getting in the way, though we shouldn't be applying any semantics-affecting optimizations until after borrowck, so perhaps this could be moved to right after post-borrowck cleanup 🤔

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll change the docs anyways.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do fake borrows get removed somewhere within analysis MIR? As ling as we can stay on analysis MIR, it should be fine, that one should not be affected by optimizations at all, right?

matches!(
self,
PlaceContext::NonMutatingUse(
NonMutatingUseContext::SharedBorrow | NonMutatingUseContext::FakeBorrow
) | PlaceContext::MutatingUse(MutatingUseContext::Borrow)
PlaceContext::NonMutatingUse(NonMutatingUseContext::SharedBorrow)
| PlaceContext::MutatingUse(MutatingUseContext::Borrow)
)
}

Expand Down
27 changes: 27 additions & 0 deletions tests/ui/lint/unaligned_references_fake_borrow.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
//@ check-pass

// Regression test for <https://github.com/rust-lang/rust/issues/137250>.

// Ensure that we don't emit unaligned packed field reference errors for the fake
// borrows that we generate during match lowering. These fake borrows are there to
// ensure in *borrow-checking* that we don't modify the value being matched, but
// they are removed after the MIR is processed by `CleanupPostBorrowck`.

#[repr(packed)]
pub struct Packed(i32);

fn f(x: Packed) {
match &x {
Packed(4) => {},
_ if true => {},
_ => {}
}

match x {
Packed(4) => {},
_ if true => {},
_ => {}
}
}

fn main() {}
Loading