Think about how to handle Self bounds on methods

[LukasKalbertodt]

I encountered a problematic situation where a trait method has a Self bound.

trait Foo {
    fn one(&self);
    
    fn two(&self)
    where
        Self: Clone;
}

// Generated impl by `#[auto_impl(&)]`
impl<'a, T: 'a + Foo> Foo for &'a T {
    fn one(&self) {
        (**self).one()
    }
    
    fn two(&self)
    where
        Self: Clone
    {
        (**self).two()
    }
}

Which results in:

error[E0277]: the trait bound `T: std::clone::Clone` is not satisfied
  --> src/main.rs:18:18
   |
18 |         (**self).two()
   |                  ^^^ the trait `std::clone::Clone` is not implemented for `T`
   |
   = help: consider adding a `where T: std::clone::Clone` bound

Ok, that makes sense, but what should we do about it? We cannot add the bound T: Clone to the method as this will result in "impl has stricter requirements than trait". We can add a T: Clone bound to the impl block, but this restricts the impl a lot. The method one() should be callable on &T where T: Foo, even if T doesn't implement Clone.

However, I cannot think of any real solution to this (not even related to this crate, but I simply can't think of a way how to impl Foo for references in a general way). So I guess we just need to add the bound to the impl. If the user has a better solution, they have to write the impl for &T themselves, as this requires logic that we cannot derive from the trait definition.

[KodrAus]

I guess the issue is how the macro attribute can scale to accommodate other kinds of bounds on the impl target, like adding the ?Sized bound in impl<'a, T: ?Sized + SomeTrait> SomeTrait for &'a T. The #[auto_impl(&, &mut, Box)] approach doesn't get us very far.

For the ?Sized specifically I think we could possibly just always add a ?Sized bound for references, since we only support methods that take references to self, but this Clone case is one where it would be nice if we could specify that bound somehow.

One option to widen support could be to just accept more of the impl in the #[auto_impl] attribute, like:

#[auto_impl(<'a, T: ?Sized + Clone> &'a T)]
#[auto_impl(<T> Box<T>)]
trait SomeTrait { ... }

But then we get into the territory of figuring out what kind of impl we're looking at and whether we need to add additional bounds to T or additional generics.

It seems like a shame to be limiting in the face of what are pretty common needs, it would be nice if we could find a syntax for #[auto_impl] that supports auto impl blocks that is neither too limiting nor inconsistently wide.

[LukasKalbertodt]

I think we could possibly just always add a ?Sized bound for references

Good point, I guess that would work.

Regarding the main problem: I think I'm against giving the user too many configurations. I think this crate is a crate for 95% of all cases. I don't think it's a problem when the remaining 5% need to be coded by hand. And in particular, including something like <'a, T: Clone> in the auto_impl attribute looks too noisy to me. I like the current approach of "minimal typing".

Also: the Clone bound is simply necessary in the case above.There is no general way how to implement the trait for references to T for non Clone Ts. The only way to omit the T: Clone bound is by specifying a custom method body (different from (**self).two()). Letting the user specify the body is too much configuration again IMO.

Right now, I think that I would always just add all Self bounds from methods to T. I actually think that this is less of a problem than I initially thought. I can't think of reasons to apply a Self bound on a trait method that is not already provided. And for provided methods, this problem should be solved by #10.

[KodrAus]

Regarding the main problem: I think I'm against giving the user too many configurations.

Yeh I agree with this. Otherwise we'd end up in configuration soup very quickly and it'd be a nightmare to maintain and use.

I'll open up a separate issue for the auto ?Sized bound on references. I think it should definitely be possible for Rc and Arc too.

[LukasKalbertodt]

Fixed in #54