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

lint on dbg! of non-reference with return value not used #68260

Open
joshtriplett opened this issue Jan 15, 2020 · 2 comments
Open

lint on dbg! of non-reference with return value not used #68260

joshtriplett opened this issue Jan 15, 2020 · 2 comments
Labels
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. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@joshtriplett
Copy link
Member

The dbg! macro takes its argument by value, so that it can return that same value. See #59943 for some background.

We should lint on code that calls dbg! on a non-reference value (thus consuming that value) and doesn't do anything with the return value. The lint could suggest taking a reference.

@joshtriplett joshtriplett added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. labels Jan 15, 2020
@Mark-Simulacrum
Copy link
Member

Is there a direct reason for doing this? i.e., has some code come up that is hurt by the consumption?

I've personally hit the "cannot use after move" error with dbg!(x) a few times, and I feel like a lint would never have been helpful, to my recollection.

@Centril Centril added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Jan 16, 2020
@Centril
Copy link
Contributor

Centril commented Jan 16, 2020

I concur with @Mark-Simulacrum and have the same experience of hitting the error.

I think it's also perfectly fine to use dbg!(val) where val is by-move as opposed to by-copy or by-reference. After all, the point of dbg!(...) is quick and dirty debugging, so even consuming the value by-move accidentally is better than getting an annoying lint in the way. Worse yet, if you have deny(warnings) on, this would cause another edit-compile-debug cycle, which seems counter-productive.

Ostensibly we could improve the diagnostics, but those are also good:

error[E0382]: use of moved value: `s`
 --> src/main.rs:4:10
  |
2 |     let s = "hello".to_owned();
  |         - move occurs because `s` has type `std::string::String`, which does not implement the `Copy` trait
3 |     dbg!(s);
  |     -------- value moved here
4 |     dbg!(s);
  |          ^ value used here after move
  |
  = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)

@Centril Centril removed the E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. label Jan 16, 2020
@JohnTitor JohnTitor added the C-feature-request Category: A feature request, i.e: not implemented / a PR. label Feb 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants