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

dropping_references lint triggers even for mutable references #125972

Open
Rua opened this issue Jun 4, 2024 · 11 comments
Open

dropping_references lint triggers even for mutable references #125972

Rua opened this issue Jun 4, 2024 · 11 comments
Assignees
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-discussion Category: Discussion or questions that doesn't represent real issues.

Comments

@Rua
Copy link
Contributor

Rua commented Jun 4, 2024

When I call std::mem::drop on a mutable reference, this is supposed to consume the reference and make it unavailable to following code. And indeed it does, because any following uses trigger a "use of moved value" error.

But when I do this, I get a warning from the compiler:

warning: calls to std::mem::drop with a reference instead of an owned value does nothing
--> foo.rs:1602:13
|
1602 | mem::drop(mutref);
| ^^^^^^^^^^-------------^
| |
| argument has type &mut Foo
|
= note: use let _ = ... to ignore the expression or result
= note: #[warn(dropping_references)] on by default

Meta

rustc --version --verbose:

rustc 1.80.0-nightly (7c52d2db6 2024-06-03)
binary: rustc
commit-hash: 7c52d2db6348b038276198e88a835125849f322e
commit-date: 2024-06-03
host: x86_64-unknown-linux-gnu
release: 1.80.0-nightly
LLVM version: 18.1.6
@Rua Rua added the C-bug Category: This is a bug. label Jun 4, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jun 4, 2024
@Urgau
Copy link
Member

Urgau commented Jun 4, 2024

This is expected, dropping a reference doesn't drop the underline value, therefore calling drop on it is misleading.

If you want to ignore the reference, to make inaccessible or avoid an unused lint, I suggest following the lint suggestion, and replace the drop(mutref); by let _ = mutref;. It has the same effect as it renders the mutref binding inaccessible but doesn't imply that the underline value is dropped.

@rustbot labels -needs-triage -C-bug +C-discussion +A-lint

@rustbot rustbot added A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-discussion Category: Discussion or questions that doesn't represent real issues. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. C-bug Category: This is a bug. labels Jun 4, 2024
@Rua
Copy link
Contributor Author

Rua commented Jun 4, 2024

Yes, my intent was to drop the reference, for soundness reasons in unsafe code. Is binding it to _ guaranteed to drop it?

@Urgau
Copy link
Member

Urgau commented Jun 4, 2024

The reason the lint exist is to notify users that the drop call does nothing, while the user might have though that it did something, otherwise there wouldn't be drop call.

This is particularly relevant when the reference is not as obvious, but is hidden behind several lines of codes.

A prime example where it matters is when using a Mutex and calling drop(ref_mutexguard) instead of drop(mutexguard) where this can lead to bug because the mutex can live longer than expected.

Yes, my intent was to drop the reference, for soundness reasons in unsafe code. Is binding it to _ guaranteed to drop it?

Dropping a reference does nothing. A reference doesn't have a Drop impl.

let _ = <expr>; is guaranteed to drop immediately, but as I said, dropping a reference does nothing, but it still renders the previous binding inaccessible (for non-Copy types), if that is your intent.

@zachs18
Copy link
Contributor

zachs18 commented Jun 4, 2024

let _ = <expr>; does not move/copy out of <expr>, so if it is a place expression (e.g. a variable) it will not be dropped, and will still be accessible later even if it does not implement Copy.

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=97306071db0da28259341950da233f4a

fn main() {
 let mut x = 42;
 let y = &mut x;
 // drop(y); // doesn't compile
 let _ = y; // compiles
 *y = 37;
}

One thing that does move/copy out of place expressions is a "trivial" expression-statement like <expr>;, e.g, this doesn't compile

fn main() {
 let mut x = 42;
 let y = &mut x;
 y;
 *y = 37;
}

You can also use an inline block to force a move, e.g. this doesn't compile

fn main() {
 let mut x = 42;
 let y = &mut x;
 let _ = { y }; // or just `{ y };`
 *y = 37;
}

@tbu-
Copy link
Contributor

tbu- commented Jun 4, 2024

to make inaccessible or avoid an unused lint, I suggest following the lint suggestion, and replace the drop(mutref); by let _ = mutref;. I has the same effect as it renders the mutref binding inaccessible but doesn't imply that the underline value is dropped.

This is incorrect. let _ = variable; has no effect other than silencing the unused variable lint. It does not make variable inaccessible.

@Urgau
Copy link
Member

Urgau commented Jun 4, 2024

Yeah, sorry about that, the lint confused me between let _ = mutref; and let _a = mutref;.
The last one which does move the value (at least for non-Copy types, which &mut <type> is).

@tbu-
Copy link
Contributor

tbu- commented Jun 4, 2024

Is there a way to make the variable inaccessible and not trigger this lint?

@Urgau
Copy link
Member

Urgau commented Jun 4, 2024

Is there a way to make the variable inaccessible and not trigger this lint?

For mutable reference there is let _a = mutref;, let _ = { mutref }; or mutref; just to name a few.
For immutable reference, I'm not sure, since they are Copy-able.

@tbu-
Copy link
Contributor

tbu- commented Jun 4, 2024

For mutable reference there is let _a = mutref;

Doesn't really work, it's still accessible through _a.

let _ = { mutref }; or mutref;

Interesting. Maybe the lint should mention these as alternatives to make the variable inaccessible?

@Urgau
Copy link
Member

Urgau commented Jun 4, 2024

let _ = { mutref }; or mutref;

Interesting. Maybe the lint should mention these as alternatives to make the variable inaccessible?

I don't know. That's not really the goal of the lint, most of the time when calling drop users want to drop the underline value, not making it inaccessible; and if the goal was to silence the unused variable lint, using let _ = ...; is still valid.

I'm worried suggesting either one will only make the suggestion more awkward for users, in particular new users, which may not understand the implication of those (subtle) syntax differences.

@tbu-
Copy link
Contributor

tbu- commented Jun 11, 2024

I'll try to make the error message more useful.

@rustbot claim

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-discussion Category: Discussion or questions that doesn't represent real issues.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants