crates/vm/src/builtins/str.rs (2)

959-979: Prefer matches! over array .contains(&..) for small enum sets.

[NumericType::Decimal, NumericType::Digit, NumericType::Numeric].contains(&NumericType::for_char(c)) constructs a stack array and does a linear search on every character. matches! is more idiomatic, reads better, and the compiler lowers it to a single jump table / bitwise test. Same applies to isdigit on Line 977.

Also: the lingering comment on Line 974 ("python's isdigit also checks if exponents are digits, these are the unicode codepoints for exponents") referred to the old hardcoded superscript list; with NumericType::Digit handling those uniformly, the tail of that sentence is stale and could be trimmed.

♻️ Proposed cleanup

     #[pymethod]
     fn isnumeric(&self) -> bool {
         !self.data.is_empty()
             && self.char_all(|c| {
-                [
-                    NumericType::Decimal,
-                    NumericType::Digit,
-                    NumericType::Numeric,
-                ]
-                .contains(&NumericType::for_char(c))
+                matches!(
+                    NumericType::for_char(c),
+                    NumericType::Decimal | NumericType::Digit | NumericType::Numeric
+                )
             })
     }

     #[pymethod]
     fn isdigit(&self) -> bool {
-        // python's isdigit also checks if exponents are digits, these are the unicode codepoints for exponents
+        // Matches CPython: Numeric_Type=Digit or Numeric_Type=Decimal (covers superscripts, etc.).
         !self.data.is_empty()
             && self.char_all(|c| {
-                [NumericType::Digit, NumericType::Decimal].contains(&NumericType::for_char(c))
+                matches!(
+                    NumericType::for_char(c),
+                    NumericType::Digit | NumericType::Decimal
+                )
             })
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/builtins/str.rs` around lines 959 - 979, Replace the
small-array-based enum membership checks in the isnumeric and isdigit methods
with matches! pattern matching to avoid allocating/linear-search semantics: in
isnumeric (fn isnumeric) change the closure that currently uses
[NumericType::Decimal, NumericType::Digit,
NumericType::Numeric].contains(&NumericType::for_char(c)) to a matches! on
NumericType::for_char(c) matching Decimal | Digit | Numeric; similarly in
isdigit (fn isdigit) replace the [NumericType::Digit,
NumericType::Decimal].contains(&NumericType::for_char(c)) check with
matches!(NumericType::for_char(c), Digit | Decimal). Also trim the stale portion
of the comment inside isdigit that mentions explicit superscript codepoints so
the comment only documents that isdigit checks digit/exponent categories if
kept.

---

948-957: Consider consolidating isalnum to use GeneralCategoryGroup::Number for consistency.

GeneralCategoryGroup::Letter ∪ DecimalNumber + c.is_numeric() correctly covers L*, Nd, Nl, and No (matching CPython's requirements). However, this method mixes ICU and Rust stdlib classification, while sibling methods (isalpha, isdecimal, isnumeric) use ICU exclusively. GeneralCategoryGroup::Number (available in icu_properties v2) expands to exactly Nd | Nl | No, making the refactor below functionally equivalent while consolidating to a single source of truth:

♻️ Optional simplification

 fn isalnum(&self) -> bool {
     !self.data.is_empty()
         && self.char_all(|c| {
             GeneralCategoryGroup::Letter
-                .union(GeneralCategoryGroup::DecimalNumber)
+                .union(GeneralCategoryGroup::Number)
                 .contains(GeneralCategory::for_char(c))
-                || c.is_numeric()
         })
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/builtins/str.rs` around lines 948 - 957, The isalnum
implementation in the isalnum method should use ICU-only classification for
consistency: replace the current check that unions GeneralCategoryGroup::Letter
and GeneralCategoryGroup::DecimalNumber and also calls c.is_numeric() with a
single GeneralCategoryGroup::Number.contains(GeneralCategory::for_char(c))
combined with the existing Letter check (i.e., Letter ∪ Number) so char_all only
uses ICU properties; update the predicate inside str::isalnum (and keep the
!self.data.is_empty() guard) to use GeneralCategoryGroup::Number and remove the
c.is_numeric() call.

crates/sre_engine/src/string.rs (1)

444-454: Consider unifying numeric classification to ICU exclusively.

The || c.is_numeric() clause uses Rust's stdlib (which checks General_Category Nd|Nl|No), while the rest of the logic relies on ICU. Both approaches are functionally equivalent, but mixing classification sources makes invariants harder to reason about. For consistency with the ICU-based approach used elsewhere, replace DecimalNumber with Number in the union and remove the is_numeric() fallback:

Optional refactoring

-        .map(|c| {
-            GeneralCategoryGroup::Letter
-                .union(GeneralCategoryGroup::DecimalNumber)
-                .contains(GeneralCategory::for_char(c))
-                || c.is_numeric()
-        })
+        .map(|c| {
+            GeneralCategoryGroup::Letter
+                .union(GeneralCategoryGroup::Number)
+                .contains(GeneralCategory::for_char(c))
+        })

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/sre_engine/src/string.rs` around lines 444 - 454, The is_uni_alnum
function mixes stdlib numeric classification (c.is_numeric()) with ICU-based
checks; change the ICU union to use GeneralCategoryGroup::Number instead of
DecimalNumber and remove the c.is_numeric() fallback so all numeric
classification comes from ICU (update the union expression in is_uni_alnum and
eliminate the || c.is_numeric() branch).

crates/vm/src/builtins/str.rs (1)

949-977: Consider centralizing the character-class predicates.

The mappings look correct, but isalnum now encodes its own union while isnumeric, isdigit, and isalpha encode related rules separately. Small helpers would keep these Python semantics in lockstep and avoid repeated per-character set construction.

♻️ Proposed helper extraction

+    #[inline]
+    fn char_isalpha(c: char) -> bool {
+        GeneralCategoryGroup::Letter.contains(GeneralCategory::for_char(c))
+    }
+
+    #[inline]
+    fn char_isnumeric(c: char) -> bool {
+        matches!(
+            NumericType::for_char(c),
+            NumericType::Decimal | NumericType::Digit | NumericType::Numeric
+        )
+    }
+
+    #[inline]
+    fn char_isdigit(c: char) -> bool {
+        matches!(
+            NumericType::for_char(c),
+            NumericType::Decimal | NumericType::Digit
+        )
+    }
+
     #[pymethod]
     fn isalnum(&self) -> bool {
-        !self.data.is_empty()
-            && self.char_all(|c| {
-                GeneralCategoryGroup::Letter
-                    .union(GeneralCategoryGroup::Number)
-                    .contains(GeneralCategory::for_char(c))
-            })
+        !self.data.is_empty() && self.char_all(|c| Self::char_isalpha(c) || Self::char_isnumeric(c))
     }
 
     #[pymethod]
     fn isnumeric(&self) -> bool {
-        !self.data.is_empty()
-            && self.char_all(|c| {
-                [
-                    NumericType::Decimal,
-                    NumericType::Digit,
-                    NumericType::Numeric,
-                ]
-                .contains(&NumericType::for_char(c))
-            })
+        !self.data.is_empty() && self.char_all(Self::char_isnumeric)
     }
 
     #[pymethod]
     fn isdigit(&self) -> bool {
-        !self.data.is_empty()
-            && self.char_all(|c| {
-                [NumericType::Digit, NumericType::Decimal].contains(&NumericType::for_char(c))
-            })
+        !self.data.is_empty() && self.char_all(Self::char_isdigit)
     }
...
     #[pymethod]
     fn isalpha(&self) -> bool {
-        !self.data.is_empty()
-            && self
-                .char_all(|c| GeneralCategoryGroup::Letter.contains(GeneralCategory::for_char(c)))
+        !self.data.is_empty() && self.char_all(Self::char_isalpha)
     }

Also applies to: 1072-1077

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/builtins/str.rs` around lines 949 - 977, Centralize the
per-character predicates by extracting reusable helper functions (e.g.,
char_is_alnum, char_is_numeric, char_is_digit, char_is_alpha) and use those
inside the existing methods (isalnum, isnumeric, isdigit, isalpha) instead of
building arrays/unions inside each char_all closure; reference the existing
char_all method, NumericType::for_char, and
GeneralCategoryGroup/GeneralCategory::for_char when implementing the helpers so
the logic is shared and avoids repeated per-call set/array construction.