Implement BCP-47 fallback for LanguageIdentifierDisplayName by sffc · Pull Request #8132 · unicode-org/icu4x

sffc · 2026-06-24T23:21:08Z

Depends on #8135

This pull request is part of the implementation for the main locale display names tracking issue: #7825.

This pull request implements full BCP-47 subtag fallback hierarchy for the new LanguageIdentifierDisplayName (and its owned version), resolving the core fallback requirements for the experimental single displaynames module.

Key Changes

BCP-47 Fallback Hierarchy: Implements step-by-step subtag fallback (e.g., language+script+region -> language+region -> language -> code) during formatting.
LossyWrap Dual-Trait Implementation (API Design):
- Following design feedback from the writeable crate, LanguageIdentifierDisplayName now implements both TryWriteable (for strict error detection and error part annotation) and Writeable / Display (for standard lenient formatting with raw code fallback) directly on the struct.
- Under the hood, LanguageIdentifierDisplayName is structured as a tuple newtype wrapping an inner struct in LossyWrap, leveraging writeable::impl_try_writeable_delegate! and writeable::impl_writeable_delegate!.
- Removed Writeable and TryWriteable from LanguageIdentifierDisplayNameOwned (callers use .as_borrowed() to format).
Renamed Fallback Error Type:
- Renamed DisplayNamesFallbackError to LanguageIdentifierNameFallbackError (correcting the spelling of "Identifier" to match the formatter).
- Removed the pub const fn new() constructor and adopted struct syntax LanguageIdentifierNameFallbackError for construction, while suppressing the clippy::exhaustive_structs warning.
Idiomatic Result Formatting:
- Leveraged the built-in TryWriteable implementation for Result<T, E> in the writeable crate to simplify the manual match-writing and Part::ERROR annotation blocks in both QualifiersWriteable and LanguageIdentifierDisplayName formatting code, making it extremely concise and clean.
Boilerplate Elimination: Reuses the standalone constructors of ScriptDisplayNameOwned, RegionDisplayNameOwned, and VariantDisplayNameOwned in the main constructor to load sub-parts, eliminating hundreds of lines of duplicate data-loading code.
Error Annotation: Annotates missing subtags in the output with Part::ERROR during strict formatting, enabling advanced styling in downstream applications.
Polished Tests: Rewrote the integration test loop and doc tests to utilize direct formatting and borrowed.try_write_to_string() for clean, idiomatic fallback verification.

🤖 This pull request was created by an AI agent working with @sffc.

Changelog

writeable: Add standard derives and repr(transparent) to LossyWrap

LossyWrap<T> now derives Clone, Copy, PartialEq, Eq, PartialOrd, Ord, and Hash, and is marked #[repr(transparent)], enabling its use inside Copy structs and with delegate macros.
Added error mapping fn to impl_try_writeable_delegate!

icu_experimental: Implement BCP-47 fallback for LanguageIdentifierDisplayName

New types: LanguageIdentifierNameFallbackError
Trait implementations:
- LanguageIdentifierDisplayName implements TryWriteable, Writeable, and Display directly using writeable::impl_delegate macros over an internal LossyWrap field.
- Removed Writeable and TryWriteable from LanguageIdentifierDisplayNameOwned (use .as_borrowed() to format).
Constructor behavior: The constructors try_new(), try_new_with_buffer_provider(), and try_new_unstable() no longer fail when the display name is not found.

…ions struct - Add `LanguageIdentifierDisplayNameOptions` with only the `language_display` option to `options.rs`. - Re-export it in `displaynames/mod.rs`. - Refactor `LanguageIdentifierDisplayNameOwned` to use `LanguageIdentifierDisplayNameOptions` instead of `DisplayNamesOptions`. - Update docstring examples and integration tests in `tests.rs` to use the new options struct. - Update `single/README.md` design documentation and add an open question about moving dialect names to a separate data marker. Co-authored-by: Gemini <176961590+gemini-code-assist[bot]@users.noreply.github.com>

… and improve Rustiness - Reuse ScriptDisplayNameOwned, RegionDisplayNameOwned, and VariantDisplayNameOwned constructors in try_new_unstable to eliminate duplicate loading boilerplate. - Replace PayloadOrFallback enum with a type alias to DataPayloadOr<M, TinyAsciiStr<8>>, reusing robust existing implementations and eliminating custom Debug impl. - Use precise DataPayloadOr<LocaleNamesLanguageMediumV1, TinyAsciiStr<4>> for language_payload, avoiding unreachable Absent case. - Implement elegant Rusty iterator chain for variant loading. - Restore all original comments in language.rs. - Preserve original test_concatenate structure in tests.rs, running both strict and lenient checks in the same loop, and adding test_fallback_parts. Co-authored-by: Gemini <176961590+gemini-code-assist[bot]@users.noreply.github.com>

