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

single_use_lifetimes warns when deriving a struct #53738

Closed
TimDiekmann opened this issue Aug 27, 2018 · 10 comments · Fixed by #61824
Closed

single_use_lifetimes warns when deriving a struct #53738

TimDiekmann opened this issue Aug 27, 2018 · 10 comments · Fixed by #61824
Assignees
Labels
A-edition-2018-lints Area: Lints supporting the 2018 edition A-lifetimes Area: Lifetimes / regions A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@TimDiekmann
Copy link
Member

When compiling

#![warn(single_use_lifetimes)]

#[derive(Debug)]
struct Foo<'a> {
    bar: &'a u32,
}

this warning is generated:

warning: lifetime parameter `'a` only used once
 --> src/lib.rs:5:12
  |
5 | struct Foo<'a> {
  |            ^^
  |

I think, #[automatically_derived] in the Debug derive does not consider this lint.

@estebank estebank added A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. A-lifetimes Area: Lifetimes / regions A-edition-2018-lints Area: Lints supporting the 2018 edition labels Aug 27, 2018
@eddyb
Copy link
Member

eddyb commented Jun 11, 2019

I tried to deny(single_use_lifetimes) in the compiler but hit this bug, which makes it a non-starter.

I'd almost change built-in deriving to not declare the lifetimes explicitly but I suspect that wouldn't work when using #[derive] nested within a block where the shadowing from explicitness would be useful.
(That's assuming built-in deriving actually uses these lifetimes in bounds, otherwise it could just use '_)
cc @matthewjasper @rust-lang/compiler

@eddyb eddyb added I-nominated T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 11, 2019
bors added a commit that referenced this issue Jun 11, 2019
Add deny(unused_lifetimes) to all the crates that have deny(internal).

@Zoxc brought up, regarding #61722, that we don't force the removal of unused lifetimes.
Turns out that it's not that bad to enable for compiler crates (I wonder why it's not `warn` by default?).

I would've liked to enable `single_use_lifetimes` as well, but #53738 makes it unusable for now.

For the `rustfmt` commit, I used rust-lang/rustfmt#1324 (comment), and manually filtered out some noise.

r? @oli-obk cc @rust-lang/compiler
@pnkfelix
Copy link
Member

cc #44752

@pnkfelix
Copy link
Member

I don't think this seems high priority. I'll leave it nominated but I'm prioritizing it as P-medium.

@oli-obk
Copy link
Contributor

oli-obk commented Jun 13, 2019

The problem is that the above code expands to something similar to

#![feature(in_band_lifetimes)]
#![warn(single_use_lifetimes)]

trait Foo {}

struct Bar<'a>(&'a i32);

impl Foo for Bar<'a> {}

which correctly warns for the 'a in the impl Foo. But since in the original code the impl is derived and reusesthe 'a from the struct declaration, the lint appears to trigger on the struct declaration.

We may either just want to not trigger in derive expansions or explicitly allow this lint in derives expansions.

@pnkfelix pnkfelix added the P-medium Medium priority label Jun 13, 2019
@pnkfelix
Copy link
Member

advertised at T-compiler meeting. removing nomination tag.

@nikomatsakis
Copy link
Contributor

I'm going to ping @zackmdavis in the hopes that they have a bit of time to look into this. My sense is that we ought to make a plan to finish up the Rust 2018 idiom lints -- but at the same time it's hard to justify a P-high label just now.

@estebank
Copy link
Contributor

estebank commented Jun 13, 2019

I tried an approach where I would look at the lifetime span to see if it came from a compiler desugaring, but the derived code doesn't keep any information about their context. A different approach will be needed in the parse step to make sure the debug subparser produced spans are appropriately marked. I won't have time to dig deeper, but wanted to share what I found.

@eddyb
Copy link
Member

eddyb commented Jun 13, 2019

@estebank AFAIK there's a perma-unstable attribute that is placed on derived impls which some error reporting code might be checking.

@pnkfelix I should've given more context - it'd be good to fix this because it would unblock something like #61735, but likely removing a far larger number of lifetimes, which may simplify e.g. the 'gcx removal.
But if we can't really soon, we can defer indefinitely, I think.

@zackmdavis zackmdavis self-assigned this Jun 13, 2019
@zackmdavis
Copy link
Member

we ought to make a plan to finish up the Rust 2018 idiom lints

#54910?

Centril added a commit to Centril/rust that referenced this issue Jun 14, 2019
in which we decline to lint single-use lifetimes in `derive`d impls

Resolves rust-lang#53738.

r? @eddyb
Centril added a commit to Centril/rust that referenced this issue Jun 14, 2019
in which we decline to lint single-use lifetimes in `derive`d impls

Resolves rust-lang#53738.

r? @eddyb
Centril added a commit to Centril/rust that referenced this issue Jun 14, 2019
in which we decline to lint single-use lifetimes in `derive`d impls

Resolves rust-lang#53738.

r? @eddyb
Centril added a commit to Centril/rust that referenced this issue Jun 15, 2019
in which we decline to lint single-use lifetimes in `derive`d impls

Resolves rust-lang#53738.

r? @eddyb
@stouset
Copy link

stouset commented Mar 12, 2020

@zackmdavis This seems to have regressed.

#[deny(single_use_lifetimes)]

#[derive(Eq)]
struct Foo<'a, T> {
    /// a reference to the underlying secret data that will be derefed
    pub data: &'a mut T,
}

produces the following warning

   Compiling playground v0.0.1 (/playground)
error: lifetime parameter `'a` only used once
 --> src/lib.rs:4:12
  |
3 | #[derive(Eq)]
  |          -- ...is used only here
4 | struct Foo<'a, T> {
  |            ^^ this lifetime...
  |
note: lint level defined here
 --> src/lib.rs:1:8
  |
1 | #[deny(single_use_lifetimes)]

mgeier added a commit to mgeier/rtrb that referenced this issue Nov 9, 2020
This causes a spurious single_use_lifetimes warning, see
rust-lang/rust#53738
toku-sa-n added a commit to toku-sa-n/accessor that referenced this issue Aug 2, 2021
toku-sa-n added a commit to toku-sa-n/accessor that referenced this issue Aug 2, 2021
* ci: deny multiple allow-by-default lints

* chore: add a changelog

* fix: wrong indentation

* ci: allow `single_use_lifetimes`

It doesn't work. See: rust-lang/rust#53738 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-edition-2018-lints Area: Lints supporting the 2018 edition A-lifetimes Area: Lifetimes / regions A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants