Skip to content

unicodedata: Fix Bidirectional, improve parser #8548

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
youknowone merged 1 commit into from
Aug 24, 2026
Merged

unicodedata: Fix Bidirectional, improve parser #8548

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 5 additions & 2 deletions Lib/test/test_unicodedata.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -306,9 +306,9 @@ def test_category(self):
self.assertRaises(TypeError, self.db.category)
self.assertRaises(TypeError, self.db.category, 'xx')

@unittest.expectedFailure # TODO: RUSTPYTHON; - 'BN' != ''
# NOTE: RUSTPYTHON; This test is from 3.15. See RustPython#8548 for motivation.
def test_bidirectional(self):
self.assertEqual(self.db.bidirectional('\uFFFE'), '')
self.assertEqual(self.db.bidirectional('\uFFFE'), '' if self.old else 'BN')
self.assertEqual(self.db.bidirectional(' '), 'WS')
self.assertEqual(self.db.bidirectional('A'), 'L')
self.assertEqual(self.db.bidirectional('\U00020000'), 'L')
Expand All @@ -329,6 +329,9 @@ def test_bidirectional(self):
# New in 16.0.0
self.assertEqual(self.db.bidirectional('\u0897'), '' if self.old else 'NSM')
self.assertEqual(self.db.bidirectional('\U0001fbef'), '' if self.old else 'ON')
# New in 17.0.0
self.assertEqual(self.db.bidirectional('\u088f'), '' if self.old else 'AL')
self.assertEqual(self.db.bidirectional('\U0001fbfa'), '' if self.old else 'ON')

self.assertRaises(TypeError, self.db.bidirectional)
self.assertRaises(TypeError, self.db.bidirectional, 'xx')
Expand Down
116 changes: 85 additions & 31 deletions crates/unicode/build.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ use std::{
thread,
};

use icu_properties::props::{EnumeratedProperty, GeneralCategory, NumericType};
use icu_properties::props::{
BidiClass, EnumeratedProperty, GeneralCategory, NamedEnumeratedProperty, NumericType,
};

