search

rust-lang / impl-trait-utils

Utilities for working with impl traits in Rust.
Apache License 2.0
89 stars 9 forks source link

feat: trait_variant::make supports rewriting of the original trait. #27

Closed sargarass closed 7 months ago

sargarass commented 7 months ago

Hello, this PR should close the issue #18.

SeaDve commented 7 months ago

I think this could be made more flexible by doing something like:

#[trait_variant::rewrite(Send + Sync)]
pub trait SomeTrait {
    ...
}
tmandry commented 7 months ago

I see there's demand for this, but I too am in favor of something that more closely resembles the shape of the make macro. #[trait_variant::rewrite(Send)] is okay. Some alternatives to rewrite: add, bound

sargarass commented 7 months ago

I see there's demand for this, but I too am in favor of something that more closely resembles the shape of the make macro. #[trait_variant::rewrite(Send)] is okay

@SeaDve, @tmandry made the update to accomplish this

odysa commented 7 months ago

I think it also closes my issue #29 Thank you for your work.

tmandry commented 7 months ago

Thanks @sargarass, the implementation looks good.

As for the name, on Zulip @Jules-Bertholet suggested trait_variant::require, which I like even better than the options presented so far. What are your thoughts?

hmacias-avaya commented 7 months ago

would this also allow adding annotations on the trait generated by make? For example, I need the trait generated by make to also have #[cfg_attr(test, automock)].

sargarass commented 7 months ago

@tmandry, Hmm, as for naming, I think that trait_variant::rewrite is much closer to trait_variant::make in the sense of 'creation,' 'generation,' etc., than trait_variant::require. Because of the similarities between them, I have not suggested to name it trait_variant::with, which also sounds good but doesn't look similar.

tmandry commented 7 months ago

The other suggestion by @traviscross was to allow #[trait_variant::make(Send)] to mean the same thing, which wouldn't require a new name. I think I'm pretty agnostic on that vs rewrite.

When I get back from vacation next week I'll work on merging one of these two, unless someone else gets to it first.

sargarass commented 7 months ago

unless someone else gets to it first.

@tmandry well...

traviscross commented 7 months ago

Over in #30 I've implemented it using the make(Send) syntax.

Given the ongoing discussion about choosing an alternate name for a different macro, I've become less ambivalent and probably would prefer to just solve this use case with make.

sargarass commented 7 months ago

unless someone else gets to it first.

@tmandry well...

@traviscross Now we both made the same work :D

sargarass commented 7 months ago

I've become less ambivalent and probably would prefer to just solve this use case with make.

As for the entire concept, I do support this idea

qrilka commented 7 months ago

Is it planned to have this in a new release soon? https://github.com/rust-lang/rust/pull/118257 adds warnings on nightly with the current release