Skip to content

<regex>: Perform insertions into character class NFA node buffers when parsing of the character class completes - #6441

Open
Julian Müller (muellerj3) wants to merge 4 commits into
microsoft:mainfrom
muellerj2:regex-delay-insertion-into-char-class-buffers
Open

<regex>: Perform insertions into character class NFA node buffers when parsing of the character class completes#6441
Julian Müller (muellerj3) wants to merge 4 commits into
microsoft:mainfrom
muellerj2:regex-delay-insertion-into-char-class-buffers

Conversation

@muellerj3

Copy link
Copy Markdown
Contributor

The builder currently eagerly mutates a character class NFA node whenever it handles the next syntactic element in the character class. In some cases there is no downside to this approach, e.g., when a single encountered character is inserted into a bitmap. But there are other cases where this has drawbacks:

  • Whenever the parser encounters a (positive) named character class (e.g., \d or [:alpha:]), the builder updates the bitmap immediately. This means that when [\d[:alpha:]] is parsed, the builder evaluates the membership of a character in the bitmap range first for class \d and then for class [:alpha:]. If this evaluation did not happen eagerly, the builder could evaluate the combined class \d[:alpha:] by passing the or'ed character class masks to the isctype() function in the traits class, as is intended by [re.grammar/9].
  • When the parser encounters a large character (code point >= 0x100), a character range with a large character, a POSIX collating symbol or a POSIX equivalence in a character class, the builder eagerly inserts characters into buffers in the NFA node. This insertion strategy means that all of these character buffers must be dynamic arrays. This complicates the implementation of these character buffers, can waste some memory as long as the regex object exists, and can lead to more allocations if there are several character classes that insert into these buffers.

This PR moves all of these insertions to the point when the parser considers the character class complete. This means that a single pass is used to update the character bitmap for all positive named character classes (except for \w due to #5242), and character buffers in the node class are immediately filled with their full contents by copying from some temporary storage.

This delayed insertion into the character buffers of the node class will actually briefly deteriorate performance: This PR does nothing about the fact that these character buffers are very simple implementations of dynamic arrays that reallocate as if all elements are inserted one by one, so the number of allocations can only increase. But after this PR, the builder sets the whole and final content of _Buf buffers in a single insert call everywhere. This means that we can solve this problem by just pulling out the dynamic array implementation in a follow-up PR.

To store individual large characters, this PR reuses the existing _Chars string in the builder class. This string is also used for character sequences in a regex, but it wasn't used while parsing a character class previously.

In the other cases, this PR adds more strings or vectors of strings as temporary storage. This changes the layout of the _Builder3 and _Parser3 classes, but this is not an ABI break because there hasn't yet been a stable release with these classes.

The same temporary storage is used for all character classes. For this reason, the new tests check that content stored in these buffers doesn't leak to the following character class because the temporary storage wasn't reset correctly.

…hen parsing of the character class completes
@muellerj3
Julian Müller (muellerj3) requested a review from a team as a code owner September 9, 2026 08:49
Copilot AI balanced review requested due to automatic review settings September 9, 2026 08:49
@github-project-automation github-project-automation Bot moved this to Initial Review in STL Code Reviews Sep 9, 2026
@azure-pipelines

This comment was marked as resolved.

This comment was marked as resolved.

Copilot AI review requested due to automatic review settings September 9, 2026 08:59

This comment was marked as resolved.

Copilot AI review requested due to automatic review settings September 9, 2026 09:03

This comment was marked as resolved.

@muellerj3

Copy link
Copy Markdown
Contributor Author

Using the element length as a dense vector index makes one custom collating name of length N allocate N string_type objects, in addition to its N characters. The vector is retained, so _Finalize_class() also scans those N slots for every later character class. Since this path accepts lengths up to UINT_MAX, valid custom traits can trigger disproportionate memory use and repeated O(maximum-length) work. Store only populated (length, buffer) groups (for example, in a sparse sorted vector) instead.

The maximum length is actually restricted by the return values of lookup_collatename() in the traits class. Collating elements in actual languages are small. The longest collating element in a human language that I know of immediately has three characters.

The tests added for #6441 cover delayed storage for large characters, ranges, collating elements, and equivalence classes, but they do not exercise the newly delayed positive named-class path. Please add coverage for multiple positive classes in one bracket expression, including the char_class_type(-1)/\w branch for a character above 255; otherwise regressions in the mask aggregation and _Fl_class_cl_all_bits handling can pass while all of these tests remain green.

I think we probably have existing coverage for this, but I will check later.

Copilot AI review requested due to automatic review settings September 9, 2026 19:55
@muellerj3

Copy link
Copy Markdown
Contributor Author

It appears that we really didn't have any test coverage for combining two positive named character classes, except for the special case that combines \w with another character class. The test coverage for combining \w with another class was added by #5438.

This comment was marked as resolved.

@StephanTLavavej Stephan T. Lavavej (StephanTLavavej) added enhancement Something can be improved regex meow is a substring of homeowner labels Sep 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Something can be improved regex meow is a substring of homeowner

Projects

Status: Initial Review

Development

Successfully merging this pull request may close these issues.

3 participants