RefCell not dropping warning

[sancho20021]

Hi all, I saw this lint: await_holding_refcell_ref which checks whether a Ref from RefCell is dropped before awaiting async code.
But what about not dropping Ref in general? I see this as a potential source of bugs, probably the most common one for RefCells. For example:

• already mutably borrowed: BorrowError davidgraeff/firestore-db-and-auth-rs#30
• Cells example panics at 'already mutably borrowed: BorrowError' slint-ui/slint#4347

I assume there could exist an automatic check which detects the last usage of Ref, and suggest the user to drop it immediately. My questions are:

1. do you think it is a useful check?
2. How easy it does it look? From the first sight, loops may complicate things

[flip1995]

If the compiler/Clippy could determine when a reference can be dropped, couldn't you then just use normal references and let the borrow checker deal with it?

In some cases a lint could possibly give hints when a Ref could be dropped earlier with some heuristics. But the above question might be an issue when actually implementing this and trying to get it to have no false positives, while still be a useful lint.

The await_holding_refcell_ref is a bit different for why it works: It doesn't try to figure out when the Ref could get dropped, it is just saying that it is held while executing the .await. Finding the when is way harder to figure out (this is something the borrow checker does for normal refs).

[sancho20021]

If the compiler/Clippy could determine when a reference can be dropped, couldn't you then just use normal references and let the borrow checker deal with it?

Hmm, probably it is really difficult, but I believe in some cases it could provide useful suggestions.
As @y21 said, similar logic for guards is already implemented, if I understand correctly, we can temporarily switch it on for Refs, and run it on some repositories to see how it behaves

[flip1995]

The lint is in nursery, meaning it has problems preventing it from going into a proper lint group.

But yeah, we could do that and look at the lintcheck results. Would you mind opening a draft PR to check this?

[sancho20021]

@flip1995 Sorry for late reply, I opened a draft PR: #13872, will see once the tests pass. But so far I already stumbled upon some false negatives, for example this error was not discovered, and if the only action with Ref is println! then it also doesn't warn

[y21]

There's the lint significant_drop_tightening which does a very similar thing (suggesting to early drop guards like MutexGuard after their last usage), though RefCell's Ref guard doesn't have the attribute clippy::has_significant_drop, so the lint doesn't work with that type right now.

Maybe we could allow users to specify additional paths (on top of the ones marked with #[clippy::has_significant_drop]) through clippy.toml if that's useful.