fn generate_unicode_3_2() {
let path = PathBuf::from(env::var("OUT_DIR").unwrap())
Expand Down Expand Up @@ -80,17 +82,23 @@ fn generate_unicode_3_2() {
write_derived(
&base,
"DerivedBidiClass-3.2.0.txt",
"BIDI_CLASS",
"BIDI_CLASS_DIFF",
"(u32, u32, BidiClass)",
NonZeroUsize::new(1).unwrap(),
&mut writer,
|start, end, id, _| {
let id = parse_bidi(id);
if id != "BidiClass::LeftToRight" {
Some((start, end, id))
} else {
None
for i in start..=end {
let legacy = BidiClass::try_from_str(id.rsplit_once("::").unwrap().1)
.expect("Unicode data contains valid variants");
let modern = char::from_u32(i).map(BidiClass::for_char);

if Some(legacy) != modern {
return Some((start, end, id));
}
}

None
},
|writer, mut values| {
values.sort_unstable_by_key(|(start, _, _)| *start);
Expand Down Expand Up @@ -197,21 +205,86 @@ fn generate_numeric_type() {
);
}

/// Generate a compressed array of Unicode 3.2 membership.
///
/// Membership + diff checks is more efficient than storing the full table for 3.2. The logic is to
/// default to the latest Unicode if a character exists in 3.2 but isn't different. Membership
/// is needed because diffs aren't enough - a character may be absent in 3.2 which is different
/// than returning a default.
fn generate_membership_3_2() {
let path = Path::new(env!("CARGO_MANIFEST_DIR"))
.join("unicode")
.join("ucd32")
.join("UnicodeData-3.2.0.txt");
let reader = BufReader::new(File::open(path).unwrap());

// Parse membership from the first data. Unfortunately, this is largely uncompressed.
let mut membership_set = BTreeSet::new();
// TODO: Oh my, is this hacky and ugly...
let mut range_membership = Vec::new();
parse_unicode_3_2(
reader,
NonZeroUsize::new(1).unwrap(),
&mut io::empty(),
|start, end, value, _| {
if value.ends_with("First>") | value.ends_with("Last>") {
// Some lines (literally 20) are compressed ranges, so we have to handle those separately
range_membership.push(start);
} else {
membership_set.insert((start, end));
}

Option::<()>::None
},
|_writer, _values| {},
Comment thread
coderabbitai[bot] marked this conversation as resolved.
);

// Second pass. Compress the ranges.
let mut iter = membership_set.iter();
let &(mut start_prev, mut end_prev) = iter.next().unwrap();
let mut membership = Vec::new();

for &(start, end) in iter {
if start <= end_prev + 1 {
end_prev = end_prev.max(end);
} else {
membership.push((start_prev, end_prev));
start_prev = start;
end_prev = end;
}
}
membership.push((start_prev, end_prev));

let (chunks, &[]) = range_membership.as_chunks::<2>() else {
panic!("Range membership is always in pairs")
};
for &chunk in chunks {
membership.push(chunk.into());
}

membership.sort_unstable_by_key(|&(start, _)| start);

let path = PathBuf::from(env::var("OUT_DIR").unwrap())
.join("generated")
.join("membership_3_2.rs");
fs::create_dir_all(path.parent().unwrap()).unwrap();
let mut writer = BufWriter::new(File::create(&path).unwrap());

writeln!(writer, "static MEMBERSHIP_3_2: &[(u32, u32)] = &").unwrap();
write!(writer, "{membership:?};").unwrap();
}

fn generate_numeric_value() {
let path = PathBuf::from(env::var("OUT_DIR").unwrap())
.join("generated")
.join("unicode_numeric_value.rs");
fs::create_dir_all(path.parent().unwrap()).unwrap();
let mut writer = BufWriter::new(File::create(&path).unwrap());

// Ideally, this would store the diffs between the two tables. However, we need 3.2.0
// membership as well as different chars. The final tables are both smaller than storing the
// full 3.2.0 value table.
let ucd32 = Path::new(env!("CARGO_MANIFEST_DIR"))
.join("unicode")
.join("ucd32");
let mut ucd32_diffs = BTreeMap::new();
let mut ucd32_member = BTreeSet::new();
let numeric_32 =
BufReader::new(File::open(ucd32.join("DerivedNumericValues-3.2.0.txt")).unwrap());
parse_unicode_3_2(
Expand All @@ -223,7 +296,6 @@ fn generate_numeric_value() {
.parse()
.expect("Unicode data contains valid properties");
ucd32_diffs.insert((start, end), value);
ucd32_member.insert((start, end));
Option::<()>::None
},
|_writer, _values| {},
Expand Down Expand Up @@ -270,26 +342,6 @@ fn generate_numeric_value() {
write!(writer, "({start}, {end}, {value:?}),").unwrap();
}
writeln!(writer, "];").unwrap();

// Compress membership table
let mut iter = ucd32_member.iter();
let &(mut start_prev, mut end_prev) = iter.next().unwrap();
let mut membership = Vec::new();

for &(start, end) in iter {
if start <= end_prev + 1 {
end_prev = end_prev.max(end);
} else {
membership.push((start_prev, end_prev));
start_prev = start;
end_prev = end;
}
}
membership.push((start_prev, end_prev));
membership.sort_unstable_by_key(|&(start, _)| start);

writeln!(writer, "static NUMERIC_VAL_EXISTS_32: &[(u32, u32)] = &").unwrap();
write!(writer, "{membership:?};").unwrap();
}

fn generate_unicode_latest() {
Expand Down Expand Up @@ -602,10 +654,12 @@ fn main() {
println!("cargo:rerun-if-changed=unicode/latest");

let t_32 = thread::spawn(generate_unicode_3_2);
let t_32_membership = thread::spawn(generate_membership_3_2);
let t_numeric_type = thread::spawn(generate_numeric_type);
let t_numeric_value = thread::spawn(generate_numeric_value);
let t_latest = thread::spawn(generate_unicode_latest);
t_32.join().unwrap();
t_32_membership.join().unwrap();
t_numeric_type.join().unwrap();
t_numeric_value.join().unwrap();
t_latest.join().unwrap();
Expand Down
69 changes: 41 additions & 28 deletions crates/unicode/src/data.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use icu_properties::props::{
};
use rustpython_wtf8::CodePoint;

include!(concat!(env!("OUT_DIR"), "/generated/membership_3_2.rs"));
include!(concat!(env!("OUT_DIR"), "/generated/unicode_3_2.rs"));
include!(concat!(env!("OUT_DIR"), "/generated/unicode_latest.rs"));
include!(concat!(env!("OUT_DIR"), "/generated/unicode_num_type.rs"));
Expand Down Expand Up @@ -70,7 +71,7 @@ impl DecompositionType {
}
}

fn lookup_property<T: Copy>(table: &[(u32, u32, T)], ch: char) -> Option<T> {
fn lookup_table<T: Copy>(table: &[(u32, u32, T)], ch: char) -> Option<T> {
let ch = ch as u32;
table
.binary_search_by(|&(start, end, _)| {
Expand All @@ -86,25 +87,34 @@ fn lookup_property<T: Copy>(table: &[(u32, u32, T)], ch: char) -> Option<T> {
.map(|i| table[i].2)
}

fn lookup_numeric_val(ch: char, modern: bool) -> Option<f64> {
fn membership_3_2(ch: u32) -> bool {
MEMBERSHIP_3_2
.binary_search_by(|&(start, end)| {
if ch > end {
Ordering::Less
} else if ch < start {
Ordering::Greater
} else {
Ordering::Equal
}
})
.is_ok()
}

fn lookup_property_diff<T: Copy>(
table: &[(u32, u32, T)],
diff: &[(u32, u32, T)],
ch: char,
modern: bool,
) -> Option<T> {
if modern {
lookup_property(NUMERIC_VALUES, ch)
lookup_table(table, ch)
} else {
cold_path();
lookup_property(NUMERIC_VALUES_DIFF, ch).or_else(|| {
NUMERIC_VAL_EXISTS_32
.binary_search_by(|&(start, end)| {
let ch = ch as u32;
if ch > end {
Ordering::Less
} else if ch < start {
Ordering::Greater
} else {
Ordering::Equal
}
})
.ok()
.and_then(|_| lookup_property(NUMERIC_VALUES, ch))
lookup_table(diff, ch).or_else(|| {
membership_3_2(ch as u32)
.then(|| lookup_table(table, ch))
.flatten()
})
}
}
Expand Down Expand Up @@ -154,7 +164,7 @@ impl Ucd {
Some(GeneralCategory::for_char(c))
} else {
cold_path();
lookup_property(GENERAL_CATEGORY, c)
lookup_table(GENERAL_CATEGORY, c)
}
.unwrap_or(GeneralCategory::Unassigned)
.short_name()
Expand All @@ -168,11 +178,13 @@ impl Ucd {
Some(BidiClass::for_char(c))
} else {
cold_path();
lookup_property(BIDI_CLASS, c)
lookup_table(BIDI_CLASS_DIFF, c)
.or_else(|| membership_3_2(c as u32).then(|| BidiClass::for_char(c)))
}
})
.unwrap_or(BidiClass::LeftToRight)
.short_name()
.as_ref()
.map(BidiClass::short_name)
.unwrap_or_default()
}

#[must_use]
Expand All @@ -192,7 +204,7 @@ impl Ucd {
//
// Currently, this implementation is incomplete because I can't figure
// out what CPython is doing.
lookup_property(EAST_ASIAN_WIDTH, c)
lookup_table(EAST_ASIAN_WIDTH, c)
}
})
.unwrap_or(EastAsianWidth::Neutral)
Expand Down Expand Up @@ -230,7 +242,7 @@ impl Ucd {
Some(CanonicalCombiningClass::for_char(c))
} else {
cold_path();
lookup_property(COMBINING_CLASS, c)
lookup_table(COMBINING_CLASS, c)
}
})
.unwrap_or(CanonicalCombiningClass::NotReordered)
Expand Down Expand Up @@ -296,7 +308,7 @@ impl Ucd {
NumericType::for_char(ch)
} else {
cold_path();
lookup_property(NUMERIC_TYPE_DIFF, ch).unwrap_or_else(|| NumericType::for_char(ch))
lookup_table(NUMERIC_TYPE_DIFF, ch).unwrap_or_else(|| NumericType::for_char(ch))
};

expected.contains(&actual).then_some(ch)
Expand All @@ -307,7 +319,7 @@ impl Ucd {
pub fn digit(&self, c: CodePoint) -> Option<u64> {
let expected = [NumericType::Decimal, NumericType::Digit];
self.numeric_type_matches(c, &expected).and_then(|ch| {
let value = lookup_numeric_val(ch, true)?;
let value = lookup_property_diff(NUMERIC_VALUES, &[], ch, true)?;
let int = value as u64;
(int as f64 == value).then_some(int)
})
Expand All @@ -318,7 +330,7 @@ impl Ucd {
pub fn decimal(&self, c: CodePoint) -> Option<u64> {
let expected = [NumericType::Decimal];
self.numeric_type_matches(c, &expected).and_then(|ch| {
let value = lookup_numeric_val(ch, self.modern)?;
let value = lookup_property_diff(NUMERIC_VALUES, NUMERIC_VALUES_DIFF, ch, self.modern)?;
let int = value as u64;
(int as f64 == value).then_some(int)
})
Expand All @@ -328,8 +340,9 @@ impl Ucd {
#[must_use]
pub fn numeric(&self, c: CodePoint) -> Option<f64> {
let expected = &NumericType::ALL_VALUES[1..];
self.numeric_type_matches(c, expected)
.and_then(|ch| lookup_numeric_val(ch, self.modern))
self.numeric_type_matches(c, expected).and_then(|ch| {
lookup_property_diff(NUMERIC_VALUES, NUMERIC_VALUES_DIFF, ch, self.modern)
})
}

#[must_use]
Expand Down
Loading
Loading