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

Deriving for generic structs #2769

Closed
2 tasks done
kuviman opened this issue Sep 15, 2021 · 4 comments · Fixed by #3023
Closed
2 tasks done

Deriving for generic structs #2769

kuviman opened this issue Sep 15, 2021 · 4 comments · Fixed by #3023
Labels
A-derive Area: #[derive]` macro API

Comments

@kuviman
Copy link
Contributor

kuviman commented Sep 15, 2021

Please complete the following tasks

  • I have searched the discussions
  • I have searched the existing issues

Clap Version

3.0.0-beta.4

Describe your use case

I would like to have a library with most common cli arguments, and then allow the main crate to specify additional, like:

#[derive(Clap)]
struct GenericArgs<T: Clap> {
    #[clap(long)]
    something_generic: Option<String>,
    #[clap(flatten)]
    app_args: T,
}

The derive macro currently does not support generic structs though

Describe the solution you'd like

The easiest solution and one that works for me would be inserting just generic args as they were in the definition into the implementation.

As you see, I have put T: Clap trait bound in the struct definition since it will be needed in the implementation.

Alternatives, if applicable

Other possibility is adding those trait bounds in the implementation, so that they are not needed in struct definition. Although this would probably not work for my usecase as I'm actually not using T directly, and instead use <T as my::GenericAppTrait>::Args that implements Clap, so putting a bound on T would not work

Additional Context

No response

@pksunkara pksunkara added the A-derive Area: #[derive]` macro API label Sep 18, 2021
@pksunkara
Copy link
Member

Note: Structopt did some work towards this after we forked it

@epage
Copy link
Member

epage commented Oct 11, 2021

Structopt issue: TeXitoi/structopt#128
Structopt PR: TeXitoi/structopt#483

Unlike a lot of the other ports from structopt. this one needs a major re-work. Structopt only has 1 trait, so they can just tack that one trait bound on everything. We have several traits and have to select trait bounds according to a generic parameters usage.

I recommend we follow the approach of serde

  • By default infer trait bounds from the fields that use them
    • e.g. #[clap(flatten)] foo: T would add a T: Args bound while #[clap(subcommand)] foo: T would add a T: Subcommand bound
  • Add a bounds attribute for overriding inferred bounds

These can be split into two different PRs with the highest priority probably on the inferring.

@kuviman
Copy link
Contributor Author

kuviman commented Nov 14, 2021

@epage

I believe there is a major difference between serde and clap in terms of how inferring bounds should work

Let's say we have a struct

struct Foo<T> {
    inner: Inner<T>,
}

In case of serde a trait bound T: Serialize is being inserted because T is contained in Inner<T>.
They do it because most likely there is impl<T: Serialize> Serialize for Inner<T>

But, in case of clap, I am not so sure that this is the most likely implementation. And, like in your example, we should probably insert Inner<T>: Subcommand instead of T: Subcommand. If we do otherwise, we can add incorrect bound. Because maybe there is actually an impl<T: Args> Subcommand for Inner<T>.

Another thing is that types implementing serde do happen to still be useful when trait bounds are not met. You cannot serialize them, but you can do something.

In case of types made for command line argument parsing, I believe their primary and only usecase is parsing command line arguments. We probably want to know if the trait is implemented properly as soon as possible, and that is when the type is defined. We want the type to implement Args/Parser/Subcommand not only sometimes, but always.

So, to me it makes more sense to put bounds on the type directly.

That being said, I have implemented basic generic support (just copy-pasting type's bounds to the impl) in #3023
Looks like structopt folks did the same

@epage
Copy link
Member

epage commented Nov 15, 2021

We have several traits and have to select trait bounds according to a generic parameters usage.

I wish I could remember what I ran into with my port of the structopt work. I see that structopt adds a trait, StructOptInternal, but that isn't relevant to us. The closest relevant situation is FromArgMatches. Maybe I forgot FromArgMatches was a supetrait for Args / Subcommand?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-derive Area: #[derive]` macro API
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants