auto-impl-rs / auto_impl Goto Github PK

I was trying to use auto_impl to delegate the implementation of Box<T> where T: Trait and Trait was a trait annotated with the async-trait procedural macro. I was using it like this:

#[async_trait]
#[auto_impl(Box)]
trait Foo: Sync {
    async fn bar(&self) -> Result<u32, Error>;
}

The ordering of the above macros matters, as it ensures that async_trait transforms Foo into the following Rust code before auto_impl parses it:

#[auto_impl(Box)]
trait Foo: Sync {
    fn bar<'life0, 'async_trait>(
        &'life0 self
    ) -> Pin<Box<dyn Future<Output = Result<u32, Error>> + Send + 'async_trait>> where
        'life0: 'async_trait,
        Self: 'async_trait;
}

Provided that the Sync bound shown above is present as a supertrait of Foo, the above code should be object safe. I expected #[auto_impl(Box)] to emit the following implementation:

impl<T: Foo + ?Sized> Foo for Box<T>
where
    Box<T>: Sync

However, it emitted the following implementation instead:

impl<T: Foo> Foo for Box<T>
where
    Box<T>: Sync

This meant that the following code was able to compile with a manual Box implementation but failed under the auto_impl-generated implementation:

fn assert_impl<T: Foo>() {}

fn foo() {
    assert_impl::<Box<dyn Foo>>(); // ERROR: Not object safe with `auto_impl`
}

Do you have an idea as to why the ?Sized bound was not applied in this case?

See ebkalderon/tower-lsp#109 for the original pull request that inspired this issue.

Just a reminder: once lifetime elision in impl headers (see tracking issue) is stable, the generated code should use that feature. So instead of:

impl<'a, T> Foo for &'a T {}

It will be (at least I think that's the syntax):

impl<T> Foo for &T {}

Same for &mut.

This does matter as the generated code ends up in the docs. And we want that code to be as idiomatic as possible.

If a trait has a provided method (body already provided), we can either include the method in the generated impl blocks or we can omit them. I don't think there is a clear "right thing to do" for every case, so we should let the user decide. Maybe something like that:

#[auto_impl(&, &mut, Box)]
trait Foo {
    fn required(&self);

    #[auto_impl(include for: &, Box)]
    fn provided(&self) {
        // ...
    }
}

The attribute #[auto_impl()] should lead to a warning on nightly. Sadly, we cannot emit warnings on stable (since the proc_macro_diagnostic API is still unstable).

I can mentor anyone interested in tackling this issue :)
Just ping me (via email, this issue, or in any other way)

Instructions: Two things need to be done:

• First, we need to extend a few things in diag.rs. There, we have a few traits that abstract over nightly/stable diagnostic. Since, as said above, proc_macro_diagnostic is still unstable, we report errors on stable via a hack. Thus you'll find a few #[cfg(feature = "nightly")] attributes there. We need to add a warn() method to SpanExt first. With the nightly feature, we should simply return a proc_macro::Diagnostic. On stable... good question. We probably want to modify our own Diagnostic type so that it can also "be a warning". But if it's a warning, the emit() will just don't do anything.

• With that out of the way, we can finally tackle the core of this issue: emit the warning. I think the easiest way to do that is to add some code to parse_types() in proxy.rs.

If anything is unclear, just go ahead and ask!

It looks like there may be some ?Sized bounds missing? I would expect the following auto impls to apply to <T: ?Sized> &T and <T: ?Sized> Box<T> but they don't.

use auto_impl::auto_impl;

#[auto_impl(&, Box)]
trait Trait {}

fn assert_impl<T: Trait>() {}

fn main() {
    assert_impl::<&dyn Trait>();
    assert_impl::<Box<dyn Trait>>();
}

error[E0277]: the size for values of type `dyn Trait` cannot be known at compilation time
 --> src/main.rs:9:5
  |
9 |     assert_impl::<&dyn Trait>();
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time
  |
  = help: the trait `std::marker::Sized` is not implemented for `dyn Trait`
  = note: to learn more, visit <https://doc.rust-lang.org/book/ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait>
  = note: required because of the requirements on the impl of `Trait` for `&dyn Trait`
note: required by `assert_impl`
 --> src/main.rs:6:1
  |
6 | fn assert_impl<T: Trait>() {}
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0277]: the size for values of type `dyn Trait` cannot be known at compilation time
  --> src/main.rs:10:5
   |
10 |     assert_impl::<Box<dyn Trait>>();
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time
   |
   = help: the trait `std::marker::Sized` is not implemented for `dyn Trait`
   = note: to learn more, visit <https://doc.rust-lang.org/book/ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait>
   = note: required because of the requirements on the impl of `Trait` for `std::boxed::Box<dyn Trait>`
