Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion Lib/test/test_format.py
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,6 @@ def test_non_ascii(self):
self.assertEqual(format(1+2j, "\u2007^8"), "\u2007(1+2j)\u2007")
self.assertEqual(format(0j, "\u2007^4"), "\u20070j\u2007")

@unittest.skip("TODO: RUSTPYTHON; formatting does not support locales. See https://github.com/RustPython/RustPython/issues/5181")
def test_locale(self):
try:
oldloc = locale.setlocale(locale.LC_ALL)
Expand Down
2 changes: 0 additions & 2 deletions Lib/test/test_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,6 @@ def test(i, format_spec, result):
test(123456, "1=20", '11111111111111123456')
test(123456, "*=20", '**************123456')

@unittest.expectedFailure # TODO: RUSTPYTHON; + 1234.57
@run_with_locale('LC_NUMERIC', 'en_US.UTF8', '')
def test_float__format__locale(self):
# test locale support for __format__ code 'n'
Expand All @@ -441,7 +440,6 @@ def test_float__format__locale(self):
self.assertEqual(locale.format_string('%g', x, grouping=True), format(x, 'n'))
self.assertEqual(locale.format_string('%.10g', x, grouping=True), format(x, '.10n'))

@unittest.expectedFailure # TODO: RUSTPYTHON; + 123456789012345678901234567890
@run_with_locale('LC_NUMERIC', 'en_US.UTF8', '')
def test_int__format__locale(self):
# test locale support for __format__ code 'n' for integers
Expand Down
196 changes: 196 additions & 0 deletions crates/common/src/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,19 @@ use rustpython_literal::format::Case;

use crate::wtf8::{CodePoint, Wtf8, Wtf8Buf};

/// Locale information for 'n' format specifier.
/// Contains thousands separator, decimal point, and grouping pattern
/// from the C library's `localeconv()`.
#[derive(Clone, Debug)]
pub struct LocaleInfo {
pub thousands_sep: String,
pub decimal_point: String,
/// Grouping pattern from `lconv.grouping`.
/// Each element is a group size. The last non-zero element repeats.
/// e.g. `[3, 0]` means groups of 3 repeating forever.
pub grouping: Vec<u8>,
}

trait FormatParse {
fn parse(text: &Wtf8) -> (Option<Self>, &Wtf8)
where
Expand Down Expand Up @@ -460,6 +473,189 @@ impl FormatSpec {
}
}

/// Returns true if this format spec uses the locale-aware 'n' format type.
pub fn has_locale_format(&self) -> bool {
matches!(self.format_type, Some(FormatType::Number(Case::Lower)))
}

/// Insert locale-aware thousands separators into an integer string.
/// Follows CPython's GroupGenerator logic for variable-width grouping.
fn insert_locale_grouping(int_part: &str, locale: &LocaleInfo) -> String {
if locale.grouping.is_empty() || locale.thousands_sep.is_empty() || int_part.len() <= 1 {
return int_part.to_string();
}

let mut group_idx = 0;
let mut group_size = locale.grouping[0] as usize;

if group_size == 0 {
return int_part.to_string();
}

// Collect groups of digits from right to left
let len = int_part.len();
let mut groups: Vec<&str> = Vec::new();
let mut pos = len;

loop {
if pos <= group_size {
groups.push(&int_part[..pos]);
break;
}

groups.push(&int_part[pos - group_size..pos]);
pos -= group_size;

// Advance to next group size
if group_idx + 1 < locale.grouping.len() {
let next = locale.grouping[group_idx + 1] as usize;
if next != 0 {
group_size = next;
group_idx += 1;
}
// 0 means repeat previous group size forever
}
}

// Groups were collected right-to-left, reverse to get left-to-right
groups.reverse();
groups.join(&locale.thousands_sep)
}

