Derive requires field types to impl Clone

[djc]

Maintainer's notes

• For Sync+Sync, see clap4 requires that options implement Send and Sync #4347

---

Please complete the following tasks

• I have searched the discussions
• I have searched the open and rejected issues

Rust Version

rustc 1.64.0 (a55dd71d5 2022-09-19)

Clap Version

4.0.2

Minimal reproducible code

#[derive(Debug, Parser)]
#[command(name = "topfew")]
struct Options {
    /// Fields to use as part of the line's key
    #[arg(long, short)]
    fields: FieldSpec,
}

#[derive(Debug)]
struct FieldSpec {
    indices: Vec<usize>,
}

impl FromStr for FieldSpec {
    type Err = Error;
    fn from_str(s: &str) -> Result<Self, Self::Err> {
        let mut indices = Vec::new();
        for f in s.split(',') {
            indices.push(usize::from_str(f)?);
        }
        Ok(FieldSpec { indices })
    }
}

Steps to reproduce the bug with the above code

cargo check

Actual Behaviour

Fails with an elaborate error pointing to many trait bounds with _AutoValueParser<FieldSpec>:

error[E0599]: the method `value_parser` exists for reference `&&&&&&_AutoValueParser<FieldSpec>`, but its trait bounds were not satisfied
    --> src/bin/tf.rs:29:5
     |
29   |     /// Fields to use as part of the line's key
     |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ method cannot be called on `&&&&&&_AutoValueParser<FieldSpec>` due to unsatisfied trait bounds
