From b489a84e865ccfe7d9acf7af481e97ba95f8422b Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Tue, 11 Aug 2026 05:36:58 +0900 Subject: [PATCH] type_methods: read the locale for the 'n' presentation type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `format(x, 'n')` never consulted the locale. The integer path inserted a separator only inside `if let Some(separator) = p.grouping`, which `'n'` never enters — it carries no `,` or `_` — and the group size came from a per-radix constant with no channel for a locale's grouping vector. The float path delegated render, group and pad end-to-end to the shared engine, which treats `'n'` as `'g'`. Under `LC_ALL=en_US.UTF-8`, `format(123456789, 'n')` was `'123456789'` and `format(1234.5, 'n')` was `'1234.5'`. Port the three routines upstream splits this across. `_get_locale` (`newformat.py:642-658`) branches on the presentation code: `'n'` takes the current locale, an explicit `,`/`_` takes that separator at a fixed group size — four for the power-of-two radices, three otherwise — and anything else carries a stop sentinel. `_group_digits` (`:738-778`) and `_fill_digits` (`:727-736`) perform the insertion for all of those, so `separate_integer_digits` goes away and the `,`/`_` specs travel the same route as `'n'`. The zero fill of a `0=` spec belongs inside the grouping rather than after it: `format(1234, '012n')` is `'0,000,001,234'`, thirteen characters from a width of twelve, because the padding digits are separated like any other. That is what `_calc_num_width`'s `n_min_width` (`:695-704`) carries, and both call sites compute it — `width - (sign + prefix)` for integers, `width - (sign + decimal point + remainder)` for floats, matching `extra_length`. The float path grows the split `_format_float` performs (`:1004-1044`): render unpadded, take the sign off, separate the integer digit run alone, re-emit the decimal point as the locale's, and leave the fraction and any exponent in the remainder, so `format(1e300, 'n')` keeps its exponent ungrouped. Zero padding is rejected for complex specs, so the per-lane `complex_component_spec` split reaches `_group_digits` only at `n_min_width` 0 and needs nothing further. `numeric_formatting` (`rlocale.py:173-178`) is new, placed beside the `_locale` module port so it shares that module's raw `localeconv()` walk: the grouping `format(x, 'n')` separates by and the grouping `locale.localeconv()` reports come out of one read. The walk keeps a `CHAR_MAX` element, which `rustpython_host_env::locale`'s reader drops — dropping it collapses "stop" onto "repeat the last group". Off unix, without `host_env`, and under sandbox the C locale's values stand in; upstream declares `localeconv` `sandboxsafe=True` and reads the host locale even there, but pyre's sandbox build replaces `_locale`'s host entry points with raising stubs and `format()` must not acquire a raising path. Six unit tests pin `_group_digits` against values taken from the vendored source: the zero-fill widths, the repeat-the-last-group rule, the stop sentinel, and a multi-byte separator surviving the buffer reverse. None are covered by `test_format.test_locale`, which asserts only that the separator appears — the test that made this visible when #1138 promoted its module into the gated set. Assisted-by: Claude --- .../src/module/_locale/interp_locale.rs | 23 +- .../src/module/_locale/mod.rs | 2 + .../src/module/_locale/rlocale.rs | 56 ++++ pyre/pyre-interpreter/src/type_methods.rs | 310 ++++++++++++++++-- 4 files changed, 348 insertions(+), 43 deletions(-) create mode 100644 pyre/pyre-interpreter/src/module/_locale/rlocale.rs diff --git a/pyre/pyre-interpreter/src/module/_locale/interp_locale.rs b/pyre/pyre-interpreter/src/module/_locale/interp_locale.rs index 07b7c88440f..ffe89dfa654 100644 --- a/pyre/pyre-interpreter/src/module/_locale/interp_locale.rs +++ b/pyre/pyre-interpreter/src/module/_locale/interp_locale.rs @@ -203,20 +203,17 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { // `_w_copy_grouping` (`interp_locale.py:36-40`): every byte // of the C grouping string up to its NUL is one group size // (a `CHAR_MAX` element stays `127`), then a trailing `0` - // is appended to a non-empty list. Read the grouping - // straight from `localeconv()` because the host helper - // stops at the first `CHAR_MAX`, dropping it. + // is appended to a non-empty list. The bytes come from + // `rlocale::charp2str`, the same read `numeric_formatting` + // groups by, rather than from the host helper, which stops + // at the first `CHAR_MAX` and drops it. The trailing `0` is + // this function's own fixup and belongs to the app-level + // list, never to the grouping the formatter walks. let grouping_of = |ptr: *const libc::c_char| -> Vec { - let mut v: Vec = Vec::new(); - if !ptr.is_null() { - let mut cur = ptr; - unsafe { - while *cur != 0 { - v.push(*cur as u8 as i64); - cur = cur.add(1); - } - } - } + let mut v: Vec = super::rlocale::charp2str(ptr) + .into_iter() + .map(i64::from) + .collect(); if !v.is_empty() { v.push(0); } diff --git a/pyre/pyre-interpreter/src/module/_locale/mod.rs b/pyre/pyre-interpreter/src/module/_locale/mod.rs index 159bb074456..d3a2a0bd2f1 100644 --- a/pyre/pyre-interpreter/src/module/_locale/mod.rs +++ b/pyre/pyre-interpreter/src/module/_locale/mod.rs @@ -3,4 +3,6 @@ //! Provides the 'C' locale defaults so locale.py's `from _locale import *` //! succeeds and Lib/locale.py exposes working `localeconv` / `setlocale`. +pub mod rlocale; + crate::pyre_module_init!(interp_locale); diff --git a/pyre/pyre-interpreter/src/module/_locale/rlocale.rs b/pyre/pyre-interpreter/src/module/_locale/rlocale.rs new file mode 100644 index 00000000000..d1b7fddf25d --- /dev/null +++ b/pyre/pyre-interpreter/src/module/_locale/rlocale.rs @@ -0,0 +1,56 @@ +//! rlocale — RPython: rpython/rlib/rlocale.py +//! +//! `numeric_formatting` is the entry point the number formatter draws its +//! locale from (`newformat.py:643-644`). It sits beside the `_locale` module +//! port so it shares the raw `localeconv()` walk with that module's own +//! `localeconv()`: the grouping `format(x, 'n')` groups by and the grouping +//! `locale.localeconv()` reports come out of the same read and cannot drift +//! apart. + +/// Every byte of a NUL-terminated C string, `CHAR_MAX` included. +/// +/// `rffi.charp2str` (`rlocale.py:175-177`) truncates at the NUL and at nothing +/// else, so a grouping terminator a locale spells as `CHAR_MAX` stays in the +/// result. `rustpython_host_env::locale`'s own reader stops at `CHAR_MAX` and +/// drops it, which would collapse the "stop" and "repeat the last group" +/// conventions onto the same vector. +#[cfg(all(unix, feature = "host_env", not(feature = "sandbox")))] +pub(super) fn charp2str(ptr: *const libc::c_char) -> Vec { + let mut out = Vec::new(); + if !ptr.is_null() { + let mut cur = ptr; + unsafe { + while *cur != 0 { + out.push(*cur as u8); + cur = cur.add(1); + } + } + } + out +} + +/// `rlocale.py:173-178 numeric_formatting`: the decimal point, thousands +/// separator and grouping string of the current locale, as the bytes +/// `localeconv()` reports them. +/// +/// Off unix, without `host_env`, and under sandbox the C locale's values stand +/// in. Upstream declares `localeconv` `sandboxsafe=True` (`rlocale.py:167`, +/// `:180-182`) and reads the host locale even there; pyre compiles the call out +/// instead, because the sandbox build replaces `_locale`'s host entry points +/// with raising stubs and `format()` must not acquire a raising path. +pub(crate) fn numeric_formatting() -> (Vec, Vec, Vec) { + #[cfg(all(unix, feature = "host_env", not(feature = "sandbox")))] + { + let conv = unsafe { libc::localeconv() }; + if !conv.is_null() { + return unsafe { + ( + charp2str((*conv).decimal_point), + charp2str((*conv).thousands_sep), + charp2str((*conv).grouping), + ) + }; + } + } + (b".".to_vec(), Vec::new(), Vec::new()) +} diff --git a/pyre/pyre-interpreter/src/type_methods.rs b/pyre/pyre-interpreter/src/type_methods.rs index 85bb6004cd8..61bf8a92778 100644 --- a/pyre/pyre-interpreter/src/type_methods.rs +++ b/pyre/pyre-interpreter/src/type_methods.rs @@ -2329,27 +2329,208 @@ fn pad_to_width( }) } -fn separate_integer_digits( - mut magnitude: String, - interval: i32, - separator: char, - displayed_digits: i32, +/// `newformat.py:642-658 Formatter._get_locale`: the decimal point, thousands +/// separator and group sizes a numeric format renders with. +/// +/// Three arms, in upstream's order. `'n'` takes the current locale. An +/// explicit `,` or `_` takes that separator at a fixed group size — four for +/// the power-of-two radices, three otherwise. Everything else separates +/// nothing and carries the `0xFF` stop sentinel. +/// +/// The locale bytes decode lossily because the body being assembled is a +/// `String`. Upstream holds them as the bytes `charp2str` returned, RPython +/// strings being byte strings; a separator that is not valid UTF-8 is the one +/// input on which the two disagree. +fn get_locale(ty: char, thousands_sep: Option) -> (String, String, Vec) { + if ty == 'n' { + let (dec, sep, grouping) = crate::module::_locale::rlocale::numeric_formatting(); + return ( + String::from_utf8_lossy(&dec).into_owned(), + String::from_utf8_lossy(&sep).into_owned(), + grouping, + ); + } + match thousands_sep { + Some(separator) => { + let group = if matches!(ty, 'b' | 'o' | 'x' | 'X') { + 4 + } else { + 3 + }; + (".".to_string(), separator.to_string(), vec![group]) + } + None => (".".to_string(), String::new(), vec![0xFF]), + } +} + +/// `newformat.py:727-736 Formatter._fill_digits`. The separator is appended +/// ahead of the digits it precedes because `group_digits` reverses the whole +/// buffer once at the end. +/// +/// Whole chunks go in where upstream appends one character at a time: the +/// final reverse puts the chunks in order and leaves each chunk's own contents +/// alone, which is also what keeps a multi-byte separator intact. +fn fill_digits( + buf: &mut Vec, + digits: &str, + d_state: i32, + n_chars: i32, + n_zeros: i32, + thousands_sep: Option<&str>, +) { + if let Some(separator) = thousands_sep + && !separator.is_empty() + { + buf.push(separator.to_string()); + } + if n_chars > 0 { + let end = d_state as usize; + buf.push(digits[end - n_chars as usize..end].to_string()); + } + if n_zeros > 0 { + buf.push("0".repeat(n_zeros as usize)); + } +} + +/// `newformat.py:738-778 Formatter._group_digits`: separate `digits` at the +/// group sizes `loc_grouping` names, zero-filling out to `min_width` while +/// doing so. +/// +/// The zero fill belongs inside the grouping rather than after it: `0=` pads +/// to the width with digits, and those digits are separated like any other. +/// A group size past the end of the vector repeats the previous one, which is +/// how a C grouping string's last element means "and every group after this". +/// `0xFF` is the sentinel [`get_locale`] carries when nothing is to be +/// separated. A locale that terminates its own string with `CHAR_MAX` needs no +/// arm of its own — `final_grouping` clamps any size to the digits left, so a +/// group of 127 consumes the rest and the loop ends. +fn group_digits( + digits: &str, + mut min_width: i32, + mut left: i32, + loc_grouping: &[u8], + loc_thousands: &str, ) -> String { - let magnitude_len = magnitude.len() as i32; - let offset = (displayed_digits % (interval + 1) == 0) as i32; - let displayed_digits = displayed_digits + offset; - let padding = displayed_digits - magnitude_len; - let separator_count = displayed_digits / (interval + 1); - let zero_count = padding - separator_count; - if zero_count > 0 { - magnitude = format!("{}{magnitude}", "0".repeat(zero_count as usize)); + let mut buf: Vec = Vec::new(); + let mut grouping_state = 0usize; + let n_ts = loc_thousands.chars().count() as i32; + let mut need_separator = false; + let mut done = false; + let mut previous = 0i32; + loop { + let group = if grouping_state >= loc_grouping.len() { + previous + } else { + let group = i32::from(loc_grouping[grouping_state]); + if group == 0xFF { + break; + } + grouping_state += 1; + previous = group; + group + }; + let final_grouping = group.min(left.max(min_width.max(1))); + let n_zeros = (final_grouping - left).max(0); + let n_chars = left.min(final_grouping).max(0); + fill_digits( + &mut buf, + digits, + left, + n_chars, + n_zeros, + need_separator.then_some(loc_thousands), + ); + need_separator = true; + left -= n_chars; + min_width -= final_grouping; + if left <= 0 && min_width <= 0 { + done = true; + break; + } + min_width -= n_ts; + } + if !done { + let group = left.max(min_width).max(1); + let n_zeros = (group - left).max(0); + let n_chars = left.min(group).max(0); + fill_digits( + &mut buf, + digits, + left, + n_chars, + n_zeros, + need_separator.then_some(loc_thousands), + ); + } + buf.reverse(); + buf.concat() +} + +#[cfg(test)] +mod group_digits_tests { + use super::{get_locale, group_digits}; + + /// Every expectation is what `newformat.py:738-778 _group_digits` returns + /// for the same arguments, taken from the vendored source rather than from + /// a second implementation of the same rule. + #[track_caller] + fn check(digits: &str, min_width: i32, grouping: &[u8], separator: &str, expected: &str) { + let got = group_digits(digits, min_width, digits.len() as i32, grouping, separator); + assert_eq!(got, expected, "digits={digits:?} min_width={min_width}"); } - let separator_count = (magnitude.len() as i32 - 1) / interval; - let magnitude_len = magnitude.len() as i32; - for i in 1..=separator_count { - magnitude.insert((magnitude_len - interval * i) as usize, separator); + + #[test] + fn separates_at_the_group_size() { + check("1234", 0, &[3], ",", "1,234"); + check("123456789", 0, &[3], ",", "123,456,789"); + check("12345678", 0, &[4], "_", "1234_5678"); + } + + #[test] + fn zero_fills_to_min_width_inside_the_grouping() { + // The zero fill of a `0=` spec is separated like any other digit, so + // the result is longer than the width that asked for it — `format(1234, + // '012n')` is thirteen characters. + check("1234", 12, &[3], ",", "0,000,001,234"); + check("1234", 20, &[3], ",", "0,000,000,000,001,234"); + check("0", 12, &[3], ",", "0,000,000,000"); + check("7", 7, &[3], ",", "000,007"); + // A width the digits already exceed adds nothing. + check("1234567", 5, &[3], ",", "1,234,567"); + } + + #[test] + fn repeats_the_last_group_size_past_the_end_of_the_vector() { + check("123456789", 0, &[3, 2], ",", "12,34,56,789"); + } + + #[test] + fn stops_at_the_sentinel() { + check("123456789", 0, &[3, 0xFF], ",", "123456,789"); + check("123456789", 0, &[0xFF], ",", "123456789"); + } + + #[test] + fn keeps_a_multi_byte_separator_intact() { + // The buffer is reversed once at the end, so a separator wider than a + // byte has to travel as one chunk. + check("1234", 0, &[3], "\u{202f}", "1\u{202f}234"); + } + + #[test] + fn get_locale_sizes_the_group_by_presentation_code() { + for ty in ['b', 'o', 'x', 'X'] { + assert_eq!(get_locale(ty, Some('_')), (".".into(), "_".into(), vec![4])); + } + for ty in ['\0', 'd'] { + assert_eq!(get_locale(ty, Some(',')), (".".into(), ",".into(), vec![3])); + } + // No separator asked for: nothing is separated. + assert_eq!( + get_locale('d', None), + (".".into(), String::new(), vec![0xFF]) + ); } - magnitude } /// `FormatSpec::format_int`, with RPython rbigint as the value owner. @@ -2377,13 +2558,16 @@ fn format_rbigint(num: &BigInt, spec: &Wtf8, type_name: &str) -> Result (10, 3, false, ""), - 'b' => (2, 4, false, if p.alt_form { "0b" } else { "" }), - 'o' => (8, 4, false, if p.alt_form { "0o" } else { "" }), - 'x' => (16, 4, false, if p.alt_form { "0x" } else { "" }), - 'X' => (16, 4, true, if p.alt_form { "0X" } else { "" }), - 'n' => (10, 3, false, ""), + // The group size a separator implies is not part of the radix: `_get_locale` + // derives it from the presentation code, and for `'n'` it comes out of the + // locale instead. + let (radix, upper, prefix) = match p.ty { + '\0' | 'd' => (10, false, ""), + 'b' => (2, false, if p.alt_form { "0b" } else { "" }), + 'o' => (8, false, if p.alt_form { "0o" } else { "" }), + 'x' => (16, false, if p.alt_form { "0x" } else { "" }), + 'X' => (16, true, if p.alt_form { "0X" } else { "" }), + 'n' => (10, false, ""), other => { return Err(crate::PyError::value_error(format!( "Unknown format code '{}' for object of type '{type_name}'", @@ -2449,13 +2633,24 @@ fn format_rbigint(num: &BigInt, spec: &Wtf8, type_name: &str) -> Result'), p.width); + } if p.fill.to_char().is_none() { // A fill with no `char` cannot come back out of the `String`-typed // engine, so render the value unpadded and pad it by code point.