rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
95.46k stars 12.29k forks source link

`dropping_references` lint triggers even for mutable references #125972

Open Rua opened 1 month ago

Rua commented 1 month ago

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
Urgau commented 1 month ago

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

Rua commented 1 month ago

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

Urgau commented 1 month ago

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 commented 1 month ago

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- commented 1 month ago

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 commented 1 month ago

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- commented 1 month ago

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

Urgau commented 1 month ago

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- commented 1 month ago

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 commented 1 month ago

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- commented 1 month ago

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

@rustbot claim