...
43   | struct FieldSpec {
     | ----------------
     | |
     | doesn't satisfy `FieldSpec: Clone`
     | doesn't satisfy `FieldSpec: From<&'s std::ffi::OsStr>`
     | doesn't satisfy `FieldSpec: From<&'s str>`
     | doesn't satisfy `FieldSpec: From<OsString>`
     | doesn't satisfy `FieldSpec: From<std::string::String>`
     | doesn't satisfy `FieldSpec: ValueEnum`
     | doesn't satisfy `FieldSpec: ValueParserFactory`
     |
    ::: /Users/djc/.cargo/registry/src/github.com-1ecc6299db9ec823/clap-4.0.2/src/builder/value_parser.rs:2017:1
     |
2017 | pub struct _AutoValueParser<T>(std::marker::PhantomData<T>);
     | ------------------------------ doesn't satisfy `_: clap::builder::via_prelude::_ValueParserViaParse`
     |
     = note: the following trait bounds were not satisfied:
             `FieldSpec: ValueEnum`
             which is required by `&&&&&_AutoValueParser<FieldSpec>: clap::builder::via_prelude::_ValueParserViaValueEnum`
             `FieldSpec: Clone`
             which is required by `&&&&&_AutoValueParser<FieldSpec>: clap::builder::via_prelude::_ValueParserViaValueEnum`
             `FieldSpec: ValueParserFactory`
             which is required by `&&&&&&_AutoValueParser<FieldSpec>: clap::builder::via_prelude::_ValueParserViaFactory`
             `FieldSpec: From<OsString>`
             which is required by `&&&&_AutoValueParser<FieldSpec>: clap::builder::via_prelude::_ValueParserViaFromOsString`
             `FieldSpec: Clone`
             which is required by `&&&&_AutoValueParser<FieldSpec>: clap::builder::via_prelude::_ValueParserViaFromOsString`
             `FieldSpec: From<&'s std::ffi::OsStr>`
             which is required by `&&&_AutoValueParser<FieldSpec>: clap::builder::via_prelude::_ValueParserViaFromOsStr`
             `FieldSpec: Clone`
             which is required by `&&&_AutoValueParser<FieldSpec>: clap::builder::via_prelude::_ValueParserViaFromOsStr`
             `FieldSpec: From<std::string::String>`
             which is required by `&&_AutoValueParser<FieldSpec>: clap::builder::via_prelude::_ValueParserViaFromString`
             `FieldSpec: Clone`
             which is required by `&&_AutoValueParser<FieldSpec>: clap::builder::via_prelude::_ValueParserViaFromString`
             `FieldSpec: From<&'s str>`
             which is required by `&_AutoValueParser<FieldSpec>: clap::builder::via_prelude::_ValueParserViaFromStr`
             `FieldSpec: Clone`
             which is required by `&_AutoValueParser<FieldSpec>: clap::builder::via_prelude::_ValueParserViaFromStr`
             `FieldSpec: Clone`
             which is required by `_AutoValueParser<FieldSpec>: clap::builder::via_prelude::_ValueParserViaParse`

Expected Behaviour

As in clap 3, this should just work.

Additional Context

I find it very surprising that From<&str> and From<String> are supported, but FromStr or TryFrom impls are not.

Debug Output

No response

[epage]

Can you provide a complete reproduction case? For example, I want to make sure I'm digging into this with the same error type you are using.

[djc]

This compiles in the playground with clap 3:

use clap::Parser;
use anyhow::Error;

#[derive(Debug, Parser)]
#[clap(name = "topfew")]
struct Options {
    /// Fields to use as part of the line's key
    #[clap(long, short)]
    fields: FieldSpec,
}

#[derive(Debug)]
struct FieldSpec {
    indices: Vec<usize>,
}

impl std::str::FromStr for FieldSpec {
    type Err = Error;
    fn from_str(s: &str) -> Result<Self, Self::Err> {
        let mut indices = Vec::new();
        for f in s.split(',') {
            indices.push(usize::from_str(f)?);
        }
        Ok(FieldSpec { indices })
    }
}

[epage]

My first step was to opt-in to clap v4's behavior with clap 3 by adding the value_parser attribute. This required adding Clone to FieldSpec

#!/usr/bin/env -S rust-script

//! ```cargo
//! [dependencies]
//! clap = { version = "3", features = ["derive"] }
//! anyhow = "1"
//! ```

use anyhow::Error;
use clap::Parser;

#[derive(Debug, Parser)]
#[clap(name = "topfew")]
struct Options {
    /// Fields to use as part of the line's key
    #[clap(long, short, value_parser)]
    fields: FieldSpec,
}

#[derive(Debug, Clone)]
struct FieldSpec {
    indices: Vec<usize>,
}

impl std::str::FromStr for FieldSpec {
    type Err = Error;
    fn from_str(s: &str) -> Result<Self, Self::Err> {
        let mut indices = Vec::new();
        for f in s.split(',') {
            indices.push(usize::from_str(f)?);
        }
        Ok(FieldSpec { indices })
    }
}

[epage]

And it turns out that was what was needed to get it working with clap v4

#!/usr/bin/env -S rust-script

//! ```cargo
//! [dependencies]
//! clap = { version = "4", features = ["derive"] }
//! anyhow = "1"
//! ```

use anyhow::Error;
use clap::Parser;

#[derive(Debug, Parser)]
#[clap(name = "topfew")]
struct Options {
    /// Fields to use as part of the line's key
    #[clap(long, short)]
    fields: FieldSpec,
}

#[derive(Debug, Clone)]
struct FieldSpec {
    indices: Vec<usize>,
}

impl std::str::FromStr for FieldSpec {
    type Err = Error;
    fn from_str(s: &str) -> Result<Self, Self::Err> {
        let mut indices = Vec::new();
        for f in s.split(',') {
            indices.push(usize::from_str(f)?);
        }
        Ok(FieldSpec { indices })
    }
}

[epage]

While the listed error message wasn't too helpful in figuring out what was going on, earlier I got this error message

error[E0277]: the trait bound `FieldSpec: Clone` is not satisfied                                                                 --> clap-4286.rs:17:5                                                                                                           |
17  |     fields: FieldSpec,                                                                                                       |     ^^^^^^ the trait `Clone` is not implemented for `FieldSpec`                                                              |
note: required by a bound in `ArgMatches::remove_one`
   --> /home/epage/.cargo/registry/src/github.com-1ecc6299db9ec823/clap-3.2.22/src/parser/matches/arg_matches.rs:309:32            |
309 |     pub fn remove_one<T: Any + Clone + Send + Sync + 'static>(&mut self, id: &str) -> Option<T> {                            |                                ^^^^^ required by this bound in `ArgMatches::remove_one`
help: consider annotating `FieldSpec` with `#[derive(Clone)]`
    |
21  | #[derive(Clone)]
    |

[djc]

So it's just the Clone bound? Why is that necessary, though? It doesn't conceptually make sense to me.

[epage]

That is an artifact of the derive API being built on top of the builder API. This is happening on two layers

• ArgMatches impls Clone
• As an implementation detail, for now we wrap the value inside ArgMatches inside an Arc but to move it out, we still need a Clone in case there are multiple references to it.

[SteveLauC]

For the issue described in my last comment, the error message will go away if you give <Foo as FromStr>::Err a type other than (), I found this solution in this comment.

Update: I filed an issue for it: #5360