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...)

[pnkfelix]

(I interpret @alexreg's response as an implicit vote for why one might want the warning when concrete type is not using it...)

[alexreg]

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...)

The abstraction is already "leaky" though, in that it requires a type parameter when the concrete type does. So I either maintain the vote for my proposal, or alternative we make error: type parameter I is part of concrete type but not used in parameter list for existential type go away.

[pnkfelix]

The abstraction is already "leaky" though, in that it requires a type parameter when the concrete type does

Your example there is using an odd definition of "leaky", from my point of view.

When a function body needs an input, then it makes sense to require it as a function parameter. In most programming languages (i.e., lexically-scoped languages, where you cannot inspect the variables in the environment of one's caller via dynamic scope), it is the only option.

When a function body does not need an input, then one may or may not include the input as a parameter on the function's signature, depending on factors like "what might I change in the future about this function body", or "where do I pass this function around as a first class value"

[alexreg]

I don't think the analogy to functions is very apt. The point is, the definition of "leaky" is that an abstraction exposes information about its concrete (implementation) details. Having a type parameter for the iterator type as in your above example is clearly an implementation detail, since the consumer of the existential type has no need to know about it.

[oli-obk]

When I implemented it, my reasoning for making it a hard error was that

error[E0392]: parameter `T` is never used
 --> src/main.rs:1:12
  |
1 | struct Foo<T>;
  |            ^ unused type parameter
  |
  = help: consider removing `T` or using a marker such as `std::marker::PhantomData`

is also a hard error. But what we have here is more along the lines of

trait Foo<T> {}

which does not error.

[pnkfelix]

Hmm. The reason unused types are an error in structs is mainly because otherwise they’d be given Bivariant variance, and @nikomatsakis and/or I felt that was likely to not be the users intention: better to force the user to indicate a variance, if only via PhantomData

I assume that existential types force Invariant variance on their type parameters. Is this true? If so, then the analogy with trait Trait<A> would be apt.

[alexreg]

I really don’t see the purpose of the error error: type parameter I is part of concrete type but not used in parameter list for existential type myself. I think that needs to go away and the current behaviour remain the same otherwise.

[pnkfelix]

What if there are lifetimes attached to I? We need some way to make those connections; it was my reason for investigating threading the I along

[pnkfelix]

(actually ... maybe i'm wrong about the necessity of making such connections explicit... I haven't worked out all the details here yet, but I did just remember that we certainly allow things like impl<'a> Trait for Struct<'a> { type Assoc = Inner<'a>; })

[alexreg]

Has any consensus been reached here lately?

[pnkfelix]

My current thinking is that we need to either get rid of the error: type parameter I is part of concrete type but not used in parameter list for existential type when you try to run this variant, or downgrade/remove the error being emitted for the code as originally written

I don't know which path of the two is the right course, but my gut says that if we can get rid of error: type parameter I is part of concrete type but not used in parameter list for existential type, then we should take that path.

[pnkfelix]

(Also I probably need to find out the answer to this question:)

I assume that existential types force Invariant variance on their type parameters. Is this true? If so, then the analogy with trait Trait<A> would be apt.

[oli-obk]

My current thinking is that we need to either get rid of the error: type parameter I is part of concrete type but not used in parameter list for existential type when you try to run this variant, or downgrade/remove the error being emitted for the code as originally written

If those are the only two options, then the decision is clear (at my level of understanding of the type theory beind it): We remove the "unused type parameter" error.

The error about

type parameter I is part of concrete type but not used in parameter list for existential type

is not born out of a sense for minimalism like the "unused generic parameter" error. The problem is that the following code does not give us a concrete type when using the name Foo. What's the size of a variable declared as let x: Foo?. Yes, we need to call foo in order to get a value of such a variable, but we could create a let x: Option<Foo> = None; without that.

existential type Foo: Bar;

fn foo<T: Bar>(t: T) -> Foo {
    t
}

It gets even more confusing if we start thinking about

let mut x = foo(42);
x = foo("bar");

which, according to the types, checks out.