Skip to content

Mark "unused binding" suggestion as maybe incorrect #120470

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Feb 7, 2024

Conversation

estebank
Copy link
Contributor

Ignoring unused bindings should be a determination made by a human, rustfix shouldn't auto-apply the suggested change.

Fix #54196.

@rustbot
Copy link
Collaborator

rustbot commented Jan 29, 2024

r? @cjgillot

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot 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 Jan 29, 2024
@compiler-errors
Copy link
Member

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Jan 29, 2024

📌 Commit 949a60c has been approved by compiler-errors

It is now in the queue for this repository.

@bors bors 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 Jan 29, 2024
@cjgillot
Copy link
Contributor

Should we do this with TryPrefixSugg too?

@compiler-errors
Copy link
Member

yeah would prefer that one too

@bors r-

@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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 30, 2024
@estebank
Copy link
Contributor Author

Done.

@bors r=compiler-errors

@bors
Copy link
Collaborator

bors commented Jan 30, 2024

📌 Commit 85ae646 has been approved by compiler-errors

It is now in the queue for this repository.

@bors bors 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 30, 2024
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jan 30, 2024
…rrors

Mark "unused binding" suggestion as maybe incorrect

Ignoring unused bindings should be a determination made by a human, `rustfix` shouldn't auto-apply the suggested change.

Fix rust-lang#54196.
@rust-log-analyzer

This comment has been minimized.

@estebank
Copy link
Contributor Author

@bors r=compiler-errors

@bors
Copy link
Collaborator

bors commented Jan 30, 2024

📌 Commit 8ebd47e has been approved by compiler-errors

It is now in the queue for this repository.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 30, 2024
…rrors

Mark "unused binding" suggestion as maybe incorrect

Ignoring unused bindings should be a determination made by a human, `rustfix` shouldn't auto-apply the suggested change.

Fix rust-lang#54196.
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 30, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#120207 (check `RUST_BOOTSTRAP_CONFIG` in `profile_user_dist` test)
 - rust-lang#120321 (pattern_analysis: cleanup the contexts)
 - rust-lang#120346 (hir: Refactor getters for owner nodes)
 - rust-lang#120396 (Account for unbounded type param receiver in suggestions)
 - rust-lang#120435 (Suggest name value cfg when only value is used for check-cfg)
 - rust-lang#120470 (Mark "unused binding" suggestion as maybe incorrect)
 - rust-lang#120495 (Remove the `abi_amdgpu_kernel` feature)
 - rust-lang#120501 (rustdoc: Correctly handle attribute merge if this is a glob reexport)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 30, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#120207 (check `RUST_BOOTSTRAP_CONFIG` in `profile_user_dist` test)
 - rust-lang#120321 (pattern_analysis: cleanup the contexts)
 - rust-lang#120346 (hir: Refactor getters for owner nodes)
 - rust-lang#120396 (Account for unbounded type param receiver in suggestions)
 - rust-lang#120435 (Suggest name value cfg when only value is used for check-cfg)
 - rust-lang#120470 (Mark "unused binding" suggestion as maybe incorrect)
 - rust-lang#120495 (Remove the `abi_amdgpu_kernel` feature)
 - rust-lang#120501 (rustdoc: Correctly handle attribute merge if this is a glob reexport)

r? `@ghost`
`@rustbot` modify labels: rollup
Nadrieril added a commit to Nadrieril/rust that referenced this pull request Jan 31, 2024
…rrors

Mark "unused binding" suggestion as maybe incorrect

Ignoring unused bindings should be a determination made by a human, `rustfix` shouldn't auto-apply the suggested change.

Fix rust-lang#54196.
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 31, 2024
Rollup of 12 pull requests

