Skip to content
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

Perform unused assignment and unused variables lints on MIR. #101500

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

cjgillot
Copy link
Contributor

@cjgillot cjgillot commented Sep 6, 2022

Based on #101399
Based on #102256
Blocked on #108993

Rough attempt to #51003.

This is my first stab at using the dataflow framework, so please be gentle 😄

In order to keep the same level of diagnostics, I had to instrument MIR a little more:

  • introduce a no-op fake read kind for let _ = syntax. This changes the behaviour of let _ = <access to unsafe field> currently type-checks #54003, I don't know if it's still desirable.
  • keep for which original local a guard local is created;
  • store in the VarBindingForm the list of introducer places and whether this was a shorthand pattern.

I am not very proud of the handling of self-assignments. The proposed scheme is in two parts: first detect probable self-assignments, by pattern matching on MIR, and second treat them specially during dataflow analysis. I welcome ideas.

I did not attempt to rewrite the dead-code lint, as #100288 does it.

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Sep 6, 2022
@rustbot
Copy link
Collaborator

rustbot commented Sep 6, 2022

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

This PR changes MIR

cc @oli-obk, @RalfJung, @JakobDegen, @davidtwco, @celinval, @vakaras

@rust-highfive
Copy link
Collaborator

r? @davidtwco

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 6, 2022
@rustbot
Copy link
Collaborator

rustbot commented Sep 6, 2022

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

  • Stabilizing library features
  • Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
  • Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
  • Changing public documentation in ways that create new stability guarantees
  • Changing observable runtime behavior of library APIs

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@cjgillot

This comment was marked as outdated.

@rust-timer

This comment was marked as outdated.

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Sep 7, 2022
@bors

This comment was marked as outdated.

@bors

This comment was marked as outdated.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 7, 2022
@rust-log-analyzer

This comment has been minimized.

@cjgillot

This comment was marked as outdated.

@rust-timer

This comment was marked as outdated.

@bors

This comment was marked as outdated.

@bors

This comment was marked as outdated.

@rust-timer

This comment was marked as outdated.

@rust-timer

This comment was marked as outdated.

@rustbot rustbot added perf-regression Performance regression. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 7, 2022
@bors
Copy link
Contributor

bors commented Mar 21, 2023

