Consider having private fields for DataLocale

[Manishearth]

Split off from #5785

After #5988, DataLocale is now in locale_core.

#5875 wanted it to have private fields, but locale fallback (in the locale crate) makes heavy use of these fields, reading from them and mutating them. The fallback iterator steps operate on &mut DataLocale. Migrating this to encapsulated private fields seems to be of little benefit.

The main benefit of private fields is to signpost users away from writing directly to the fields (they should use the Locale/LanguageIdentifier From impls). We're unlikely to change the composition of this type any time soon, the stability benefit is not why we want this. Perhaps we can mark these hidden? Or do nothing?

cc @zbraniecki @sffc @robertbastian

[sffc]

I think keep them the way they are, in a non_exhaustive struct. We can add things, but it would likely break behavior anyway if we removed things.

[robertbastian]

We should align them with the other types in icu_locale_core. Exhaustive struct, public fields, macro constructor, from/into other types, etc.

[sffc]

Exhaustive struct

No, because this concept of a "data locale" is not yet in a spec; it is a design doc that got some review. Even if/when we turn it into a spec, it should sit as a preview for a while.

public fields

Yes

macro constructor

Seems unnecessary, but we could add it.

from/into other types

Yes

[robertbastian]

No, because this concept of a "data locale" is not yet in a spec; it is a design doc that got some review. Even if/when we turn it into a spec, it should sit as a preview for a while.

Data Locale 1.0 never had to change...

[zbraniecki]

I am with Shane here - I don't think what we now have as Data Locale is what we would want to expose as stable public struct. It seems keeping it hidden as an internal struct is correct.

[robertbastian]

• @robertbastian My opinion was to see if anyone agrees. We should make them easier to construct. For locales, we have the tuple of language/region/script.
• @sffc How would you get these outside the crate?
• @robertbastian You could call .langauge to get a ref. And we can do that because they implement Copy. So we can make mutable accessors.
• @sffc What's better about than mutable accessors than public fields?
• @robertbastian I guess it just needs to be non_exhaustive for now. But then it's painful to construct.
• @sffc We need the constructor either way.
• @robertbastian Are you confident that we will keep the 5 fields in 3.0?
• @sffc It's harmless to keep the 5 fields until 3.0. We don't currently use the subdivision fields in our data. We could drop them, but it's also fine to keep them. I could see DataLocale only having language/script/region. The 1.0 data model for DataLocale had language/script/region plus variants and extensions.-
• @zbraniecki I think you answered my question, what are the odds that we use these fields in the next two years? If we see a path to using these fields, then yeah, let's keep them. They should be annotated/documented to help future generations know why they are there.

Consensus to keep the status quo, and add more convenience constructors. Maybe add a macro.