Constructor signatures in Rust and FFI

[sffc]

In #1833 we're starting to establish a pattern for constructor signatures in Rust. I want to establish how that extends to FFI.

There are some constraints that apply to FFI but not Rust:

1. No type parameters (must be expanded into multiple functions)*
2. No non-exhaustive structs, unless they are opaque
3. Opaque structs require extra allocations so we should not use them with impunity
4. No macros for generating APIs (not supported in syn; ask @Manishearth for details)*

* Diplomat itself could be extended to reduce duplication and improve code maintainability/readability on an as-needed basis.

I'm going to approach this question assuming that constructors typically take 3 parameters: locale/preferences, data provider, and configuration options.

I. Locale/Preferences

Options:

1. Transparent (exhaustive) preferences struct
   • Pro: Cheap to construct
   • Pro: Consistent with Rust
   • Con: Cannot easily add additional preferences (exhaustive)
2. Opaque preferences struct with getters/setters
   • Pro: Consistent with Rust
   • Pro: Extensible (can add more preferences)
   • Con: More expensive (extra allocations)
   • Con: More boilerplate (getters/setters)
3. Locale object
   • Pro: Easy to implement
   • Pro: Less boilerplate
   • Con: Requires using Locale which may harm code size and/or type safety
   • Con: Doesn't support non-BCP-47 preferences
