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

clap 4.3: Implementation of FnOnce is not general enough #4939

Open
2 tasks done
lukesneeringer opened this issue May 24, 2023 · 3 comments
Open
2 tasks done

clap 4.3: Implementation of FnOnce is not general enough #4939

lukesneeringer opened this issue May 24, 2023 · 3 comments
Labels
C-bug Category: Updating dependencies

Comments

@lukesneeringer
Copy link

Please complete the following tasks

Rust Version

1.69.0

Clap Version

4.3.0

Minimal reproducible code

use anyhow::Result;
use clap::Parser;

#[derive(Clone, Debug)]
struct Offset {}

impl Offset {
    pub fn parse(s: impl AsRef<str>) -> Result<Self> {
        Ok(Self {})
    }
}

#[derive(Clone, Debug, Parser)]
struct Command {
    #[arg(long, value_parser = Offset::parse)]
    pub offset: Offset,
}

fn main() {
    println!("Hello, world!");
}

Steps to reproduce the bug with the above code

cargo build

Actual Behaviour

error: implementation of `FnOnce` is not general enough
  --> src/main.rs:13:24
   |
13 | #[derive(Clone, Debug, Parser)]
   |                        ^^^^^^ implementation of `FnOnce` is not general enough
   |
   = note: `fn(&'2 str) -> Result<Offset, anyhow::Error> {Offset::parse::<&'2 str>}` must implement `FnOnce<(&'1 str,)>`, for any lifetime `'1`...
   = note: ...but it actually implements `FnOnce<(&'2 str,)>`, for some specific lifetime `'2`
   = note: this error originates in the derive macro `Parser` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0308]: mismatched types
   --> src/main.rs:13:24
    |
13  | #[derive(Clone, Debug, Parser)]
    |                        ^^^^^^ one type is more general than the other
    |
    = note: expected trait `for<'a> Fn<(&'a str,)>`
               found trait `Fn<(&str,)>`
note: the lifetime requirement is introduced here
   --> /Users/luke/.cargo/registry/src/github.com-1ecc6299db9ec823/clap_builder-4.3.0/src/builder/arg.rs:975:48
    |
