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

Change the encoding of supertrait clauses to support associated types #91

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lukenels
Copy link

@lukenels lukenels commented May 3, 2023

This pull request is related to the discussion in #87.

When auto_impl is used to implement trait that has a supertrait, it needs to enforce that the inner type already implements the supertrait. However, the current encoding does not work correctly when the supertrait has an associated type that the subtrait refers to in one of its methods.

To make this concrete, take the following example.

use auto_impl::auto_impl;

pub trait Foo {
    type MyType;
}

pub trait Bar: Foo {
    fn bar(&self, value: Self::MyType);
}

auto_impl will expand this code into the following, which fails to compile since the compiler doesn't know that Arc<T>::Foo is the same type as T::Foo.

pub trait Foo {
    type MyType;
}

pub trait Bar: Foo {
    fn bar(&self, value: Self::MyType);
}

const _: () = {
    extern crate alloc;
    impl<T: Bar + ?Sized> Bar for alloc::sync::Arc<T>
    where
        alloc::sync::Arc<T>: Foo,
    {
        fn bar(&self, value: Self::MyType) {
            T::bar(self, value)
        }
    }
};

This commit changes how auto_impl encodes supertrait where constraints like alloc::sync::Arc<T>: Foo to refer to the inner type parameter instead, e.g., T: Foo. With this change, if we also annotate trait Foo with auto_impl, then the associated type example will compile, because the compiler has access to the implementation of Foo for Arc<T> when T: Foo:

use auto_impl::auto_impl;

pub trait Foo {
    type MyType;
}

pub trait Bar: Foo {
    fn bar(&self, value: Self::MyType);
}

This change is not backwards compatible in certain edge scenarios. For example, the new encoding does not permit using auto_impl to implement Bar for Arc<T> but having a user-defined / custom implementation of Arc<T> for its supertrait Foo.

It also causes auto_impl on Bar to fail to compile eagerly when there is no corresponding implementation of Foo, whereas the previous behavior would allow this to compile (but would produce a constraint that fails to be satisfied if the implementation of Bar is ever attempted to be used).

These seem like niche scenarios, but this does constitute a functional (though minor) change. However, this this trade-off may be worth since it's otherwise not possible to use associated types from supertraits with auto_impl.

This commit also updates the unit tests accordingly; for example, adding auto_impl on supertraits where it was missing before in order to fix the new build errors.

When `auto_impl` is used to implement trait that has a supertrait, it needs to
enforce that the inner type already implements the supertrait. However,
the current encoding does not work correctly when the supertrait has an
associated type that the subtrait refers to in one of its methods.

To make this concrete, take the following example.

```rust
use auto_impl::auto_impl;

pub trait Foo {
    type MyType;
}

pub trait Bar: Foo {
    fn bar(&self, value: Self::MyType);
}
```

`auto_impl` will expand this code into the following, which fails to
compile since the compiler doesn't know that `Arc<T>::Foo` is the same
type as `T::Foo`.

```rust
pub trait Foo {
    type MyType;
}

pub trait Bar: Foo {
    fn bar(&self, value: Self::MyType);
}

const _: () = {
    extern crate alloc;
    impl<T: Bar + ?Sized> Bar for alloc::sync::Arc<T>
    where
        alloc::sync::Arc<T>: Foo,
    {
        fn bar(&self, value: Self::MyType) {
            T::bar(self, value)
        }
    }
};
```

This commit changes how `auto_impl` encodes supertrait `where`
constraints like `alloc::sync::Arc<T>: Foo` to refer to the inner type
parameter instead, e.g., `T: Foo`. With this change, if we also annotate
trait `Foo` with `auto_impl`, then the associated type example will
compile, because the compiler has access to the implementation of `Foo`
for `Arc<T>` when `T: Foo`:

```rust
use auto_impl::auto_impl;

pub trait Foo {
    type MyType;
}

pub trait Bar: Foo {
    fn bar(&self, value: Self::MyType);
}
```

This change is _not_ backwards compatible in certain edge scenarios.
For example, the new encoding does not permit using `auto_impl` to
implement `Bar` for `Arc<T>` but having a user-defined / custom
implementation of `Arc<T>` for its supertrait `Foo`.

It also causes `auto_impl` on `Bar` to fail to compile _eagerly_
when there is no corresponding implementation of `Foo`, whereas the
previous behavior would allow this to compile (but would produce a
constraint that fails to be satisfied if the implementation of `Bar`
is ever attempted to be used).

These seem like niche scenarios, but this does constitute a functional
(though minor) change. However, this this trade-off may be worth
since it's otherwise not possible to use associated types from
supertraits with `auto_impl`.

This commit also updates the unit tests accordingly; for example,
adding `auto_impl` on supertraits where it was missing before in
order to fix the new build errors.
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 this pull request may close these issues.

None yet

1 participant