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

False positive for await_holding_refcell_ref #6353

Open
Daniel-B-Smith opened this issue Nov 20, 2020 · 4 comments
Open

False positive for await_holding_refcell_ref #6353

Daniel-B-Smith opened this issue Nov 20, 2020 · 4 comments
Labels
C-bug Category: Clippy is not doing the correct thing E-hard Call for participation: This a hard problem and requires more experience or effort to work on I-false-positive Issue: The lint was triggered on code it shouldn't have

Comments

@Daniel-B-Smith
Copy link

I tried this code:

let mut state = self.state.borrow_mut();
let handles = state.cancel_all_in_flight();
drop(state);
let to_cancel = handles.len();
let cancelled = stream::iter(handles).then(|f| f).count().await;

I expected to see this happen: No error since the RefMut is explicitly dropped before the await

Instead, this happened: Brought up a warning

This happens in the recently promoted beta.

This should be pretty easy to fix. As a short term mitigation, I will downgrade the check from correctness and should hopefully be able to work on the fix next week.

@Daniel-B-Smith Daniel-B-Smith added the C-bug Category: Clippy is not doing the correct thing label Nov 20, 2020
bors added a commit that referenced this issue Nov 20, 2020
Downgrade the await holding lints from correctness

We found a false positive in these lints (see #6353 for more details). As a short-term mitigation, this downgrades the lints from correctness to limit the noise.
bors added a commit that referenced this issue Nov 20, 2020
Downgrade the await holding lints from correctness

We found a false positive in these lints (see #6353 for more details). As a short-term mitigation, this downgrades the lints from correctness to limit the noise.

changelog: downgrade AWAIT_HOLDING_REFCELL_REF and AWAIT_HOLDING_LOCK to pedantic. From rustup earlier, where I forgot the changlog: deprecate [`panic_params`] (uplifted)
@phansch phansch added the I-false-positive Issue: The lint was triggered on code it shouldn't have label Dec 18, 2020
@khuey
Copy link
Contributor

khuey commented Jan 6, 2021

Any progress here?

@Daniel-B-Smith
Copy link
Author

Apologies for the lack of communication. From conversations on Zulip, it is in fact not an easy fix to identify variables that have been explicitly dropped. I intended to add the documentation quickly, but got caught up in things. I just sent out #6585. Unfortunately, I don't have the cycles to dig into this deeply right now.

@flip1995 flip1995 added the E-hard Call for participation: This a hard problem and requires more experience or effort to work on label Jan 17, 2021
bors added a commit that referenced this issue Jan 17, 2021
Explicitly document false positives

Adds documentation for known false positives for the `await_holding*` lints.

Issues:

#6353
#6446
bors added a commit that referenced this issue Jan 17, 2021
Explicitly document false positives

Adds documentation for known false positives for the `await_holding*` lints.

Issues:

#6353
#6446

changelog: document FPs for the ``await_holding_*`` lints
@Ralith
Copy link

Ralith commented May 21, 2022

This lint seems to have been enabled by default in 1.61 and is still producing false positives, breaking my CI.

@Ralith
Copy link

Ralith commented Jun 18, 2022

Bump. This is still breaking my CI. Can we get it rolled back?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing E-hard Call for participation: This a hard problem and requires more experience or effort to work on I-false-positive Issue: The lint was triggered on code it shouldn't have
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants