Skip to content

<regex>: regex_traits::transform_primary should yield primary sort keys appropriate for the imbued locale - #5444

Merged
Stephan T. Lavavej (StephanTLavavej) merged 15 commits into
microsoft:mainfrom
muellerj2:regex-transform_primary
May 10, 2025
Merged

Stephan T. Lavavej (StephanTLavavej) merged 15 commits into
microsoft:mainfrom
muellerj2:regex-transform_primary

Conversation

@muellerj2

@muellerj2 Julian Müller (muellerj2) commented Apr 26, 2025

Copy link
Copy Markdown

Fixes #5435. Fixes #5291.

The actual work is done in two new functions __std_regex_transform_primary_char/wchar_t, which are basically 1:1 copies of _Strxfrm() and _Wcsxfrm() but pass different flags to __crtLCMapStringA/W. I also took the liberty to correct the SAL annotations.

__crtLCMapStringA/W are declared in awint.hpp which includes yvals.h. I'm uncertain if this is the best approach, but I undefined _ENFORCE_ONLY_CORE_HEADERS so that awint.hpp can be included.

transform_primary has to check the types of the collate facets using RTTI, so I made the function always returns an empty string when dynamic RTTI is disabled/_CPPRTTI is undefined. The implementation itself is heavily based on collate::do_transform (including the change in #5431). It also needs access to the internals of collate, so I made _Regex_traits a friend of it.

There is a behavior change for the C locale: As I explained in more detail in #5435, the traits requirement in [re.req]/20 is actually misleading, since it is wrong for precisely one locale: the C locale (or the POSIX locale, see the collation order definition here: https://pubs.opengroup.org/onlinepubs/9799919799/basedefs/V1_chap07.html#tag_07_03_02_06). Since the equivalence classes are derived from POSIX and the definition of regex_traits::transform_primary also alludes to "primary sort keys" which indirectly reference terminology from the POSIX standard (https://pubs.opengroup.org/onlinepubs/9799919799/basedefs/V1_chap07.html#tag_07_03_02), I think we should do as POSIX says: "A" should not match [[=a=]].

This has consequences:

  • When I implemented <regex>: Properly parse and match collating symbols and equivalences #5392, I assumed [re.req]/20, so I didn't add any character translation using translate and translate_nocase when parsing equivalences. Now we have to add such logic in _Parser::_Do_ex_class2 to handle potentially case-sensitive sort keys when case-insensitive regexes are used (else "A" would even fail to match [[=A=]]).
  • A number of test cases (some of my own making) failed, because they all assumed that lower and upper case characters are equivalent in the C locale.

Since matching and parsing of equivalences no longer go through collate::transform, related tests no longer have to be skipped under IDL mismatch.

Comment thread stl/src/regex.cpp
@StephanTLavavej Stephan T. Lavavej (StephanTLavavej) added bug Something isn't working LWG Library Working Group issue regex meow is a substring of homeowner labels Apr 27, 2025
Comment thread tests/libcxx/expected_results.txt
Comment thread stl/src/regex.cpp
Comment thread stl/inc/locale Outdated
Comment thread stl/inc/regex
Comment thread stl/inc/regex Outdated
Comment thread stl/inc/regex Outdated
Comment thread stl/inc/regex
Comment thread stl/src/regex.cpp Outdated
Comment thread tests/std/tests/GH_005204_regex_collating_ranges/test.cpp Outdated
Comment thread tests/std/tests/GH_005204_regex_collating_ranges/test.cpp Outdated
Comment thread tests/libcxx/expected_results.txt
@StephanTLavavej

Copy link
Copy Markdown
Member

Thanks! 😻 I pushed some fixes, the most significant being INT_MAX, please double-check.

⚠️ Note to self: I'll need to perform MSVC-internal changes to add the new file regex.cpp, and I will likely need to push changes to deal with /clr:pure.

@StephanTLavavej

Copy link
Copy Markdown
Member

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

Stephan T. Lavavej (StephanTLavavej) added a commit to StephanTLavavej/STL that referenced this pull request May 9, 2025
@StephanTLavavej

Copy link
Copy Markdown
Member

As foretold by the ancient prophecy, I had to push a commit to fix /clr:pure.

@StephanTLavavej

Copy link
Copy Markdown
Member

I resolved a trivial adjacent-add conflict with #5437 in GH_005204_regex_collating_ranges.

@StephanTLavavej
Stephan T. Lavavej (StephanTLavavej) merged commit 91e4631 into microsoft:main May 10, 2025
@github-project-automation github-project-automation Bot moved this from Merging to Done in STL Code Reviews May 10, 2025
@StephanTLavavej

Copy link
Copy Markdown
Member

Thanks for implementing this new LWG issue and fixing this ancient bug! 🐈‍⬛ 💚 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working LWG Library Working Group issue regex meow is a substring of homeowner

Projects

None yet

5 participants