☔ The latest upstream changes (presumably #109092) made this pull request unmergeable. Please resolve the merge conflicts.

@cjgillot cjgillot force-pushed the mir-liveness branch 2 times, most recently from 20ad900 to 8322e70 Compare April 9, 2023 13:40
@bors
Copy link
Contributor

bors commented Apr 21, 2023

☔ The latest upstream changes (presumably #96840) made this pull request unmergeable. Please resolve the merge conflicts.

@rust-log-analyzer

This comment has been minimized.

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Apr 21, 2023
@bors
Copy link
Contributor

bors commented Apr 22, 2023

☔ The latest upstream changes (presumably #106934) made this pull request unmergeable. Please resolve the merge conflicts.

@rustbot
Copy link
Collaborator

rustbot commented Jul 8, 2023

Some changes might have occurred in exhaustiveness checking

cc @Nadrieril

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Jul 13, 2023

☔ The latest upstream changes (presumably #113646) made this pull request unmergeable. Please resolve the merge conflicts.

@Dylan-DPC
Copy link
Member

@cjgillot any updates on this

@Dylan-DPC

This comment was marked as outdated.

@Dylan-DPC Dylan-DPC closed this Mar 10, 2024
@Dylan-DPC Dylan-DPC removed the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Mar 10, 2024
@Dylan-DPC
Copy link
Member

oh nvm this was blocked

@Dylan-DPC Dylan-DPC reopened this Mar 10, 2024
@Dylan-DPC Dylan-DPC added the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Mar 10, 2024
@rustbot
Copy link
Collaborator

rustbot commented Apr 7, 2024

Some changes occurred in match lowering

cc @Nadrieril

@@ -738,6 +738,7 @@ pub enum PatKind<'tcx> {
/// Is this the leftmost occurrence of the binding, i.e., is `var` the
/// `HirId` of this pattern?
is_primary: bool,
is_shorthand: bool,
Copy link
Member

Choose a reason for hiding this comment

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

Could you document what "shorthand" means?

@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-tools failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
## Running ui tests in tests/pass for x86_64-unknown-linux-gnu
   Compiler: "MIRI_ENV_VAR_TEST"="0" "MIRI_TEMP"="/tmp/miri-uitest-c103fq" "RUST_BACKTRACE"="1" /checkout/obj/build/x86_64-unknown-linux-gnu/stage1/bin/miri "--error-format=json" "-Dwarnings" "-Dunused" "-Ainternal_features" "-Zui-testing" "--target" "x86_64-unknown-linux-gnu" "--out-dir" OUT_DIR

FAILED TEST: tests/pass/align_repeat_into_well_aligned_array.rs
command: MIRI_ENV_VAR_TEST="0" MIRI_TEMP="/tmp/miri-uitest-c103fq" RUST_BACKTRACE="1" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage1/bin/miri" "--error-format=json" "-Dwarnings" "-Dunused" "-Ainternal_features" "-Zui-testing" "--target" "x86_64-unknown-linux-gnu" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/ui/tests/pass" "tests/pass/align_repeat_into_well_aligned_array.rs" "-Zmiri-symbolic-alignment-check" "--edition" "2021"
error: pass test got exit status: 1, but expected 0

error: actual output differed from expected
Execute `./miri test --bless` to update `tests/pass/align_repeat_into_well_aligned_array.stderr` to the actual output
Execute `./miri test --bless` to update `tests/pass/align_repeat_into_well_aligned_array.stderr` to the actual output
--- tests/pass/align_repeat_into_well_aligned_array.stderr
+++ <stderr output>
+error: variable `a` is assigned to, but never used
+   |
+LL |     let mut a = Params::new();
+   |         ^^^^^
+   |
+   |
+   = note: consider using `_a` instead
+   = note: `-D unused-variables` implied by `-D unused`
+   = help: to override `-D unused` add `#[allow(unused_variables)]`
+error: value assigned to `a` is never read
+  --> $DIR/align_repeat_into_well_aligned_array.rs:LL:CC
+   |
+   |
+LL |     a.key_block = [0; BLOCKBYTES];
+   |
+   |
+   = help: maybe it is overwritten before being read?
+   = note: `-D unused-assignments` implied by `-D unused`
+   = help: to override `-D unused` add `#[allow(unused_assignments)]`
+error: miri cannot be run on programs that fail compilation
+
+error: aborting due to 3 previous errors
+
---

error: there were 1 unmatched diagnostics
##[error] --> tests/pass/align_repeat_into_well_aligned_array.rs:6:9
  |
6 |     let mut a = Params::new();
  |         ^^^^^ Error: variable `a` is assigned to, but never used

error: there were 1 unmatched diagnostics
##[error] --> tests/pass/align_repeat_into_well_aligned_array.rs:9:5
  |
  |
9 |     a.key_block = [0; BLOCKBYTES];
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Error: value assigned to `a` is never read

full stderr:
full stderr:
error: variable `a` is assigned to, but never used
   |
LL |     let mut a = Params::new();
   |         ^^^^^
   |
   |
   = note: consider using `_a` instead
   = note: `-D unused-variables` implied by `-D unused`
   = help: to override `-D unused` add `#[allow(unused_variables)]`
error: value assigned to `a` is never read
##[error]  --> tests/pass/align_repeat_into_well_aligned_array.rs:9:5
   |
   |
LL |     a.key_block = [0; BLOCKBYTES];
   |
   |
   = help: maybe it is overwritten before being read?
   = note: `-D unused-assignments` implied by `-D unused`
   = help: to override `-D unused` add `#[allow(unused_assignments)]`
error: miri cannot be run on programs that fail compilation

error: aborting due to 3 previous errors

---
Error: 
   0: ui tests in tests/pass for x86_64-unknown-linux-gnu failed
   1: tests failed

Backtrace omitted. Run with RUST_BACKTRACE=1 environment variable to display it.
Run with RUST_BACKTRACE=full to include source snippets.
error: test failed, to rerun pass `--test ui`
Caused by:
  process didn't exit successfully: `/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/x86_64-unknown-linux-gnu/release/deps/ui-f4b6be260663ac84 --quiet` (exit status: 1)
Build completed unsuccessfully in 0:02:54
  local time: Sun Apr  7 19:22:39 UTC 2024

@bors
Copy link
Contributor

bors commented Apr 8, 2024

☔ The latest upstream changes (presumably #123569) made this pull request unmergeable. Please resolve the merge conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc perf-regression Performance regression. S-blocked Status: Blocked on something else such as an RFC or other implementation work. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant 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.