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

Warn when a borrow expression is unused #76264

Closed
ecstatic-morse opened this issue Sep 3, 2020 · 4 comments · Fixed by #86426
Closed

Warn when a borrow expression is unused #76264

ecstatic-morse opened this issue Sep 3, 2020 · 4 comments · Fixed by #86426
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-feature-request Category: A feature request, i.e: not implemented / a PR. D-papercut Diagnostics: An error or lint that needs small tweaks. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-help-wanted Call for participation: Help is requested to fix this issue.

Comments

@ecstatic-morse
Copy link
Contributor

ecstatic-morse commented Sep 3, 2020

Borrowing an expression is side-effect free, so there should never be a reason to write the first instead of the second.

&expr;
// vs
expr;

If you really wanted to do this you could assign the result to an unused variable:

let _ = &expr;

Therefore, we should add a lint for unused borrows to the compiler. Why is this important? A misplaced semi-colon before an && operator in a multi-line boolean expression causes subsequent clauses to be silently ignored.

let trex = TyrannosaurusRex::new();
let is_a_dog = has_a_tail(trex)
  && has_bad_breath(trex)
  && is_a_carnivore(trex); // Misplaced semi-colon (perhaps due to reordering of lines)
  && is_adorable(trex);

if is_a_dog {
    give_hug(trex); // Ouch!
}
@ecstatic-morse ecstatic-morse added A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. A-diagnostics Area: Messages for errors, warnings, and lints D-papercut Diagnostics: An error or lint that needs small tweaks. labels Sep 3, 2020
@ecstatic-morse ecstatic-morse added the C-feature-request Category: A feature request, i.e: not implemented / a PR. label Sep 18, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Feb 27, 2021
… r=lcnr

Lint for unused borrows as part of `UNUSED_MUST_USE`

Resolves rust-lang#76264.

This means an `unused_must_use` warning for statements like `&expr;` and `&mut expr;`. `unused_must_use` was chosen because it already lints for logical operators (`&&` and `||`) whose result is unused. Unused borrows actually appear fairly often in `rustc`'s test suite, since we have tests that rely on side-effects of the index operator (panicking). These cannot be written as `expr[..];`, since the result is unsized, but they can be written as `let _ = &expr[..];`, which is what this PR does.

See the linked issue for the motivating example. This lint also found a benign mistake in the derived impl of `HashStable`.
@RalfJung
Copy link
Member

RalfJung commented May 24, 2021

FWIW, the corresponding PR #76894 has been closed due to inactivity. However, it was basically ready, just needed tending to some conflicts and test failures, so if someone wants to pick it up that shouldn't be too hard.

@scottmcm scottmcm added E-help-wanted Call for participation: Help is requested to fix this issue. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. labels May 24, 2021
@inashivb
Copy link
Contributor

Hi @RalfJung ! I'd like to pick it up if its OK?

@RalfJung
Copy link
Member

@inashivb sure, go ahead. :)

(I'm not necessarily the best reviewer for this PR, and I am pretty swamped currently, so I'd appreciate if someone else reading along could help with reviewing.)

@inashivb
Copy link
Contributor

@rustbot claim

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Jun 19, 2021
…=Aaron1011

Lint for unused borrows as part of UNUSED_MUST_USE

close rust-lang#76264

base on rust-lang#76894

r? `@RalfJung`
@bors bors closed this as completed in 39260f6 Jun 19, 2021
flip1995 pushed a commit to flip1995/rust-clippy that referenced this issue Jul 1, 2021
Lint for unused borrows as part of UNUSED_MUST_USE

close rust-lang/rust#76264

base on rust-lang/rust#76894

r? `@RalfJung`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-feature-request Category: A feature request, i.e: not implemented / a PR. D-papercut Diagnostics: An error or lint that needs small tweaks. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-help-wanted Call for participation: Help is requested to fix this issue.
Projects
None yet
4 participants