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 unused_lifetimes warning #77217

Closed
Hawk777 opened this issue Sep 26, 2020 · 11 comments
Closed

False positive unused_lifetimes warning #77217

Hawk777 opened this issue Sep 26, 2020 · 11 comments
Assignees
Labels
A-async-await Area: Async & Await A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. C-bug Category: This is a bug.

Comments

@Hawk777
Copy link

Hawk777 commented Sep 26, 2020

I tried this code:

#![warn(unused_lifetimes)]

pub struct Thingy<'x, X> {
	x: &'x mut X,
	y: &'static str,
}

impl<'x, X> Thingy<'x, X> {
	pub fn new(x: &'x mut X) -> Self {
		Self{x, y: "hello"}
	}

	pub async fn foo(&mut self) -> &'static str {
		self.y
	}
}

I got a warning “lifetime parameter 'x never used”, on the impl line. But it clearly is used. The warning goes away if I delete the function foo, or make it a regular function instead of an asynchronous function.

Meta

rustc --version --verbose:

rustc 1.46.0
binary: rustc
commit-hash: unknown
commit-date: unknown
host: x86_64-unknown-linux-gnu
release: 1.46.0
LLVM version: 10.0

I saw #61115 and rust-lang/rust-clippy#3988, but those were both closed at least a year ago and this is still happening in 1.46.0 which was just released recently, so I assume it’s not the same bug.

@Hawk777 Hawk777 added the C-bug Category: This is a bug. label Sep 26, 2020
@lcnr
Copy link
Contributor

lcnr commented Sep 26, 2020

The issue is that the field x is never marked as used, which is required to mark 'x as used, as x is the only field using 'x.

This is not ideal, but not sure on where exactly we want to lint here. If we have functions f and g with f calling g and g calling f it is probably good to lint for both f and g so I don't think just marking everything used in unused things as used is the right solution here.

@jonas-schievink jonas-schievink added the A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. label Sep 26, 2020
@Hawk777
Copy link
Author

Hawk777 commented Sep 26, 2020

Then why does the warning not show up if foo is not async? x would be equally unused in that case, but there's no unused lifetime warning.

@Hawk777
Copy link
Author

Hawk777 commented Sep 27, 2020

Furthermore, this code, which clearly does use x, still has the unused lifetime warning, but of course not the unused field warning:

#![warn(unused_lifetimes)]

pub struct Thingy<'x> {
	x: &'x u32,
	y: &'static str,
}

impl<'x> Thingy<'x> {
	pub fn new(x: &'x u32) -> Self {
		Self{x, y: "hello"}
	}

	pub async fn foo(&self) -> &'static str {
		self.y
	}

	pub async fn bar(&self) -> u32 {
		*self.x
	}
}

Meanwhile, if I change both async fns to fn, the warning goes away. It seems to be triggered by the presence of any async fn in the impl.

@lcnr
Copy link
Contributor

lcnr commented Sep 27, 2020

Yeah, that is an actual bug 👍 thanks

@lcnr lcnr added the A-async-await Area: Async & Await label Sep 27, 2020
@tmandry tmandry added this to On deck in wg-async work via automation Sep 29, 2020
@tmandry tmandry added the AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. label Sep 29, 2020
@nikomatsakis
Copy link
Contributor

nikomatsakis commented Sep 29, 2020

So this is probably related to the desugaring somehow:

// When we create the opaque type for this async fn, it is going to have
// to capture all the lifetimes involved in the signature (including in the
// return type). This is done by introducing lifetime parameters for:
//
// - all the explicitly declared lifetimes from the impl and function itself;
// - all the elided lifetimes in the fn arguments;
// - all the elided lifetimes in the return type.
//
// So for example in this snippet:
//
// ```rust
// impl<'a> Foo<'a> {
// async fn bar<'b>(&self, x: &'b Vec<f64>, y: &str) -> &u32 {
// // ^ '0 ^ '1 ^ '2
// // elided lifetimes used below
// }
// }
// ```
//
// we would create an opaque type like:
//
// ```
// type Bar<'a, 'b, '0, '1, '2> = impl Future<Output = &'2 u32>;
// ```
//
// and we would then desugar `bar` to the equivalent of:
//
// ```rust
// impl<'a> Foo<'a> {
// fn bar<'b, '0, '1>(&'0 self, x: &'b Vec<f64>, y: &'1 str) -> Bar<'a, 'b, '0, '1, '_>
// }
// ```
//
// Note that the final parameter to `Bar` is `'_`, not `'2` --
// this is because the elided lifetimes from the return type
// should be figured out using the ordinary elision rules, and
// this desugaring achieves that.
//
// The variable `input_lifetimes_count` tracks the number of
// lifetime parameters to the opaque type *not counting* those
// lifetimes elided in the return type. This includes those
// that are explicitly declared (`in_scope_lifetimes`) and
// those elided lifetimes we found in the arguments (current
// content of `lifetimes_to_define`). Next, we will process
// the return type, which will cause `lifetimes_to_define` to
// grow.

