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

Coproducts and generics + trait bounds? #133

Open
jeff-hiner opened this issue Nov 6, 2018 · 8 comments
Open

Coproducts and generics + trait bounds? #133

jeff-hiner opened this issue Nov 6, 2018 · 8 comments

Comments

@jeff-hiner
Copy link

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
Copy link
Collaborator

ExpHP commented Nov 6, 2018

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
Copy link
Author

jeff-hiner commented Nov 6, 2018

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
Copy link
Owner

lloydmeta commented Nov 7, 2018

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 #133 (comment) or your example in #133 (comment) ? 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
Copy link
Author

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
Copy link
Collaborator

ExpHP commented Nov 7, 2018

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
Copy link
Author

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
Copy link
Collaborator

ExpHP commented Nov 8, 2018

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
Copy link
Owner

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants