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.

Sign up for GitHub

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

Jump to bottom

Handling Arbitrary on structs containing the Never type (!) #331

Open
nipunn1313 opened this issue Jun 10, 2023 · 4 comments · May be fixed by #332
Open

Handling Arbitrary on structs containing the Never type (!) #331

nipunn1313 opened this issue Jun 10, 2023 · 4 comments · May be fixed by #332
Labels
feature-request This issue is requesting new functionality

Comments

@nipunn1313
Copy link
Contributor

Hello! Here's a motivating example

fn main() {
    let x = any::<Wrapper<!>>();
}

#[derive(proptest_derive::Arbitrary, Debug)]
enum Wrapper<T> {
    A(T),
    B,
    C,
}

I'd like to be able to create such a Wrapper struct such that for Wrapper<!>, the proptest arbitrary picks between the other two options. This of course fails with

the trait `Arbitrary` is not implemented for `!`

Which makes sense! Impling arbitrary on ! doesn't particularly make sense since it can't be instantiated. Perhaps I can implement Arbitrary manually with separate impls for <T: Arbitrary> and T = !.

type WrapperStrategy<T: Arbitrary> = impl Strategy<Value = Wrapper<T>>;
impl<T: Arbitrary> Arbitrary for Wrapper<T> {
    type Parameters = T::Parameters;
    type Strategy = WrapperStrategy<T>;

    fn arbitrary_with(args: Self::Parameters) -> Self::Strategy {
        unimplemented!()
    }
}

impl Arbitrary for Wrapper<!> {
    type Parameters = ();
    type Strategy = WrapperStrategy<!>;

    fn arbitrary_with(args: Self::Parameters) -> Self::Strategy {
        unimplemented!()
    }
}

Unfortunately, this also doesn't work because

19 | impl<T: Arbitrary> Arbitrary for Wrapper<T> {
   | ------------------------------------------- first implementation here
...
28 | impl Arbitrary for Wrapper<!> {
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `Wrapper<!>`
   |
   = note: upstream crates may add a new impl of trait `_IMPL_ARBITRARY_FOR_Wrapper::_proptest::arbitrary::Arbitrary` for type `!` in future versions

I briefly tried using https://doc.rust-lang.org/beta/unstable-book/language-features/negative-impls.html to impl !Arbitrary for ! {} - but this does not work for avoiding this coherence detection.

Any ideas on how this could work?
Perhaps proptest_derive could be augmented to detect this scenario and loosen the requirement that T: Arbitrary. Another idea could be something in the same vein as #[proptest(skip)] on Wrapper::A(T) that only skips it for the uninhabited case.
@matthew-russo matthew-russo added the help-request This issue is asking for advice/help on using proptest label Jun 10, 2023
@matthew-russo
Copy link
Member

matthew-russo commented Jun 10, 2023 •
edited

I think you're looking to restrict generation to just the two variants that don't contain a T which can be done via

#![feature(never_type)]

#[derive(proptest_derive::Arbitrary, Debug)]
enum Wrapper<T> {
    A(T),
    B,
    C,
}

fn main() {
    let x = proptest::prop_oneof![
        Just(Wrapper::<!>::B),
        Just(Wrapper::<!>::C),
    ];
}

does this address your use case or is there some dependency between Arbitrary and the use of ! for your use case?

@nipunn1313
Copy link
Contributor Author

Ideally, we're able to instantiate both Wrapper<!> and an inhabited variant (eg Wrapper<String>).

For the Wrapper<String> - I'd want arbitrary to pick from all of the choices, and for the Wrapper<!> - I'd want it to pick from the two inhabited choices.

It is possible (as you mentioned) to make two different strategies, but it doesn't seem possible to impl Arbitrary for Wrapper<!>. My codebase uses both Wrapper<String> and Wrapper<!> in many places. It would be possible (but annoying) to override the arbitrary strategy - but it would be nicer to be able to impl Arbitrary.

One workaround is to make our own never type - then it allows impling arbitrary.

#[derive(Clone, Debug)]
enum Never {}
impl Arbitrary for Wrapper<Never> {
    type Parameters = ();
    type Strategy = impl Strategy<Value = Self>;

    fn arbitrary_with(args: Self::Parameters) -> Self::Strategy {
        proptest::sample::select(vec![
            Wrapper::<Never>::B,
            Wrapper::<Never>::C,
        ])
    }
}

I think it would be nice if we could get proptest-rs to work nicely with coherence rules to allow for impl Arbitrary for Wrapper<!>. I think this could be possible with both

I also additionally think it would be nice if #[derive(Arbitrary)] supported cases where ! was a type param, though I suspect that would be difficult to implement.

@matthew-russo
Copy link
Member

I see. I think a negative impl for ! is reasonable given its impossible to generate a value.

I also additionally think it would be nice if #[derive(Arbitrary)] supported cases where ! was a type param, though I suspect that would be difficult to implement.

I'm not as convinced on adding this to the derive impl at the moment but I'll think it over a bit more

@matthew-russo matthew-russo added feature-request This issue is requesting new functionality and removed help-request This issue is asking for advice/help on using proptest labels Jun 10, 2023
@matthew-russo matthew-russo linked a pull request Jun 10, 2023 that will close this issue
Open
@matthew-russo
Copy link
Member

Let me know if this works for you. you'll need to use proptest with features unstable and never-arbitrary

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
feature-request This issue is requesting new functionality
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Arbitrary : negative impl of Arbitrary on never type (!) matthew-russo/proptest
2 participants
@nipunn1313 @matthew-russo