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

Coproducts and generics + trait bounds? #133

Open jeff-hiner opened 6 years ago

jeff-hiner commented 6 years ago

I'm working with some code that filters specific coproducts out of an mpsc and running into problems. (Originally this was a huge enum of disjoint structs and I'm trying to rewrite it using frunk because I'm tired of not having anonymous enum variants.) I've distilled this down to an example that I feel should compile, but Rust is obviously unhappy that T could be a type that doesn't belong to the Coproduct and I'm getting trait bound errors. Is there a trait bound I can add to T to make this work, or should this be solved by CNil implementing the trait and always returning None?

extern crate frunk_core;

pub type SimpleCoprod = Coprod!(
    i32,
    String,
);

/// Take and return Option
pub fn foo<T>(f: SimpleCoprod) -> Option<T> {
    f.take()
}
error[E0277]: the trait bound `frunk_core::coproduct::CNil: frunk_core::coproduct::CoproductTaker<T, _>` is not satisfied
  --> src/noworky.rs:10:7
   |
10 |     f.take()
   |       ^^^^ the trait `frunk_core::coproduct::CoproductTaker<T, _>` is not implemented for `frunk_core::coproduct::CNil`
   |
   = note: required because of the requirements on the impl of `frunk_core::coproduct::CoproductTaker<T, frunk_core::indices::There<_>>` for `frunk_core::coproduct::Coproduct<std::string::String, frunk_core::coproduct::CNil>`
   = note: required because of the requirements on the impl of `frunk_core::coproduct::CoproductTaker<T, frunk_core::indices::There<frunk_core::indices::There<_>>>` for `frunk_core::coproduct::Coproduct<i32, frunk_core::coproduct::Coproduct<std::string::String, frunk_core::coproduct::CNil>>`
ExpHP commented 6 years ago

Assuming that T is always supposed to be a member of SimpleCoprod:

#[macro_use]
extern crate frunk_core;

use frunk_core::coproduct::CoproductTaker;

pub type SimpleCoprod = Coprod!(
    i32,
    String,
);

pub fn foo<T, Index>(f: SimpleCoprod) -> Option<T>
where SimpleCoprod: CoproductTaker<T, Index>,
{
    f.take()
}

If that is not the case, unfortunately, it is impossible to write a function that does one thing if T is a member of the Coproduct and something else if it is not. This is because the recursive impl (the one on Coproduct<U, Rest> that checks if Rest impls the trait) would always succeed and overlap with the success base case impl for Coproduct<T, Rest>.

In fact, these impls already "overlap" due to the possibility that T = U, but frunk is able to work around it in that case by adding a dummy Index type parameter and exploiting type inference. (If you ever tried to take something out of a Coproduct with two copies of T, i.e. a case of legitimate overlap, then you'll see where this workaround breaks down.) Frunk cannot apply such a workaround here, because all coproducts end in CNil, and therefore it will overlap with every other impl.

jeff-hiner commented 6 years ago

Ahh, gotcha. I misunderstood how the CNil base case was supposed to work. Thanks for your help!

I'm not sure how many folks are using generics in tandem with frunk like this, but it might be helpful to port the above snippet into the rustdoc for Coproduct or CoproductTaker, with a mention that the trait bounds may need to be propagated upwards to anyone extracting the generated newtype and calling foo. This example is a bit contrived, but is something often encountered in Future implementations (notably the trait bounds are necessary on both):

extern crate frunk;

use frunk::coproduct::CoproductTaker;

type SimpleCoprod = Coprod!(i32, String,);

/// Take and return Option
fn foo<T, Index>(f: SimpleCoprod) -> Option<T>
where
    SimpleCoprod: CoproductTaker<T, Index>,
{
    f.take()
}

enum StateMachine {
    Pending,
    Success(SimpleCoprod),
    Error(String)
}

fn bar<T, Index>(w: StateMachine) -> Option<T> where
    SimpleCoprod: CoproductTaker<T, Index>,
{
    if let StateMachine::Success(coprod) = w {
        foo(coprod)
    } else {
        None
    }
}
lloydmeta commented 6 years ago

it might be helpful to port the above snippet into the rustdoc for Coproduct or CoproductTaker, with a mention that the trait bounds may need to be propagated upwards to anyone extracting the generated newtype and calling foo.

FWIW, I'm always +1 on more docs :)

EDIT: Just to avoid ambiguity, are you talking about @ExpHP 's example in https://github.com/lloydmeta/frunk/issues/133#issuecomment-436125462 or your example in https://github.com/lloydmeta/frunk/issues/133#issuecomment-436315936 ? Either one is fine by me; and needless to say, you're more than welcome to take a stab at adding said rustdocs 😄

jeff-hiner commented 6 years ago

Oh I was just thinking @ExpHP 's example with a note, just so if people see really gross template errors they have another example to try out. I'll see if I can get a PR into here in the next day or two to add it.

ExpHP commented 6 years ago

This feels to me more like a problem of "how to use Rust," though. (I mean, you always have to propagate bounds up in generic functions, and the docs show the bounds you need)

Or perhaps the confusion is with something more subtle? (maybe the output shown by rustdoc is missing something important or draws focus to the wrong things? frunk does some heavy reexporting and this sometimes reveals bugs in rustdoc) Can you point to the pages that you were looking at when you were having trouble?

jeff-hiner commented 6 years ago

I was using a combination of the frunk docs for coproduct and CoproductTaker as well as the normal search for "the trait bound is not satisfied" and "trait bounds rust" that turned up references back into the Book and reminders from StackOverflow that traits need to be in scope to be usable. The latter was a red herring in this case, as the trait was in scope already.

I think what kicked me is that normally rustc will hold your hand a bit, suggesting the specific trait bound you need to add and where to add it. I got about 3-4 pages (200+ lines) worth of "used in... used in..." template soup, and a lot of Here<_> and There<_> and CNil mojo, but no trait bound hint. Honestly with newtypes or macros that expand to several lines of template magic it probably would have presented the full macro expansion as the thing to paste instead of the newtype, which obviously isn't the right thing to use. At that point I knew I had to put some kind of trait bound in there, but with flashbacks of C++ template errors I wasn't able to extract the generic name/format from the template soup. It wasn't clear what traits the Coprod! macro implemented, and cargo doc for my crate didn't show something like "impl CoproductTaker<T, Index>" for my newtype-- not that I'd expect rustdoc to be able to extract that anyway. The closest thing in the rustdoc is impl<Head, Tail> CoproductTaker<Head, Here> for Coproduct<Head, Tail> which also isn't directly useful.

I just think a single example of the correct trait bound syntax in both CoproductTaker and CoproductSelector would save a lot of trouble. The metaprogramming going on here is awesome, but it's Serious Black Magic to most folks using the library. A naive user isn't going to realize the correct bound is CoproductTaker<T, Index> unless it's specifically called out in the rustdoc.

ExpHP commented 6 years ago

Ahh, I see, it's harder to figure out the right bound from the page of the trait now that rustdoc hides the declaration by default.

(N.B. it does currently appear somewhere, though not on those pages; the place where I pulled the right bound from is from the docs of the inherent method, which has precisely the required signature.)

lloydmeta commented 6 years ago

The metaprogramming going on here is awesome, but it's Serious Black Magic to most folks using the library

😢 yeah, this is not good; I'm sorry to hear that this was your experience. I could be wrong, but it feels more in line with the Rust community norm to err on the side of providing too many docs.

Sorry for the noise but I just want to give another +1 to adding @ExpHP 's example (or something like it) to a more visible/obvious place in the Rust docs, wherever that may be for the end-developer/user.