but I don't exactly know how yet. Maybe applying the desugaring to the example will help to see it.

@Hawk777
Copy link
Author

Hawk777 commented Oct 3, 2020

I can also trigger a single_use_lifetimes warning with a free function:

#![warn(single_use_lifetimes)]
pub async fn foo<'a>(x: &'a u32) -> &'a u32 {
	x
}

Again, this only happens with an async fn, not with a regular fn, so I suspect it’s probably the same underlying cause (though if you disagree I’m happy to file a separate ticket).

@ghost
Copy link

ghost commented Dec 6, 2020

I also encountered this bug, and if I add #[allow(unused_lifetimes)] on the async fn (instead of the impl), the warning disappears, in both my code and the code of this issue.
I also read #78522. Is that the same bug? The warnings are both fired on the async fn itself.

@Vooblin
Copy link
Contributor

Vooblin commented Dec 7, 2020

@hyd-dev Yes, it is the same bug and I fixed it in this #79689 PR.

@Vooblin
Copy link
Contributor

Vooblin commented Dec 7, 2020

@rustbot claim

JohnTitor added a commit to JohnTitor/rust that referenced this issue Jan 14, 2021
Update tests of "unused_lifetimes" lint for async functions and corresponding source code

Before this PR the following code would cause an error:
```
#![deny(unused_lifetimes)]
async fn f<'a>(_: &'a i32) {}
fn main() {}
```
It was happening because of the desugaring of return type in async functions. As a result of the desugaring, the return type contains all lifetimes involved in the function signature. And these lifetimes were interpreted separately from the same in the function scope => so they are unused.

Now, all lifetimes from the return type are interpreted as used. It is also not perfect, but at least this lint doesn't cause wrong errors now.

This PR connected to issues rust-lang#78522, rust-lang#77217
m-ou-se added a commit to m-ou-se/rust that referenced this issue Jan 14, 2021
Update tests of "unused_lifetimes" lint for async functions and corresponding source code

Before this PR the following code would cause an error:
```
#![deny(unused_lifetimes)]
async fn f<'a>(_: &'a i32) {}
fn main() {}
```
It was happening because of the desugaring of return type in async functions. As a result of the desugaring, the return type contains all lifetimes involved in the function signature. And these lifetimes were interpreted separately from the same in the function scope => so they are unused.

Now, all lifetimes from the return type are interpreted as used. It is also not perfect, but at least this lint doesn't cause wrong errors now.

This PR connected to issues rust-lang#78522, rust-lang#77217
bors added a commit to rust-lang-ci/rust that referenced this issue Jan 14, 2021
Update tests of "unused_lifetimes" lint for async functions and corresponding source code

Before this PR the following code would cause an error:
```
#![deny(unused_lifetimes)]
async fn f<'a>(_: &'a i32) {}
fn main() {}
```
It was happening because of the desugaring of return type in async functions. As a result of the desugaring, the return type contains all lifetimes involved in the function signature. And these lifetimes were interpreted separately from the same in the function scope => so they are unused.

Now, all lifetimes from the return type are interpreted as used. It is also not perfect, but at least this lint doesn't cause wrong errors now.

This PR connected to issues rust-lang#78522, rust-lang#77217
@tmandry
Copy link
Contributor

tmandry commented Jan 15, 2021

Presumably fixed in #79689, leaving this open to try the original example in the next nightly.

@tmandry tmandry moved this from On deck to In progress in wg-async work Jan 15, 2021
@tmandry
Copy link
Contributor

tmandry commented Jan 28, 2021

Triage: Confirmed that this is fixed in nightly.

@tmandry tmandry closed this as completed Jan 28, 2021
wg-async work automation moved this from In progress to Done Jan 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-async-await Area: Async & Await A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. C-bug Category: This is a bug.
Projects
Development

No branches or pull requests

6 participants