confusing "unused type parameter" diagnostic

[chisophugis]

I really don't get this diagnostic. It says that U is unused, but removing U causes a " cannot find type U in this scope" diagnostic. Is U used or not? I feel like this diagnostic could be improved.

(btw, I arrived at this code while reading https://doc.rust-lang.org/book/2018-edition/ch13-01-closures.html#limitations-of-the-cacher-implementation)

struct Cacher<T, U: Copy>
    where T: Fn(U) -> U
{
    calculation: T,
    value: Option<u32>,
}


fn main() {
    let c = Cacher::new(|x| x);
    println!("Hello, world!");
}

(Playground)

Errors:

   Compiling playground v0.0.1 (file:///playground)
error[E0392]: parameter `U` is never used
 --> src/main.rs:2:18
  |
2 | struct Cacher<T, U: Copy>
  |                  ^ unused type parameter
  |
  = help: consider removing `U` or using a marker such as `std::marker::PhantomData`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0392`.
error: Could not compile `playground`.

To learn more, run the command again with --verbose.

[RustyYato]

Type bounds don't matter when checking if a type parameter is used to fix this you can use std::marker::PhantomData like the error message tells you. You can always type in "rustc --explain <ERROR_CODE>" to get an explanation for most errors and some reasons for why it is an error.

use std::marker::PhantomData;

struct Cacher<T, U: Copy>
    where T: Fn(U) -> U
{
    calculation: T,
    value: Option<u32>,
    __phantom: PhantomData<U> // the name "__phantom" doesn't matter
}

struct Cacher<T, U: Copy>
    where T: Fn(U) -> U
{
    fn new(calculation: T) -> Self {
        Self { calculation, value: None, __phantom: PhantomData }
    }
}

fn main() {
    let c = Cacher::new(|x| x);
    println!("Hello, world!");
}

But since you are building a cacher, you probably want to change value to a Option<U>.

struct Cacher<T, U: Copy>
    where T: Fn(U) -> U
{
    calculation: T,
    value: Option<U>,
}

For more information about this error, you can look into issue 35146

Now, going a bit further, we could generalize this Cacher using a Map (HashMap or BTreeMap), so that it can keep track of multiple inputs, but this is left as an exercise for the reader.

edit: moved urls into links

[chisophugis]

Oh, duh. I should have just changed it to Option<U> which fixes everything. Still, I think this diagnostic is confusing.

To me, the word "unused" implies "you can delete it and there will be no name resolution errors", which is not the case here. And I think this confused me and that confusion prevented me from seeing the obvious fix.

It sounds like what is meant by the diagnostic is something like "no member transitively contains or points to an object of type U".

Also, instead of immediately jumping to the somewhat esoteric PhantomData, the diagnostic could also suggest to make sure that you didn't forget to use U in one of the member types. Also, the compiler should probably not suggest removing U in this case since that causes "cannot find type U in this scope" errors.

Here is a strawman proposed diagnostic:

   Compiling playground v0.0.1 (file:///playground)
error[E0392]: parameter `U` is never used
 --> src/main.rs:2:18
  |
2 | struct Cacher<T, U: Copy>
  |                  ^ no member type stores or points to this type
  |
  = help: check if any member should be storing or pointing to `U` (you can use `std::marker::PhantomData` to fake this)

[RustyYato]

Generally unused type parameters are used with respect to lifetimes, for example, implementing a slice struct with raw pointers. There clearly should be a lifetime, but we can't store it in the struct.

struct Slice<'a, T> {
    ptr: *const T,
    __phantom: Phantom<&'a T>
}

Here the PhantomData is used to control the variance of both 'a and 'T', and this pattern is fairly common when implementing data structures from first principles.
RFC 2357 aims to deprecate PhantomData, but I don't think such a large scale RFC will be accepted until we get more discussion about it.

[RustyYato]

You can read more about variance and PhantomData in the rust nomicon. The rust nomicon delves into unsafe code, and how it should be used, and some of the many considerations needed for writing good unsafe code. Especially the section about PhantomData.

[chisophugis]

Thanks. I understand the need for PhantomData. The code I was writing was totally pedestrian, safe rust code though, so seeing PhantomData mentioned in the diagnostic was distracting and confusing. I think we should try to avoid mixing the more esoteric PhantomData situation with pedestrian mistakes like the one in this bug.

[estebank]

A first approximation of good output would be:

error[E0392]: parameter `U` is never used
 --> src/main.rs:2:18
  |
2 | struct Cacher<T, U: Copy>
  |                  ^ unused type parameter
  |
  = help: consider removing `U`
  |
2 | struct Cacher<T>
  |               --
  = help: alternatively, consider using a marker such as `std::marker::PhantomData`
  |
7 |     __marker: std::marker::PhantomData<U>,
  |

Ideally, we would detect that U is being used in where clause and either not suggest it

error[E0392]: parameter `U` is never used
 --> src/main.rs:2:18
  |
2 | struct Cacher<T, U: Copy>
  |                  ^ unused type parameter
  |
  = help: consider using a marker such as `std::marker::PhantomData`
  |
7 |     __marker: std::marker::PhantomData<U>,
  |

or suggest removing the bound

error[E0392]: parameter `U` is never used
 --> src/main.rs:2:18
  |
2 | struct Cacher<T, U: Copy>
  |                  ^ unused type parameter
  |
  = help: consider removing `U` and its references
  |
2 | struct Cacher<T>
3 | {
  |
  = help: alternatively, consider using a marker such as `std::marker::PhantomData`
  |
7 |     __marker: std::marker::PhantomData<U>,
  |

---

I don't see how we could suggest the correct fix without being misleading more times than not (even though I'm thinking of a couple of heuristics we could use):

error[E0392]: parameter `U` is never used
 --> src/main.rs:2:18
  |
2 | struct Cacher<T, U: Copy>
  |                  ^ unused type parameter
  |
  = help: you might have meant to use `U` here
  |
6 |     value: Option<U>,
  |                   -

[chisophugis]

Is there a more precise way to say "U is not used in struct members"? The core misleading thing I think is saying "unused" when there are uses of the name.

[RustyYato]

@chisophugis This seems like the best way to improve the situation, as it points at the problem directly and then we could make suggestions on how to fix it like what @estebank suggested.

[Guang1234567]

Hello @KrishnaSannasi:

I implement this Cacher whithout Copy trait. But confuse is it correct?

Because change the member by mem::replace(current, Some(v));.

Could you give me some suggestion? Thanks.

//   Cacher.rs

use std::marker::PhantomData;
use std::mem;
use std::thread;
use std::time::Duration;

struct Cacher<T, U, R>
    where
        T: Fn(U) -> R
{
    calculation: T,
    value: Option<R>,
    __phantom: PhantomData<U>,
}

impl<T, U, R> Cacher<T, U, R>
    where
        T: Fn(U) -> R
{
    fn new(calculation: T) -> Self {
        Self {
            calculation,
            value: None,
            __phantom: PhantomData,
        }
    }

    fn value(&mut self, arg: U) -> &R {
        let current = &mut self.value;
        match current {
            Some(v) => v,
            None => {
                let v = (self.calculation)(arg);
                mem::replace(current, Some(v));
                let current = current.as_ref();
                current.unwrap()
            }
        }
    }
}


#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn it_works() {
        let mut expensive_result = Cacher::new(|input: i32| {
            println!("calculating slowly...");
            thread::sleep(Duration::from_secs(2));
            input * 2
        });

        println!(
            "Today, do {} pushups!",
            expensive_result.value(1)
        );

        println!(
            "Next, do {} situps!",
            expensive_result.value(1)
        );

        let correctOutput = &2;
        assert_eq!(correctOutput, expensive_result.value(1));
    }
}

Test command

     cargo test --package minigrep --lib cache::tests::it_works -- --nocapture 

Output

~/dev_kit/src_code/minigrep >>>>   cargo test --package minigrep --lib cache::tests::it_works -- --nocapture 
    Finished dev [unoptimized + debuginfo] target(s) in 0.00s
     Running target/debug/deps/minigrep-81d8885cf42afcb9

running 1 test
calculating slowly...
Today, do 2 pushups!
Next, do 2 situps!
test cache::tests::it_works ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 6 filtered out

[RustyYato]

You could just do *current = Some(v) instead of replace, also I would delay the Fn bound to the value function and remove the type parameter R from the type.

Here is what I would do

struct Cacher<T, R>
{
    calculation: T,
    value: Option<R>,
}

impl<T, R> Cacher<T, R> {
    fn new(calculation: T) -> Self {
        Self {
            calculation,
            value: None,
        }
    }

    fn value<U>(&mut self, arg: U) -> &R
    where T: FnMut(U) -> R
    {
        match self.value.as_ref() {
            Some(v) => v,
            None => {
                let v = (self.calculation)(arg);
                self.value = Some(v);
                self.value.as_ref().unwrap()
            }
        }
    }
}

[Guang1234567]

@KrishnaSannasi
Thanks.

Just like in cpp *current = Some(v) .

fn value<U>(&mut self, arg: U) -> &R
where T: FnMut(U) -> R
 {
        let current: &mut Option<R> = &mut self.value;
        match current {
            Some(v) => v,
            None => {
                let v: R = (self.calculation)(arg);
                *current=Some(v);
                current.as_ref().unwrap()
            }
        }
    }

It works!

BTW, Could mention it in offical tutorial? Thanks.

---

But

    fn value<U>(&mut self, arg: U) -> &R
        where T: FnMut(U) -> R
    {
        match self.value.as_ref() {
            Some(v) => v,
            None => {
                let v = (self.calculation)(arg);
                self.value = Some(v);
                self.value.as_ref().unwrap()
            }
        }
    }

It has compile error

error[E0506]: cannot assign to `self.value` because it is borrowed
  --> src/cache/mod.rs:27:17
   |
20 |     fn value<U>(&mut self, arg: U) -> &R
   |                 - let's call the lifetime of this reference `'1`
...
23 |         match self.value.as_ref() {
   |               ---------- borrow of `self.value` occurs here
24 |             Some(v) => v,
   |                        - returning this value requires that `self.value` is borrowed for `'1`
...
27 |                 self.value = Some(v);
   |                 ^^^^^^^^^^ assignment to borrowed `self.value` occurs here

error: aborting due to previous error

For more information about this error, try `rustc --explain E0506`.
error: Could not compile `minigrep`.

[RustyYato]

pub fn value<U>(&mut self, arg: U) -> &R
    where T: FnMut(U) -> R
{
    if self.value.as_ref().is_none() {
        self.value = Some((self.calculation)(arg));
    }
    
    self.value.as_ref().unwrap()
}

Note that the unwrap if fine because we are guaranteed to initialize value before it, and the panic will get optimized away see goldbolt for asm.

This lifetime error likely won't happen by polonius when that lands, nll isn't great at handling return values.