clippy::drop_ref will trigger on &mut T as well as &T

[Lokathor]

the drop_ref lint will trigger on &mut T values being dropped, even though dropping them does have an effect.

[luser]

I ran into this today. :-/

[mbrubeck]

In one sense, dropping an &mut T does not have an effect: It doesn't do anything at runtime, and removing the drop(mut_ref) call does not change the semantics of any program that compiles.

On the other hand, drop(mut_ref) can be a useful sort of static assertion, since it can prevent you from accidentally adding code in the future that uses the reference after it should no longer be used. In safe Rust the compiler already prevents this, but it could be useful in unsafe code that uses a mixture of raw pointers and references.

[mbrubeck]

I think keeping this lint enabled by default for &mut T is a good idea. The cases I discussed above where drop(reference) is useful as a compile-time assertion are fairly rare, and only appear in unsafe code where it's worth explicitly opting out of the lint and explaining why. In most Rust code, dropping a reference is not likely to do anything useful.

For example, I talked with @luser about the case he mentioned above, and it turned out that the lint was correct: the drop call had no effect and should be removed.

[jgarvin]

I ran into this and still think it's useful for reasons beyond just being an assertion, but it's possible I'm misunderstanding something (I am pretty new to rust), but if I am misunderstanding something it might be worth documenting what is wrong with my reasoning for other newbies who come across this.

Rust requires that you only have one mutable reference at a time to a given object. If you are writing unsafe code, it's definitely possible to write code that violates this language rule. To avoid violating the language rule you may choose to explicitly drop a mutable reference early. Imagine:

fn foo(node: *mut Node) {
    let mut node = unsafe { &mut *node };
    // .. do stuff with node
    let p = node.next;
    drop(node);
    let p = unsafe { &mut *p };
    // ... do stuff with p
}

If it is possible for node.next to be a circular pointer back to the same node, the drop prevents undefined behavior. It seems like it would be possible to avoid these false positives by not triggering it for &muts formed by a dereference inside an unsafe block? Is there a reason this drop would somehow be unnecessary?

[mbrubeck]

Rust requires that you only have one mutable reference at a time to a given object.

Under non-lexical lifetimes, a reference is only "live" until the point where it is last used. You don't need to explicitly drop it after it is no longer used. For a more detailed model, see the stacked borrows paper.

So the drop in this example has no effect, and you can remove it without introducing UB. However, it's a good example of the "static assertion" use case, since it could prevent you from adding code that introduces UB in the future (by incorrectly using the node reference later in the function).

[NicholasGorski]

Is there a future where this is changed? This issue was re-opened and remains that way, so adding my +1 to it.

I am in the mixed reference and pointer scenario, and the drop complements an unsafe block:

std::mem::drop(our_ref); // Drops the &mut, not the value.

unsafe {
    // SAFETY: This function will perform a foreign write to the value,
    // invalidating all outstanding references. We have dropped our
    // only reference, and will re-fetch the value afterward to continue.
    self.foo();
}

It's nice to see the SAFETY comment explicitly supported in code. The code is already mentally taxing, and adding clippy attributes detracts from the clarity of the relationship.

Would it make sense to issue the warning when the reference is a function parameter, but permit it otherwise? Since parameters are noalias ("protected" in stacked/tree borrow parlance), dropping them can never help static assert why some following code is safe.