975 |     pub fn value_parser(mut self, parser: impl IntoResettable<super::ValueParser>) -> Self {
    |                                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    = note: this error originates in the derive macro `Parser` (in Nightly builds, run with -Z macro-backtrace for more info)

Expected Behaviour

Successful compilation.

Additional Context

This program compiled successfully with clap 4.2, and broke in clap 4.3.

Debug Output

Same as "actual behavior" above.

@lukesneeringer lukesneeringer added the C-bug Category: Updating dependencies label May 24, 2023
@epage
Copy link
Member

epage commented May 24, 2023

This program compiled successfully with clap 4.2, and broke in clap 4.3.

I just ran with

#!/usr/bin/env rust-script

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

use anyhow::Result;
use clap::Parser;

#[derive(Clone, Debug)]
struct Offset {}

impl Offset {
    pub fn parse(s: impl AsRef<str>) -> Result<Self> {
        Ok(Self {})
    }
}

#[derive(Clone, Debug, Parser)]
struct Command {
    #[arg(long, value_parser = Offset::parse)]
    pub offset: Offset,
}

fn main() {
    println!("Hello, world!");
}

and I'm seeing the failure

even if I drop to

#!/usr/bin/env rust-script

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

use anyhow::Result;
use clap::Parser;

#[derive(Clone, Debug)]
struct Offset {}

impl Offset {
    pub fn parse(s: impl AsRef<str>) -> Result<Self> {
        Ok(Self {})
    }
}

#[derive(Clone, Debug, Parser)]
struct Command {
    #[arg(long, value_parser = Offset::parse)]
    pub offset: Offset,
}

fn main() {
    println!("Hello, world!");
}

I see the failure.

@mkatychev
Copy link

mkatychev commented Dec 16, 2023

Rust Version: 1.74.1
Clap Version: 4.4.11

@epage @lukesneeringer I did a bit of diving into the the problem since I ran into something similar on latest clap & latest rust. I made the smallest reproducible example I could:

Problem

#[derive(Clone)]
struct Offset;

pub fn offset_parse(_: impl AsRef<str>) -> Result<Offset, String> {
    Ok(Offset)
}

fn main() {
    let arg = clapArg::new("offset").value_parser(offset_parse);
    let _cmd = clap::Command::new("command").arg(arg);
}

the compiler (nightly or stable) still seems to have a hard time at showing the actual origin of the problem when dealing with higher ranked trait bounds, the actual source of the error is the line below:

F: Fn(&str) -> Result<T, E> + Clone + Send + Sync + 'static,

Partial solution

I found a reddit comment that touches very closely on the problem encountered in this issue and managed to reproduce most of the pattern:
https://old.reddit.com/r/rust/comments/f1spyt/higher_kinded_lifetime_on_return_types/fh8n2b2/

However, the last step in the post using the nested function that returns an impl still seems to be tricky to invoke:

As it turns out, rustc is quite bad with type inference when it comes to HRTB - for some weird reason here it cannot > prove that our closure is general enough, even though it is. Well, once again, we have to help the compiler:

fn fancy_copy_2(value: i32) -> i32 {
    fn build_callback(value: i32) -> impl FnOnce(&Source) -> Ref {
        move |s| {
            s.new_ref(value)
        }
    }

    call_2(build_callback(value))
}

I've made a separate feature branch to test out the issue that includes the modified impl<F, T, E> TypedValueParser for F:

pub trait Callback<'a> {
type Ok: Send + Sync + Clone + 'a;
type Error: Into<Box<dyn std::error::Error + Send + Sync + 'static>>;
fn call(&self, value: &'a str) -> Result<Self::Ok, Self::Error>;
}
impl<'a, F, T, E> Callback<'a> for F
where
F: Fn(&'a str) -> Result<T, E>,
T: Send + Sync + Clone + 'a,
E: Into<Box<dyn std::error::Error + Send + Sync + 'static>>,
{
type Ok = T;
type Error = E;
fn call(&self, value: &'a str) -> Result<Self::Ok, Self::Error> {
self(value)
}
}
impl<F, T, E> TypedValueParser for F
where
for<'a> F: Callback<'a, Ok = T, Error = E> + Clone + Send + Sync + 'static,
E: Into<Box<dyn std::error::Error + Send + Sync + 'static>>,
T: Send + Sync + Clone,
{
type Value = T;

As well as a unit test to reproduce the issue:

#[test]
fn value_parser_hrtb() {
#[derive(Clone)]
struct Offset;
fn offset_parse(_: impl AsRef<str>) -> Result<Offset, String> {
Ok(Offset)
}
let arg = Arg::new("offset").value_parser(offset_parse);
let _cmd = Command::new("command").arg(arg);
}

There is still a compilation error but I do believe this is closer to being compiled:

$ cargo test

error: implementation of `Callback` is not general enough
  --> clap_builder/src/builder/tests.rs:67:15
   |
67 |     let arg = Arg::new("offset").value_parser(offset_parse);
   |               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `Callback` is not general enough
   |
   = note: `fn(&str) -> std::result::Result<Offset, String> {offset_parse::<&str>}` must implement `Callback<'0>`, for any lifetime `'0`...
   = note: ...but it actually implements `Callback<'1>`, for some specific lifetime `'1`

@Patryk27
Copy link

Patryk27 commented Dec 17, 2023

I've experimented with this for a while now and I think this is some limitation in the compiler - I've reported it here, we'll see:
rust-lang/rust#119045

I think the best solution for now is to use a closure, i.e.:

let arg = Arg::new("offset").value_parser(|a: &str| offset_parse(a));

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Updating dependencies
Projects
None yet
Development

No branches or pull requests

4 participants