Should we be exposing PluralOperands over FFI?

[nordzilla]

Internally in ICU4X, our select() function takes a type that can be turned into a PluralOperands struct as an argument. I think this makes a lot of sense, and provides a lot of versatility.

Our FFI-exposed select() funciton, however, takes a PluralOperands struct directly (rather than something that can be turned into a PluralOperands).

The PluralOperands struct represents a numeric value in a very specific format, and this puts a lot of burden on users to be able to have their own functionality that can convert integer, floating-point, or string types into our PluralOperands format.

While testing ICU4X in Gecko, I decided it would be easiest to expose the PluralOperands::from_str() function over the FFI, so that I could easily create a PluralOperands object from a string representation of a double that I get back from NumberFormat.

While this is working fine for me to test our code in Gecko, I do not think that this is the final way our API should work. Currently, the easiest way to invoke select() across FFI is to either write your own logic to convert a number into our PluralOperands format, or to

1. Stringify your number.
2. Use our FFI call to get a PluralOperands object from your string.
3. Call select() and pass in your PluralOperands object.

Our current implementation also has the quality that PluralOperands::from_str() only works on string representations of numbers that use ASCII digits with no separators and a '.' decimal, e.g. 123456.789. Some locales may format this as 123.456,789 ("es") or 123 456.789 ("br"), or ١٢٣٬٤٥٦٫٧٨٩ ("ar").

Currently, in Gecko, regardless of my PluralRules locale, I am having to call NumberFormat::format() specifically with the "en" locale, with separators turned off, so that I can pass that formatted number to ICU4X to turn into PluralOperands.

I feel like we should expose more direct ways to call select() over FFI that put the burden of creating the PluralOperands object on the internal implementation details, rather than on the caller.

• ICU4XPluralCategory select(<integer type>)

• ICU4XPluralCategory select(<float type>)

• ICU4XPluralCategory selectFormatted(UTF8String)

Where selectFormatted() would be able to accept all of "123456.789", "123,456.789", "123.456,789", "123 456.789", "١٢٣٬٤٥٦٫٧٨٩" etc. and still give you back the correct PluralCategory.

ICU4C exposes select() in a similar fashion:

• uplrules_select()

• uplrules_selectFormatted()

The two select() methods with numeric types could be exposed right away, but the selectFormatted() function would require adding functionality to ICU4X that could turn a formatted string into a PluralOperands internally.

[sffc]

The C API of ICU4C has only those two functions you referenced, and only the second one supports trailing zeros, which requires invoking NumberFormat.

In ICU4X, we address this problem much more elegantly with FixedDecimal. I would like if we could stick with the FixedDecimal input instead of PluralOperands over FFI, as well as possibly the convenience functions for int and string. (Follow-up: #811)

More specifically:

ICU4XPluralCategory select(<integer type>)

Fine with me as a convenience function.

ICU4XPluralCategory select(<float type>)

No, because passing floats to PluralRules is misguided. #811

ICU4XPluralCategory selectFormatted(UTF8String)
"123456.789", "123,456.789", "123.456,789", "123 456.789", "١٢٣٬٤٥٦٫٧٨٩"

I see your point, but this isn't the right way to implement a "selectFormatted" because:

1. We should avoid APIs that parse localized strings, especially in a core function like this (see my blog post). I don't want this thread to devolve into a discussion on parsing, so I'll just say that if we did ever introduce it into ICU4X, it should be confined to a very specific module or crate that does just that one task and nothing more.
2. We have FixedDecimalFormat, which exposes two actual types (not UTF8String) that we could use:
   1. FormattedFixedDecimal, which is returned from the .format() function
   2. FixedDecimal itself, which already supports Into<PluralOperands>

Note that the ICU4C function takes in a "UFormattedNumber", which is roughly equivalent to our "FormattedFixedDecimal", except that I think in ICU4X we should probably stick with FixedDecimal for this purpose.

Note: I have worked extensively on ICU4C PluralRules, and am responsible for both UFormattedNumber as well as the uplrules_selectFormatted() function you referenced. I want to make a faster and more modular solution in ICU4X via FixedDecimal, which is one of my first contributions to this project.

[sffc]

2021-06-17:

• @Manishearth - I believe PluralOperands is used directly as well as indirectly.
• @nordzilla - It would be nicer to turn your number into a FixedDecimal and then select from that, rather than having to go through the additional intermediate step of PluralOperands.
• @Manishearth - I think it would be okay to have multiple select functions.
• @sffc - I like to avoid creating the temporary short-lived FFI struct
• @Manishearth - The select_from_string version avoids an extra box allocation relative to FixedDecimal
• @sffc - I think we should add select_from_string as well as select_from_fixed_decimal