lloydmeta / frunk

Funktional generic type-level programming in Rust: HList, Coproduct, Generic, LabelledGeneric, Validated, Monoid and friends.
https://beachape.com/frunk/
MIT License
1.29k stars 58 forks source link

Wrappers around trait methods (suggestions to improve ergonomics) #90

Closed ExpHP closed 6 years ago

ExpHP commented 6 years ago

I find ergonomics to be a crucial part of any library's design, and I have some proposals for frunk that I would like to test the waters with.

I should preface this with some caveats:

  1. Not all of what I say is common practice. I'm just proposing design patterns that I've found to be successful in the past to see if frunk might be willing to adopt them.
  2. Ergonomical hacks often come at the cost of decreased learnability, in part because sometimes the abstraction may leak, and in part because it tends to lead to having multiple similar-looking things in the documentation. The impact of the latter can be reduced by having sufficient-quality docs that direct the reader to the right places and help clarify the different roles of similar-looking things.

I apologize if this is a lot to respond to! I just have strong opinions on this topic, and to really adopt frunk I'd like to know that these changes are either possible in the future, or that there is good reason for not making them.

Wrapping method traits with inherent methods

What

Something like this:


impl<A, Rest> HCons<A, Rest> {
    ...

    // NOTE: this is where the primary documentation for `get` would lie.
    // The docs on Selector would be reduced in scope to mostly just
    // describe it as a trait you can use in `where` bounds when you need
    // to call `get` in a generic function
    #[inline(always)]
    pub fn get<T, I>(&self) -> &T
    where Self: Selector<T, I>,
    { Selector::get(self) }

    #[inline(always)]
    pub fn pluck<T, I>(self) -> T
    where Self: Plucker<T, I>,
    { Plucker::pluck(self) }

    #[inline(always)]
    pub fn into_reverse(self) -> <Self as IntoReverse>::Output
    where Self: IntoReverse,
    { IntoReverse::into_reverse(self) }

    ...
}

impl HNil {
    ...
    // (same stuff; this can easily be done with a macro)
    ...
}

Aside: In funky I went even further, and tried to unify the documentation of HCons and HNil by using some trick to define a trait for documentation only (it cannot be used). At the end of the day though I'm not actually sure whether this makes the documentation better or worse, so for now I am not proposing that as an idea for frunk.

Why

Making turbofish usable

Existing motivation

Traits in a library like frunk are often forced to put type parameters on the trait rather than on the method. An example I added in PR #89 unfortunately showcases how awkward it can be to use a type annotation with coproduct.uninject when doing exhaustive matches:

let res: Result<i32, _> = co.uninject();
let co = match res {
    Ok(x) => return (2 * x) as f32,
    Err(co) => co,
};

(and the alternative of using UFCS is even worse)

If uninject were an inherent method designed with turbofish usage in mind, we could support the following usage:

let co = match co.uninject::<i32, _> {
    Ok(x) => return (2 * x) as f32,
    Err(co) => co,
};

Of course, there are still other ergonomic issues with uninject as evidenced by the Err(co) => co, but I feel that this is more a problem with the language itself.

Future motivation

Once I can provide sufficient evidence that the feature is useful, I'd like to add reified indices to frunk, so that you can do things like this:

let index = list_a.index_of::<T, >();
let value = list_b.at(index)

Imagine trying to do that without turbofish support! :fearful:

Less need to import traits

Having to use a glob import to use basic functionality of a type is, in my opinion, a design smell. use ::frunk::hlist::{HCons, NHil}; is all that a user should need to get 99% of the functionality of HLists.

In my ideal design, the only time it is necessary to import these traits should be if the user needs to write where bounds for a function that is generic over HLists. (actually, even then it is still not necessary, as they can use frunk::hlist::{self, HCons, HNil} and put where T: hlist::Sculptor<S, I> on the generic function).

Bonus 1: The import tree can be made shallower; since HCons and HNil are for the most part all you need, frunk can reexport those things at the root, allowing people to write use frunk::{HCons, HNil}.

Bonus 2: If we are willing to take the overhaul even further, we can get rid of prefixes like "Coprod" from the traits (e.g. Injector instead of CoprodInjector) now that we don't need to worry about name clashes. (I am assuming that is the reason why those prefixes were there...)

Wrapping "constructor" traits with free functions

CoprodInjector currently has this usability wart that you need to name the type of the coproduct in order to inject into it. This is a pain to deal with in cases where the type could otherwise be inferred. I think it would be ideal if this were exposed as a free function:

Before:

use frunk::*;

// even though only one thing uses this type,
// we still need to make a type alias so that we can inject into it later
type Gnarly = Coprod!(Oh, Dear, Thats, Something);

fn takes_gnarly(co: Gnarly) -> Something {
    ...
}

takes_gnarly(Gnarly::inject(dear))

After:

use frunk::coproduct;

fn takes_gnarly(co: Coprod!(Oh, Dear, Thats, Something)) -> Something {
    ...
}

// Now we can let type inference do its job, hurrah!
takes_gnarly(coproduct::inject(dear))
Centril commented 6 years ago

I'm favor of these additions in general; supporting turbofish, reified indices and fewer imports seem like good ideas. 👍 But I'd like us to not remove things as the traits often work with things as tuples and types that are not in frunk.

ExpHP commented 6 years ago

I'm not proposing removing the traits, either; merely shifting them out of the spotlight. Almost all of them still need to exist in at least some form, anyways, as most of the methods cannot be implemented without trait-based polymorphism (so public traits are going to need to appear in trait bounds).

Centril commented 6 years ago

SGTM 👍

lloydmeta commented 6 years ago

I just have strong opinions on this topic

Awesome, and definitely welcome :)

I'm also + 💯 on all of these proposals to make frunk more ergonomic to use. From my side, the reason why these didn't exist is simple: I simply did not know/think about these techniques.

ExpHP commented 6 years ago

I ran into a problem:

list.as_ref().map(|x| x.clone())

Today, this works by having as_ref produce an &HCons, and having impl HMappable for &HCons. However, it will be impossible to define map as an inherent method on both HCons and &HCons receivers. Unfortunately this will be a huge leak in the abstraction IMO if map can always be called on plain HLists, but requires a trait to be called on borrows.

I was going to suggest possibly changing the output type of as_ref to something else (like Hlist!(&A, &B, &C, ...) or pub struct HRef<'a, H, Tail>(pub &'a HCons<H, Tail>);). However, such an idea would be a complete nonstarter here, because that as_ref method actually comes from the AsRef trait. Overriding it and changing the type would not only be a terrible idea in every way imaginable; but it would also destroy something beautiful.


Hm, here's perhaps a better idea: Simply don't add inherent methods for mapand fold, and instead just put these traits into a prelude.

There are still plenty of other methods (get, pluck, inject, etc.) that will be able to reap the benefits of becoming inherent methods, and there's also a fairly clear and intuitive line drawn for when traits need to be imported (basically whenever you need to use functions).