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

Generic type bounds are not handled #38

Closed
MrJohz opened this issue Oct 14, 2020 · 3 comments · Fixed by #41
Closed

Generic type bounds are not handled #38

MrJohz opened this issue Oct 14, 2020 · 3 comments · Fixed by #41

Comments

@MrJohz
Copy link
Contributor

MrJohz commented Oct 14, 2020

I have a use-case where I would like to wrap the content for individual pages with a generic "page + metadata" content struct (e.g. to provide i18n, script information, etc).

#[derive(ramhorns::Content)]
struct SamplePage {
    username: String,
    age: u64,
}

#[derive(ramhorns::Content)]
struct PageWithMetadata<T: ramhorns::Content> {
    #[ramhorns(flatten)]
    page: T,
    metadata: (),
}

However, this fails with a series of fairly-indecipherable errors:

error[E0658]: associated type bounds are unstable
 --> src/main.rs:8:25
  |
8 | struct PageWithMetadata<T: ramhorns::Content> {
  |                         ^^^^^^^^^^^^^^^^^^^^
  |
  = note: see issue #52662 <https://github.com/rust-lang/rust/issues/52662> for more information

error[E0107]: wrong number of type arguments: expected 1, found 0
 --> src/main.rs:8:8
  |
8 | struct PageWithMetadata<T: ramhorns::Content> {
  |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected 1 type argument

error[E0229]: associated type bindings are not allowed here
 --> src/main.rs:8:25
  |
8 | struct PageWithMetadata<T: ramhorns::Content> {
  |                         ^^^^^^^^^^^^^^^^^^^^ associated type not allowed here

Reading briefly through the source, it seems like the generic part is not being parsed or broken down further, and simply being quoted in when necessary, which works for simple type parameters (e.g. <T>) but not when there are bounds on those types.

I also tried this by using a where T: ramhorns::Content block:

error[E0277]: the trait bound `T: ramhorns::Content` is not satisfied
 --> src/main.rs:7:10
  |
7 | #[derive(ramhorns::Content)]
  |          ^^^^^^^^^^^^^^^^^ the trait `ramhorns::Content` is not implemented for `T`
8 | struct PageWithMetadata<T> where T: ramhorns::Content {
  |                                     ----------------- required by this bound in `PageWithMetadata`
  |
  = note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info)
help: consider restricting type parameter `T`
  |
8 | struct PageWithMetadata<T: ramhorns::Content> where T: ramhorns::Content {
  |                          ^^^^^^^^^^^^^^^^^^^

Is this supported at all/is there an obvious alternative way to achieve this? My other way of solving this would be by putting the metadata inside the SamplePage struct, but this involves more repetition and messy APIs, so I was hoping to avoid this.

Thanks!

@maciejhirsz
Copy link
Owner

This is something the derive macro doesn't handle yet. The correct use, I believe, would be:

#[derive(ramhorns::Content)]
struct SamplePage {
    username: String,
    age: u64,
}

#[derive(ramhorns::Content)]
struct PageWithMetadata<T> { // no trait bounds here
    #[ramhorns(flatten)]
    page: T,
    metadata: (),
}

The derive macro would then have to conditionally impl Content for PageWithMetadata<T> where T: Content. This is not particularly hard to do, it just needs to keep track of generic types and make sure the bounds are applied to them in the produced code.

@MrJohz
Copy link
Contributor Author

MrJohz commented Oct 16, 2020

That makes sense. I can try and put together a pull request for that, if that would be helpful?

@maciejhirsz
Copy link
Owner

That makes sense. I can try and put together a pull request for that, if that would be helpful?

I'm more than happy to accept a PR for this :). The generics are currently only copied dumbly into the output stream currently. This works with lifetimes, but obviously breaks here, so that's the part needs to be more robust. Field rendering should work fine without changes I believe.

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

Successfully merging a pull request may close this issue.

2 participants