note: required by `assert_impl`
  --> src/main.rs:6:1
   |
6  | fn assert_impl<T: Trait>() {}
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^

I'm having some trouble getting auto_impl to work when a trait defines a method that takes an argument of some associated type, where the associated type is defined in a supertrait:

 use auto_impl::auto_impl;

pub trait Foo {
    type MyType;
}

#[auto_impl(Arc)]
pub trait Bar: Foo {
    fn bar(&self, value: Self::MyType);
}

When I try to compile this, I get the following error:

error[E0308]: mismatched types
  --> src/main.rs:11:19
   |
9  | #[auto_impl(Arc)]
   | -----------------
   | |
   | this type parameter
   | arguments to this function are incorrect
10 | pub trait Bar: Foo {
11 |     fn bar(&self, value: Self::MyType);
   |                   ^^^^^ expected type parameter `T`, found struct `Arc`
   |
   = note: expected associated type `<T as Foo>::MyType`
              found associated type `<Arc<T> as Foo>::MyType`
note: associated function defined here
  --> src/main.rs:11:8
   |
11 |     fn bar(&self, value: Self::MyType);
   |        ^^^

For more information about this error, try `rustc --explain E0308`.
error: could not compile `autoimpl-demo` due to previous error

I've tried a few different things (e.g., using <Self as Foo>::MyType instead, putting auto_impl on both traits, etc.) but can't seem to get it to work. Any tips / workarounds? Thanks!

> rustc --version
rustc 1.64.0 (a55dd71d5 2022-09-19)

auto_impl should support trait methods of the form

fn foo(&self, mut bar: Bar) {
    // default implementation that mutates bar
}

The use-case I run into most often is boilerplate for:

trait MyTrait {
...
}

impl MyTrait for Box<dyn MyTrait> {
// shim methods
}

Would it be possible to extend auto_impl to this case?

It would also need to support generating impls with autotraits/lifetimes:

impl MyTrait for Box<dyn MyTrait + Sync /* and/or 'static, Send, etc */> {
...

The current testing situation is not terrible, but it can certainly be improved:

• More compile-pass tests
• Make compile-tests more granular
• Add compile-fail tests
  • Compare actual error message (or code) with expected one (see this comment for some thoughts on this)
• Make compile-tests and compile-fail tests run with cargo test in the root directory
• More unit tests
  • Partially blocked by the fact that most functions from proc-macro panic when not inside a real macro invocation (see my comment here)

So we need our own type parameter (and lifetime parameter in case of & and &mut). Right now they have the beautiful names __AutoImplProxyT and __auto_impl_proxy_lifetime. This is done because the part of proc macro API that has now been stabilized doesn't include def site hygiene but only call site hygiene. Meaning: the names we use are in the same "namespace" as all names of the user. And to avoid having naming conflicts we use these terrible names.

This is far from optimal. For example, these names pop up in the documentation and error messages:

... this ain't pretty.

My idea to solve this problem is to do a bit more work in auto_impl and choose a name that is not already taken. Luckily, we can inspect all other parameters that will be in scope and choose a name different from all of them. As far as I can tell, that's the best solution for good docs and errors. In that case we need to decide:

1. If T/'a is already taken, how to choose the name? U and 'b? And then? Or fallback to our terrible name from above?
2. Should we do something different if compiled on nightly, like using def site hygiene and always use T and 'a?
3. Should we simply use the new anonymous/in-band lifetimes (impl<T> Foo for &'_ T) to get rid of our lifetime parameter?

My (not strong) opinion right now:

1. Use U and 'b and keep using the remaining alphabet. If we exhausted the alphabet, we can fallback to the terrible names. The latter will probably never happen.
2. If we have a solution for stable, I think we don't need to use def site hygiene on nightly. impl<T, T> will look strange in the documentation, even if the compiler knows that the two Ts are different.
3. I think I'm in favor of using the anonymous lifetime, because proc-macros and in band lifetimes will be stabilized at roughly the same time, so we don't really lose "users" by requiring a new compiler version.

I just noticed that I never thought about how this crate works with super traits. I guess that this:

trait Supi {}

#[auto_impl(Box, &)]
trait Foo: Supi {}

Should generate these impl blocks:

impl<T: Foo> Foo for Box<T>
where
    Box<T>: Supi,
{}

impl<'a, T: Foo> Foo for &'a T
where
    &'a T: Supi,
{}

Right? Or am I missing something? That logic should work for all proxy types, even Fn.

I can mentor anyone interested in tackling this issue :)
Just ping me (via email, this issue, or in any other way)

Instructions: the important code for this issue is the function header in gen.rs (which, by the way, should be renamed to gen_header...). Here we generate everything outside of the braces {} of the impl block we are generating. What we need to do for this issue is basically "just" adding a bound to the where clause. Currently we use the where_clause variable as returned by trait_def.generics.split_for_impl().

I think what we should do is generate our custom tokens for the where clause. This needs to be almost at the very end (just before the last quote! of the function). Here, we need to start with an token stream only holding where. Then we need to iterate through the predicates adding each predicate to the token stream (and don't forget the comma!). Finally, we add our own predicate: quote! { #self_ty: ??? } where ??? should be a list of all super traits.

A list of super traits can be obtained through trait_def.supertraits. It might be possible to simply use that field in the quote! macro.

Finally, compile-pass tests should be added. (And mabye one compile-fail test where the super trait is not actually implemented for the proxy type).

If anything is unclear, just go ahead and ask!

#[auto_impl(FnMut)]
trait FnTrait3 {
    fn execute(self);
}

Expands to:

impl<__AutoImplProxyT: ::std::ops::FnMut()> FnTrait3 for __AutoImplProxyT {
    fn execute(self) {
        self()
    }
}

Which errors because self is bound immutably but used mutably. To fix this, we could treat this as a special case and generate (mut self) instead of (self). Maybe there is another way. Or we could simply always generate ({self})() as body for Fn traits: the {} act as the identify function, so self can be used mutably or immutably.

I can mentor anyone interested in tackling this issue :)
Just ping me (via email, this issue, or in any other way)

Instructions: code generation of methods is done in gen_method_item in gen.rs. The important part for this issue is here (just search for proxy_type.is_fn()). The quote!{ } macro there is what generates code. Currently it's quote! { self(#args) } which is wrong. This probably just have to be changed as discussed above (the {} trick). Additionally, the compile test value_self_for_fn_mut.rs should be modified.

If anything is unclear, just go ahead and ask!

I think this could be handy. It would just add an impl Trait for ! {}. This should work fine for all traits, that:

• don't have associated constants or types
• only have methods which take some sort of receiver (self, &self or &mut self).

All methods can then be implemented with self as body, because ! is coercable to all other types. So we can basically implement the trait for ! when we know that none of the trait items will ever be used :P

impl Trait for Box<dyn Trait> { ... }

Any chance to support these?

At the moment, the generated code uses types prefixed with ::std::. By switching to ::core:: and alloc::, #[auto_impl] could work with no_std projects as well.

• std's Arc, Rc, and Box types are simply re-exports from alloc.
• Skimming through the codebase, I noticed some other types which should be swapped out with their ::core counterparts: e.g: ::std::ops::Fn -> ::core::ops::Fn, ::std::marker::Sized -> ::core::marker::Sized, etc...

A new release including the syn 2 PR hasn't been published to crates.io, though this comment said it would be.

Kindly requesting this as I'm looking to prune my various Cargo.locks. Thanks!

It would be neat to be able to use this on stable (once use_extern_macros is stabilized). Currently the following features are activated:

• #![feature(in_band_lifetimes)]: tracking issue, expected to be stabilized for Rust 2018. If not, we can easily remove the feature by changing one line of code. => probably won't be stabilized, so this has been removed from this crate for now.
• #![feature(proc_macro_span)]: used only for one Span::join() call. I think it's easy to remove join() with something we can use on stable. => guarded by nightly feature (see #26)
• #![feature(proc_macro_diagnostic)]: this is the big part! I don't think the Diagnostics API will be stabilized in near future. So in order to compile on stable, we need to replace all nice diagnostics with simple panics. This shouldn't be too hard. => guarded by nightly feature (see #26)

We still use Cargo's cargo-features = ["edition"] (EDIT: it's stabilized on nightly and beta RC1 now) with edition ="2018" which entails a couple of features. But all of these should be stabilized for Rust 2018 (EDIT: they are now).

• Implicit extern crates (see rust-lang/rust#53128)
• #![feature(extern_prelude)] (see rust-lang/rust#53128)
• #![feature(crate_visibility_modifier)] (see rust-lang/rust#53120) (TODO: this might not make it into Rust 2018, maybe switch to pub(crate) for now?) -> replaced by pub(crate)
• #![feature(crate_in_paths)] (see rust-lang/rust#53130)

Generic Associated types are stable for more than a year now:
https://blog.rust-lang.org/2022/10/28/gats-stabilization.html

#[auto_impl::auto_impl(&, Arc, Box)]
pub trait Trait {
    type Type<'a, T>: core::iter::Iterator<Item = T> + 'a;
}

Output

error[E0107]: missing generics for associated type `Trait::Type`
   --> main.rs:221:14
    |
221 |         type Type<'a, T>: core::iter::Iterator<Item = T> + 'a;
    |              ^^^^ expected 1 lifetime argument
    |
note: associated type defined here, with 1 lifetime parameter: `'a`
   --> main.rs:221:14
    |
221 |         type Type<'a, T>: core::iter::Iterator<Item = T> + 'a;
    |              ^^^^ --
help: add missing lifetime argument
    |
221 |         type Type<'b><'a, T>: core::iter::Iterator<Item = T> + 'a;

• Add a dummy project that exercises each of the impls in various ways (I think there might still be some cases that aren't covered properly)
• Add a proper readme and docs
• Check if we actually need a separate internals crate. It'd be nice if we don't. I put that there to make it easier to unit test.

So we can report errors much more better using spans to the actual code that prevents auto_impl from being valid.

Consider the following code. It currently does not compile (on version 1.0.0).

#[auto_impl::auto_impl(&)]
trait Foo {
    fn foo<const I: i32>(&self);
}

fn main() {}

Using cargo expand I believe the macro expands to:

impl<'a, T: 'a + Foo + ?::core::marker::Sized> Foo for &'a T {
    fn foo<const I: i32>(&self) {
        T::foo(self)
    }
}

The T::foo(self) should be T::foo::<I>(self)

Currently, this:

#[auto_impl(Fn)]
trait Greeter {
    fn greet<T: Display>(&self, name: T);
}

Generates this:

impl<U: ::std::ops::Fn(T)> Greeter for U {
    fn greet<T: Display>(&self, name: T) {
        ({ self })(name)
    }
}

This doesn't compile as T is only declared on the method but already used in the impl header. We could make this specific thing work by moving T: Display to the impl header.

But it's questionable whether we should support generic functions at all. impl Trait in argument position could be complicated. We would have to transform impl Trait generics into standard generics. If we would only support standard generics, that would... be kinda sad.

Since we never claimed to support generics for Fn* types, I won't classify this as bug. But it would be nice to support in the future.

Before the next release, proper documentation (in code and in the README) should exist. The README is already pretty nice, but might need updating.

I found this project which could help reduce compile times of auto_impl. Currently we use the "full" feature of syn which leads to a quite high compile time of that crate. syn-mid promises to solve that problem by not parsing function bodies. Sounds like we should use it for auto_impl.

I didn't have time to investigate further, though. Hence this issue as a reminder.

This is a list of a few things that should be cleaned up:

• use fully qualified paths for Box and Fn traits in generated code
• support mutable and by-value receivers for Box
• properly reject by-value receivers for &mut
• support associated methods (where there's no self receiver)
• clean up the attribute parser and make sure it works when there's no whitespace (the indexing right now will probably break without whitespace padding on things)

Hey there!

I see you put significant effort into error reporting in your crate. It so happened that I'm very interested in the situation around error reporting in proc-macros and I made a crate just for it.

It turns out that most of the code in your diag.rs module is very very similar to the code in my crate (thread local storage of errors, emitting effectively puts the new error into this storage). Thus I would like to help you move from your DIY solution to my crate, I believe code should be reused as much as possible.

What do you think? Just say yes and I'll make a PR in, say, 4 days (I'm kind of busy right now).

P.S. The latest published version doesn't support "note" messages quite yet but I'm on the verge of new release that will support them quite reasonably.

Maybe just #[auto_impl(&)] and #[auto_impl(&mut)] would do it. We should be able to support associated types ad let ref coersion break in cases it isn't supported.

We already have compile-fail tests, but they in fact only check if compilation indeed fails. It does not check the error message at all. This is pretty unfortunate, because any other random failure can make the test pass.

See this PR for discussion on how one would do that. We probably want to use a crate that does it for us, but it's not clear which one.

I just noticed that this crate is currently licensed as MIT only. The vast majority of crates in the Rust eco-system are dual license as MIT/Apache-2. @KodrAus is there a specific reason you licensed it as only MIT? AFAIK, yes, MIT is strictly more liberal than Apache-2, but they are still somewhat incompatible and Apache-2 apparently helps with some trademark thingies?

Anyway, I would immediately relicense everything I contributed as MIT/Apache-2.

We should decide this ASAP before more contributors help out and make everything more complicated ^_^

#[auto_impl(&)]
trait Foo {
    fn foo<T>();
    fn bar<U>(&self);
}

Is expanded into:

impl<'a, V: 'a + Foo> Foo for &'a V {
    fn foo<T>() {
        V::foo()         // <-- cannot infer type for `T`
    }
    fn bar<U>(&self) {
        (**self).bar()   // <-- cannot infer type for `U`
    }
}

And results in two compiler errors. So we need to change the method body into an explicit call all of the time. In this case:

• V::foo::<T>(), and
• V::bar::<T>(*self)

I can mentor anyone interested in tackling this issue :)
Just ping me (via email, this issue, or in any other way)

