generic existential types should (at most) warn, not error, if a type parameter is unused

[pnkfelix]

Consider the following code (play):

#![feature(existential_type)]

fn main() {
    let y = choose(vec![1, 2, 3, 4].into_iter());
    for (i, elem) in y.enumerate() {
        println!("processing y[{}]: {:?}", i, elem);
    }
}

existential type MaybeWrapped<I>: Iterator<Item = usize>;

fn choose<I: Iterator<Item = usize>>(x: I) -> MaybeWrapped<I> {
    // If you switch to variant below, it works.
    // return Wrapped(x);
    return x.collect::<Vec<usize>>().into_iter();
}

/// Trivial wrapper around an iterator.
struct Wrapped<I: Iterator>(I);
impl<I: Iterator> Iterator for Wrapped<I> {
    type Item = I::Item;
    fn next(&mut self) -> Option<I::Item> { self.0.next() }
}

The compiler errors, with the diagnostic

error[E0091]: type parameter `I` is unused
  --> src/main.rs:10:31
   |
10 | existential type MaybeWrapped<I>: Iterator<Item = usize>;
   |                               ^ unused type parameter

error: aborting due to previous error

But it should not be illegal to leave a type parameter unused. It is useful, from the view-point of the implementor, to have the option of returning concrete types that do not use the given type.

(Also, I find the diagnostic itself a bit less helpful than it could be; when I first read it, I thought it was complaining about not seeing I on the trait-bound in existential type <NAME>: <TRAIT_BOUND>; I needed @eddyb to point out to me that the issue is in the body of fn choose itself.)

[eddyb]

Relevant code:

rust/src/librustc_typeck/check/mod.rs

Line 5275 in 994cdd9

struct_span_err!(tcx.sess, span, E0091, "type parameter `{}` is unused", param.name)

[Centril]

This is a bug. The current behavior with the error deviates from the specification in RFC 2071.
In particular, see:

existential type Foo<T>: Debug;

[...]
Similarly to impl Trait under RFC 1951, existential type implicitly captures all generic type parameters in scope. In practice, this means that existential associated types may contain generic parameters from their impl:
[...]

I don't think this should even be a warning.

cc @cramertj

[cramertj]

Agreed, this shouldn't warn.

[Centril]

cc rust-lang/rfcs#2071 #34511

[Centril]

"Assigning" this to @alexreg to fix per the spec in RFC 2071.

[alexreg]

I'm not quite sure I get this. Why can't we just remove the type parameter? It's not needed in this case, even if we wrap the iterator in a Wrapped value.

[Centril]

@alexreg perhaps not in this case; but there's still a bug as compared to the specification of RFC 2071 so it should be fixed.

[pnkfelix]

@alexreg

I went too far when I was trying to simplify the example (to cut down the number of lines; whoops).

here is a variant example that does need the type parameter for MaybeWrapped (play):

#![feature(existential_type)]

fn main() {
    let v = vec![1, 2, 3, 4];
    let y = demoit(v.iter().cloned());
    for (i, elem) in y.enumerate() {
        println!("processing y[{}]: {:?}", i, elem);
    }

    let v = vec![1, 2, 3, 4];
    let y = choose(v.iter().cloned());
    for (i, elem) in y.enumerate() {
        println!("processing y[{}]: {:?}", i, elem);
    }
}

existential type WhyArgNeeded<I>: Iterator<Item = usize>;

fn demoit<I: Iterator<Item = usize>>(x: I) -> WhyArgNeeded<I> {
    // If you remove `I` from `WhyArgNeeded`, this will break.
    return Wrapped(x);
}

existential type MaybeWrapped<I>: Iterator<Item = usize>;

fn choose<I: Iterator<Item = usize>>(x: I) -> MaybeWrapped<I> {
    // `demoit` above shows that variant below would work.
    // return Wrapped(x);
    return x.collect::<Vec<usize>>().into_iter();
}

/// Trivial wrapper around an iterator.
struct Wrapped<I: Iterator>(I);
impl<I: Iterator> Iterator for Wrapped<I> {
    type Item = I::Item;
    fn next(&mut self) -> Option<I::Item> { self.0.next() }
}

(In particular, here we need the type parameter in MaybeWrapped<I>, because the I is going to carry the lifetime of the vec we are iterating over.)

[pnkfelix]

I love playing devil's advocate, so: Here's an argument for "why a warning might be desirable": we warn today about unused parameters in methods/functions/closures, even when the parameter's presence is forced upon us in order to satisfy external APIs.

• I.e. we try to force programmers to either 1. revise their interfaces to be more minimal, or 2. mark that they are opting into ignoring the parameter (i.e. by locally naming it with a leading underscore, or by putting in the appropriate #[allow(..)] directive, etc).

Now, do I think these two cases (of functions vs existential types) are really comparable? Meh, not really.

But its possible that some developers will find it useful to get feedback like this, and would prefer to explicitly write (via e.g. existential type MaybeWrapped<_I>: Iterator<Item = usize>;) when they want to make it clear that they know that the concrete implementation is not currently using the given type parameter.

[Centril]

@pnkfelix My view here is mostly that it is way too early to tell if we should warn or not with no substantial usage experience to talk of. I'd like to first stabilize at some point and then we can always add lints if it turned out to be necessary. Being a tad more empirical doesn't hurt =P

However -- in the interim, the RFC does not specify that there should be any warnings, so we should first implement what is decided and then implement other things once other decisions are made.

[alexreg]

@pnkfelix Yeah, that makes more sense now. error: type parameter I is part of concrete type but not used in parameter list for existential type is the key error if you remove the type parameter, which is in fact sensible.

Regarding your "devil's advocate" argument, I'm actually of the opinion the error should remain when the concrete type is not using it, and disappear entirely when it is using it (no warning/lint).

[pnkfelix]

@alexreg wrote:

Regarding your "devil's advocate" argument, I'm actually of the opinion the error should remain when the concrete type is not using it, and disappear entirely when it is using it (no warning/lint).

Wait, that makes it sound like you are saying that the issue as I originally described is not a bug?

I had hoped my example illustrated why this is a bug: requiring someone to remove the type parameter based on the particulars of the concrete type makes this abstraction "leaky." It would be analogous to requiring a function to actually reference all of its arguments in the function body (which is what my devil's advocate argument was implying would be a bad thing...)