Fix the issue of unused assignment from MIR liveness checking#149072
Fix the issue of unused assignment from MIR liveness checking#149072bors merged 2 commits intorust-lang:mainfrom
Conversation
|
Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt |
rustbot
added
S-waiting-on-review
| // If this place was dropped and has non-trivial drop, | ||
| // skip reporting field assignments. | ||
| if !is_direct && is_maybe_drop_guard { | ||
| continue; |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Why would there be an unused_assign for that line irrespective of whether there is a Drop impl?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
In the past fields weren't checked, only whole variables, right? Why was that changed?
There was a problem hiding this comment.
The indirect statement is added here:
it seems to be trying to record the access for a variable with a field:
let mut waiter_queue = .... ;
...
waiter_queue.set_state_on_drop_to = ..@cjgillot correct me if I'm wrong.
There was a problem hiding this comment.
another issue which related to Drop, but this PR does not fix it:
maybe we need a more general fix.
chenyukang
force-pushed
the
yukang-fix-unused-148960
branch
from
57c1dcf to
4930d3e
Compare
chenyukang
force-pushed
the
yukang-fix-unused-148960
branch
from
b7c9e8d to
00f3155
Compare
|
This seems reasonable to me, even if we want a principled fix, this is useful for the time being. @bors r+ rollup |
bors
added
S-waiting-on-bors
…0, r=davidtwco Fix the issue of unused assignment from MIR liveness checking Fixes rust-lang#148960 Fixes rust-lang#148418 r? `@davidtwco` cc `@cjgillot` My first try on MIR related code, so it may not be the best fix.
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
theemathas
added
the
beta-nominated
rustbot
added
the
beta-accepted
cuviper
removed
the
beta-nominated
[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
9 participants
Fixes #148960
Fixes #148418
r? @davidtwco
cc @cjgillot
My first try on MIR related code, so it may not be the best fix.