illicitonion / num_enum

Apache License 2.0
264 stars 34 forks source link

Bug: discriminant is marked as collided when using ranged alternatives with `default` or `catch_all` attributes. #144

Closed AlexSherbinin closed 5 months ago

AlexSherbinin commented 8 months ago

I have tried this code:

#[derive(FromPrimitive)]
#[repr(u8)]
enum Number {
    Zero = 0,
    #[num_enum(alternatives = [2..=3])]
    OneTwoThree = 1,
    #[num_enum(catch_all)]
    Other(u8)
}

And got following error:

error: The discriminant '2' collides with a value attributed to a previous variant
   |
15 |     Other(u8)
   |     ^^^^^

When there is no collision between Other and OneTwoThree variant

AlexSherbinin commented 8 months ago

Noticed that it's ok for default variants to emit this error because with this attribute it's allowed to have a discriminant. Anyway it stays catch_all only bug.

illicitonion commented 5 months ago

Thanks for the issue report, and apologies for the delay in getting back to you!

I think this is working as intended - because Other doesn't have an explicit discriminant listed, Rust will use the next available u8 as the discriminant for it, which is indeed 2. You can test this with this small program:

#[repr(u8)]
enum Number {
    Zero = 0,
    OneTwoThree = 1,
    Other(u8)
}

fn main() {
    let one = Number::OneTwoThree;
    let other = Number::Other(10);
    println!("disciminant of one: {:?}", std::mem::discriminant(&one));
    println!("discriminant of other: {:?}", std::mem::discriminant(&other));
}

which outputs this:

% cargo run 2>/dev/null
disciminant of one: Discriminant(1)
discriminant of other: Discriminant(2)

If you do you want the discriminant 2 to represent OneTwoThree, you could fix your enum in this case by explicitly setting Other's discriminant to a higher value, e.g.

#[derive(FromPrimitive)]
#[repr(u8)]
enum Number {
    Zero = 0,
    #[num_enum(alternatives = [2..=3])]
    OneTwoThree = 1,
    #[num_enum(catch_all)]
    Other(u8) = 4,
}

which I believe should make the error from num_enum go away.

Without that explicit discriminant for Other, it's ambiguous whether seeing a discriminant of 2 means "This is a OneTwoThree" or "This is an Other". Rust thinks it's an Other, num_enum would think it was a OneTwoThree, and that kind of ambiguity/disagreement is where sneaky bugs sneak in!