Instructions: code generation of methods is done in gen_method_item in gen.rs. The important part for this issue are the last two arms of the last match statement in the function (SelfType::Value and SelfType::Ref | SelfType::Mut). These generate incorrect code. You can see the generated code in the quote! {} macro.

The most difficult part is probably to generate the list of generic parameters to call the other method. In the example above it's simply <T>, but it could be more complicated. In gen_method_item(), we have the variable sig which is a syn::MethodSig. We are interested in sig.decl.generics which stores the generics of the method we are generating. Sadly, we can't just use that: e.g. fn foo<T: Clone, U>() would have <T: Clone, U> as generics and we can't call foo::<T: Clone, U>(), but need to call foo::<T, U>(). So we might have to remove all bounds. But with some luck, we can use syn::Generics::split_for_impl. The second element of the returned tuple should be what we want. But we need to test that!

Finally, one or more compile-pass tests should be added which test this exact code.

If anything is unclear, just go ahead and ask!

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.

It looks like this crate has all the needed machinery in place to be extended to work with any newtypes and not just smart pointers, allowing traits to opt-in to easily work with anything that implements From<T> + Deref<Target = T> (which all the smart pointers also already fit).

One approach could be to generate blanket implementation with trait bounds specified above, but another could be having an explicit internal wrapper which would be used by #[auto_impl(newtype)] on one side and by something like #[derive(AutoImplNewtype)] on another.

