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

Constants of type BitFlags cannot be used in match expressions #60

Open
pamburus opened this issue Jun 8, 2024 · 12 comments
Open

Constants of type BitFlags cannot be used in match expressions #60

pamburus opened this issue Jun 8, 2024 · 12 comments

Comments

@pamburus
Copy link

pamburus commented Jun 8, 2024

Constants of type BitFlags cannot be used as a pattern in match expressions because it has manually implemented PartialEq trait.
To be able to use constants of a type in match expressions, the type must derive PartialEq trait.

Here is the exact error message:

to use a constant of type `BitFlags<MyFlag, u16>` in a pattern, `BitFlags<MyFlag, u16>` must be annotated with `#[derive(PartialEq)]`
the traits must be derived, manual `impl`s are not sufficient
see https://doc.rust-lang.org/stable/std/marker/trait.StructuralPartialEq.html for details

But it looks like there is no need in manual implementation of PartialEq trait.
It anyway just compares the inner values

impl<T, N: PartialEq> PartialEq for BitFlags<T, N> {
    #[inline(always)]
    fn eq(&self, other: &Self) -> bool {
        self.val == other.val
    }
}

Please consider deriving PartialEq.

Thank you.

@meithecatte
Copy link
Owner

I don't remember all the design constraints at this point – I'd have to go digging in the git history a bit more.

One issue is that the current implementation doesn't require T: PartialEq, and #[derive(PartialEq)] will add that useless bound – and I don't see how to avoid that.

This creates compatibility issues – examples currently in the test suite will stop compiling.

@pamburus
Copy link
Author

pamburus commented Jun 8, 2024

How can it perform self.val == other.val without requiring T: PartialEq?

@meithecatte
Copy link
Owner

It has N: PartialEq, but not T: PartialEq.

@pamburus
Copy link
Author

pamburus commented Jun 8, 2024

Ah.. So, the problem can go out of marker: PhantomData<T>?

@pamburus
Copy link
Author

pamburus commented Jun 8, 2024

Hmm, however according to this https://doc.rust-lang.org/src/core/marker.rs.html#749 it should not.

@meithecatte
Copy link
Owner

See rust-lang/rust#26925

@pamburus
Copy link
Author

pamburus commented Jun 8, 2024

Oh, that is a pretty serious limitation, and given the low priority, it looks like it's unlikely to ever be fixed.

@pamburus
Copy link
Author

pamburus commented Jun 8, 2024

But is it so bad to require PartialEq for T?

@meithecatte
Copy link
Owner

Well, it would require bumping the version to 0.8.0

@pamburus
Copy link
Author

pamburus commented Jun 8, 2024

Actually, I am not sure this will help.
Probably PhantomData should also derive PartialEq, but it doesn't.
It looks like an experiment should be done first.

@meithecatte
Copy link
Owner

Yes, I'd suggest you clone the repo and try getting it to work in your project with enumflags2 = { path = "..." }

@pamburus
Copy link
Author

pamburus commented Jun 8, 2024

Tried. Yes, it requires T to implement PartialEq, as expected due to the issue you mentioned earlier.
But everything else works fine. #[derive(PartialEq)] works without issues.
And I was able to use constants as patterns in match expressions with this fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants