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

Do not mix type parameters and impl Trait #2

Closed
dtolnay opened this issue Oct 13, 2018 · 5 comments

Comments

Projects
None yet
2 participants
@dtolnay
Copy link

commented Oct 13, 2018

ndarray-csv/src/lib.rs

Lines 79 to 83 in a69bdf9

/// Read CSV data into a new ndarray with the given shape
pub fn read<A>(shape: (usize, usize), reader: &mut Reader<impl Read>) -> Result<Array2<A>, Error>
where
A: Copy,
for<'de> A: serde::Deserialize<'de>,


Mixing type parameters and impl Trait in the same signature makes it impossible to use with turbofish. Turbofish is commonly needed for signatures where some type parameter appears only in the return type.

use std::io::{Cursor, Read};

fn read<A>(_reader: impl Read) -> A {
    unimplemented!()
}

fn main() {
    let reader = Cursor::new(Vec::new());
    let _: u8 = read(reader); // okay

    let _ = read::<u8>(reader); // not supported
}
@paulkernfeld

This comment has been minimized.

Copy link
Owner

commented Oct 13, 2018

This makes sense. Would you recommend switching to all impl Trait or all type parameters?

@paulkernfeld

This comment has been minimized.

Copy link
Owner

commented Oct 13, 2018

As a more general note, is there ever a situation where it might be worth it to mix type parameters and impl Trait even though it makes it impossible to use the turbofish operator?

@dtolnay

This comment has been minimized.

Copy link
Author

commented Oct 13, 2018

You wouldn't be able to express the signature of read with all impl Trait because A appears in the return type.

Using all type parameters can be annoying when you need to turbofish one of the type parameters but not the other. For example if you had fn read<R, A> then the caller would need to write ndarray_csv::read::<_, u8> to specify A but infer R.

You could consider either a trait object (to avoid having an R type parameter entirely) or an extension trait method (which places R on the impl and A on the method, allowing turbofish for one or both independently). An extension trait could work much like the csv::Reader::deserialize method: reader.read_array::<u8>(2, 3).

As a more general note, is there ever a situation where it might be worth it to mix type parameters and impl Trait even though it makes it impossible to use the turbofish operator?

Turbofish is typically not needed for signatures where type parameters appear only in the argument types, so mixing is not problematic for those. Turbofish is mostly for methods with a type parameter in the return type such as Iterator::collect, str::parse, serde_json::from_str.

@paulkernfeld

This comment has been minimized.

Copy link
Owner

commented Oct 16, 2018

Hmm, this issue is a bit deeper than I had realized. I like your extension trait suggestion!

Is there somewhere that I could go to read more about how to choose generics in Rust?

@paulkernfeld

This comment has been minimized.

Copy link
Owner

commented Oct 16, 2018

Okay, I have now implemented the extension trait suggestion. I think I did it correctly, but please let me know if I have gotten something wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.