Either way, this could allow to solve the common issues with reimplementing traits for each newtype.

For a few weeks now, the daily CI builds fail on nightly. Everything works up till the doctest which fails with:

error: extern location for quote does not exist: /home/travis/build/auto-impl-rs/auto_impl/target/debug/deps/libquote-db9ca8cbcca99a73.rlib
error[E0463]: can't find crate for `quote`

I investigated a bit and found the cause of this failure:

• cargo build (and cargo test --no-run) prepare the files correctly. They create libquote-db9ca8cbcca99a73.rlib in the right location.
• cargo test --tests now runs our own test harnesses. They need to get the path to libauto_impl****.so. This is done via the build_plan crate and by executing cargo build -Z unstable-options --build-plan. Executing this command deletes the libquote-db9ca8cbcca99a73.rlib file! (And apparently a bunch of other files as well)
• cargo test --doc then tries to run rustdoc which is unable to find the libquote-db9ca8cbcca99a73.rlib file.

So the problem is cargo build -Z unstable-options --build-plan removing the rlib file. I'm not yet sure if that's intended or not. When I first created those tests, it worked fine. But I guess it was clear that the current solution is hacky and would break at some point.

I'm trying to fix this, but wanted to create this issue already to write down my findings.

Apparently this is tracked here: rust-lang/cargo#6954

