Skip to content

unused_variables lint should consider destructors #29427

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
sanxiyn opened this issue Oct 28, 2015 · 4 comments
Closed

unused_variables lint should consider destructors #29427

sanxiyn opened this issue Oct 28, 2015 · 4 comments
Labels
A-lints Area: Lints (warnings about flaws in source code) such as unused_mut.

Comments

@sanxiyn
Copy link
Member

sanxiyn commented Oct 28, 2015

use std::sync::Mutex;

fn main() {
    let mutex = Mutex::new(());
    let lock = mutex.lock();
}
test.rs:5:9: 5:13 warning: unused variable: `lock`, #[warn(unused_variables)] on by default
test.rs:5     let lock = mutex.lock();
                  ^~~~

The standard workaround is to assign to _lock. It is so standard that documentation examples do so.

@sanxiyn sanxiyn added the A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. label Oct 28, 2015
@wthrowe
Copy link
Contributor

wthrowe commented Oct 28, 2015

Usually taking a lock without using it is a bug. (Intentionally poisoning a mutex that you're not otherwise using is sufficiently unusual that I think making people silence a lint is OK.)

Most C++ compilers do this (it's kind of required because of how C++'s RAII guards are designed), and I find that it makes their unused variable warnings nearly useless because nothing that contains a heap allocation is ever considered dead. With the exceptions of things like lock guards (which are handled differently in Rust), I can only think of two C++ classes I've encountered where running the destructor logically counted as a use (a profiler object that just recorded how long it existed and an object that chdir()ed on construction and then went back to the original directory on deletion (yay global state!)). I'd consider having to mark those well worth it to get warnings about all the dead strings and vector-like objects.

Relates to #21775 (where it sounds like the solution is likely going to be to just modify the warning text).

@sanxiyn
Copy link
Member Author

sanxiyn commented Oct 29, 2015

My motivation for this issue was _recursion_lock in Rust.

You are probably right that these cases are sufficiently rare that prefixing an underscore is a good enough solution.

@steveklabnik
Copy link
Member

Since new lints have a big impact on users of rustc, the policy is that they should go through the RFC process like other user-facing changes. As such, I'm going to give this one a close, but if anyone comes across this ticket and wants this lint, consider adding it to clippy and/or writing up an RFC. Thanks!

@adrianheine
Copy link
Contributor

Since new lints have a big impact on users of rustc, the policy is that they should go through the RFC process like other user-facing changes. As such, I'm going to give this one a close, but if anyone comes across this ticket and wants this lint, consider adding it to clippy and/or writing up an RFC. Thanks!

This is not about a new lint, though, but about false positives in an existing lint. For the record, I currently work around this with explicit drop calls.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lints Area: Lints (warnings about flaws in source code) such as unused_mut.
Projects
None yet
Development

No branches or pull requests

4 participants