…d chain try_interpolate

sffc · 2026-06-25T02:12:20Z

I've gone over this multiple times. I'm happy with this now and ready for a review.

dpulls · 2026-06-25T06:02:01Z

🎉 All dependencies have been resolved !

Manishearth

Got a little confused around the error types for a bit but no, this is correct.

…tag structs

…ifier_not_found and cleanup imports

…docs, and refine doctests Co-authored-by: Gemini <176961590+gemini-code-assist[bot]@users.noreply.github.com>

sffc-bot

🤖 AI agent working with @sffc addressed actionable review comments in commit 7ae252c3ab (replaced AbsentOrFallback with Option, improved error docs, added Debug to with_fallback, and refined doc-tests).

…lback

LossyWrap is a transparent newtype wrapper around a TryWriteable. Adding standard derives and repr(transparent) allows wrapping values in LossyWrap without stripping Copy, Clone, or equality traits, enabling use in Copy structs and delegate macros. Co-authored-by: Gemini <176961590+gemini-code-assist[bot]@users.noreply.github.com>

…e .with_fallback() Structure LanguageIdentifierDisplayName as an outer LossyWrap wrapper around an inner struct, implementing TryWriteable, Writeable, and Display directly using writeable delegate macros. Remove the .with_fallback() method and update all doc tests and unit tests. Co-authored-by: Gemini <176961590+gemini-code-assist[bot]@users.noreply.github.com>

sffc · 2026-07-07T03:59:34Z

I changed it to implement both Writeable and TryWriteable on the same type, instead of making a function that returns impl Writeable + '_.

robertbastian · 2026-07-07T13:43:04Z

