Skip to content

Fix the issue of unused assignment from MIR liveness checking - #149072

Merged
bors merged 2 commits into
rust-lang:mainfrom
chenyukang:yukang-fix-unused-148960
Nov 22, 2025
Merged

Fix the issue of unused assignment from MIR liveness checking#149072
bors merged 2 commits into
rust-lang:mainfrom
chenyukang:yukang-fix-unused-148960

Conversation

@chenyukang

@chenyukangchenyukang commented Nov 19, 2025

Copy link
Copy Markdown
Member

Fixes#148960
Fixes#148418

r? @davidtwco

cc @cjgillot

My first try on MIR related code, so it may not be the best fix.

@rustbot

Copy link
Copy Markdown
Collaborator

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@rustbotrustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 19, 2025
// If this place was dropped and has non-trivial drop,
// skip reporting field assignments.
if !is_direct && is_maybe_drop_guard {
continue;

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

this is for avoiding report unused_assign for this line of code:
https://github.com/rust-lang/rust/blob/main/library/std/src/io/mod.rs#L393

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why would there be an unused_assign for that line irrespective of whether there is a Drop impl?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

If there's no subsequent code that explicitly reads g.len, this assignment appears "unused".

I think the liveness checking in MIR only knows that the entire g will be dropped, but doesn't know the drop process will read g.len. The is_direct here is used to distinguish this case and avoid false positives for field assignments that Drop may use.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In the past fields weren't checked, only whole variables, right? Why was that changed?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

The indirect statement is added here:

https://github.com/chenyukang/rust/blob/4930d3e6129b17538e687f97c3b3f3e0dcaea76f/compiler/rustc_mir_transform/src/liveness.rs#L656-L661

it seems to be trying to record the access for a variable with a field:

letmut waiter_queue = .... ; ...waiter_queue.set_state_on_drop_to = ..

@cjgillot correct me if I'm wrong.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

another issue which related to Drop, but this PR does not fix it:

#148418

maybe we need a more general fix.

@chenyukangchenyukangNov 20, 2025

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I added a new commit for this PR to fix #148418

00f3155

I double checked the changes for testcase tests/ui/drop/or-pattern-drop-order.stderr, it's now the same with stable branch.

@cjgillotcjgillot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I wonder about a more principled fix. Instead of trying guess is this is a drop guard when emitting the diagnostic, should we change the dataflow analysis to make the Drop terminator mark live on interesting types? Maybe as a follow-up...

View changes since this review

Comment threadcompiler/rustc_mir_transform/src/liveness.rs
Comment threadcompiler/rustc_mir_transform/src/liveness.rs Outdated
@chenyukang
chenyukangforce-pushed the yukang-fix-unused-148960 branch from 57c1dcf to 4930d3eCompareNovember 19, 2025 05:30
@chenyukang
chenyukangforce-pushed the yukang-fix-unused-148960 branch from b7c9e8d to 00f3155CompareNovember 20, 2025 12:01
@davidtwco

Copy link
Copy Markdown
Member

This seems reasonable to me, even if we want a principled fix, this is useful for the time being.

@bors r+ rollup

@bors

bors commented Nov 22, 2025

Copy link
Copy Markdown
Collaborator

📌 Commit 00f3155 has been approved by davidtwco

It is now in the queue for this repository.

@borsbors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 22, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 22, 2025
…0, r=davidtwco
Fix the issue of unused assignment from MIR liveness checking
Fixesrust-lang#148960Fixesrust-lang#148418
r? `@davidtwco`
cc `@cjgillot`
My first try on MIR related code, so it may not be the best fix.
This was referenced Nov 22, 2025
bors added a commit that referenced this pull request Nov 22, 2025
Rollup of 9 pull requests
Successful merges:
- #148407 (Warn against calls which mutate an interior mutable `const`-item)
- #149065 (Address annotate-snippets test differences)
- #149072 (Fix the issue of unused assignment from MIR liveness checking)
- #149077 (feat: Enable annotate-snippets' simd feature)
- #149168 (Fix ICE when collecting opaques from trait method declarations)
- #149180 (Couple of refactors to SharedEmitter)
- #149185 (Handle cycles when checking impl candidates for `doc(hidden)`)
- #149194 (Move safe computation out of unsafe block)
- #149204 (Fix typo in HashMap performance comment)
r? `@ghost`
`@rustbot` modify labels: rollup
@bors
bors merged commit cef578b into rust-lang:mainNov 22, 2025
11 checks passed
@rustbotrustbot added this to the 1.93.0 milestone Nov 22, 2025
rust-timer added a commit that referenced this pull request Nov 22, 2025
Rollup merge of #149072 - chenyukang:yukang-fix-unused-148960, r=davidtwco
Fix the issue of unused assignment from MIR liveness checking
Fixes#148960Fixes#148418
r? ``@davidtwco``
cc ``@cjgillot``
My first try on MIR related code, so it may not be the best fix.
@theemathastheemathas added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Nov 22, 2025
@apiraino

Copy link
Copy Markdown
Contributor

Beta backport accepted as per compiler team on Zulip. A backport PR will be authored by the release team at the end of the current development cycle. Backport labels handled by them.

@rustbot label +beta-accepted

@rustbotrustbot added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Nov 27, 2025
@cuvipercuviper mentioned this pull request Dec 3, 2025
@cuvipercuviper modified the milestones: 1.93.0, 1.92.0Dec 3, 2025
@cuvipercuviper removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Dec 3, 2025
bors added a commit that referenced this pull request Dec 3, 2025
[beta] backports
- rustdoc: Use configured target modifiers when collecting doctests #148068
- fix(rustdoc): Color doctest errors #148834
- Fix the issue of unused assignment from MIR liveness checking #149072
- Skip unused variables warning for unreachable code #149096
- In `BTreeMap::eq`, do not compare the elements if the sizes are different. #149125
- Handle cycles when checking impl candidates for `doc(hidden)` #149185
- Generalize branch references #148395
- only the commit updating CI scripts
- Change default branch references #148564
r? cuviper
flip1995 pushed a commit to flip1995/rust-clippy that referenced this pull request Aug 17, 2026
Rollup of 9 pull requests
Successful merges:
- rust-lang/rust#148407 (Warn against calls which mutate an interior mutable `const`-item)
- rust-lang/rust#149065 (Address annotate-snippets test differences)
- rust-lang/rust#149072 (Fix the issue of unused assignment from MIR liveness checking)
- rust-lang/rust#149077 (feat: Enable annotate-snippets' simd feature)
- rust-lang/rust#149168 (Fix ICE when collecting opaques from trait method declarations)
- rust-lang/rust#149180 (Couple of refactors to SharedEmitter)
- rust-lang/rust#149185 (Handle cycles when checking impl candidates for `doc(hidden)`)
- rust-lang/rust#149194 (Move safe computation out of unsafe block)
- rust-lang/rust#149204 (Fix typo in HashMap performance comment)
r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

beta-acceptedAccepted for backporting to the compiler in the beta channel.S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

unused_assignments not shown for vec overwrite - Rust 1.92.0-beta.3 Regression (?) in unused_assignments lint when Drop is implemented

9 participants

@chenyukang@rustbot@davidtwco@bors@apiraino@cjgillot@bjorn3@cuviper@theemathas