Successful merges:

 - rust-lang#120207 (check `RUST_BOOTSTRAP_CONFIG` in `profile_user_dist` test)
 - rust-lang#120321 (pattern_analysis: cleanup the contexts)
 - rust-lang#120323 (On E0277 be clearer about implicit `Sized` bounds on type params and assoc types)
 - rust-lang#120355 (document `FromIterator for Vec` allocation behaviors)
 - rust-lang#120396 (Account for unbounded type param receiver in suggestions)
 - rust-lang#120430 (std: thread_local::register_dtor fix proposal for FreeBSD.)
 - rust-lang#120435 (Suggest name value cfg when only value is used for check-cfg)
 - rust-lang#120470 (Mark "unused binding" suggestion as maybe incorrect)
 - rust-lang#120472 (Make duplicate lang items fatal)
 - rust-lang#120490 (Don't hash lints differently to non-lints.)
 - rust-lang#120495 (Remove the `abi_amdgpu_kernel` feature)
 - rust-lang#120501 (rustdoc: Correctly handle attribute merge if this is a glob reexport)

Failed merges:

 - rust-lang#120346 (hir: Refactor getters for owner nodes)

r? `@ghost`
`@rustbot` modify labels: rollup
@matthiaskrgr
Copy link
Member

@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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 31, 2024
@Nadrieril
Copy link
Member

Nadrieril commented Jan 31, 2024

This is failing some cargo tests. Run ./x.py test tools/cargo to reproduce:

failures:

---- fix::fix_shared_cross_workspace stdout ----
running `/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/cargo fix --allow-no-vcs`
thread 'fix::fix_shared_cross_workspace' panicked at tests/testsuite/fix.rs:1513:10:

test failed running `/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/cargo fix --allow-no-vcs`
error: Expected lines did not match (ignoring order):
0   23        Checking foo v0.1.0 (/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/tmp/cit/t1329/foo/foo)
1   22        Checking bar v0.1.0 (/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/tmp/cit/t1329/foo/bar)
2        -       Fixed [..]foo/src/shared.rs (2 fixes)
3   19        Finished dev [unoptimized + debuginfo] target(s) in 0.32s
    3    +
    4    +
    5    +  |
    6    +  |
    7    +  |
    8    +  |
    9    + --> foo/src/shared.rs:1:14
    10   +warning: unused variable: `x`
    11   +warning: unused variable: `x`
    12   + --> bar/src/../../foo/src/shared.rs:1:14
    13   +1 | pub fn fixme(x: Box<&dyn Fn() -> ()>) {}
    14   +1 | pub fn fixme(x: Box<&dyn Fn() -> ()>) {}
    15   +warning: `bar` (lib test) generated 1 warning
    16   +warning: `foo` (lib test) generated 1 warning
    17   +  = note: `#[warn(unused_variables)]` on by default
    18   +  = note: `#[warn(unused_variables)]` on by default
    19   +       Fixed bar/src/../../foo/src/shared.rs (1 fix)
    20   +warning: `foo` (lib) generated 1 warning (1 duplicate)
    21   +warning: `bar` (lib) generated 1 warning (1 duplicate)
    22   +  |              ^ help: if this is intentional, prefix it with an underscore: `_x`
    23   +  |              ^ help: if this is intentional, prefix it with an underscore: `_x`



---- messages::deduplicate_messages_basic stdout ----
running `/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/cargo test --no-run -j1`
thread 'messages::deduplicate_messages_basic' panicked at tests/testsuite/messages.rs:72:10:

test failed running `/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/cargo test --no-run -j1`
error: stderr did not match:
1   1        Compiling foo [..]
2   2     warning: unused variable: `x`
3   3      --> src/lib.rs:3:25
4   4       |
5   5     3 |                     let x = 1;
6   6       |                         ^ help: if this is intentional, prefix it with an underscore: `_x`
7   7       |
8   8       = note: `#[warn(unused_variables)]` on by default
9   9     
10       -warning: `foo` (lib) generated 1 warning (run `cargo fix --lib -p foo` to apply 1 suggestion)
    10   +warning: `foo` (lib) generated 1 warning
11  11    warning: `foo` (lib test) generated 1 warning (1 duplicate)
12  12        Finished [..]
13  13      Executable unittests src/lib.rs (target/debug/deps/foo-[..])


other output:



