Ignore fake borrows for packed field check #137257
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
rustbot has assigned @petrochenkov. Use |
rustbot
added
S-waiting-on-review
|
cc @RalfJung who added this check |
|
r? oli-obk who reviewed it |
RalfJung
reviewed
Feb 19, 2025
| @@ -1394,9 +1394,8 @@ impl PlaceContext { | |||
| pub fn is_borrow(self) -> bool { | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 🤔
There was a problem hiding this comment.
I'll change the docs anyways.
There was a problem hiding this comment.
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?
|
r=me with docs. I was just being overly conservative, looking at the uses of |
compiler-errors
force-pushed
the
fake-borrow-of-packed-field
branch
from
44c5f7e to
0713bbc
Compare
|
@bors r=oli-obk rollup |
bors
added
S-waiting-on-bors
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Feb 21, 2025
…ked-field, r=oli-obk Ignore fake borrows for packed field check We should not 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 (which is why this only occurs when there's a match guard, in this case `if true`), but they are removed after the MIR is processed by `CleanupPostBorrowck`, since they're really just there to cause borrowck errors if necessary. I modified `PlaceContext::is_borrow` since that's used by the packed field check: https://github.com/rust-lang/rust/blob/17c1c329a5512d718b67ef6797538b154016cd34/compiler/rustc_mir_transform/src/check_packed_ref.rs#L40 It's only used in one other place, in the SROA optimization (by which fake borrows are removed, so it doesn't matter): https://github.com/rust-lang/rust/blob/17c1c329a5512d718b67ef6797538b154016cd34/compiler/rustc_mir_dataflow/src/value_analysis.rs#L922 Fixes rust-lang#137250
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Feb 21, 2025
…ked-field, r=oli-obk Ignore fake borrows for packed field check We should not 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 (which is why this only occurs when there's a match guard, in this case `if true`), but they are removed after the MIR is processed by `CleanupPostBorrowck`, since they're really just there to cause borrowck errors if necessary. I modified `PlaceContext::is_borrow` since that's used by the packed field check: https://github.com/rust-lang/rust/blob/17c1c329a5512d718b67ef6797538b154016cd34/compiler/rustc_mir_transform/src/check_packed_ref.rs#L40 It's only used in one other place, in the SROA optimization (by which fake borrows are removed, so it doesn't matter): https://github.com/rust-lang/rust/blob/17c1c329a5512d718b67ef6797538b154016cd34/compiler/rustc_mir_dataflow/src/value_analysis.rs#L922 Fixes rust-lang#137250
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Feb 21, 2025
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#136458 (Do not deduplicate list of associated types provided by dyn principal) - rust-lang#136474 ([`compiletest`-related cleanups 3/7] Make the distinction between sources root vs test suite sources root in compiletest less confusing) - rust-lang#136592 (Make sure we don't overrun the stack in canonicalizer) - rust-lang#136787 (Remove `lifetime_capture_rules_2024` feature) - rust-lang#137180 (Give `global_asm` a fake body to store typeck results, represent `sym fn` as a hir expr to fix `sym fn` operands with lifetimes) - rust-lang#137257 (Ignore fake borrows for packed field check) - rust-lang#137348 (More sophisticated span trimming for suggestions) - rust-lang#137399 (fix ICE in layout computation with unnormalizable const) r? `@ghost` `@rustbot` modify labels: rollup
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Feb 22, 2025
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#136458 (Do not deduplicate list of associated types provided by dyn principal) - rust-lang#136474 ([`compiletest`-related cleanups 3/7] Make the distinction between sources root vs test suite sources root in compiletest less confusing) - rust-lang#136592 (Make sure we don't overrun the stack in canonicalizer) - rust-lang#136787 (Remove `lifetime_capture_rules_2024` feature) - rust-lang#137207 (Add #[track_caller] to Duration Div impl) - rust-lang#137245 (Tweak E0277 when predicate comes indirectly from ?) - rust-lang#137257 (Ignore fake borrows for packed field check) - rust-lang#137399 (fix ICE in layout computation with unnormalizable const) r? `@ghost` `@rustbot` modify labels: rollup
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Feb 22, 2025
Rollup merge of rust-lang#137257 - compiler-errors:fake-borrow-of-packed-field, r=oli-obk Ignore fake borrows for packed field check We should not 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 (which is why this only occurs when there's a match guard, in this case `if true`), but they are removed after the MIR is processed by `CleanupPostBorrowck`, since they're really just there to cause borrowck errors if necessary. I modified `PlaceContext::is_borrow` since that's used by the packed field check: https://github.com/rust-lang/rust/blob/17c1c329a5512d718b67ef6797538b154016cd34/compiler/rustc_mir_transform/src/check_packed_ref.rs#L40 It's only used in one other place, in the SROA optimization (by which fake borrows are removed, so it doesn't matter): https://github.com/rust-lang/rust/blob/17c1c329a5512d718b67ef6797538b154016cd34/compiler/rustc_mir_dataflow/src/value_analysis.rs#L922 Fixes rust-lang#137250
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We should not 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 (which is why this only occurs when there's a match guard, in this case
if true), but they are removed after the MIR is processed byCleanupPostBorrowck, since they're really just there to cause borrowck errors if necessary.I modified
PlaceContext::is_borrowsince that's used by the packed field check:rust/compiler/rustc_mir_transform/src/check_packed_ref.rs
Line 40 in 17c1c32
It's only used in one other place, in the SROA optimization (by which fake borrows are removed, so it doesn't matter):
rust/compiler/rustc_mir_dataflow/src/value_analysis.rs
Line 922 in 17c1c32
Fixes #137250