Skip to content

Unused variables suggest _prefix for patterns with ref bindings when _ wildcard would suffice #59153

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

Closed
estebank opened this issue Mar 13, 2019 · 7 comments
Labels
A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@estebank
Copy link
Contributor

error: unused variable: `e`
   --> src/librustc_resolve/error_reporting.rs:234:41
    |
234 |             if let PathSource::Expr(ref e) = source {
    |                                         ^ help: consider prefixing with an underscore: `_e`
    |
    = note: `-D unused-variables` implied by `-D warnings`

The correct suggestion is either to remove the ref e and supply _e or to change the code to ref e: _.

@estebank estebank added C-bug Category: This is a bug. A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` labels Mar 13, 2019
@ehuss
Copy link
Contributor

ehuss commented Mar 13, 2019

I think this may be the same as #54180.

EDIT: Or maybe not, ignore me.

@estebank
Copy link
Contributor Author

It's very similar. It'll probably be fixed by the same code.

@petrochenkov
Copy link
Contributor

remove the ref e and supply _e

This may cause a "can't move out from source" error later.

... or to change the code to ref e: _

It's longer and noisier, why is this better than ref _e?
The existing diagnostic looks like the best alternative to me.

@pnkfelix
Copy link
Member

pnkfelix commented Mar 13, 2019

... or to change the code to ref e: _

It's longer and noisier, why is this better than ref _e?
The existing diagnostic looks like the best alternative to me.

Longer, noisier, and invalid: playground

The only thing I can think of that is actually valid code that looks like that in this context is Variant { 0: ref _e } (playground)

but like @petrochenkov , I do not see why one would suggest that over Variant(ref _e)

@pnkfelix
Copy link
Member

Or maybe the intent is to suggest Variant(_) rather than Variant(ref e) ?

@pnkfelix pnkfelix added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Mar 13, 2019
@pnkfelix
Copy link
Member

In any case, if we are indeed talking about tuple-structs, I don't see how the given suggestion as implemented is "incorrect." It may not be ideal (if I'm right that Variant(_) should always be a valid option here), but striving for the ideal is a feature request, not a bug fix

@pnkfelix pnkfelix added C-enhancement Category: An issue proposing an enhancement or a PR with one. and removed C-bug Category: This is a bug. labels Mar 13, 2019
@pnkfelix pnkfelix changed the title Unused variables produce incorrect suggestion for patterns with ref bindings Unused variables suggest _prefix for patterns with ref bindings when _ wildcard would suffice Mar 13, 2019
@estebank
Copy link
Contributor Author

This is what I get for creating tickets sleep deprived. I misread the error as having the problem in #54180. We can close this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants