fix(intl): locale-aware group/decimal separators + zero-arg bigint toLocaleString grouping (#7429, #7428)

changelog.d/7797-locale-separators.md

@@ -0,0 +1,13 @@
+**`toLocaleString` now uses each locale's own digit separators, and groups when called with no arguments** (#7429, #7428).
+
+**#7429 — the separator pair was a `de`-vs-everything-else branch.** Written when `de-DE` was the only non-`en` locale under test, and duplicated at three sites. So every locale that does not group with `,` was wrong, not only French. Measured against Node v26.5.1 across 21 locales, Perry produced `,`/`.` for all of them except German; the correct data is three groups — `.`/`,` for `de`/`es`/`it`/`pt`/`nl`/`tr`, a space plus `,` for `fr`/`ru`/`pl`/`nb`/`sv`/`fi`/`cs`/`hu`/`uk`, and `,`/`.` for `en`/`ja`/`ko`/`zh`. `locale_separators` now returns the CLDR pair for the primary language subtag, and the three call sites share it.
+
+The French case is the one the issue caught, and it is the sharp one: `fr-FR` groups with **U+202F** (narrow no-break space) while `fr-CA` uses **U+00A0**. Those render identically in a terminal, so the region is consulted for French and only for French. Getting it wrong is silent everywhere except a byte-for-byte oracle diff — and the first cut of this fix did get it wrong, slicing `locale.get(..6)` against the five-byte `"fr-FR"`, which returns `None` and quietly demoted every French locale to the U+00A0 arm. The oracle diff caught it; nothing else would have.
+
+Locales not named in the table keep the previous `,`/`.` default, so this widens correctness without changing any locale it does not list.
+
+**#7428 — zero-argument `bigint.toLocaleString()` produced no grouping**, while `toLocaleString(undefined)` was already correct. Codegen lowers the zero-arg form to `Expr::DateToLocaleString`, which lands in `js_object_default_to_locale_string`; that function has arms for numbers, Dates and Temporal values, and a BigInt fell past them into Object.prototype's "Invoke(O, 'toString')" tail — and `BigInt.prototype.toString` has no grouping. Any call carrying locales/options goes down the generic method-call path to the real thunk instead, so the two forms never met. That asymmetry is why the bug survived: the natural way to write a test for it (`toLocaleString(undefined)`) exercises the other path. A BigInt arm now formats through the same ECMA-402 machinery with the default locale.
+
+`test_gap_intl_locale_separators_7429_7428.ts` covers both, asserting separators as **code points** rather than as formatted strings, so U+202F, U+00A0 and a plain space cannot be confused; and it exercises `Intl.NumberFormat` with a fractional value so a locale that got the group separator right and the decimal wrong still fails.
+
+Verified: the new gap test passes byte-for-byte against Node v26.5.1, all 21 locales plus the `Intl.NumberFormat` rows match, and `test_gap_intl` (7) and `test_gap_bigint` (4) stay green. `cargo test -p perry-runtime --lib` is unchanged; the one intermittent failure seen during validation is `gc::tests::root_words::bare_address_in_shadow_slot_survives_a_real_collection`, which reproduces at 2/10 runs on **clean `main`** with these changes reverted and rebuilt — it is #7365, not this change.

crates/perry-runtime/src/intl/number_format.rs

@@ -664,9 +664,8 @@ fn number_parts_core(r: &NfResolved, value: f64) -> Vec<(&'static str, String)>
         return currency_instance_parts(r, value);
     }
 
-    let de_style = r.locale.eq_ignore_ascii_case("de") || r.locale.starts_with("de-");
-    let group_sep = if de_style { '.' } else { ',' };
-    let decimal_sep = if de_style { ',' } else { '.' };
+    // #7429: CLDR separators for the resolved locale, not a de-vs-rest guess.
+    let (group_sep, decimal_sep) = locale_separators(&r.locale);
 
     let mut parts: Vec<(&'static str, String)> = Vec::new();
     let is_zero = value == 0.0;
@@ -881,6 +880,49 @@ fn locale_lang(locale: &str) -> &str {
     locale.split(['-', '_']).next().unwrap_or(locale)
 }
 
+/// The `(group, decimal)` separator pair for a locale — CLDR's `symbols-*`
+/// `group` and `decimal` for its primary language subtag.
+///
+/// #7429: this used to be a single `de`-vs-everything-else branch, written when
+/// `de-DE` was the only non-`en` locale under test. Every other locale that
+/// does not group with `,` was therefore wrong, not just French: measured
+/// against Node v26.5.1, `es`/`it`/`pt`/`nl`/`tr` want `.` like German, and
+/// `fr`/`ru`/`pl`/`nb`/`sv`/`fi`/`cs`/`hu`/`uk` group with a SPACE.
+///
+/// The space is not one character. `fr-FR` uses U+202F (narrow no-break space)
+/// while `fr-CA` uses U+00A0, which is why the region is consulted for French
+/// and only for French — every other space-grouping locale here is U+00A0 in
+/// CLDR. Getting that wrong is invisible in a terminal and loud in a
+/// byte-for-byte oracle diff, which is exactly how #7429 was found.
+///
+/// Locales absent from the table keep the previous default (`,` and `.`), so
+/// this widens correctness without changing any locale it does not name.
+fn locale_separators(locale: &str) -> (char, char) {
+    const NNBSP: char = '\u{202f}';
+    const NBSP: char = '\u{00a0}';
+    match locale_lang(locale) {
+        // `.` group, `,` decimal.
+        "de" | "es" | "it" | "pt" | "nl" | "tr" | "id" | "da" | "ro" | "el" | "vi" | "ca" => {
+            ('.', ',')
+        }
+        // Space group, `,` decimal. French splits by region: fr-FR is U+202F,
+        // fr-CA (and the rest of these) U+00A0.
+        "fr" => {
+            // `"fr-FR"` is five bytes; slicing `..6` returns None and silently
+            // demotes every French locale to the U+00A0 arm.
+            let region_fr = locale.eq_ignore_ascii_case("fr")
+                || locale
+                    .get(..5)
+                    .is_some_and(|p| p.eq_ignore_ascii_case("fr-fr"));
+            (if region_fr { NNBSP } else { NBSP }, ',')
+        }
+        "ru" | "pl" | "nb" | "no" | "sv" | "fi" | "cs" | "sk" | "hu" | "uk" | "lv" | "lt"
+        | "et" | "bg" => (NBSP, ','),
+        // `,` group, `.` decimal — en, ja, ko, zh, he, th, and the default.
+        _ => (',', '.'),
+    }
+}
+
 /// Prefix text some locales place *before* the number for a unit (e.g. the
 /// Japanese/Korean/Chinese "speed" reading of `kilometer-per-hour`'s long
 /// form: "時速 -987 キロメートル"). Only a handful of compound units have a
@@ -1101,9 +1143,8 @@ fn bigint_number_parts_exact(
     negative: bool,
     abs_digits: &str,
 ) -> Vec<(&'static str, String)> {
-    let de_style = r.locale.eq_ignore_ascii_case("de") || r.locale.starts_with("de-");
-    let group_sep = if de_style { '.' } else { ',' };
-    let decimal_sep = if de_style { ',' } else { '.' };
+    // #7429: CLDR separators for the resolved locale, not a de-vs-rest guess.
+    let (group_sep, decimal_sep) = locale_separators(&r.locale);
     set_round_ctx(&r.rounding_mode, negative);
 
     let mut parts: Vec<(&'static str, String)> = Vec::new();

crates/perry-runtime/src/object/native_call_method/object_proto.rs

@@ -92,6 +92,29 @@ pub(crate) unsafe fn js_object_default_to_locale_string(receiver: f64) -> f64 {
     if crate::temporal::is_temporal_value(receiver) {
         return crate::temporal::dispatch::call_method(receiver, "toLocaleString", &[]);
     }
+    // #7428: a BigInt receiver must format through
+    // `BigInt.prototype.toLocaleString`, i.e. with the DEFAULT locale's digit
+    // grouping — `(12345678901234567890n).toLocaleString()` is
+    // `"12,345,678,901,234,567,890"`, not the bare digits.
+    //
+    // Without this arm a BigInt falls through to the generic tail below, whose
+    // job is Object.prototype.toLocaleString's "Invoke(O, 'toString')" — and
+    // `BigInt.prototype.toString` has no grouping. That tail is correct for the
+    // receivers it is written for; a BigInt simply is not one of them, the same
+    // way a number and a Date are handled above rather than left to it.
+    //
+    // Only the ZERO-ARG form reaches here at all: codegen lowers
+    // `x.toLocaleString()` to `Expr::DateToLocaleString`, while any call
+    // carrying locales/options goes down the generic method-call path to
+    // `bigint_proto_to_locale_string_thunk`. That asymmetry is why the explicit
+    // `toLocaleString(undefined)` was already correct while the bare call was
+    // not — the two forms never met.
+    #[cfg(feature = "intl-namespace")]
+    if jsval.is_bigint() {
+        let undef = f64::from_bits(crate::value::TAG_UNDEFINED);
+        let s = crate::intl::bigint_to_locale_string(receiver, undef, undef);
+        return f64::from_bits(JSValue::string_ptr(s).bits());
+    }
     // Symbols are POINTER-tagged, so `!jsval.is_pointer()` would be false for
     // them — check before the pointer guard so the branch is reachable.
     let is_symbol = unsafe { crate::symbol::js_is_symbol(receiver) } != 0;

test-files/test_gap_intl_locale_separators_7429_7428.ts

@@ -0,0 +1,66 @@
+// Gap: locale-aware digit grouping for `toLocaleString` (#7429, #7428).
+//
+// Two defects, both byte-visible only against the oracle:
+//
+// #7429 — the group/decimal separator pair was a single `de`-vs-everything
+// branch, written when `de-DE` was the only non-`en` locale under test. Every
+// other locale that does not group with `,` was wrong, not only French. The
+// French case is the sharp one because the separator is a NARROW no-break
+// space (U+202F) for `fr-FR` and a regular no-break space (U+00A0) for
+// `fr-CA` — two characters that render identically in a terminal and differ in
+// a byte-for-byte diff, which is why this test asserts the code points
+// explicitly rather than eyeballing the formatted strings.
+//
+// #7428 — `bigint.toLocaleString()` with NO arguments produced no grouping at
+// all, while `toLocaleString(undefined)` was correct. Codegen lowers the
+// zero-arg form to `Expr::DateToLocaleString`, which reaches
+// `js_object_default_to_locale_string`; that function had arms for numbers,
+// Dates and Temporal values but not for BigInt, so a BigInt fell through to
+// Object.prototype's "Invoke(O, 'toString')" tail. The two call forms never
+// met, which is exactly why the bug survived: the obvious spelling in a test
+// (`toLocaleString(undefined)`) exercises the other path.
+
+const big = 12345678901234567890n;
+
+// #7428: the zero-argument form must group with the default locale, and must
+// agree with the explicitly-undefined form.
+console.log("zeroarg:" + big.toLocaleString());
+console.log("undef:" + big.toLocaleString(undefined));
+console.log("small-zeroarg:" + (9876543n).toLocaleString());
+
+// #7429: separators per locale. Printed as code points so the two space
+// characters cannot be confused with each other or with a plain ASCII space.
+const locales = [
+  "en-US",
+  "de-DE",
+  "fr-FR",
+  "fr-CA",
+  "es-ES",
+  "it-IT",
+  "ru-RU",
+  "pl-PL",
+  "sv-SE",
+  "pt-BR",
+  "nl-NL",
+  "tr-TR",
+  "cs-CZ",
+  "ja-JP",
+];
+
+for (const loc of locales) {
+  const s = (9876543210n).toLocaleString(loc);
+  const seps = s.replace(/[0-9]/g, "");
+  const codes: string[] = [];
+  for (let i = 0; i < seps.length; i++) {
+    codes.push("U+" + seps.charCodeAt(i).toString(16).toUpperCase().padStart(4, "0"));
+  }
+  console.log(loc + " " + JSON.stringify(s) + " " + codes.join(","));
+}
+
+// The same table through `Intl.NumberFormat`, which shares the resolver, with
+// a fractional value so the DECIMAL separator is exercised too — `fr` groups
+// with U+202F and separates decimals with a comma, so a locale that got the
+// group right and the decimal wrong would still pass the integer-only rows.
+for (const loc of ["en-US", "de-DE", "fr-FR", "ru-RU", "nl-NL"]) {
+  console.log("nf:" + loc + " " + JSON.stringify(new Intl.NumberFormat(loc).format(1234567.891)));
+}