Fix panicking_unwrap FP on field access with implicit deref#16196
Merged
llogiq merged 1 commit intorust-lang:masterfrom Dec 5, 2025
Merged
Fix panicking_unwrap FP on field access with implicit deref#16196llogiq merged 1 commit intorust-lang:masterfrom
panicking_unwrap FP on field access with implicit deref#16196llogiq merged 1 commit intorust-lang:masterfrom
Conversation
rustbot
added
the
S-waiting-on-review
Collaborator
|
Lintcheck changes for c905503
This comment will be updated if you push new changes |
llogiq
approved these changes
Dec 5, 2025
rustbot
removed
the
S-waiting-on-review
ada4a
reviewed
Dec 5, 2025
Comment on lines
+183
to
+195
| let field_projections = place | ||
| .projections | ||
| .iter() | ||
| .filter(|proj| matches!(proj.kind, ProjectionKind::Field(_, _))) | ||
| .collect::<Vec<_>>(); | ||
| is_potentially_local_place(*local_id, place) | ||
| // If there were projections other than field projections, err on the side of caution and say that they | ||
| // _might_ be mutating something. | ||
| // | ||
| // The reason we use `<=` and not `==` is that a mutation of `struct` or `struct.field1` should count as | ||
| // mutation of the child fields such as `struct.field1.field2` | ||
| && place.projections.len() <= field_indices.len() | ||
| && iter::zip(&place.projections, field_indices.iter().copied().rev()).all(|(proj, field_idx)| { | ||
| && field_projections.len() <= field_indices.len() | ||
| && iter::zip(&field_projections, field_indices.iter().copied().rev()).all(|(proj, field_idx)| { |
Contributor
There was a problem hiding this comment.
Why does this work though?... It definitely no longer aligns with what the comment is describing
Contributor
There was a problem hiding this comment.
I think what happened here is that I got my conditions wrong: when an non-field projections appears, we're supposed to return true, as it might be modifying something somehow -- but what the code was actually doing is returning false inside the .all(), meaning that the whole function would return false as well.
I came up with the following patch, which goes through each case a bit more thouroughly -- what do you think?
{
if !is_potentially_local_place(*local_id, place) {
// This can't be a local place -- no point checking any further
return false;
}
let mut field_indices = field_indices.iter().rev().copied();
for proj in &place.projections {
let ProjectionKind::Field(f_idx, _) = proj.kind else {
// This is a projection we didn't expect -- it _might_ be mutating something
return true;
};
let Some(field_idx) = field_indices.next() else {
// The field projections are going deeper than our `Local` -- a mutation in there
// can't change whether the `Local` is the success or error variant
return false;
};
if f_idx != field_idx {
// The projection is going into an unrelated field -- it won't mutate our `Local`
return false;
}
}
// There are no more projections, and we haven't strayed off the field access chain that our `Local`
// is located on, so a mutation of this place might mutate our `Local`
true
}
Member
Author
There was a problem hiding this comment.
I don't really get your point, but for the case of the issue, there is a deref before the field access
Contributor
There was a problem hiding this comment.
What I'm trying to say is that just filtering out non-field projections might be dangerous, as they might end up modifying the Local... somehow. So I think we should instead early-return when encountering them. The patch above achieves that using the first let-else
flip1995
added
the
beta-nominated
rust-bors bot
pushed a commit
to rust-lang/rust
that referenced
this pull request
Feb 11, 2026
[stable] prepare Rust 1.93.1 This includes the following backported fixes: - Don't try to recover keyword as non-keyword identifier #150590 - Fix `panicking_unwrap` FP on field access with implicit deref rust-lang/rust-clippy#16196 - Revert "Update wasm-related dependencies in CI" #152259 And these are just to avoid recent CI issues: - Remove rustdoc GUI flaky test #152116 - Remove the 4 failing tests from rustdoc-gui #152194 r? @rust-lang/release @rustbot ping relnotes-interest-group
rust-bors bot
pushed a commit
to rust-lang/rust
that referenced
this pull request
Feb 11, 2026
[stable] prepare Rust 1.93.1 This includes the following backported fixes: - Don't try to recover keyword as non-keyword identifier #150590 - Fix `panicking_unwrap` FP on field access with implicit deref rust-lang/rust-clippy#16196 - Revert "Update wasm-related dependencies in CI" #152259 And these are just to avoid recent CI issues: - Remove rustdoc GUI flaky test #152116 - Remove the 4 failing tests from rustdoc-gui #152194 r? @rust-lang/release @rustbot ping relnotes-interest-group
wip-sync
pushed a commit
to NetBSD/pkgsrc-wip
that referenced
this pull request
Feb 14, 2026
Pkgsrc changes: * drop dependency on compat80 for some targets (no longer applicable) * re-enable armv7 in the do-cross.mk file. Upstream changes: Version 1.93.1 (2026-02-12) =========================== - [Don't try to recover keyword as non-keyword identifier] (rust-lang/rust#150590), fixing an ICE that especially [affected rustfmt] (rust-lang/rustfmt#6739). - [Fix `clippy::panicking_unwrap` false-positive on field access with implicit deref] (rust-lang/rust-clippy#16196). - [Revert "Update wasm-related dependencies in CI"] (rust-lang/rust#152259), fixing file descriptor leaks on the `wasm32-wasip2` target.
tmeijn
pushed a commit
to tmeijn/dotfiles
that referenced
this pull request
Feb 15, 2026
This MR contains the following updates: | Package | Update | Change | |---|---|---| | [rust](https://github.com/rust-lang/rust) | patch | `1.93.0` → `1.93.1` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>rust-lang/rust (rust)</summary> ### [`v1.93.1`](https://github.com/rust-lang/rust/blob/HEAD/RELEASES.md#Version-1931-2026-02-12) [Compare Source](rust-lang/rust@1.93.0...1.93.1) \=========================== <a id="1.93.1"></a> - [Don't try to recover keyword as non-keyword identifier](rust-lang/rust#150590), fixing an ICE that especially [affected rustfmt](rust-lang/rustfmt#6739). - [Fix `clippy::panicking_unwrap` false-positive on field access with implicit deref](rust-lang/rust-clippy#16196). - [Revert "Update wasm-related dependencies in CI"](rust-lang/rust#152259), fixing file descriptor leaks on the `wasm32-wasip2` target. </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4xMC4zIiwidXBkYXRlZEluVmVyIjoiNDMuMTAuMyIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiUmVub3ZhdGUgQm90IiwiYXV0b21hdGlvbjpib3QtYXV0aG9yZWQiLCJkZXBlbmRlbmN5LXR5cGU6OnBhdGNoIl19-->
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
Closes #16188
changelog: [
panicking_unwrap] fix FP on field access with implicit deref