+writeable::impl_try_writeable_delegate!(
+    LanguageIdentifierDisplayName<'_>,
+    |&self| &self.0.0,
+    Error = LanguageIdentifierNameFallbackError
+);
+
+writeable::impl_writeable_delegate!(LanguageIdentifierDisplayName<'_>, |&self| &self.0);
+
+writeable::impl_display_with_writeable!(LanguageIdentifierDisplayName<'_>);


I don't think you need a separate inner type. just implement TryWriteable directly on LanguageIdentifierDisplayName, and then

Suggested change

writeable::impl_try_writeable_delegate!(

LanguageIdentifierDisplayName<'_>,

|&self| &self.0.0,

Error = LanguageIdentifierNameFallbackError

);

writeable::impl_writeable_delegate!(LanguageIdentifierDisplayName<'_>, |&self| &self.0);

writeable::impl_display_with_writeable!(LanguageIdentifierDisplayName<'_>);

writeable::impl_writeable_delegate!(LanguageIdentifierDisplayName<'_>, |&self| LossyWrap(&self.0));

writeable::impl_display_with_writeable!(LanguageIdentifierDisplayName<'_>);

Unfortunately this pattern doesn't work because write_to_string borrows from the temporary LossyWrap. The Writeable trait doesn't have a way to track the inner lifetime.

robertbastian · 2026-07-07T13:54:59Z

+            match VariantDisplayNameOwned::try_new_unstable(provider, prefs, variant)
+                .allow_identifier_not_found()?
+            {
+                Some(obj) => Ok(DataPayloadOr::from_payload(obj.payload)),
+                None => Ok(DataPayloadOr::from_other(variant)),
+            }


Suggested change

match VariantDisplayNameOwned::try_new_unstable(provider, prefs, variant)

.allow_identifier_not_found()?

{

Some(obj) => Ok(DataPayloadOr::from_payload(obj.payload)),

None => Ok(DataPayloadOr::from_other(variant)),

}

Ok(VariantDisplayNameOwned::try_new_unstable(provider, prefs, variant)

.allow_identifier_not_found()?

.map_or_default(

|| DataPayloadOr::from_other(variant),

|r| DataPayloadOr::from_payload(r.payload)))

also elsewhere

map_or_default is too new

rust-lang/rust#138099

Co-authored-by: Robert Bastian <4706271+robertbastian@users.noreply.github.com>

sffc · 2026-07-07T21:49:08Z

There are a lot of comments so I'm going to click resolve on the ones that are either fixed or have follow-up issues filed.

sffc · 2026-07-07T21:55:41Z

I think the last round of comments was more about subjective style and commenting choices than substance, with the possible exception of the error handling, because I agree it's better style to track errors.

Variable names and things like that being self-inconsistent in the same file is, well, regrettable but 🤷 not really worth the ping-pong to fix IMO.

This PR has been in a mergeable state 3 times. Let's please favor follow-up issues.

robertbastian

Variable names and things like that being self-inconsistent in the same file is, well, regrettable but 🤷 not really worth the ping-pong to fix IMO.

You could just not send code like that for review. It might involve hand-writing it.

sffc force-pushed the sffc/displaynames-fallback branch 7 times, most recently from d5b4878 to a4d3b58 Compare June 25, 2026 01:14

sffc force-pushed the sffc/displaynames-fallback branch from a4d3b58 to cef16f4 Compare June 25, 2026 01:30

rewriting test again

ba6c561

sffc commented Jun 25, 2026

View reviewed changes

sffc and others added 4 commits June 24, 2026 18:50

🤖 [displaynames] Address PR comments: leverage Result TryWriteable an…

ad8a36f

…d chain try_interpolate

tweaks

e99e539

tweaks

5befee0

sffc requested review from Manishearth and robertbastian June 25, 2026 02:12

sffc marked this pull request as ready for review June 25, 2026 02:12

sffc requested review from a team and snktd as code owners June 25, 2026 02:12

sffc removed the request for review from snktd June 25, 2026 02:12

Merge branch 'main' into sffc/displaynames-fallback

49433b8

Manishearth approved these changes Jun 25, 2026

View reviewed changes

robertbastian requested changes Jun 25, 2026

View reviewed changes

Manishearth mentioned this pull request Jun 25, 2026

Introduce dedicated LanguageIdentifierDisplayNameOptions struct #8135

Merged

sffc added 2 commits June 25, 2026 16:53

🤖 [displaynames] Switch fallback representation to strongly typed sub…

86f09dd

…tag structs

🤖 [displaynames] Address PR reviewer feedback: consistent allow_ident…

8d9274d

…ifier_not_found and cleanup imports

return impl Writeable instead of LossyWrap

f827fc9

robertbastian reviewed Jun 26, 2026

View reviewed changes

Comment thread components/experimental/src/displaynames/single/language.rs Outdated

🤖 [displaynames] Replace AbsentOrFallback with Option, improve error …

7ae252c

…docs, and refine doctests Co-authored-by: Gemini <176961590+gemini-code-assist[bot]@users.noreply.github.com>

sffc-bot reviewed Jul 7, 2026

View reviewed changes

sffc and others added 3 commits July 6, 2026 20:40

Merge remote-tracking branch 'origin/main' into sffc/displaynames-fal…

23cca92

…lback

sffc mentioned this pull request Jul 7, 2026

In the displaynames module: FooOwned+Foo or Foo+FooBorrowed? #8180

Open

single_result

429d795

sffc mentioned this pull request Jul 7, 2026

Implement and test fully-borrowed language identifier display name #8181

Closed

sffc added 2 commits July 7, 2026 11:07

assert_try_writeable_eq

3222d47

rm PayloadOrFallback

be2f6ba

sffc requested a review from robertbastian July 7, 2026 09:10

robertbastian reviewed Jul 7, 2026

View reviewed changes

sffc and others added 7 commits July 7, 2026 14:02

Add error mapping fn to impl_try_writeable_delegate

30bbb19

use LanguageIdentifierNameFallbackError more consistently

1b322cf

Update components/experimental/src/displaynames/single/language.rs

81ad745

Co-authored-by: Robert Bastian <4706271+robertbastian@users.noreply.github.com>

streamline comments

1ac53e1

remove redundant comment

b64ba01

language_payload_or

e095f55

no more peeking

6554e58

sffc requested a review from robertbastian July 7, 2026 21:54

robertbastian approved these changes Jul 7, 2026

View reviewed changes

sffc merged commit a796abb into unicode-org:main Jul 7, 2026
36 checks passed

sffc deleted the sffc/displaynames-fallback branch July 7, 2026 22:06

sffc mentioned this pull request Jul 7, 2026

Evaluate stack size for LanguageIdentifierDisplayName[Owned] and tweak Variant field #8190

Closed

Uh oh!

Conversation

sffc commented Jun 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Key Changes

Changelog

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sffc commented Jun 25, 2026

Uh oh!

dpulls Bot commented Jun 25, 2026

Uh oh!

Manishearth left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sffc-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sffc commented Jul 7, 2026

Uh oh!

Uh oh!

Uh oh!

robertbastian Jul 7, 2026

Choose a reason for hiding this comment

Uh oh!

sffc Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

robertbastian Jul 7, 2026

Choose a reason for hiding this comment

Uh oh!

sffc Jul 7, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sffc commented Jul 7, 2026

Uh oh!

sffc commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

robertbastian left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

sffc commented Jun 24, 2026 •

edited

Loading

sffc Jul 7, 2026 •

edited

Loading

sffc commented Jul 7, 2026 •

edited

Loading