icu4x
icu4x copied to clipboard
Consider changing PluralCategory discriminants to be 0 for Other
In https://github.com/unicode-org/icu4x/pull/4496 I added ULE discriminants that match the Serde discriminants in PluralCategory, which is 0 == Zero. Although that is cute, I think we should try to stick with 0 == the undefined state, which is Other. At this point it would be a breaking change to any data struct that currently uses PluralCategory so it should be made in 2.0.
If we get too busy in 2.0 this is an easy thing to drop.
This is a data struct bump, so I think it's independent of 2.0?
True since we want to keep supporting V1 data structs then I guess this can be decoupled from 2.0
Discuss with:
- @sffc
- @zbraniecki
- @robertbastian
- @echeran
I don't think this is worth the cost of having a second data struct, which comes with a second code path for construction. This increases code size, and complexity in general.
- @sffc - Ideally I think that discriminant
0should be the default/auto value. - @robertbastian - I don't see the issue, Rust users are not casting the enum to u8, and nobody is inspecting our raw ULE bytes. The change is churn
- @Manishearth I'm ok with doing this if we do it without forking the data structs. Which is unlikely to happen.
- @echeran - We go back and forth between default values and enums in CodePointTrie, for example, and we've had issues with the default values in that crate.
- noted: it's also in the ABI as Zero = 0 for ICU4XPluralCategory
Decision: no change