/// Apply locale-aware grouping and decimal point replacement to a formatted number.
fn apply_locale_formatting(magnitude_str: String, locale: &LocaleInfo) -> String {
let mut parts = magnitude_str.splitn(2, '.');
let int_part = parts.next().unwrap();
let grouped = Self::insert_locale_grouping(int_part, locale);

if let Some(frac_part) = parts.next() {
format!("{grouped}{}{frac_part}", locale.decimal_point)
} else {
grouped
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

/// Format an integer with locale-aware 'n' format.
pub fn format_int_locale(
&self,
num: &BigInt,
locale: &LocaleInfo,
) -> Result<String, FormatSpecError> {
self.validate_format(FormatType::Decimal)?;
let magnitude = num.abs();

let raw_magnitude_str = match self.format_type {
Some(FormatType::Number(Case::Lower)) => self.format_int_radix(magnitude, 10),
_ => return self.format_int(num),
}?;

let magnitude_str = Self::apply_locale_formatting(raw_magnitude_str, locale);

let format_sign = self.sign.unwrap_or(FormatSign::Minus);
let sign_str = match num.sign() {
Sign::Minus => "-",
_ => match format_sign {
FormatSign::Plus => "+",
FormatSign::Minus => "",
FormatSign::MinusOrSpace => " ",
},
};

self.format_sign_and_align(&AsciiStr::new(&magnitude_str), sign_str, FormatAlign::Right)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, let's look at the AsciiStr implementation
rg -n "impl.*AsciiStr" crates/common/src/format.rs -A 30 | head -100

Repository: RustPython/RustPython

Length of output: 1208


🏁 Script executed:

# Check the specific lines mentioned in the comment
sed -n '560,570p' crates/common/src/format.rs

Repository: RustPython/RustPython

Length of output: 362


🏁 Script executed:

# Also check lines 604 and 656
sed -n '600,610p' crates/common/src/format.rs
sed -n '652,662p' crates/common/src/format.rs

Repository: RustPython/RustPython

Length of output: 774


🏁 Script executed:

# Look for the char_len method definition
rg -n "fn char_len" crates/common/src/format.rs -A 5

Repository: RustPython/RustPython

Length of output: 305


🏁 Script executed:

# Check how locale separators are inserted into the magnitude_str
rg -n "thousands_sep\|decimal_point" crates/common/src/format.rs -B 2 -A 2

Repository: RustPython/RustPython

Length of output: 47


🏁 Script executed:

# Look for locale definition and separator types
rg -n "struct.*Locale\|decimal_point\|thousands_sep" crates/common/src/ -A 3

Repository: RustPython/RustPython

Length of output: 47


🏁 Script executed:

# Search for Locale struct and locale-related code
find crates -name "*.rs" | xargs grep -l "locale\|Locale" | head -5

Repository: RustPython/RustPython

Length of output: 204


🏁 Script executed:

# Search more broadly for where locale is used in format.rs
rg -n "locale\." crates/common/src/format.rs -B 2 -A 2

Repository: RustPython/RustPython

Length of output: 1144


🏁 Script executed:

# Look for how magnitude_str is constructed
rg -n "magnitude_str.*format\|let.*magnitude_str" crates/common/src/format.rs -B 2 -A 2 | head -50

Repository: RustPython/RustPython

Length of output: 47


🏁 Script executed:

# Find LocaleInfo definition
rg -n "struct LocaleInfo\|^pub struct.*Locale" crates/common/src/format.rs -A 10

Repository: RustPython/RustPython

Length of output: 47


🏁 Script executed:

# Check the complete insert_locale_grouping function
sed -n '482,535p' crates/common/src/format.rs

Repository: RustPython/RustPython

Length of output: 1970


🏁 Script executed:

# Look at the format_float_locale function to see how it constructs magnitude_str
sed -n '567,600p' crates/common/src/format.rs

Repository: RustPython/RustPython

Length of output: 1303


🏁 Script executed:

# Check the locale module to understand the locale structure
head -100 crates/stdlib/src/locale.rs

Repository: RustPython/RustPython

Length of output: 3356


🏁 Script executed:

# Find format_sign_and_align implementation
rg -n "fn format_sign_and_align" crates/common/src/format.rs -A 20

Repository: RustPython/RustPython

Length of output: 851


🏁 Script executed:

# Check how char_len is used in format_sign_and_align
sed -n '900,1000p' crates/common/src/format.rs | grep -n "char_len\|width"

Repository: RustPython/RustPython

Length of output: 184


🏁 Script executed:

# Find the complete format_sign_and_align function
rg -n "fn format_sign_and_align" crates/common/src/format.rs -A 30

Repository: RustPython/RustPython

Length of output: 1253


🏁 Script executed:

# Let's look for where LocaleInfo is defined
rg -n "struct LocaleInfo\|pub.*LocaleInfo" crates/common/src/ -A 5

Repository: RustPython/RustPython

Length of output: 47


🏁 Script executed:

# Check if there are other implementations of CharLen
rg -n "impl CharLen" crates/common/src/format.rs

Repository: RustPython/RustPython

Length of output: 101


🏁 Script executed:

# Let's look for a Unicode-aware version or other CharLen implementations
rg -n "impl.*CharLen" crates/ -A 3

Repository: RustPython/RustPython

Length of output: 1007


🏁 Script executed:

# Check if locales can actually have non-ASCII separators
rg -n "thousands_sep\|decimal_point" crates/stdlib/src/locale.rs -B 2 -A 2 | head -30

Repository: RustPython/RustPython

Length of output: 47


🏁 Script executed:

# Let me check the full context - look for where apply_locale_formatting is called and how its result is used
sed -n '567,610p' crates/common/src/format.rs

Repository: RustPython/RustPython

Length of output: 1569


🏁 Script executed:

# Let's verify that the magnitude_str can contain non-ASCII characters
# Check what types are used for thousands_sep and decimal_point
rg -n "pub struct LocaleInfo\|thousands_sep:\|decimal_point:" crates/common/src/ -A 2

Repository: RustPython/RustPython

Length of output: 47


🏁 Script executed:

# Let's trace through what happens - check if the string from apply_locale_formatting
# is used directly or if there's any normalization
sed -n '527,545p' crates/common/src/format.rs

Repository: RustPython/RustPython

Length of output: 687


🏁 Script executed:

# Check how sign_str.len() is used - does it account for multi-byte characters?
sed -n '985,990p' crates/common/src/format.rs

Repository: RustPython/RustPython

Length of output: 331


🏁 Script executed:

# Find the LocaleInfo struct definition
rg -n "^pub struct LocaleInfo\|^struct LocaleInfo" crates/common/src/format.rs -A 10

Repository: RustPython/RustPython

Length of output: 47


🏁 Script executed:

# Check where LocaleInfo is created/initialized
rg -n "LocaleInfo {" crates/common/src/format.rs -B 2 -A 5

Repository: RustPython/RustPython

Length of output: 152


🏁 Script executed:

# Let's trace how locale.thousands_sep is set - look at the locale module more carefully
sed -n '1,150p' crates/stdlib/src/locale.rs | grep -A 5 -B 5 "thousands_sep"

Repository: RustPython/RustPython

Length of output: 559


🏁 Script executed:

# Search for LocaleInfo definition differently
rg "LocaleInfo" crates/common/src/format.rs | head -5

Repository: RustPython/RustPython

Length of output: 312


🏁 Script executed:

# Get context around the first LocaleInfo reference
rg -n "LocaleInfo" crates/common/src/format.rs | head -1

Repository: RustPython/RustPython

Length of output: 91


🏁 Script executed:

# Look for where LocaleInfo is defined - broader search
rg -n "struct LocaleInfo" crates/ -A 10

Repository: RustPython/RustPython

Length of output: 772


🏁 Script executed:

# Now let's verify: check what sign_str can be and if it has the same byte-length issue
sed -n '590,605p' crates/common/src/format.rs

Repository: RustPython/RustPython

Length of output: 611


🏁 Script executed:

# Check the similar code in format_int_locale and format_complex_locale for the same issue
sed -n '620,650p' crates/common/src/format.rs

Repository: RustPython/RustPython

Length of output: 1283


🏁 Script executed:

# Let's also verify if there are other CharLen implementations that correctly handle Unicode
sed -n '1520,1535p' crates/vm/src/builtins/str.rs

Repository: RustPython/RustPython

Length of output: 429


🏁 Script executed:

# Final verification: check if French locale actually uses non-breaking space
python3 << 'EOF'
import locale
# Check what the French locale uses for thousands separator
# Note: this is for documentation purposes - we know from Unicode that:
# - \u{00A0} is non-breaking space (NBSP) - 2 bytes in UTF-8
# - French locale typically uses NBSP as thousands separator
# - This is confirmed by CPython behavior

print("UTF-8 byte length of non-breaking space: ", len("\u00A0".encode('utf-8')))
print("Character count of '1\u00A0234': ", len("1\u00A02 34"))
print("Byte length of '1\u00A0234': ", len("1\u00A0234".encode('utf-8')))
EOF

Repository: RustPython/RustPython

Length of output: 166


🏁 Script executed:

# More direct verification
python3 << 'EOF'
nbsp_string = "1\u00A0234"  # 1, NBSP, 2, 3, 4
print(f"String: {repr(nbsp_string)}")
print(f"Character count: {len(nbsp_string)}")
print(f"Byte count: {len(nbsp_string.encode('utf-8'))}")
print(f"Difference: {len(nbsp_string.encode('utf-8')) - len(nbsp_string)} bytes extra")
EOF

Repository: RustPython/RustPython

Length of output: 142


AsciiStr returns byte length instead of character count, causing incorrect width padding with non-ASCII locale separators.

In format_sign_and_align() (line 988), the width calculation uses magnitude_str.char_len() which AsciiStr implements to return self.inner.len() (byte length). However, magnitude_str from apply_locale_formatting() can contain multi-byte UTF-8 characters from locale separators—for example, French locales use non-breaking space (\u{00A0}, 2 bytes in UTF-8) as the thousands separator. This causes the fill character calculation to be off by 1+ bytes when formatting with width specs like "{:15n}".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/common/src/format.rs` at line 564, The padding is computed using
AsciiStr.char_len() (byte length), which miscounts multi-byte locale separators
in magnitude_str from apply_locale_formatting(); update format_sign_and_align
and its caller site so magnitude_str is passed/treated as a Unicode &str (not
AsciiStr) and compute visual length with a Unicode-aware count (e.g.,
s.chars().count() or UnicodeSegmentation::graphemes(true).count()) when
calculating width/fill; replace references to AsciiStr::new(&magnitude_str) and
char_len() in format_sign_and_align with the Unicode-aware length and adjust any
types/signatures that assumed AsciiStr accordingly.

}

/// Format a float with locale-aware 'n' format.
pub fn format_float_locale(
&self,
num: f64,
locale: &LocaleInfo,
) -> Result<String, FormatSpecError> {
self.validate_format(FormatType::FixedPoint(Case::Lower))?;
let precision = self.precision.unwrap_or(6);
let magnitude = num.abs();

let raw_magnitude_str = match &self.format_type {
Some(FormatType::Number(case)) => {
let precision = if precision == 0 { 1 } else { precision };
Ok(float::format_general(
precision,
magnitude,
*case,
self.alternate_form,
false,
))
}
_ => return self.format_float(num),
}?;

let magnitude_str = Self::apply_locale_formatting(raw_magnitude_str, locale);

let format_sign = self.sign.unwrap_or(FormatSign::Minus);
let sign_str = if num.is_sign_negative() && !num.is_nan() {
"-"
} else {
match format_sign {
FormatSign::Plus => "+",
FormatSign::Minus => "",
FormatSign::MinusOrSpace => " ",
}
};

self.format_sign_and_align(&AsciiStr::new(&magnitude_str), sign_str, FormatAlign::Right)
}

/// Format a complex number with locale-aware 'n' format.
pub fn format_complex_locale(
&self,
num: &Complex64,
locale: &LocaleInfo,
) -> Result<String, FormatSpecError> {
// Reuse format_complex_re_im with 'g' type to get the base formatted parts,
// then apply locale grouping. This matches CPython's format_complex_internal:
// 'n' → 'g', add_parens=0, skip_re=0.
let locale_spec = FormatSpec {
format_type: Some(FormatType::GeneralFormat(Case::Lower)),
..*self
};
let (formatted_re, formatted_im) = locale_spec.format_complex_re_im(num)?;

// Apply locale grouping to both parts
let grouped_re = if formatted_re.is_empty() {
formatted_re
} else {
// Split sign from magnitude, apply grouping, recombine
let (sign, mag) = if formatted_re.starts_with('-')
|| formatted_re.starts_with('+')
|| formatted_re.starts_with(' ')
{
formatted_re.split_at(1)
} else {
("", formatted_re.as_str())
};
format!(
"{sign}{}",
Self::apply_locale_formatting(mag.to_string(), locale)
)
};

// formatted_im is like "+1234j" or "-1234j" or "1234j"
// Split sign, magnitude, and 'j' suffix
let im_str = &formatted_im;
let (im_sign, im_rest) = if im_str.starts_with('+') || im_str.starts_with('-') {
im_str.split_at(1)
} else {
("", im_str.as_str())
};
let im_mag = im_rest.strip_suffix('j').unwrap_or(im_rest);
let im_grouped = Self::apply_locale_formatting(im_mag.to_string(), locale);
let grouped_im = format!("{im_sign}{im_grouped}j");

// No parentheses for 'n' format (CPython: add_parens=0)
let magnitude_str = format!("{grouped_re}{grouped_im}");

self.format_sign_and_align(&AsciiStr::new(&magnitude_str), "", FormatAlign::Right)
}
Comment on lines +607 to +657

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

format_complex_locale bypasses complex-format flag validation.

At Line 656, the locale path returns formatted output without enforcing the same checks as format_complex (Line 864-Line 870). = alignment and zero-padding should raise AlignmentFlag and ZeroPadding consistently.

💡 Proposed fix
     // No parentheses for 'n' format (CPython: add_parens=0)
     let magnitude_str = format!("{grouped_re}{grouped_im}");
-
-    self.format_sign_and_align(&AsciiStr::new(&magnitude_str), "", FormatAlign::Right)
+    if let Some(FormatAlign::AfterSign) = &self.align {
+        return Err(FormatSpecError::AlignmentFlag);
+    }
+    match &self.fill.unwrap_or(' '.into()).to_char() {
+        Some('0') => Err(FormatSpecError::ZeroPadding),
+        _ => self.format_sign_and_align(&AsciiStr::new(&magnitude_str), "", FormatAlign::Right),
+    }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/common/src/format.rs` around lines 607 - 657, format_complex_locale
skips the same complex-format flag validation performed in format_complex,
allowing invalid '=' alignment and zero-padding to pass; fix by enforcing the
same checks before formatting: in format_complex_locale (inside impl for
FormatSpec) replicate or call the same validation logic used by format_complex
to return Err(FormatSpecError::AlignmentFlag) when the alignment is '='
(FormatAlign::AfterSign) and Err(FormatSpecError::ZeroPadding) when
zero-fill/zero-padding rules are violated (e.g., fill == '0' with a sign-aware
width), or better yet extract that logic into a small helper (e.g.,
validate_complex_format_flags(&self) -> Result<(), FormatSpecError>) and invoke
it at the start of format_complex_locale so the later calls to
format_complex_re_im and format_sign_and_align behave consistently.


pub fn format_bool(&self, input: bool) -> Result<String, FormatSpecError> {
let x = u8::from(input);
match &self.format_type {
Expand Down
11 changes: 9 additions & 2 deletions crates/vm/src/builtins/complex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -321,8 +321,15 @@ impl PyComplex {
if spec.is_empty() {
return Ok(zelf.as_object().str(vm)?.as_wtf8().to_owned());
}
FormatSpec::parse(spec.as_str())
.and_then(|format_spec| format_spec.format_complex(&zelf.value))
let format_spec =
FormatSpec::parse(spec.as_str()).map_err(|err| err.into_pyexception(vm))?;
let result = if format_spec.has_locale_format() {
let locale = crate::format::get_locale_info();
format_spec.format_complex_locale(&zelf.value, &locale)
} else {
format_spec.format_complex(&zelf.value)
};
result
.map(Wtf8Buf::from_string)
.map_err(|err| err.into_pyexception(vm))
}
Expand Down
11 changes: 9 additions & 2 deletions crates/vm/src/builtins/float.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,8 +259,15 @@ impl PyFloat {
if spec.is_empty() {
return Ok(zelf.as_object().str(vm)?.as_wtf8().to_owned());
}
FormatSpec::parse(spec.as_str())
.and_then(|format_spec| format_spec.format_float(zelf.value))
let format_spec =
FormatSpec::parse(spec.as_str()).map_err(|err| err.into_pyexception(vm))?;
let result = if format_spec.has_locale_format() {
let locale = crate::format::get_locale_info();
format_spec.format_float_locale(zelf.value, &locale)
} else {
format_spec.format_float(zelf.value)
};
result
.map(Wtf8Buf::from_string)
.map_err(|err| err.into_pyexception(vm))
}
Expand Down
11 changes: 9 additions & 2 deletions crates/vm/src/builtins/int.rs
Original file line number Diff line number Diff line change
Expand Up @@ -499,8 +499,15 @@ impl PyInt {
if spec.is_empty() && !zelf.class().is(vm.ctx.types.int_type) {
return Ok(zelf.as_object().str(vm)?.as_wtf8().to_owned());
}
FormatSpec::parse(spec.as_str())
.and_then(|format_spec| format_spec.format_int(&zelf.value))
let format_spec =
FormatSpec::parse(spec.as_str()).map_err(|err| err.into_pyexception(vm))?;
let result = if format_spec.has_locale_format() {
let locale = crate::format::get_locale_info();
format_spec.format_int_locale(&zelf.value, &locale)
} else {
format_spec.format_int(&zelf.value)
};
result
.map(Wtf8Buf::from_string)
.map_err(|err| err.into_pyexception(vm))
}
Expand Down
58 changes: 58 additions & 0 deletions crates/vm/src/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,64 @@ use crate::{
use crate::common::format::*;
use crate::common::wtf8::{Wtf8, Wtf8Buf};

/// Get locale information from C `localeconv()` for the 'n' format specifier.
#[cfg(unix)]
pub(crate) fn get_locale_info() -> LocaleInfo {
use core::ffi::CStr;
unsafe {
let lc = libc::localeconv();
if lc.is_null() {
return LocaleInfo {
thousands_sep: String::new(),
decimal_point: ".".to_string(),
grouping: vec![],
};
}
let thousands_sep = CStr::from_ptr((*lc).thousands_sep)
.to_string_lossy()
.into_owned();
let decimal_point = CStr::from_ptr((*lc).decimal_point)
.to_string_lossy()
.into_owned();
let grouping = parse_grouping((*lc).grouping);
LocaleInfo {
thousands_sep,
decimal_point,
grouping,
}
}
}

#[cfg(not(unix))]
pub(crate) fn get_locale_info() -> LocaleInfo {
LocaleInfo {
thousands_sep: String::new(),
decimal_point: ".".to_string(),
grouping: vec![],
}
}

/// Parse C `lconv.grouping` into a `Vec<u8>`.
/// Reads bytes until 0 or CHAR_MAX, then appends 0 (meaning "repeat last group").
#[cfg(unix)]
unsafe fn parse_grouping(grouping: *const libc::c_char) -> Vec<u8> {
let mut result = Vec::new();
if grouping.is_null() {
return result;
}
unsafe {
let mut ptr = grouping;
while ![0, libc::c_char::MAX].contains(&*ptr) {
result.push(*ptr as u8);
ptr = ptr.add(1);
}
}
if !result.is_empty() {
result.push(0);
}
result
Comment on lines +49 to +67

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

C POSIX localeconv grouping lconv.grouping CHAR_MAX 0 terminator semantics

💡 Result:

In struct lconv (from localeconv()), grouping/mon_grouping is a char * that points to a sequence of byte values (often treated like an unsigned-byte array) describing how to group digits to the left of the radix point:

  • grouping[0] = size of the rightmost group; grouping[1] = next group to the left; etc. [1]
  • If an element is 0, it means repeat the previous group size for all remaining more-significant digits. Because 0 is also '\0', this is typically where the sequence ends (e.g. "\3\0" means “group by 3 forever”). [1]
  • If an element is CHAR_MAX, it means stop grouping: no further separators are inserted for any more-significant digits. [1]
  • If grouping is the empty string (""), that means no grouping information / no grouping (implementation/locale dependent, but common). [2]

Notes:

  • Some platforms use -1 (in a char) instead of CHAR_MAX due to char signedness; robust code often compares after casting to unsigned char. [3]

Sources: Linux locale(7) description of grouping semantics (including 0 and CHAR_MAX) [1]; NetBSD localeconv(3) notes on CHAR_MAX as “unavailable” in some fields/locales [2]; Gnulib portability note about -1 vs CHAR_MAX on some platforms [3].

[1] (man7.org) [2] (man.netbsd.org) [3] (gnu.org)

Citations:


🏁 Script executed:

# First, let's check if there are any clippy or format issues with the current implementation
cd crates/vm/src && head -70 format.rs | tail -25

Repository: RustPython/RustPython

Length of output: 674


🏁 Script executed:

cd crates/vm && grep -n "parse_grouping\|get_locale_info" src/format.rs | head -20

Repository: RustPython/RustPython

Length of output: 296


🏁 Script executed:

# Verify the suggested fix more carefully by checking the actual current state
cd crates/vm/src && cat -n format.rs | sed -n '49,68p'

Repository: RustPython/RustPython

Length of output: 761


parse_grouping conflates CHAR_MAX with repeat-last behavior, violating C/POSIX semantics.

When the C grouping sequence ends with CHAR_MAX, it means "stop grouping entirely". The current implementation appends 0 unconditionally, which incorrectly translates this to "repeat previous group forever". Additionally, the doc comment is misleading.

Suggested fix
 unsafe fn parse_grouping(grouping: *const libc::c_char) -> Vec<u8> {
     let mut result = Vec::new();
     if grouping.is_null() {
         return result;
     }
-    unsafe {
-        let mut ptr = grouping;
-        while ![0, libc::c_char::MAX].contains(&*ptr) {
-            result.push(*ptr as u8);
-            ptr = ptr.add(1);
-        }
-    }
-    if !result.is_empty() {
+    let mut ptr = grouping;
+    let mut repeat_last = false;
+    while !ptr.is_null() {
+        let v = *ptr;
+        if v == 0 {
+            repeat_last = true;
+            break;
+        }
+        if v == libc::c_char::MAX {
+            repeat_last = false;
+            break;
+        }
+        result.push(v as u8);
+        ptr = ptr.add(1);
+    }
+    if repeat_last && !result.is_empty() {
         result.push(0);
     }
     result
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/format.rs` around lines 49 - 67, parse_grouping currently
always appends a trailing 0 (meaning "repeat last") even when the C sequence
terminated with CHAR_MAX (meaning "stop grouping entirely"). Modify
parse_grouping to distinguish the terminator: iterate over grouping (pointer
grouping) until you reach 0 or libc::c_char::MAX, keep the bytes you read in
result, then if the terminator was a NUL (0) append a trailing 0 to indicate
"repeat last", but if the terminator was CHAR_MAX do not append anything (stop
grouping). Update the doc comment to reflect that CHAR_MAX means "stop grouping"
and NUL means "repeat last". Use the existing symbols parse_grouping, grouping,
result, ptr, libc::c_char::MAX and the checked terminator to implement this
behavior.

}

impl IntoPyException for FormatSpecError {
fn into_pyexception(self, vm: &VirtualMachine) -> PyBaseExceptionRef {
match self {
Expand Down
Loading