The methods we generate in the impl blocks are super simple and just call another function. Therefore one might want to add #[inline] or #[inline(always)] to those methods. However, in C++ land I learned that the programmer usually is way dumber than the compiler and that one should just let the compiler do its thing most of the time.

I'm not sure what we should do here.

The crate docs are pretty sad. While the method signature isn't going to be very helpful we should at least include most of what's in the readme in there.

In-band lifetimes will probably be stabilized soon. Currently, auto_impl doesn't work with those and Fn-traits. From the comments here:

Now it get's a bit complicated. The types of the function signature
could contain "local" lifetimes, meaning that they are not declared in
the trait definition (or are 'static). We need to extract all local
lifetimes to declare them with HRTB (e.g. for<'a>).

In Rust 2015 that was easy: we could just take the lifetimes explicitly
declared in the function signature. Those were the local lifetimes.
Unfortunately, with in-band lifetimes, things get more complicated. We
need to take a look at all lifetimes inside the types (arbitrarily deep)
and check if they are local or not.

In cases where lifetimes are omitted (e.g. &str), we don't have a
problem. If we just translate that to for<> Fn(&str), it's fine: all
omitted lifetimes in an Fn() type are automatically declared as HRTB.

Edit: given the recent discussion about in-band lifetimes, we might not need to worry about this, as they might not be stabilized. So this issue is rather small priority until it's clear if and in what form in-band lifetimes will land.

There is a new Span constructor in town. We could probably nicely use it in auto_impl and then probably remove the nightly feature completely.