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

Support custom bounds for Display-like traits #93

Closed
tyranron opened this issue Oct 14, 2019 · 11 comments
Closed

Support custom bounds for Display-like traits #93

tyranron opened this issue Oct 14, 2019 · 11 comments

Comments

@tyranron
Copy link
Collaborator

tyranron commented Oct 14, 2019

Synopsis

Current #[derive(Display)] is smart enough with its default behavior. However, due to absence of compile-time reflection, it's impossible to handle all the cases really gracefully.

Consider the following:

#[derive(Display)]
enum Foo<A, B> {
    A(A),
    B(B),
}

#[derive(Display)]
enum Bar<A, B, C> {
    Foo(Foo<A, B>),
    C(C),
}

The Bar won't compile with an error:

`A` cannot be formatted with the default formatter`

This is because we're unable to infer Display requirements for Foo<A, B> in Foo() variant knowing only tokens.

Fortunately, such situations are tend to be resolved with custom bound attributes (see #[serde(bound = "T: Serialize")], for example) in Rust ecosystem.

Proposal

Let's extend #[derive(Display)] with #[display(bound)] attribute, which may be used in the following manner:

#[derive(Display)]
enum Foo<A, B> {
    A(A),
    B(B),
}

#[derive(Display)]
#[display(bound = "A, B")]
enum Bar<A, B, C> {
    Foo(Foo<A, B>),
    C(C),
}

// Expands to:
impl<A, B, C> fmt::Display for Bar<A, B, C>
where A: Display,
      B: Display,
      C: Display,
{ ... }

If bound is not specified (no semicolon), we apply the bound of the derived trait.

#[derive(Display)]
enum Foo<A, B> {
    A(A),
    #[display(fmt = "{0:?} + {0:o}", _0)]
    B(B),
}

#[derive(Display)]
#[display(bound = "A, B: Debug + Octal")]
enum Bar<A, B, C> {
    Foo(Foo<A, B>),
    C(C),
}

// Expands to:
impl<A, B, C> fmt::Display for Bar<A, B, C>
where A: Display,
      B: Debug + Octal,
      C: Display,
{ ... }
@JelteF
Copy link
Owner

JelteF commented Oct 14, 2019

I believe I understand the problem, but I think a much simpler and more elegant solution would be to automatically derive:

impl<A, B, C> fmt::Display for Bar<A, B, C>
where Foo<A, B>: Display,
      C: Display,
{ ... }

It could be that I'm misunderstanding though.

@JelteF
Copy link
Owner

JelteF commented Oct 14, 2019

To confirm my solution works as well I just tried the following code and it compiles (without the where it doesn't):

#[derive(Display)]
struct Foo<A>(A);

struct Bar<A>(Foo<A>);
impl<A> ::core::fmt::Display for Bar<A>
where
    Foo<A>: ::core::fmt::Display,
{
    #[allow(unused_variables)]
    #[inline]
    fn fmt(&self, f: &mut ::core::fmt::Formatter) -> ::core::fmt::Result {
        write!(f, "{}", self.0)
    }
}

@tyranron
Copy link
Collaborator Author

@JelteF

WAT

Didn't even know that is possible 😵

@tyranron
Copy link
Collaborator Author

tyranron commented Oct 15, 2019

@JelteF however, still specifying bounds explicitly may be very desirable in quite exotic cases like the following:

#[derive(Display)]
#[display(fmt = "Foo's tree: {}", "_0.generate_tree_view()")]
#[display(bound = "A: TreeViewGenerator")]
struct Foo<A>(A);

These cases are too rare, however, having the ability to manually specify the bounds for the implementation is vital, as for me.

So, the plan is to land (one step - one PR):

  1. Improved default bounds for Display-like traits when generics are involved (Foo<A, B>: Display).
  2. #[display(bound)] attribute for resolving any other cases.

@JelteF
Copy link
Owner

JelteF commented Oct 15, 2019

Sounds perfect to me!

@tyranron
Copy link
Collaborator Author

tyranron commented Oct 21, 2019

From #95 (comment):

I also think, that we can rework the code further, so that for fields/enumerators of type &T a trait bound T: Display is added.

@ffuugoo would you be so kind to explain this point further with primitive examples which illustrate the problem?

@ffuugoo
Copy link
Contributor

ffuugoo commented Oct 21, 2019

Given:

#[derive(Display)]
struct Foo<'a, T> {
    field: &'a T,
}

Result:

error[E0277]: `T` doesn't implement `std::fmt::Display`
   |
   |     field: &'a T,
   |     ^^^^^ `T` cannot be formatted with the default formatter
   |
   = help: the trait `std::fmt::Display` is not implemented for `T`
   = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead
   = help: consider adding a `where T: std::fmt::Display` bound
   = note: required because of the requirements on the impl of `std::fmt::Display` for `&T`
   = note: required by `std::fmt::Display::fmt`

@ffuugoo
Copy link
Contributor

ffuugoo commented Oct 21, 2019

I just realized, that this case is, actually, no different from generic SomeType<T> case. I.e., we can add &T: Display bound and it should just work.

@tyranron
Copy link
Collaborator Author

tyranron commented Oct 21, 2019

@ffuugoo I guess cases like &'a T and &'a mut T we can treat like special. And apply bound T: Display directly. I think it won't break anything.

@tyranron
Copy link
Collaborator Author

@ffuugoo even more, I think we should take this pattern as common:

#[derive(Display)]
struct Foo<'a, T> {
    field: &'a Bar<T>,
}

// expands to:
impl<'a, T> Display for Foo<'a, T>
where Bar<T>: Display
{ }

@ffuugoo
Copy link
Contributor

ffuugoo commented Oct 22, 2019

@ffuugoo even more, I think we should take this pattern as common...

Done in fe1dccc.

JelteF pushed a commit that referenced this issue Oct 26, 2019
First half of #93.

Modified `Display` macro, so that for each field of the following types _or of reference to 
such types_ a where clause is generated bounding field's type by `Display` trait:

* `T`
* `T::AssociatedType` and `<T as Trait>::AssociatedType`
* `::path::SomeOtherType<T>` and `::path::SomeOtherType<&T>`

E.g., for a structure `Foo` defined like this:

```Rust
#[derive(Display)]
struct Foo<'a, T1, T2: Trait, T3> {
    field1: T1,
    field2: <T2 as Trait>::Type,
    field3: Bar<T3>,
    field4: &'a T1,
    field5: &'a <T2 as Trait>::Type,
    field6: &'a Bar<T3>,
}
```

Following where clauses would be generated (notice, that for reference types the 
_referred_ type is bound):

* `T1: Display`
* `<T2 as Trait>::Type: Display`
* `Bar<T3>: Display`
JelteF pushed a commit that referenced this issue Nov 2, 2019
Second half of #93. This PR is based on top of #95.

Implemented [display(bound = "...")] attribute for Display macro as proposed by @tyranron in #93.
@tyranron tyranron closed this as completed Nov 2, 2019
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