4. DataLocale object (#1995)
   • Pro: Easy to implement
   • Pro: Less boilerplate
   • Pro: Code size friendly
   • Con: Less type safety relative to preferences structs
   • Con: Doesn't support non-Unicode-extension preferences

II. Data Provider

Options:

1. Two constructors, one for AnyProvider and one for BufferProvider
   • Pro: Future-proof
   • Con: Does not support caching data providers (see #919 for details)
2. One constructor with call-site-specific data provider wrapper like FixedDecimalFormatProvider
   • Pro: Supports caching and alternative data provider implementations
   • Con: Requires adding a new wrapper whenever the data keys are changed
3. Three constructors (options 1 and 2)
   • Pro: Maximum flexibility
   • Con: More boilerplate

III. Configuration Options

Options:

1. Transparent (exhaustive) options struct
   • Pro: Cheap to construct
   • Con: Requires creating versioned structs and versioned constructors
2. Opaque options struct with getters/setters
   • Pro: Extensible
   • Con: More expensive at runtime
   • Con: More boilerplate

Thoughts? @zbraniecki @Manishearth @robertbastian

[sffc]

Discussion:

• @echeran - Not everything has a locale, but everything has a provider, so maybe the provider should go first
• @Manishearth - Generally more specific arguments should go later
• @sffc - The main thing is that the options should go last. I'm okay moving the provider to first.

Things we need to do with constructors:

1. Fix argument order
2. Use preferences or other change to the locale argument (pending final decision on the shape of this argument)
3. Add "overloads" for the 3 types of data providers
4. Ensure that options are taken by value

[sffc]

Discussion with @sffc @Manishearth:

• @Manishearth - An advantage of transparent options is that Diplomat can create better APIs. A disadvantage of opaque is that we need to allocate, but it's not really that big of a problem because we call it only once.
• @sffc - For the options bag, I lean toward transparent options with specific names for each component corresponding to the fields inside of the struct, so that adding more fields creates more transparent structs, but those structs have clear names. If there is only 1 field in the transparent struct, pass it directly into the constructor.
• @sffc - I lean toward having 2 data provider types (Any and Buffer).
• @Manishearth - We're already generating a lot of functions. I don't want to have more than 1 type of argument that can be passed for the locale/preferences.
• @sffc - I agree, but I also eventually see these becoming function overloads in languages that support it.
• @sffc - I prefer to use DataLocale for the constructors.
• @Manishearth - DataLocale becomes the type for interfacing with ICU4X, and Locale is for people who need more power or are doing their own i18n. That makes sense to me.
• @sffc - I'll wait on @zbraniecki for the final decision on the type of the locale/preference argument over FFI.

[sffc]

It appears that #1833 still has unresolved questions.

In light of that, let me be clear on what I am proposing for constructor signatures in ICU4X 1.0.

In Rust:

impl FooFormat {
    pub fn try_new_any(p: &P, locale: &DataLocale, options: FooOptions)
        -> Result<FooFormat, DataError>
    where
        P: AnyProvider

    pub fn try_new_buffer(p: &P, locale: &DataLocale, options: FooOptions)
        -> Result<FooFormat, DataError>
    where
        P: BufferProvider

    pub fn try_new_unstable(p: &P, locale: &DataLocale, options: FooOptions)
        -> Result<FooFormat, DataError>
    where
        P: ResourceProvider<Marker1> + ResourceProvider<Marker2>
}

#[non_exhaustive]
pub struct FooOptions {
    pub option1: i32,
    // ...
}

In FFI:

class ICU4XFooFormat {
  public:
    DiplomatResult<FooFormat, DataError> try_new_any(
      const ICU4XAnyProvider& provider,
      const ICU4XDataLocale& locale,
      const ICU4XFooOptions& options
    );

    DiplomatResult<FooFormat, DataError> try_new_buffer(
      const ICU4XBufferProvider& provider,
      const ICU4XDataLocale& locale,
      const ICU4XFooOptions& options
    );
}

class ICU4XFooOptions; // opaque

[sffc]

I should also note a potential path for Rust constructors involving traits

trait TryNewBufferProvider<O> {
    fn try_new(locale: DataLocale, p: &impl BufferProvider, options: O)
}

impl TryFromBufferProvider<FooOptions> for FooFormat { ... }

[sffc]

• @robertbastian - If we take DataLocale in our APIs, why do we have Locale?
• @robertbastian - Can we collapse the constructors for and/or/unit into a single buffer or any constructor?
• @sffc - No because of data slicing
• @markusicu - What is the difference between Locale and DataLocale?
• @sffc - DataLocale has only language identifier and keywords.
• @markusicu - Are you sure we won't need transform keywords ever?
• @sffc - DataLocale has private fields and we can extend it in the future if needed
• @hsivonen - I'm not sure if the model works for collation. You can override collator fallback behavior via options. It's inconvenient API to have to stick stuff into the locale. For collator it's a bit tricky to draw the line between user preferences and developer settings (for example, "numeric" behavior).
• @markusicu - I don't have a clear recommendation, but I'm a bit uneasy because I think it's hard to make and agree on a categorization of a user choice and a developer choice. I like the clear line of using UTS 35 Unicode keywords as user preferences and everything else as options. But that's a clunky API. So it's a choice between being a better API and clarity.
• @zbraniecki - I think people will look at our docs and see the options they can fiddle with. For preferences, people are probably not going to be fiddling with it per call site. They'll just be passing a locale. For places where they actually do set the preferences, it's more systematic. They merge preferences from different sources. I don't think people are going to "look at a spreadsheet and hunt down" where a particular setting lives.
• @zbraniecki - I think we need to make a decision per-case, and I think that's ok.
• @zbraniecki - We can start with the unicode keyword distinction. The issue is that this is an accidental distinction. There are more preferences that could be added later, and then do we move it from options to preferences?
• @zbraniecki - I'm trying to propose a dictinction, and I'm hearing from the group that it's not clear-cut. But lots of things are fuzzy in i18n.
• @zbraniecki - What concerns me about @sffc's proposal is that it may be difficult to change. I'm worried of landing on a less-optimal solution now and a solution in 2.0 not improving enough to justify the change.
• @zbraniecki - The core of my proposal is the merging part.
• @sffc - (1) we can continue adding private fields to DataLocale. (2) it's nice that all constructors take the same type. (3) we can iterate on the options bag in 1.x based on feedback from users. (4) in 2.0 I won't block a change on the basis of being disruptive.
• @hsivonen - If we have an ECMA-402 scope, I'd note that ECMA-402 has most things in the options bag.
• @markusicu - What happens if we add an Option that is also a Keyword?
• @sffc - For both Henri and Markus's case, we can clearly define and test special resolution algorithms on a per-constructor basis.
• @zbraniecki - CLDR has some examples of fallbacks in algorithm that can be overriden by data.

[sffc]

• @robertbastian - Should we take the locale by value?
• @sffc - I prefer by reference and cloning only when we need to, when loading data.
• @robertbastian - Should we take a trait, like Into<DataLocale>?
• @sffc - The concrete type helps with type resolution. You can .into() from anything that implements it.
• @robertbastian - If we merge DataRequest and DataLocale, we can simplicify the code.
• @sffc - Let's take that part of the conversation offline.

[sffc]

Consensus: No objection to what I proposed in https://github.com/unicode-org/icu4x/issues/2136#issuecomment-1183797247.

[sffc]

I reviewed all component and property APIs. To-do list:

• [x] AnyCalendar: make &provider the first argument
• [x] Japanese and JapaneseExtended: add Any/Buffer
• [x] JapaneseEraStyle: see if enum can be deleted
• [x] Collator: add Any/Buffer
• [x] DatagenProvider: implement AnyProvider directly
• [x] DateTimeFormatterOptions: Remove hour cycle preferences
• [x] TimeZoneFormatter: add Any/Buffer
• [x] TimeZoneDataPayloads: see if these can be made private
• [x] DateTimeFormatter: take options by value
• [x] TimeFormatter: remove preferences
• [x] TimeFormatter: add Any/Buffer
• [x] TypedDateFormatter: add Any/Buffer
• [x] TypedDateTimeFormatter: add Any/Buffer
• [x] TypedZonedDateTimeFormatter: add Any/Buffer
• [x] icu_decimal::error module should be private (the error type is re-exported)
• [x] ListFormatter: add Any/Buffer
• [x] LocaleCanonicalizer: add Any/Buffer (or do this at the same time as #767)
• [x] CanonicalComposition: add Any/Buffer
• [x] CanonicalDecomposition: add Any/Buffer
• [x] ComposingNormalizer: add Any/Buffer
• [x] DecomposingNormalizer: add Any/Buffer
• [x] EnumeratedProperty enum: see if it can be deleted or made private
• [ ] ~BidiClassAdapter: add Any/Buffer~ (EDIT: There is no provider here)
• [x] LocaleFallbackProvider: add Any/Buffer
• [x] LocaleFallbacker: add Any/Buffer
• [x] LocaleFallbackerWithConfig: see if it works to borrow the DataLocale
• [x] BlobDataProvider: constructor should be try_new_...
• [x] StaticDataProvider: constructor should be try_new_...
• [x] All four segmenter types: add Any/Buffer
• [x] CustomTimeZone::try_set_metazone does not return a Result so it is a misnomer

Questions:

1. TimeZoneFormatter: do we want to generate a bunch of Any/Buffer constructors for all the load functions, or just try_from_config?
2. CldrCalendar trait: should this trait be doc(hidden) or do we want people to be able to implement it?
3. icu_decimal::provider: should all the types be labled "V1" or only the main one?
4. icu_list::List: should it be named FormattedList?
5. DateFormatter, TimeFormatter, ListFormatter: these take a single flat option; should they be wrapped in a non-exhaustive options struct? Or at least be named with the flat option in the method name?
6. Should icu_provider::serde::borrow_de_utils be doc-hidden?

[sffc]

For FFI purposes, it would make things a lot simpler if we had just one provider type. It would be an enum between a buffer provider and an any provider.

This doesn't solve Serde code size issues directly, but we can circumvent that problem by having a feature on the icu_capi crate to enable or disable the BufferProvider version of it.

[Manishearth]

Hmmm. Not ideal but I guess it works.

[sffc]

None of our choices are ideal:

1. Enum toggling between Buffer and Any with a feature on icu_capi to enable/disable Buffer mode
2. Add two versions of all the constructors over FFI
3. Add constructor-specific data provider opaque wrappers

We have some precedent of using features to reduce code size (deserialize_json). So I think it's fine.

We could even do the same thing on the Rust side to get from 3 constructors down to 2 constructors. But I'm not proposing that we do that in 1.0. We should consider it for 2.0 alongside the new Preferences stuff.

[sffc]

• @robertbastian - How should someone give their own BakedProvider over FFI? Maybe datagen could output something to help.
• @Manishearth - That sounds workable with some asterisks. Or people could just modify the icu_capi code.

Proposal: Have a single ICU4XDataProvider with features to independently enable the AnyProvider and the BufferProvider versions.

LGTM: @robertbastian @Manishearth @sffc @nordzilla

[sffc]

This is finally fixed.