---- messages::deduplicate_messages_mismatched_warnings stdout ----
�[1m�[31merror�[0m�[1m:�[0m test failed, to rerun pass `--test testsuite`
running `/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/cargo test --no-run -j1`
thread 'messages::deduplicate_messages_mismatched_warnings' panicked at tests/testsuite/messages.rs:116:10:

test failed running `/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/cargo test --no-run -j1`
error: stderr did not match:
1   1        Compiling foo v0.0.1 [..]
2   2     warning: unused variable: `x`
3   3      --> src/lib.rs:3:25
4   4       |
5   5     3 |                     let x = 1;
6   6       |                         ^ help: if this is intentional, prefix it with an underscore: `_x`
7   7       |
8   8       = note: `#[warn(unused_variables)]` on by default
9   9     
10       -warning: `foo` (lib) generated 1 warning (run `cargo fix --lib -p foo` to apply 1 suggestion)
    10   +warning: `foo` (lib) generated 1 warning
11  11    warning: variable `MY_VALUE` should have a snake case name
12  12     --> src/lib.rs:8:25
13  13      |
14  14    8 |                     let MY_VALUE = 1;
15  15      |                         ^^^^^^^^ help: convert the identifier to snake case: `my_value`
16  16      |
17  17      = note: `#[warn(non_snake_case)]` on by default
18  18    
19  19    warning: `foo` (lib test) generated 2 warnings (1 duplicate)
20  20        Finished [..]
21  21      Executable unittests src/lib.rs (target/debug/deps/foo-[..])


other output:




failures:
    fix::fix_shared_cross_workspace
    messages::deduplicate_messages_basic
    messages::deduplicate_messages_mismatched_warnings

test result: FAILED. 3020 passed; 3 failed; 178 ignored; 0 measured; 0 filtered out; finished in 286.95s

@bors r-
@rustbot author

EDIT: raced while I was trying to reproduce x)

@estebank
Copy link
Contributor Author

estebank commented Feb 1, 2024

Just looked into this. This is inconvenient. I found the test changes that I need to make, but I would have to land a disabling of the tests in the cargo repo, then update the cargo remote, merge this, and then reenable the tests, right?

estebank added a commit to estebank/cargo that referenced this pull request Feb 1, 2024
`rustc` will start marking the suggestions for prefacing unused bindings
with underscores as "maybe incorrect", which makes them no longer auto
applicable by `rustfix`.

Change done at rust-lang/rust#120470.
bors added a commit to rust-lang/cargo that referenced this pull request Feb 1, 2024
Change tests to support changes to suggestion

`rustc` will start marking the suggestions for prefacing unused bindings with underscores as "maybe incorrect", which makes them no longer auto applicable by `rustfix`.

Change done at rust-lang/rust#120470.
@Nadrieril
Copy link
Member

Can't you commit the cargo changes in this PR directly? That's how we do it with clippy and rust-analyzer.

@estebank
Copy link
Contributor Author

estebank commented Feb 1, 2024

@Nadrieril clippy and rust-analyzer both live in repo and get merged, while cargo is a sub-repo, so commits to it need to be upstreamed first. Thankfully, @weihanglo is on the case and already merged my test changes to the cargo repo and has a PR up to update the subrepo, so this PR shouldn't linger for too long.

Ignoring unused bindings should be a determination made by a human, `rustfix` shouldn't auto-apply the suggested change.

Fix rust-lang#54196.
@estebank
Copy link
Contributor Author

estebank commented Feb 6, 2024

@bors r=compiler-errors

@bors
Copy link
Collaborator

bors commented Feb 6, 2024

📌 Commit aef18c9 has been approved by compiler-errors

It is now in the queue for this repository.

@bors bors 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 6, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 7, 2024
…rrors

Mark "unused binding" suggestion as maybe incorrect

Ignoring unused bindings should be a determination made by a human, `rustfix` shouldn't auto-apply the suggested change.

Fix rust-lang#54196.
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 7, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#119162 (Add unstable `-Z direct-access-external-data` cmdline flag for `rustc`)
 - rust-lang#119592 (resolve: Unload speculatively resolved crates before freezing cstore)
 - rust-lang#120103 (Make it so that async-fn-in-trait is compatible with a concrete future in implementation)
 - rust-lang#120455 ( Add FileCheck annotations to MIR-opt SROA tests)
 - rust-lang#120470 (Mark "unused binding" suggestion as maybe incorrect)
 - rust-lang#120619 (Assert that params with the same *index* have the same *name*)
 - rust-lang#120633 (pattern_analysis: gather up place-relevant info)
 - rust-lang#120726 (Don't use bashism in checktools.sh)

r? `@ghost`
`@rustbot` modify labels: rollup
Nadrieril added a commit to Nadrieril/rust that referenced this pull request Feb 7, 2024
…rrors

Mark "unused binding" suggestion as maybe incorrect

Ignoring unused bindings should be a determination made by a human, `rustfix` shouldn't auto-apply the suggested change.

Fix rust-lang#54196.
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 7, 2024
Rollup of 13 pull requests

Successful merges:

 - rust-lang#110482 (Add armv8r-none-eabihf target for the Cortex-R52.)
 - rust-lang#119162 (Add unstable `-Z direct-access-external-data` cmdline flag for `rustc`)
 - rust-lang#120302 (various const interning cleanups)
 - rust-lang#120455 ( Add FileCheck annotations to MIR-opt SROA tests)
 - rust-lang#120470 (Mark "unused binding" suggestion as maybe incorrect)
 - rust-lang#120479 (Suggest turning `if let` into irrefutable `let` if appropriate)
 - rust-lang#120564 (coverage: Split out counter increment sites from BCB node/edge counters)
 - rust-lang#120633 (pattern_analysis: gather up place-relevant info)
 - rust-lang#120664 (Add parallel rustc ui tests)
 - rust-lang#120721 (fix `llvm_out` to use the correct LLVM root)
 - rust-lang#120726 (Don't use bashism in checktools.sh)
 - rust-lang#120733 (MirPass: make name more const)
 - rust-lang#120735 (Remove some `unchecked_claim_error_was_emitted` calls)

Failed merges:

 - rust-lang#120727 (exhaustiveness: Prefer "`0..MAX` not covered" to "`_` not covered")

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 7, 2024
Rollup of 13 pull requests

Successful merges:

 - rust-lang#110482 (Add armv8r-none-eabihf target for the Cortex-R52.)
 - rust-lang#119162 (Add unstable `-Z direct-access-external-data` cmdline flag for `rustc`)
 - rust-lang#120302 (various const interning cleanups)
 - rust-lang#120455 ( Add FileCheck annotations to MIR-opt SROA tests)
 - rust-lang#120470 (Mark "unused binding" suggestion as maybe incorrect)
 - rust-lang#120479 (Suggest turning `if let` into irrefutable `let` if appropriate)
 - rust-lang#120564 (coverage: Split out counter increment sites from BCB node/edge counters)
 - rust-lang#120633 (pattern_analysis: gather up place-relevant info)
 - rust-lang#120664 (Add parallel rustc ui tests)
 - rust-lang#120726 (Don't use bashism in checktools.sh)
 - rust-lang#120733 (MirPass: make name more const)
 - rust-lang#120735 (Remove some `unchecked_claim_error_was_emitted` calls)
 - rust-lang#120746 (Record coroutine kind in coroutine generics)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f5a36cb into rust-lang:master Feb 7, 2024
@rustbot rustbot added this to the 1.78.0 milestone Feb 7, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 7, 2024
Rollup merge of rust-lang#120470 - estebank:issue-54196, r=compiler-errors

Mark "unused binding" suggestion as maybe incorrect

Ignoring unused bindings should be a determination made by a human, `rustfix` shouldn't auto-apply the suggested change.

Fix rust-lang#54196.
weihanglo pushed a commit to weihanglo/cargo that referenced this pull request Feb 16, 2024
`rustc` will start marking the suggestions for prefacing unused bindings
with underscores as "maybe incorrect", which makes them no longer auto
applicable by `rustfix`.

Change done at rust-lang/rust#120470.
stupendoussuperpowers pushed a commit to stupendoussuperpowers/cargo that referenced this pull request Feb 28, 2024
`rustc` will start marking the suggestions for prefacing unused bindings
with underscores as "maybe incorrect", which makes them no longer auto
applicable by `rustfix`.

Change done at rust-lang/rust#120470.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.

cargo fix renames unused local variables from foo to _foo
8 participants