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

Add lint for lifetimes or generic types not required by trait / struct / enum #18549

Closed
shepmaster opened this issue Nov 2, 2014 · 9 comments
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut.

Comments

@shepmaster
Copy link
Member

This code

struct Alpha;
impl<'a, T> Alpha {
  fn yeah(arg: &'a T) {}
}

Should generate two lint warnings on the impl line

  1. The lifetime 'a is not needed by the struct.
  2. The type T is not needed by the struct.

This should apply similarly to trait impls where the lifetime / types are not needed by either the trait or struct.

Update as of rustc 1.0.0-dev (82cc57b6a 2015-01-13 09:54:08 -0800)

Type parameters now have a warning like "the type parameter T is not constrained by the impl trait, self type, or predicates", so that part is good. I think we still want something for lifetimes.

Original report below

$ rustc --version=verbose
rustc 0.13.0-dev (3327ecca4 2014-11-01 22:41:48 +0000)
binary: rustc
commit-hash: 3327ecca422046699315122345c6c050ab73804b
commit-date: 2014-11-01 22:41:48 +0000
host: x86_64-apple-darwin
release: 0.13.0-dev

This code (playpen link)

struct Parser;

impl<Z> Parser {
    fn parse_item(&self) {}

    fn parse_items(&self) {
        self.parse_item();
    }
}

fn main() {}

Fails to compile:

$ rustc --test repro.rs
repro.rs:7:9: 7:26 error: unable to infer enough type information about `_`; type annotations required
repro.rs:7         self.parse_item();
                   ^~~~~~~~~~~~~~~~~

Removing the <Z> allows the code to compile. This is a reduced example from my full app, where the type parameter is actually used. 😀

@ghost
Copy link

ghost commented Nov 2, 2014

For the record, here's what 0.12 would say about this code:

<anon>:7:9: 7:26 error: unable to infer enough type information to locate the impl of the trait `core::kinds::Sized` for the type `<generic #1>`; type annotations required
<anon>:7         self.parse_item();
                 ^~~~~~~~~~~~~~~~~
error: aborting due to previous error

@sfackler
Copy link
Member

sfackler commented Nov 2, 2014

@shepmaster Could you post a bit more code to show how Z is used in the real version? It seems to me like the compiler is correct in being confused here. There is a parse_item function for all types, and nothing in the signature of parse_items to lock down which version you actually want to call.

@shepmaster
Copy link
Member Author

@sfackler Here's the full shebang. Specifically the call to parse_eq on line 322.

Your comment makes me think that I have a large conceptual mismatch. I was under the impression that only functions that actively used the type parameter would be specialized (in this example, nothing would happen). Is it more correct to think the entire impl that gets specialized?

I would have expected that the version of parse_item would be the one that matches parse_items - can function calls on self go across different specializations of the type parameter?

I think that the error message part about _ could also use some loving, but I'm not sure I understand the problem enough to suggest an alternative.

@sfackler
Copy link
Member

sfackler commented Nov 4, 2014

Everything in the impl block is parameterized. I've actually never seen type parameters added to impl blocks themselves that aren't part of the trait or type definition. It's far more common to parameterize the individual methods that need it.

@shepmaster
Copy link
Member Author

It's far more common to parameterize the individual methods that need it.

Moving the type parameters to each function worked well to get me unstuck, thanks!. However, is there a way of avoiding so much repetition? That was my original goal of moving it up to the impl.

// This is easier for me to understand
impl<T : PartialEq> Foo {
  fn method1(&self, a: T);
  fn method2(&self, a: T);
  fn method3(&self, a: T);
  fn method4(&self, a: T);
  fn method5(&self, a: T);
} 
// But it needs to be this way?
impl Foo {
  fn method1<T : PartialEq>(&self, a: T);
  fn method2<T : PartialEq>(&self, a: T);
  fn method3<T : PartialEq>(&self, a: T);
  fn method4<T : PartialEq>(&self, a: T);
  fn method5<T : PartialEq>(&self, a: T);
} 

@shepmaster
Copy link
Member Author

@sfackler I've seen this same problem occur a few times now on Stack Overflow questions. I propose we transmute this issue to become something like "Add lint for lifetimes or generic types not required by trait / struct / enum"

@sfackler
Copy link
Member

👍

@shepmaster shepmaster changed the title "unable to infer enough type information about _" when type parameter is used Add lint for lifetimes or generic types not required by trait / struct / enum Dec 29, 2014
@steveklabnik steveklabnik added the A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. label Jan 29, 2015
@steveklabnik
Copy link
Member

Triage: still gives T is not constrained error, but nothing about the lifetimes.

@steveklabnik
Copy link
Member

Since new lints have a big impact on users of rustc, the policy is that they should go through the RFC process like other user-facing changes. As such, I'm going to give this one a close, but if anyone comes across this ticket and wants this lint, consider adding it to clippy and/or writing up an RFC. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut.
Projects
None yet
Development

No branches or pull requests

3 participants