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

Default value seems to be cached, resulting in wrong default #5127

Open
2 tasks done
ctron opened this issue Sep 15, 2023 · 3 comments
Open
2 tasks done

Default value seems to be cached, resulting in wrong default #5127

ctron opened this issue Sep 15, 2023 · 3 comments
Labels
A-derive Area: #[derive]` macro API C-bug Category: Updating dependencies S-blocked Status: Blocked on something else such as an RFC or other implementation work.

Comments

@ctron
Copy link

ctron commented Sep 15, 2023

Please complete the following tasks

Rust Version

rustc 1.72.0 (5680fa18f 2023-08-23)

Clap Version

4.4.3

Minimal reproducible code

use clap::Parser;
use std::marker::PhantomData;

trait Provider {
    const VALUE: usize;
}

#[derive(Clone, Debug)]
struct A;

impl Provider for A {
    const VALUE: usize = 1;
}

#[derive(Clone, Debug)]
struct B;

impl Provider for B {
    const VALUE: usize = 2;
}

#[derive(Clone, Debug, clap::Args)]
struct Example<T: Provider> {
    #[arg(default_value_t = T::VALUE)]
    pub value: usize,

    #[arg(skip)]
    _marker: PhantomData<T>,
}

#[derive(Clone, Debug, clap::Parser)]
enum Cli {
    A(Example<A>),
    B(Example<B>),
}

fn main() {
    // prints 1, as expected
    //Cli::parse_from(["example", "a", "--help"]);
    // prints 2, bug!
    Cli::parse_from(["example", "b", "--help"]);
}

Steps to reproduce the bug with the above code

Run the above example.

Important is that you have the same structure, where a type argument provides the default value, in more than one branch.

Actual Behaviour

Initializes with the wrong default value. Shown in help and used as value.

Expected Behaviour

Use the correct the default value.

Additional Context

No response

Debug Output

No response

@ctron ctron added the C-bug Category: Updating dependencies label Sep 15, 2023
@ctron
Copy link
Author

ctron commented Sep 15, 2023

Here's a test for this:

#[test]
fn default_values_type_arg() {
    trait DefaultProvider {
        const VALUE: usize;
    }

    #[derive(PartialEq, Debug)]
    struct A;
    #[derive(PartialEq, Debug)]
    struct B;

    impl DefaultProvider for A {
        const VALUE: usize = 1;
    }

    impl DefaultProvider for B {
        const VALUE: usize = 2;
    }

    #[derive(Args, PartialEq, Debug)]
    struct Example<T: DefaultProvider> {
        #[arg(default_value_t = T::VALUE)]
        value: usize,

        #[arg(skip)]
        _marker: PhantomData<T>,
    }

    #[derive(Parser, PartialEq, Debug)]
    enum Cli {
        A(Example<A>),
        B(Example<B>),
    }

    impl Cli {
        fn value(&self) -> usize {
            match self {
                Cli::A(args) => args.value,
                Cli::B(args) => args.value,
            }
        }
    }

    assert_eq!(
        A::VALUE,
        Cli::try_parse_from(["test", "a"]).unwrap().value()
    );
    assert_eq!(
        B::VALUE,
        Cli::try_parse_from(["test", "b"]).unwrap().value()
    );

    let help_a = utils::get_subcommand_long_help::<Cli>("a");
    assert!(help_a.contains("[default: 1]"));
    let help_b = utils::get_subcommand_long_help::<Cli>("b");
    assert!(help_b.contains("[default: 2]"));
}

@ctron
Copy link
Author

ctron commented Sep 15, 2023

I think the issue is that default values are created with a static OnceLock. Which of course will initialize only once.

@ctron
Copy link
Author

ctron commented Sep 15, 2023

Looks like that's why: rust-lang/rust#22991

ctron added a commit to ctron/trustification that referenced this issue Sep 15, 2023
ctron added a commit to ctron/trustification that referenced this issue Sep 15, 2023
ctron added a commit to ctron/trustification that referenced this issue Sep 15, 2023
github-merge-queue bot pushed a commit to trustification/trustification that referenced this issue Sep 15, 2023
@epage epage added A-derive Area: #[derive]` macro API S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Sep 15, 2023
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 C-bug Category: Updating dependencies S-blocked Status: Blocked on something else such as an RFC or other implementation work.
Projects
None yet
Development

No branches or pull requests

2 participants