perf: narrow \d to ASCII [0-9] in metadata-matched regex patterns - #446
Merged
Conversation
.NET's \d (without RegexOptions.ECMAScript) matches the full Unicode
"decimal digit" category -- hundreds of non-ASCII digit characters -- which
is both a broader and more expensive character-class check than a plain
[0-9] range test, and a semantic divergence from upstream: Java's \d
defaults to ASCII-only ([0-9]) unless Pattern.UNICODE_CHARACTER_CLASS is
set, and these metadata patterns were authored/verified upstream against
that ASCII-only default.
Correctness trace (why this is safe): every metadata-derived pattern that
is actually matched against phone-number input -- nationalNumberPattern,
leadingDigits (territory- and format-level), internationalPrefix,
nationalPrefixForParsing, and a numberFormat's pattern attribute (national
and, separately, the copy read again for intlNumberFormat_) -- is only
ever matched, at run time, against a string PhoneNumberUtil has already
normalized to ASCII 0-9:
- PhoneNumberUtil.Normalize/NormalizeDigits and its callers
(MaybeStripInternationalPrefixAndNormalize, ParseHelper,
MaybeStripNationalPrefixAndCarrierCode) run before any metadata
pattern match in the parse path.
- IsValidNumber*, Format*, and ShortNumberInfo's checks all match
against GetNationalSignificantNumber(Impl), which is rebuilt from the
numeric NationalNumber field of an already-parsed PhoneNumber -- ASCII
by construction, never the raw input string.
- AsYouTypeFormatter accrues into `nationalNumber` exclusively through
NormalizeAndAccrueDigitsAndPlusSign, which normalizes each character
(including full-width digits) as it is typed, before any pattern match.
- PhoneNumberMatcher's post-candidate-extraction verification
(leniency.Verify) parses the candidate via ParseAndKeepRawInput first,
so it goes through the same normalization as any other Parse call.
PhoneNumberMatcher's free-text *candidate-scanning* patterns (matching
phone-number-shaped substrings in arbitrary raw text) are hand-written C#
constants in PhoneNumberUtil.cs/PhoneNumberMatcher.cs (ValidPhoneNumber,
ExtnPattern, and PhoneNumberMatcher's own GeneratedRegex patterns) -- not
sourced from BuildMetadataFromXml's XML parsing -- and are deliberately
left untouched; they must stay Unicode-digit-aware since they run before
normalization.
Only the four resource XML files' *replacement-role* fields
(nationalPrefixTransformRule, nationalPrefixFormattingRule,
carrierCodeFormattingRule, format/intlFormat text,
preferredInternationalPrefix, preferredExtnPrefix) are left going through
the original ValidateRE -- they're never matched against input, only used
as Regex.Replace templates, and (verified by inspection) never contain \d
in the shipped metadata anyway.
Implementation: NarrowDigitClassToAscii is a small regex-structure-aware
rewriter (tracks escape pairs and character-class entry/exit, including
the POSIX literal-']'-as-first-member idiom) rather than a blind string
replace: a bare \d becomes [0-9], but a \d already inside a class (e.g.
[\d-]) substitutes the bare 0-9 in place ([0-9-]), never the broken nested
[[0-9]-]. \D narrows to [^0-9] outside a class; inside a class it throws,
since it does not occur anywhere in the four shipped XML files today and
cannot in general be soundly unioned into an existing bracket expression.
ValidateAndNarrowPatternRE wraps this ahead of the existing ValidateRE
compile-check, wired in at exactly the six matched-as-pattern call sites
above; every other ValidateRE call site is untouched.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J8DATgGMQCmTpkg9dkdxks
…tations
Adds direct coverage of BuildMetadataFromXml.NarrowDigitClassToAscii
against real patterns pulled verbatim from resources/PhoneNumberMetadata.xml
(nationalNumberPattern, internationalPrefix, nationalPrefixForParsing,
numberFormat pattern), plus synthetic cases for shapes that don't currently
occur in the shipped metadata but the rewrite must still handle correctly:
\d already inside a character class (including with other members, a
leading '^', and the POSIX literal-']'-as-first-member idiom), an escaped
backslash immediately before a literal "d" (must not be read as \d), and
\D inside a class (must throw rather than silently emit something wrong).
Every narrowed pattern is also round-tripped through Regex's own
constructor, and one case additionally asserts the narrowed pattern
matches/rejects the same ASCII inputs as the original.
Also updates the handful of existing BuildMetadataFromXml/PhoneNumberUtil
tests that asserted a metadata pattern's literal string value (e.g.
"\\d{3}") to the now-narrowed "[0-9]{3}" -- these are metadata loaded via
the same BuildMetadataFromXml path production code uses, so the change in
expected literal text is the intended, correct effect of the narrowing,
not a regression.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J8DATgGMQCmTpkg9dkdxks
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #446 +/- ##
==========================================
+ Coverage 87.26% 87.31% +0.05%
==========================================
Files 41 41
Lines 3831 3871 +40
Branches 978 989 +11
==========================================
+ Hits 3343 3380 +37
- Misses 284 286 +2
- Partials 204 205 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Owner
Author
Local benchmark numbersThe automated performance comment only posts when
Generated by Claude Code |
This was referenced Sep 10, 2026
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
\dto[0-9]in every metadata-derived regex pattern that's actually matched against phone-number input: territoryleadingDigits,internationalPrefix,nationalPrefixForParsing,numberFormat'spatternattribute (national and theintlNumberFormat_copy), per-formatleadingDigits, andnationalNumberPattern.\d(withoutRegexOptions.ECMAScript) matches the full Unicode "decimal digit" category — hundreds of non-ASCII characters — which is both broader and more expensive to match than a plain[0-9]range check. Upstream Java's\ddefaults to ASCII-only ([0-9]) unlessPattern.UNICODE_CHARACTER_CLASSis set, and these patterns were authored/verified against that ASCII-only default, so this also closes a semantic gap from upstream, not just a perf one.PhoneNumberUtil.Normalize/NormalizeDigits(which converts fullwidth/other-script digits to ASCII) or againstGetNationalSignificantNumber, which is rebuilt from the numericNationalNumberproto field and is ASCII by construction. Traced through the parse path (MaybeStripInternationalPrefixAndNormalize,ParseHelper/MaybeStripNationalPrefixAndCarrierCode),AsYouTypeFormatter(NormalizeAndAccrueDigitsAndPlusSign), andPhoneNumberMatcher(ParseAndVerify/ParseAndKeepRawInput) — full detail in commita1204a2.PhoneNumberMatcher's free-text candidate-scanning regexes andPhoneNumberUtil's extension-parsing pattern are hand-written, not XML-metadata-derived, and are deliberately left untouched since they run on raw, un-normalized text.BuildMetadataFromXml.NarrowDigitClassToAscii) is structure-aware, not a blind string replace: it tracks escape pairs and character-class entry/exit so\dalready nested inside a class (e.g.[\d-]) narrows to[0-9-]rather than producing broken nested brackets, and handles the POSIX literal-]-as-first-member idiom.\Dnarrows to[^0-9]outside a class; inside a class it throws rather than silently mishandling it (doesn't occur in shipped metadata today — verified by grep across all four resource XML files).Test plan
dotnet build csharp— clean on all three TFMs (netstandard2.0, net8.0, net10.0), zero warnings underTreatWarningsAsErrors+ trim/AOT analyzersdotnet test csharp/PhoneNumbers.slnx— 434 + 37 tests pass on net8.0 and net10.0, including newTestNarrowDigitClassToAsciitheory tests covering real patterns fromresources/PhoneNumberMetadata.xml, the nested-class case,[^\d], POSIX-literal-bracket shapes, escaped-backslash-before-d, and a match-semantics-preservation checkdotnet run -c Release --framework net10.0 -- --filter "*PhoneNumberWorkflowBenchmark*"/*ColdStartBenchmark*— modest, directionally consistent steady-state win (~5-9% onParseOnly/ParseNationalFormat/ValidateOnly/FormatOnly, several exceeding their reported confidence margins), cold start flat as expected (this is a matching-cost change, not a JIT-cost change)