Skip to content

General ascii fast paths for character classes #644

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.

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
merged 7 commits into from
Apr 4, 2023

Conversation

milseman
Copy link
Member

@milseman milseman commented Apr 3, 2023

When the portion of the string being matched is ASCII, use fast ASCII character class membership tests.

=== Regressions ======================================================================
- NotFoundAll                             7.12ms	7.03ms	89µs		1.3%
- EagarQuantWithTerminalWhole             2.64ms	2.62ms	18µs		0.7%
=== Improvements =====================================================================
- DiceRollsInTextAll                      50.3ms	64.9ms	-14.6ms		-22.5%
- EmailBuiltinCharacterClassAll           15.7ms	24.8ms	-9.06ms		-36.5%
- WordsAll                                14.8ms	22.5ms	-7.67ms		-34.1%
- BasicBuiltinCharacterClassAll           9.26ms	15.2ms	-5.96ms		-39.2%
- CompilerMessagesAll                     117ms	123ms	-5.91ms		-4.8%
- NumbersAll                              8.07ms	11.9ms	-3.87ms		-32.4%
- DiceNotation                            5.42ms	7.02ms	-1.6ms		-22.8%
- GraphemeBreakNoCapAll                   5.49ms	7ms	-1.51ms		-21.6%
- EmailRFCNoMatchesAll                    137ms	138ms	-1.16ms		-0.8%
- EmailRFCAll                             63ms	64ms	-999µs		-1.6%
- IntersectionCCC                         22.1ms	22.8ms	-671µs		-2.9%
- EmailLookaheadAll                       40.4ms	40.9ms	-487µs		-1.2%
- SubtractionCCC                          21.7ms	22.1ms	-403µs		-1.8%
- EmojiRegexAll                           73.4ms	73.8ms	-403µs		-0.5%
- InvertedCCC                             21.3ms	21.7ms	-351µs		-1.6%
- IPv4Address                             2.58ms	2.88ms	-304µs		-10.6%
- symDiffCCC                              49.4ms	49.7ms	-280µs		-0.6%
- AnchoredNotFoundWhole                   9.08ms	9.26ms	-182µs		-2.0%
- CssAll                                  3.84ms	4.02ms	-177µs		-4.4%
- CaseInsensitiveCCC                      11.9ms	12ms	-154µs		-1.3%
- BasicCCC                                10.7ms	10.8ms	-127µs		-1.2%
- HangulSyllableAll                       6.89ms	7.01ms	-121µs		-1.7%
- BasicRangeCCC                           11.1ms	11.3ms	-115µs		-1.0%
- EmailLookaheadNoMatchesAll              41.4ms	41.5ms	-109µs		-0.3%
- IPv6Address                             4.1ms	4.19ms	-82.7µs		-2.0%
- MACAddress                              3.05ms	3.11ms	-58.1µs		-1.9%
- LinesAll                                3.15ms	3.19ms	-40.1µs		-1.3%
- HangulSyllableFirst                     3.34ms	3.38ms	-38.7µs		-1.1%

@milseman milseman requested a review from natecook1000 April 3, 2023 16:44
@milseman
Copy link
Member Author

milseman commented Apr 3, 2023

Note: the times are post #642

Copy link
Member Author

@milseman milseman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comments to myself and @natecook1000

return (first: base, next: next, crLF: true)
}

return (first: base, next: next, crLF: false)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For all of this, when we successfully return we should assert the result matches what we would get from the character view

// TODO: bitvectors
switch cc {
case .any, .anyGrapheme, .anyScalar:
// TODO: should any scalar not consume CR-LF in scalar semantic mode?
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@natecook1000 could you check this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is okay as is, since anyScalar isn't actually supported. See #647.

if _isASCIINumber(asciiValue) {
return (next, true)
}
return (next, false)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return (next, _isASCIINumber...

Copy link
Member

@natecook1000 natecook1000 Apr 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you want to make this change?

Edit: This is in reply to your comment about changing this to return (next, isAscii...) — don't know why that isn't visible here.

}

case .whitespace:
switch asciiValue {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@natecook1000 can you add tests here as well? We probably need to back up for CR-LF

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests in #648

Copy link
Member

@natecook1000 natecook1000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good with your changes!

// TODO: bitvectors
switch cc {
case .any, .anyGrapheme, .anyScalar:
// TODO: should any scalar not consume CR-LF in scalar semantic mode?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is okay as is, since anyScalar isn't actually supported. See #647.

}

case .whitespace:
switch asciiValue {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests in #648

@milseman
Copy link
Member Author

milseman commented Apr 3, 2023

Switching coding convention/style to inline the quick-check and outline the slow path, along with recognizing the yes/no/maybe nature of quick checks, gives us further benefits. Added assertions (to check behavior parity) and added the first entry in the programmer's manual.

This change resulted in a robust improvement in EmailBuiltinCharacterClassAll (10% more shrinkage consistent across many runs), while other benchmarks were largely unaffected.

New overall results:

=== Regressions ======================================================================
- EmailRFCNoMatchesAll                    140ms	133ms	6.75ms		5.1%
- EmailRFCAll                             64.4ms	61.9ms	2.52ms		4.1%
- EmojiRegexAll                           73.7ms	71.2ms	2.47ms		3.5%
- symDiffCCC                              49.9ms	48.6ms	1.31ms		2.7%
- EmailLookaheadNoMatchesAll              41.7ms	40.4ms	1.3ms		3.2%
- EmailLookaheadAll                       40.7ms	39.5ms	1.16ms		2.9%
- InvertedCCC                             21.8ms	20.9ms	929µs		4.5%
- BasicRangeCCC                           11.4ms	11ms	373µs		3.4%
- ReluctantQuantWithTerminalWhole         9.42ms	9.09ms	333µs		3.7%
- BasicCCC                                11ms	10.6ms	330µs		3.1%
- EmailLookaheadList                      10ms	9.72ms	316µs		3.3%
- ReluctantQuantWhole                     14.2ms	13.8ms	305µs		2.2%
- CaseInsensitiveCCC                      12.1ms	11.9ms	223µs		1.9%
- AnchoredNotFoundWhole                   9.17ms	8.97ms	200µs		2.2%
- LiteralSearchAll                        6.78ms	6.58ms	199µs		3.0%
- IntersectionCCC                         22.3ms	22.1ms	195µs		0.9%
- SubtractionCCC                          21.8ms	21.6ms	189µs		0.9%
- NotFoundAll                             7.22ms	7.03ms	188µs		2.7%
- LiteralSearchNotFoundAll                6.56ms	6.4ms	156µs		2.4%
- IPv6Address                             4.14ms	3.98ms	151µs		3.8%
- HangulSyllableAll                       6.99ms	6.85ms	138µs		2.0%
- MACAddress                              3.07ms	2.96ms	106µs		3.6%
- EagarQuantWithTerminalWhole             2.67ms	2.56ms	104µs		4.1%
- LinesAll                                3.19ms	3.12ms	77.1µs		2.5%
- HangulSyllableFirst                     3.36ms	3.3ms	64µs		1.9%
=== Improvements =====================================================================
- DiceRollsInTextAll                      48.2ms	62.2ms	-14ms		-22.6%
- EmailBuiltinCharacterClassAll           13ms	24.6ms	-11.6ms		-47.0%
- WordsAll                                14.2ms	21.7ms	-7.47ms		-34.5%
- BasicBuiltinCharacterClassAll           8.73ms	14.6ms	-5.87ms		-40.2%
- NumbersAll                              7.67ms	11.5ms	-3.85ms		-33.4%
- DiceNotation                            5.21ms	6.73ms	-1.52ms		-22.6%
- GraphemeBreakNoCapAll                   5.35ms	6.75ms	-1.4ms		-20.7%
- CompilerMessagesAll                     117ms	119ms	-1.37ms		-1.2%
- IPv4Address                             2.52ms	2.76ms	-238µs		-8.6%
- CssAll                                  3.82ms	3.94ms	-120µs		-3.0%

@milseman milseman marked this pull request as draft April 3, 2023 21:09
@milseman
Copy link
Member Author

milseman commented Apr 3, 2023

Converting to draft as I still have a little more refactoring to do

@milseman milseman marked this pull request as ready for review April 4, 2023 00:22
@milseman
Copy link
Member Author

milseman commented Apr 4, 2023

@swift-ci please test

@milseman milseman force-pushed the general_ascii_fast_paths branch from 7cadebe to cf8fd52 Compare April 4, 2023 00:33
@milseman
Copy link
Member Author

milseman commented Apr 4, 2023

@swift-ci please test

Copy link
Member

@natecook1000 natecook1000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few small notes below, otherwise good to go!

Comment on lines 134 to 157
if case .definite(let result) = _quickMatchBuiltinCC(
cc,
in: input,
at: currentPosition,
isInverted: isInverted,
isStrictASCII: isStrictASCII,
isScalarSemantics: isScalarSemantics
) {
assert(result == _thoroughMatchBuiltinCC(
cc,
in: input,
at: currentPosition,
isInverted: isInverted,
isStrictASCII: isStrictASCII,
isScalarSemantics: isScalarSemantics))
return result
}
return _thoroughMatchBuiltinCC(
cc,
in: input,
at: currentPosition,
isInverted: isInverted,
isStrictASCII: isStrictASCII,
isScalarSemantics: isScalarSemantics)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Future] It would be great if we had a macro to expand into this pattern, since we have to essentially repeat the same function call three times. Something like:

return #quickMatch(
  _thoroughMatchBuiltinCC(
    cc,
    in: input,
    at: currentPosition,
    isInverted: isInverted,
    isStrictASCII: isStrictASCII,
    isScalarSemantics: isScalarSemantics))

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that would be nice. When I did the string re-gutting, I had some (always-inline) functions that took two different closures and applied this kind of pattern, but it turned too unwieldy and for the UTF-8 switch it ended up being better to just stamp the code out manually. A macro would be ideal here.

if _isASCIINumber(asciiValue) {
return (next, true)
}
return (next, false)
Copy link
Member

@natecook1000 natecook1000 Apr 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you want to make this change?

Edit: This is in reply to your comment about changing this to return (next, isAscii...) — don't know why that isn't visible here.

at: currentPosition,
isInverted: isInverted,
isStrictASCII: isStrictASCII,
isScalarSemantics: isScalarSemantics)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice 👍🏻

@@ -0,0 +1,143 @@

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For a new file, we need to:

  • include the Swift.org preamble
  • add it to CMakeLists.txt

@milseman
Copy link
Member Author

milseman commented Apr 4, 2023

I made an ASCII.swift file in the Unicode folder, and also put some of the quick checks on String directly so we could avoid @_effects(releasenone).

@swift-ci please test

@milseman milseman merged commit a7ba701 into swiftlang:main Apr 4, 2023
@milseman milseman deleted the general_ascii_fast_paths branch April 4, 2023 15:19
milseman added a commit that referenced this pull request Apr 5, 2023
* Atomically load the lowered program (#610)

Since we're atomically initializing the compiled program in
`Regex.Program`, we need to pair that with an atomic load.

Resolves #609.

* Add tests for line start/end word boundary diffs (#616)

The `default` and `simple` word boundaries have different behaviors
at the start and end of strings/lines. These tests validate that we
have the correct behavior implemented. Related to issue #613.

* Add tweaks for Android

* Fix documentation typo (#615)

* Fix abstract for Regex.dotMatchesNewlines(_:). (#614)

The old version looks like it was accidentally duplicated from
anchorsMatchLineEndings(_:) just below it.

* Remove `RegexConsumer` and fix its dependencies (#617)

* Remove `RegexConsumer` and fix its dependencies

This eliminates the RegexConsumer type and rewrites its users to call
through to other, existing functionality on Regex or in the Algorithms
implementations. RegexConsumer doesn't take account of the dual
subranges required for matching, so it can produce results that are
inconsistent with matches(of:) and ranges(of:), which were rewritten
earlier.

rdar://102841216

* Remove remaining from-end algorithm methods

This removes methods that are left over from when we were considering
from-end algorithms. These aren't tested and may not have the correct
semantics, so it's safer to remove them entirely.

* Improve StringProcessing and RegexBuilder documentation (#611)

This includes documentation improvements for core types/methods,
RegexBuilder types along with their generated variadic initializers,
and adds some curation. It also includes tests of the documentation
code samples.

* Set availability for inverted character class test (#621)

This feature depends on running with a Swift 5.7 stdlib, and fails
when that isn't available.

* Add type annotations in RegexBuilder tests

These changes work around a change to the way result builders are
compiled that removes the ability for result builder closure outputs
to affect the overload resolution elsewhere in an expression.

Workarounds for rdar://104881395 and rdar://104645543

* Workaround for fileprivate array issue

A recent compiler change results in fileprivate arrays sometimes
not keeping their buffers around long enough. This change avoids that
issue by removing the fileprivate annotations from the affected type.

* Fix an issue where named character classes weren't getting converted in the result builder. <rdar://104480703>

* Stop at end of search string in TwoWaySearcher (#631)

When searching for a substring that doesn't exist, it was possible
for TwoWaySearcher to advance beyond the end of the search string,
causing a crash. This change adds a `limitedBy:` parameter to that
index movement, avoiding the invalid movement.

Fixes rdar://105154010

* Correct misspelling in DSL renderer (#627)

vertial -> vertical

rdar://104602317

* Fix output type mismatch with RegexBuilder (#626)

Some regex literals (and presumably other `Regex` instances) lose
their output type information when used in a RegexBuilder closure due
to the way the concatenating builder calls are overloaded. In
particular, any output type with labeled tuples or where the sum of
tuple components in the accumulated and new output types is greater
than 10 will be ignored.

Regex internals don't make this distinction, however, so there ends up
being a mismatch between what a `Regex.Match` instance tries to
produce and the output type of the outermost regex. For example, this
code results in a crash, because `regex` is a `Regex<Substring>`
but the match tries to produce a `(Substring, number: Substring)`:

    let regex = Regex {
        ZeroOrMore(.whitespace)
        /:(?<number>\d+):/
        ZeroOrMore(.whitespace)
    }
    let match = try regex.wholeMatch(in: " :21: ")
    print(match!.output)

To fix this, we add a new `ignoreCapturesInTypedOutput` DSLTree node
to mark situations where the output type is discarded. This status
is propagated through the capture list into the match's storage,
which lets us produce the correct output type. Note that we can't just
drop the capture groups when building the compiled program because
(1) different parts of the regex might reference the capture group
and (2) all capture groups are available if a developer converts the
output to `AnyRegexOutput`.

    let anyOutput = AnyRegexOutput(match)
    // anyOutput[1] == "21"
    // anyOutput["number"] == Optional("21")

Fixes #625. rdar://104823356

Note: Linux seems to crash on different tests when the two customTest
overloads have `internal` visibility or are called. Switching one of the
functions to be generic over a RegexComponent works around the issue.

* Revert "Merge pull request #628 from apple/result_builder_changes_workaround"

This reverts commit 7e059b7, reversing
changes made to 3ca8b13.

* Use `some` syntax in variadics

This supports a type checker fix after the change in how result
builder closure parameters are type-checked.

* Type checker workaround: adjust test

* Further refactor to work around type checker regression

* Align availability macro with OS versions (#641)

* Speed up general character class matching (#642)

Short-circuit Character.isASCII checks inside built in character class matching.

Also, make benchmark try a few more times before giving up.

* Test for \s matching CRLF when scalar matching (#648)

* General ascii fast paths for character classes (#644)

General ASCII fast-paths for builtin character classes

* Remove the unsupported `anyScalar` case (#650)

We decided not to support the `anyScalar` character class, which would
match a single Unicode scalar regardless of matching mode. However,
its representation was still included in the various character class
types in the regex engine, leading to unreachable code and unclear
requirements when changing or adding new code. This change removes
that representation where possible.

The `DSLTree.Atom.CharacterClass` enum is left unchanged, since it
is marked `@_spi(RegexBuilder) public`. Any use of that enum case
is handled with a `fatalError("Unsupported")`, and it isn't produced
on any code path.

---------

Co-authored-by: Nate Cook <natecook@apple.com>
Co-authored-by: Butta <repo@butta.fastem.com>
Co-authored-by: Ole Begemann <ole@oleb.net>
Co-authored-by: Alex Martini <amartini@apple.com>
Co-authored-by: Alejandro Alonso <alejandro_alonso@apple.com>
Co-authored-by: David Ewing <dewing@apple.com>
Co-authored-by: Dave Ewing <96321608+DaveEwing@users.noreply.github.com>
milseman added a commit to milseman/swift-experimental-string-processing that referenced this pull request Apr 5, 2023
* Atomically load the lowered program (swiftlang#610)

Since we're atomically initializing the compiled program in
`Regex.Program`, we need to pair that with an atomic load.

Resolves swiftlang#609.

* Add tests for line start/end word boundary diffs (swiftlang#616)

The `default` and `simple` word boundaries have different behaviors
at the start and end of strings/lines. These tests validate that we
have the correct behavior implemented. Related to issue swiftlang#613.

* Add tweaks for Android

* Fix documentation typo (swiftlang#615)

* Fix abstract for Regex.dotMatchesNewlines(_:). (swiftlang#614)

The old version looks like it was accidentally duplicated from
anchorsMatchLineEndings(_:) just below it.

* Remove `RegexConsumer` and fix its dependencies (swiftlang#617)

* Remove `RegexConsumer` and fix its dependencies

This eliminates the RegexConsumer type and rewrites its users to call
through to other, existing functionality on Regex or in the Algorithms
implementations. RegexConsumer doesn't take account of the dual
subranges required for matching, so it can produce results that are
inconsistent with matches(of:) and ranges(of:), which were rewritten
earlier.

rdar://102841216

* Remove remaining from-end algorithm methods

This removes methods that are left over from when we were considering
from-end algorithms. These aren't tested and may not have the correct
semantics, so it's safer to remove them entirely.

* Improve StringProcessing and RegexBuilder documentation (swiftlang#611)

This includes documentation improvements for core types/methods,
RegexBuilder types along with their generated variadic initializers,
and adds some curation. It also includes tests of the documentation
code samples.

* Set availability for inverted character class test (swiftlang#621)

This feature depends on running with a Swift 5.7 stdlib, and fails
when that isn't available.

* Add type annotations in RegexBuilder tests

These changes work around a change to the way result builders are
compiled that removes the ability for result builder closure outputs
to affect the overload resolution elsewhere in an expression.

Workarounds for rdar://104881395 and rdar://104645543

* Workaround for fileprivate array issue

A recent compiler change results in fileprivate arrays sometimes
not keeping their buffers around long enough. This change avoids that
issue by removing the fileprivate annotations from the affected type.

* Fix an issue where named character classes weren't getting converted in the result builder. <rdar://104480703>

* Stop at end of search string in TwoWaySearcher (swiftlang#631)

When searching for a substring that doesn't exist, it was possible
for TwoWaySearcher to advance beyond the end of the search string,
causing a crash. This change adds a `limitedBy:` parameter to that
index movement, avoiding the invalid movement.

Fixes rdar://105154010

* Correct misspelling in DSL renderer (swiftlang#627)

vertial -> vertical

rdar://104602317

* Fix output type mismatch with RegexBuilder (swiftlang#626)

Some regex literals (and presumably other `Regex` instances) lose
their output type information when used in a RegexBuilder closure due
to the way the concatenating builder calls are overloaded. In
particular, any output type with labeled tuples or where the sum of
tuple components in the accumulated and new output types is greater
than 10 will be ignored.

Regex internals don't make this distinction, however, so there ends up
being a mismatch between what a `Regex.Match` instance tries to
produce and the output type of the outermost regex. For example, this
code results in a crash, because `regex` is a `Regex<Substring>`
but the match tries to produce a `(Substring, number: Substring)`:

    let regex = Regex {
        ZeroOrMore(.whitespace)
        /:(?<number>\d+):/
        ZeroOrMore(.whitespace)
    }
    let match = try regex.wholeMatch(in: " :21: ")
    print(match!.output)

To fix this, we add a new `ignoreCapturesInTypedOutput` DSLTree node
to mark situations where the output type is discarded. This status
is propagated through the capture list into the match's storage,
which lets us produce the correct output type. Note that we can't just
drop the capture groups when building the compiled program because
(1) different parts of the regex might reference the capture group
and (2) all capture groups are available if a developer converts the
output to `AnyRegexOutput`.

    let anyOutput = AnyRegexOutput(match)
    // anyOutput[1] == "21"
    // anyOutput["number"] == Optional("21")

Fixes swiftlang#625. rdar://104823356

Note: Linux seems to crash on different tests when the two customTest
overloads have `internal` visibility or are called. Switching one of the
functions to be generic over a RegexComponent works around the issue.

* Revert "Merge pull request swiftlang#628 from apple/result_builder_changes_workaround"

This reverts commit 7e059b7, reversing
changes made to 3ca8b13.

* Use `some` syntax in variadics

This supports a type checker fix after the change in how result
builder closure parameters are type-checked.

* Type checker workaround: adjust test

* Further refactor to work around type checker regression

* Align availability macro with OS versions (swiftlang#641)

* Speed up general character class matching (swiftlang#642)

Short-circuit Character.isASCII checks inside built in character class matching.

Also, make benchmark try a few more times before giving up.

* Test for \s matching CRLF when scalar matching (swiftlang#648)

* General ascii fast paths for character classes (swiftlang#644)

General ASCII fast-paths for builtin character classes

* Remove the unsupported `anyScalar` case (swiftlang#650)

We decided not to support the `anyScalar` character class, which would
match a single Unicode scalar regardless of matching mode. However,
its representation was still included in the various character class
types in the regex engine, leading to unreachable code and unclear
requirements when changing or adding new code. This change removes
that representation where possible.

The `DSLTree.Atom.CharacterClass` enum is left unchanged, since it
is marked `@_spi(RegexBuilder) public`. Any use of that enum case
is handled with a `fatalError("Unsupported")`, and it isn't produced
on any code path.

---------

Co-authored-by: Nate Cook <natecook@apple.com>
Co-authored-by: Butta <repo@butta.fastem.com>
Co-authored-by: Ole Begemann <ole@oleb.net>
Co-authored-by: Alex Martini <amartini@apple.com>
Co-authored-by: Alejandro Alonso <alejandro_alonso@apple.com>
Co-authored-by: David Ewing <dewing@apple.com>
Co-authored-by: Dave Ewing <96321608+DaveEwing@users.noreply.github.com>
milseman added a commit that referenced this pull request Apr 5, 2023
* Atomically load the lowered program (#610)

Since we're atomically initializing the compiled program in
`Regex.Program`, we need to pair that with an atomic load.

Resolves #609.

* Add tests for line start/end word boundary diffs (#616)

The `default` and `simple` word boundaries have different behaviors
at the start and end of strings/lines. These tests validate that we
have the correct behavior implemented. Related to issue #613.

* Add tweaks for Android

* Fix documentation typo (#615)

* Fix abstract for Regex.dotMatchesNewlines(_:). (#614)

The old version looks like it was accidentally duplicated from
anchorsMatchLineEndings(_:) just below it.

* Remove `RegexConsumer` and fix its dependencies (#617)

* Remove `RegexConsumer` and fix its dependencies

This eliminates the RegexConsumer type and rewrites its users to call
through to other, existing functionality on Regex or in the Algorithms
implementations. RegexConsumer doesn't take account of the dual
subranges required for matching, so it can produce results that are
inconsistent with matches(of:) and ranges(of:), which were rewritten
earlier.

rdar://102841216

* Remove remaining from-end algorithm methods

This removes methods that are left over from when we were considering
from-end algorithms. These aren't tested and may not have the correct
semantics, so it's safer to remove them entirely.

* Improve StringProcessing and RegexBuilder documentation (#611)

This includes documentation improvements for core types/methods,
RegexBuilder types along with their generated variadic initializers,
and adds some curation. It also includes tests of the documentation
code samples.

* Set availability for inverted character class test (#621)

This feature depends on running with a Swift 5.7 stdlib, and fails
when that isn't available.

* Add type annotations in RegexBuilder tests

These changes work around a change to the way result builders are
compiled that removes the ability for result builder closure outputs
to affect the overload resolution elsewhere in an expression.

Workarounds for rdar://104881395 and rdar://104645543

* Workaround for fileprivate array issue

A recent compiler change results in fileprivate arrays sometimes
not keeping their buffers around long enough. This change avoids that
issue by removing the fileprivate annotations from the affected type.

* Fix an issue where named character classes weren't getting converted in the result builder. <rdar://104480703>

* Stop at end of search string in TwoWaySearcher (#631)

When searching for a substring that doesn't exist, it was possible
for TwoWaySearcher to advance beyond the end of the search string,
causing a crash. This change adds a `limitedBy:` parameter to that
index movement, avoiding the invalid movement.

Fixes rdar://105154010

* Correct misspelling in DSL renderer (#627)

vertial -> vertical

rdar://104602317

* Fix output type mismatch with RegexBuilder (#626)

Some regex literals (and presumably other `Regex` instances) lose
their output type information when used in a RegexBuilder closure due
to the way the concatenating builder calls are overloaded. In
particular, any output type with labeled tuples or where the sum of
tuple components in the accumulated and new output types is greater
than 10 will be ignored.

Regex internals don't make this distinction, however, so there ends up
being a mismatch between what a `Regex.Match` instance tries to
produce and the output type of the outermost regex. For example, this
code results in a crash, because `regex` is a `Regex<Substring>`
but the match tries to produce a `(Substring, number: Substring)`:

    let regex = Regex {
        ZeroOrMore(.whitespace)
        /:(?<number>\d+):/
        ZeroOrMore(.whitespace)
    }
    let match = try regex.wholeMatch(in: " :21: ")
    print(match!.output)

To fix this, we add a new `ignoreCapturesInTypedOutput` DSLTree node
to mark situations where the output type is discarded. This status
is propagated through the capture list into the match's storage,
which lets us produce the correct output type. Note that we can't just
drop the capture groups when building the compiled program because
(1) different parts of the regex might reference the capture group
and (2) all capture groups are available if a developer converts the
output to `AnyRegexOutput`.

    let anyOutput = AnyRegexOutput(match)
    // anyOutput[1] == "21"
    // anyOutput["number"] == Optional("21")

Fixes #625. rdar://104823356

Note: Linux seems to crash on different tests when the two customTest
overloads have `internal` visibility or are called. Switching one of the
functions to be generic over a RegexComponent works around the issue.

* Revert "Merge pull request #628 from apple/result_builder_changes_workaround"

This reverts commit 7e059b7, reversing
changes made to 3ca8b13.

* Use `some` syntax in variadics

This supports a type checker fix after the change in how result
builder closure parameters are type-checked.

* Type checker workaround: adjust test

* Further refactor to work around type checker regression

* Align availability macro with OS versions (#641)

* Speed up general character class matching (#642)

Short-circuit Character.isASCII checks inside built in character class matching.

Also, make benchmark try a few more times before giving up.

* Test for \s matching CRLF when scalar matching (#648)

* General ascii fast paths for character classes (#644)

General ASCII fast-paths for builtin character classes

* Remove the unsupported `anyScalar` case (#650)

We decided not to support the `anyScalar` character class, which would
match a single Unicode scalar regardless of matching mode. However,
its representation was still included in the various character class
types in the regex engine, leading to unreachable code and unclear
requirements when changing or adding new code. This change removes
that representation where possible.

The `DSLTree.Atom.CharacterClass` enum is left unchanged, since it
is marked `@_spi(RegexBuilder) public`. Any use of that enum case
is handled with a `fatalError("Unsupported")`, and it isn't produced
on any code path.

---------

Co-authored-by: Nate Cook <natecook@apple.com>
Co-authored-by: Butta <repo@butta.fastem.com>
Co-authored-by: Ole Begemann <ole@oleb.net>
Co-authored-by: Alex Martini <amartini@apple.com>
Co-authored-by: Alejandro Alonso <alejandro_alonso@apple.com>
Co-authored-by: David Ewing <dewing@apple.com>
Co-authored-by: Dave Ewing <96321608+DaveEwing@users.noreply.github.com>
milseman added a commit that referenced this pull request May 25, 2023
* Atomically load the lowered program (#610)

Since we're atomically initializing the compiled program in
`Regex.Program`, we need to pair that with an atomic load.

Resolves #609.

* Add tests for line start/end word boundary diffs (#616)

The `default` and `simple` word boundaries have different behaviors
at the start and end of strings/lines. These tests validate that we
have the correct behavior implemented. Related to issue #613.

* Add tweaks for Android

* Fix documentation typo (#615)

* Fix abstract for Regex.dotMatchesNewlines(_:). (#614)

The old version looks like it was accidentally duplicated from
anchorsMatchLineEndings(_:) just below it.

* Remove `RegexConsumer` and fix its dependencies (#617)

* Remove `RegexConsumer` and fix its dependencies

This eliminates the RegexConsumer type and rewrites its users to call
through to other, existing functionality on Regex or in the Algorithms
implementations. RegexConsumer doesn't take account of the dual
subranges required for matching, so it can produce results that are
inconsistent with matches(of:) and ranges(of:), which were rewritten
earlier.

rdar://102841216

* Remove remaining from-end algorithm methods

This removes methods that are left over from when we were considering
from-end algorithms. These aren't tested and may not have the correct
semantics, so it's safer to remove them entirely.

* Improve StringProcessing and RegexBuilder documentation (#611)

This includes documentation improvements for core types/methods,
RegexBuilder types along with their generated variadic initializers,
and adds some curation. It also includes tests of the documentation
code samples.

* Set availability for inverted character class test (#621)

This feature depends on running with a Swift 5.7 stdlib, and fails
when that isn't available.

* Add type annotations in RegexBuilder tests

These changes work around a change to the way result builders are
compiled that removes the ability for result builder closure outputs
to affect the overload resolution elsewhere in an expression.

Workarounds for rdar://104881395 and rdar://104645543

* Workaround for fileprivate array issue

A recent compiler change results in fileprivate arrays sometimes
not keeping their buffers around long enough. This change avoids that
issue by removing the fileprivate annotations from the affected type.

* Fix an issue where named character classes weren't getting converted in the result builder. <rdar://104480703>

* Stop at end of search string in TwoWaySearcher (#631)

When searching for a substring that doesn't exist, it was possible
for TwoWaySearcher to advance beyond the end of the search string,
causing a crash. This change adds a `limitedBy:` parameter to that
index movement, avoiding the invalid movement.

Fixes rdar://105154010

* Correct misspelling in DSL renderer (#627)

vertial -> vertical

rdar://104602317

* Fix output type mismatch with RegexBuilder (#626)

Some regex literals (and presumably other `Regex` instances) lose
their output type information when used in a RegexBuilder closure due
to the way the concatenating builder calls are overloaded. In
particular, any output type with labeled tuples or where the sum of
tuple components in the accumulated and new output types is greater
than 10 will be ignored.

Regex internals don't make this distinction, however, so there ends up
being a mismatch between what a `Regex.Match` instance tries to
produce and the output type of the outermost regex. For example, this
code results in a crash, because `regex` is a `Regex<Substring>`
but the match tries to produce a `(Substring, number: Substring)`:

    let regex = Regex {
        ZeroOrMore(.whitespace)
        /:(?<number>\d+):/
        ZeroOrMore(.whitespace)
    }
    let match = try regex.wholeMatch(in: " :21: ")
    print(match!.output)

To fix this, we add a new `ignoreCapturesInTypedOutput` DSLTree node
to mark situations where the output type is discarded. This status
is propagated through the capture list into the match's storage,
which lets us produce the correct output type. Note that we can't just
drop the capture groups when building the compiled program because
(1) different parts of the regex might reference the capture group
and (2) all capture groups are available if a developer converts the
output to `AnyRegexOutput`.

    let anyOutput = AnyRegexOutput(match)
    // anyOutput[1] == "21"
    // anyOutput["number"] == Optional("21")

Fixes #625. rdar://104823356

Note: Linux seems to crash on different tests when the two customTest
overloads have `internal` visibility or are called. Switching one of the
functions to be generic over a RegexComponent works around the issue.

* Revert "Merge pull request #628 from apple/result_builder_changes_workaround"

This reverts commit 7e059b7, reversing
changes made to 3ca8b13.

* Use `some` syntax in variadics

This supports a type checker fix after the change in how result
builder closure parameters are type-checked.

* Type checker workaround: adjust test

* Further refactor to work around type checker regression

* Align availability macro with OS versions (#641)

* Speed up general character class matching (#642)

Short-circuit Character.isASCII checks inside built in character class matching.

Also, make benchmark try a few more times before giving up.

* Test for \s matching CRLF when scalar matching (#648)

* General ascii fast paths for character classes (#644)

General ASCII fast-paths for builtin character classes

* Remove the unsupported `anyScalar` case (#650)

We decided not to support the `anyScalar` character class, which would
match a single Unicode scalar regardless of matching mode. However,
its representation was still included in the various character class
types in the regex engine, leading to unreachable code and unclear
requirements when changing or adding new code. This change removes
that representation where possible.

The `DSLTree.Atom.CharacterClass` enum is left unchanged, since it
is marked `@_spi(RegexBuilder) public`. Any use of that enum case
is handled with a `fatalError("Unsupported")`, and it isn't produced
on any code path.

* Fix range-based quantification fast path (#653)

The fast path for quantification incorrectly discards the last save
position when the quantification used up all possible trips, which is
only possible with range-based quantifications (e.g. `{0,3}`). This
bug shows up when a range-based quantifier matches the maximum - 1
repetitions of the preceding pattern.

For example, the regex `/a{0,2}a/` should succeed as a full match any
of the strings "aa", "aaa", or "aaaa". However, the pattern fails
to match "aaa", since the save point allowing a single "a" to match
the first `a{0,2}` part of the regex is discarded.

This change only discards the last save position when advancing the
quantifier fails due to a failure to match, not maxing out the number
of trips.

* Add in ASCII fast-path for anyNonNewline (#654)

* Avoid long expression type checks (#657)

These changes remove several seconds of type-checking time from the
RegexBuilder test cases, bringing all expressions under 150ms (on
the tested computer).

* Processor cleanup (#655)

Clean up and refactor the processor

* Simplify instruction fetching

* Refactor metrics out, and void their storage in release builds

*Put operations onto String

* Fix `firstRange(of:)` search (#656)

Calls to `ranges(of:)` and `firstRange(of:)` with a string parameter
actually use two different string searching algorithms. `ranges(of:)`
uses the "z-searcher" algorithm, while `firstRange(of:)` uses a
two-way search. Since it's better to align on a single path for these
searches, the z-searcher has lower requirements, and the two-way
search implementation has a correctness bug, this change removes
the two-way search algorithm and uses z-search for `firstRange(of:)`.

The correctness bug in `firstRange(of:)` appears only when searching
for the second (or later) occurrence of a substring, which you have
to be fairly deliberate about. In the example below, the substring
at offsets `7..<12` is missed:

    let text = "ADACBADADACBADACB"
    //          =====  -----=====
    let pattern = "ADACB"
    let firstRange = text.firstRange(of: pattern)!
    // firstRange ~= 0..<5
    let secondRange = text[firstRange.upperBound...].firstRange(of: pattern)!
    // secondRange ~= 12..<17

This change also removes some unrelated, unused code in Split.swift,
in addition to removing an (unused) usage of `TwoWaySearcher`.

rdar://92794248

* Bug fix and hot path for quantified `.` (#658)

Bug fix in newline hot path, and apply hot path to quantified dot

* Run scalar-semantic benchmark variants (#659)

Run scalar semantic benchmarks

* Refactor operations to be on String (#664)

Finish refactoring logic onto String

* Provide unique generic method parameter names (#669)

This is getting warned on in the 5.9 compiler, will be an error
starting in Swift 6.

* Enable quantification optimizations for scalar semantics (#671)

*  Quantified scalar semantic matching

* Remove redundant test

---------

Co-authored-by: Nate Cook <natecook@apple.com>
Co-authored-by: Butta <repo@butta.fastem.com>
Co-authored-by: Ole Begemann <ole@oleb.net>
Co-authored-by: Alex Martini <amartini@apple.com>
Co-authored-by: Alejandro Alonso <alejandro_alonso@apple.com>
Co-authored-by: David Ewing <dewing@apple.com>
Co-authored-by: Dave Ewing <96321608+DaveEwing@users.noreply.github.com>
milseman added a commit to milseman/swift-experimental-string-processing that referenced this pull request May 25, 2023
* Atomically load the lowered program (swiftlang#610)

Since we're atomically initializing the compiled program in
`Regex.Program`, we need to pair that with an atomic load.

Resolves swiftlang#609.

* Add tests for line start/end word boundary diffs (swiftlang#616)

The `default` and `simple` word boundaries have different behaviors
at the start and end of strings/lines. These tests validate that we
have the correct behavior implemented. Related to issue swiftlang#613.

* Add tweaks for Android

* Fix documentation typo (swiftlang#615)

* Fix abstract for Regex.dotMatchesNewlines(_:). (swiftlang#614)

The old version looks like it was accidentally duplicated from
anchorsMatchLineEndings(_:) just below it.

* Remove `RegexConsumer` and fix its dependencies (swiftlang#617)

* Remove `RegexConsumer` and fix its dependencies

This eliminates the RegexConsumer type and rewrites its users to call
through to other, existing functionality on Regex or in the Algorithms
implementations. RegexConsumer doesn't take account of the dual
subranges required for matching, so it can produce results that are
inconsistent with matches(of:) and ranges(of:), which were rewritten
earlier.

rdar://102841216

* Remove remaining from-end algorithm methods

This removes methods that are left over from when we were considering
from-end algorithms. These aren't tested and may not have the correct
semantics, so it's safer to remove them entirely.

* Improve StringProcessing and RegexBuilder documentation (swiftlang#611)

This includes documentation improvements for core types/methods,
RegexBuilder types along with their generated variadic initializers,
and adds some curation. It also includes tests of the documentation
code samples.

* Set availability for inverted character class test (swiftlang#621)

This feature depends on running with a Swift 5.7 stdlib, and fails
when that isn't available.

* Add type annotations in RegexBuilder tests

These changes work around a change to the way result builders are
compiled that removes the ability for result builder closure outputs
to affect the overload resolution elsewhere in an expression.

Workarounds for rdar://104881395 and rdar://104645543

* Workaround for fileprivate array issue

A recent compiler change results in fileprivate arrays sometimes
not keeping their buffers around long enough. This change avoids that
issue by removing the fileprivate annotations from the affected type.

* Fix an issue where named character classes weren't getting converted in the result builder. <rdar://104480703>

* Stop at end of search string in TwoWaySearcher (swiftlang#631)

When searching for a substring that doesn't exist, it was possible
for TwoWaySearcher to advance beyond the end of the search string,
causing a crash. This change adds a `limitedBy:` parameter to that
index movement, avoiding the invalid movement.

Fixes rdar://105154010

* Correct misspelling in DSL renderer (swiftlang#627)

vertial -> vertical

rdar://104602317

* Fix output type mismatch with RegexBuilder (swiftlang#626)

Some regex literals (and presumably other `Regex` instances) lose
their output type information when used in a RegexBuilder closure due
to the way the concatenating builder calls are overloaded. In
particular, any output type with labeled tuples or where the sum of
tuple components in the accumulated and new output types is greater
than 10 will be ignored.

Regex internals don't make this distinction, however, so there ends up
being a mismatch between what a `Regex.Match` instance tries to
produce and the output type of the outermost regex. For example, this
code results in a crash, because `regex` is a `Regex<Substring>`
but the match tries to produce a `(Substring, number: Substring)`:

    let regex = Regex {
        ZeroOrMore(.whitespace)
        /:(?<number>\d+):/
        ZeroOrMore(.whitespace)
    }
    let match = try regex.wholeMatch(in: " :21: ")
    print(match!.output)

To fix this, we add a new `ignoreCapturesInTypedOutput` DSLTree node
to mark situations where the output type is discarded. This status
is propagated through the capture list into the match's storage,
which lets us produce the correct output type. Note that we can't just
drop the capture groups when building the compiled program because
(1) different parts of the regex might reference the capture group
and (2) all capture groups are available if a developer converts the
output to `AnyRegexOutput`.

    let anyOutput = AnyRegexOutput(match)
    // anyOutput[1] == "21"
    // anyOutput["number"] == Optional("21")

Fixes swiftlang#625. rdar://104823356

Note: Linux seems to crash on different tests when the two customTest
overloads have `internal` visibility or are called. Switching one of the
functions to be generic over a RegexComponent works around the issue.

* Revert "Merge pull request swiftlang#628 from apple/result_builder_changes_workaround"

This reverts commit 7e059b7, reversing
changes made to 3ca8b13.

* Use `some` syntax in variadics

This supports a type checker fix after the change in how result
builder closure parameters are type-checked.

* Type checker workaround: adjust test

* Further refactor to work around type checker regression

* Align availability macro with OS versions (swiftlang#641)

* Speed up general character class matching (swiftlang#642)

Short-circuit Character.isASCII checks inside built in character class matching.

Also, make benchmark try a few more times before giving up.

* Test for \s matching CRLF when scalar matching (swiftlang#648)

* General ascii fast paths for character classes (swiftlang#644)

General ASCII fast-paths for builtin character classes

* Remove the unsupported `anyScalar` case (swiftlang#650)

We decided not to support the `anyScalar` character class, which would
match a single Unicode scalar regardless of matching mode. However,
its representation was still included in the various character class
types in the regex engine, leading to unreachable code and unclear
requirements when changing or adding new code. This change removes
that representation where possible.

The `DSLTree.Atom.CharacterClass` enum is left unchanged, since it
is marked `@_spi(RegexBuilder) public`. Any use of that enum case
is handled with a `fatalError("Unsupported")`, and it isn't produced
on any code path.

* Fix range-based quantification fast path (swiftlang#653)

The fast path for quantification incorrectly discards the last save
position when the quantification used up all possible trips, which is
only possible with range-based quantifications (e.g. `{0,3}`). This
bug shows up when a range-based quantifier matches the maximum - 1
repetitions of the preceding pattern.

For example, the regex `/a{0,2}a/` should succeed as a full match any
of the strings "aa", "aaa", or "aaaa". However, the pattern fails
to match "aaa", since the save point allowing a single "a" to match
the first `a{0,2}` part of the regex is discarded.

This change only discards the last save position when advancing the
quantifier fails due to a failure to match, not maxing out the number
of trips.

* Add in ASCII fast-path for anyNonNewline (swiftlang#654)

* Avoid long expression type checks (swiftlang#657)

These changes remove several seconds of type-checking time from the
RegexBuilder test cases, bringing all expressions under 150ms (on
the tested computer).

* Processor cleanup (swiftlang#655)

Clean up and refactor the processor

* Simplify instruction fetching

* Refactor metrics out, and void their storage in release builds

*Put operations onto String

* Fix `firstRange(of:)` search (swiftlang#656)

Calls to `ranges(of:)` and `firstRange(of:)` with a string parameter
actually use two different string searching algorithms. `ranges(of:)`
uses the "z-searcher" algorithm, while `firstRange(of:)` uses a
two-way search. Since it's better to align on a single path for these
searches, the z-searcher has lower requirements, and the two-way
search implementation has a correctness bug, this change removes
the two-way search algorithm and uses z-search for `firstRange(of:)`.

The correctness bug in `firstRange(of:)` appears only when searching
for the second (or later) occurrence of a substring, which you have
to be fairly deliberate about. In the example below, the substring
at offsets `7..<12` is missed:

    let text = "ADACBADADACBADACB"
    //          =====  -----=====
    let pattern = "ADACB"
    let firstRange = text.firstRange(of: pattern)!
    // firstRange ~= 0..<5
    let secondRange = text[firstRange.upperBound...].firstRange(of: pattern)!
    // secondRange ~= 12..<17

This change also removes some unrelated, unused code in Split.swift,
in addition to removing an (unused) usage of `TwoWaySearcher`.

rdar://92794248

* Bug fix and hot path for quantified `.` (swiftlang#658)

Bug fix in newline hot path, and apply hot path to quantified dot

* Run scalar-semantic benchmark variants (swiftlang#659)

Run scalar semantic benchmarks

* Refactor operations to be on String (swiftlang#664)

Finish refactoring logic onto String

* Provide unique generic method parameter names (swiftlang#669)

This is getting warned on in the 5.9 compiler, will be an error
starting in Swift 6.

* Enable quantification optimizations for scalar semantics (swiftlang#671)

*  Quantified scalar semantic matching

* Remove redundant test

---------

Co-authored-by: Nate Cook <natecook@apple.com>
Co-authored-by: Butta <repo@butta.fastem.com>
Co-authored-by: Ole Begemann <ole@oleb.net>
Co-authored-by: Alex Martini <amartini@apple.com>
Co-authored-by: Alejandro Alonso <alejandro_alonso@apple.com>
Co-authored-by: David Ewing <dewing@apple.com>
Co-authored-by: Dave Ewing <96321608+DaveEwing@users.noreply.github.com>
milseman added a commit that referenced this pull request Jun 4, 2023
* Atomically load the lowered program (#610)

Since we're atomically initializing the compiled program in
`Regex.Program`, we need to pair that with an atomic load.

Resolves #609.

* Add tests for line start/end word boundary diffs (#616)

The `default` and `simple` word boundaries have different behaviors
at the start and end of strings/lines. These tests validate that we
have the correct behavior implemented. Related to issue #613.

* Add tweaks for Android

* Fix documentation typo (#615)

* Fix abstract for Regex.dotMatchesNewlines(_:). (#614)

The old version looks like it was accidentally duplicated from
anchorsMatchLineEndings(_:) just below it.

* Remove `RegexConsumer` and fix its dependencies (#617)

* Remove `RegexConsumer` and fix its dependencies

This eliminates the RegexConsumer type and rewrites its users to call
through to other, existing functionality on Regex or in the Algorithms
implementations. RegexConsumer doesn't take account of the dual
subranges required for matching, so it can produce results that are
inconsistent with matches(of:) and ranges(of:), which were rewritten
earlier.

rdar://102841216

* Remove remaining from-end algorithm methods

This removes methods that are left over from when we were considering
from-end algorithms. These aren't tested and may not have the correct
semantics, so it's safer to remove them entirely.

* Improve StringProcessing and RegexBuilder documentation (#611)

This includes documentation improvements for core types/methods,
RegexBuilder types along with their generated variadic initializers,
and adds some curation. It also includes tests of the documentation
code samples.

* Set availability for inverted character class test (#621)

This feature depends on running with a Swift 5.7 stdlib, and fails
when that isn't available.

* Add type annotations in RegexBuilder tests

These changes work around a change to the way result builders are
compiled that removes the ability for result builder closure outputs
to affect the overload resolution elsewhere in an expression.

Workarounds for rdar://104881395 and rdar://104645543

* Workaround for fileprivate array issue

A recent compiler change results in fileprivate arrays sometimes
not keeping their buffers around long enough. This change avoids that
issue by removing the fileprivate annotations from the affected type.

* Fix an issue where named character classes weren't getting converted in the result builder. <rdar://104480703>

* Stop at end of search string in TwoWaySearcher (#631)

When searching for a substring that doesn't exist, it was possible
for TwoWaySearcher to advance beyond the end of the search string,
causing a crash. This change adds a `limitedBy:` parameter to that
index movement, avoiding the invalid movement.

Fixes rdar://105154010

* Correct misspelling in DSL renderer (#627)

vertial -> vertical

rdar://104602317

* Fix output type mismatch with RegexBuilder (#626)

Some regex literals (and presumably other `Regex` instances) lose
their output type information when used in a RegexBuilder closure due
to the way the concatenating builder calls are overloaded. In
particular, any output type with labeled tuples or where the sum of
tuple components in the accumulated and new output types is greater
than 10 will be ignored.

Regex internals don't make this distinction, however, so there ends up
being a mismatch between what a `Regex.Match` instance tries to
produce and the output type of the outermost regex. For example, this
code results in a crash, because `regex` is a `Regex<Substring>`
but the match tries to produce a `(Substring, number: Substring)`:

    let regex = Regex {
        ZeroOrMore(.whitespace)
        /:(?<number>\d+):/
        ZeroOrMore(.whitespace)
    }
    let match = try regex.wholeMatch(in: " :21: ")
    print(match!.output)

To fix this, we add a new `ignoreCapturesInTypedOutput` DSLTree node
to mark situations where the output type is discarded. This status
is propagated through the capture list into the match's storage,
which lets us produce the correct output type. Note that we can't just
drop the capture groups when building the compiled program because
(1) different parts of the regex might reference the capture group
and (2) all capture groups are available if a developer converts the
output to `AnyRegexOutput`.

    let anyOutput = AnyRegexOutput(match)
    // anyOutput[1] == "21"
    // anyOutput["number"] == Optional("21")

Fixes #625. rdar://104823356

Note: Linux seems to crash on different tests when the two customTest
overloads have `internal` visibility or are called. Switching one of the
functions to be generic over a RegexComponent works around the issue.

* Revert "Merge pull request #628 from apple/result_builder_changes_workaround"

This reverts commit 7e059b7, reversing
changes made to 3ca8b13.

* Use `some` syntax in variadics

This supports a type checker fix after the change in how result
builder closure parameters are type-checked.

* Type checker workaround: adjust test

* Further refactor to work around type checker regression

* Align availability macro with OS versions (#641)

* Speed up general character class matching (#642)

Short-circuit Character.isASCII checks inside built in character class matching.

Also, make benchmark try a few more times before giving up.

* Test for \s matching CRLF when scalar matching (#648)

* General ascii fast paths for character classes (#644)

General ASCII fast-paths for builtin character classes

* Remove the unsupported `anyScalar` case (#650)

We decided not to support the `anyScalar` character class, which would
match a single Unicode scalar regardless of matching mode. However,
its representation was still included in the various character class
types in the regex engine, leading to unreachable code and unclear
requirements when changing or adding new code. This change removes
that representation where possible.

The `DSLTree.Atom.CharacterClass` enum is left unchanged, since it
is marked `@_spi(RegexBuilder) public`. Any use of that enum case
is handled with a `fatalError("Unsupported")`, and it isn't produced
on any code path.

* Fix range-based quantification fast path (#653)

The fast path for quantification incorrectly discards the last save
position when the quantification used up all possible trips, which is
only possible with range-based quantifications (e.g. `{0,3}`). This
bug shows up when a range-based quantifier matches the maximum - 1
repetitions of the preceding pattern.

For example, the regex `/a{0,2}a/` should succeed as a full match any
of the strings "aa", "aaa", or "aaaa". However, the pattern fails
to match "aaa", since the save point allowing a single "a" to match
the first `a{0,2}` part of the regex is discarded.

This change only discards the last save position when advancing the
quantifier fails due to a failure to match, not maxing out the number
of trips.

* Add in ASCII fast-path for anyNonNewline (#654)

* Avoid long expression type checks (#657)

These changes remove several seconds of type-checking time from the
RegexBuilder test cases, bringing all expressions under 150ms (on
the tested computer).

* Processor cleanup (#655)

Clean up and refactor the processor

* Simplify instruction fetching

* Refactor metrics out, and void their storage in release builds

*Put operations onto String

* Fix `firstRange(of:)` search (#656)

Calls to `ranges(of:)` and `firstRange(of:)` with a string parameter
actually use two different string searching algorithms. `ranges(of:)`
uses the "z-searcher" algorithm, while `firstRange(of:)` uses a
two-way search. Since it's better to align on a single path for these
searches, the z-searcher has lower requirements, and the two-way
search implementation has a correctness bug, this change removes
the two-way search algorithm and uses z-search for `firstRange(of:)`.

The correctness bug in `firstRange(of:)` appears only when searching
for the second (or later) occurrence of a substring, which you have
to be fairly deliberate about. In the example below, the substring
at offsets `7..<12` is missed:

    let text = "ADACBADADACBADACB"
    //          =====  -----=====
    let pattern = "ADACB"
    let firstRange = text.firstRange(of: pattern)!
    // firstRange ~= 0..<5
    let secondRange = text[firstRange.upperBound...].firstRange(of: pattern)!
    // secondRange ~= 12..<17

This change also removes some unrelated, unused code in Split.swift,
in addition to removing an (unused) usage of `TwoWaySearcher`.

rdar://92794248

* Bug fix and hot path for quantified `.` (#658)

Bug fix in newline hot path, and apply hot path to quantified dot

* Run scalar-semantic benchmark variants (#659)

Run scalar semantic benchmarks

* Refactor operations to be on String (#664)

Finish refactoring logic onto String

* Provide unique generic method parameter names (#669)

This is getting warned on in the 5.9 compiler, will be an error
starting in Swift 6.

* Enable quantification optimizations for scalar semantics (#671)

*  Quantified scalar semantic matching

* Remove redundant test

---------

Co-authored-by: Nate Cook <natecook@apple.com>
Co-authored-by: Butta <repo@butta.fastem.com>
Co-authored-by: Ole Begemann <ole@oleb.net>
Co-authored-by: Alex Martini <amartini@apple.com>
Co-authored-by: Alejandro Alonso <alejandro_alonso@apple.com>
Co-authored-by: David Ewing <dewing@apple.com>
Co-authored-by: Dave Ewing <96321608+DaveEwing@users.noreply.github.com>
milseman added a commit that referenced this pull request Dec 15, 2023
* Atomically load the lowered program (#610)

Since we're atomically initializing the compiled program in
`Regex.Program`, we need to pair that with an atomic load.

Resolves #609.

* Add tests for line start/end word boundary diffs (#616)

The `default` and `simple` word boundaries have different behaviors
at the start and end of strings/lines. These tests validate that we
have the correct behavior implemented. Related to issue #613.

* Add tweaks for Android

* Fix documentation typo (#615)

* Fix abstract for Regex.dotMatchesNewlines(_:). (#614)

The old version looks like it was accidentally duplicated from
anchorsMatchLineEndings(_:) just below it.

* Remove `RegexConsumer` and fix its dependencies (#617)

* Remove `RegexConsumer` and fix its dependencies

This eliminates the RegexConsumer type and rewrites its users to call
through to other, existing functionality on Regex or in the Algorithms
implementations. RegexConsumer doesn't take account of the dual
subranges required for matching, so it can produce results that are
inconsistent with matches(of:) and ranges(of:), which were rewritten
earlier.

rdar://102841216

* Remove remaining from-end algorithm methods

This removes methods that are left over from when we were considering
from-end algorithms. These aren't tested and may not have the correct
semantics, so it's safer to remove them entirely.

* Improve StringProcessing and RegexBuilder documentation (#611)

This includes documentation improvements for core types/methods,
RegexBuilder types along with their generated variadic initializers,
and adds some curation. It also includes tests of the documentation
code samples.

* Set availability for inverted character class test (#621)

This feature depends on running with a Swift 5.7 stdlib, and fails
when that isn't available.

* Add type annotations in RegexBuilder tests

These changes work around a change to the way result builders are
compiled that removes the ability for result builder closure outputs
to affect the overload resolution elsewhere in an expression.

Workarounds for rdar://104881395 and rdar://104645543

* Workaround for fileprivate array issue

A recent compiler change results in fileprivate arrays sometimes
not keeping their buffers around long enough. This change avoids that
issue by removing the fileprivate annotations from the affected type.

* Fix an issue where named character classes weren't getting converted in the result builder. <rdar://104480703>

* Stop at end of search string in TwoWaySearcher (#631)

When searching for a substring that doesn't exist, it was possible
for TwoWaySearcher to advance beyond the end of the search string,
causing a crash. This change adds a `limitedBy:` parameter to that
index movement, avoiding the invalid movement.

Fixes rdar://105154010

* Correct misspelling in DSL renderer (#627)

vertial -> vertical

rdar://104602317

* Fix output type mismatch with RegexBuilder (#626)

Some regex literals (and presumably other `Regex` instances) lose
their output type information when used in a RegexBuilder closure due
to the way the concatenating builder calls are overloaded. In
particular, any output type with labeled tuples or where the sum of
tuple components in the accumulated and new output types is greater
than 10 will be ignored.

Regex internals don't make this distinction, however, so there ends up
being a mismatch between what a `Regex.Match` instance tries to
produce and the output type of the outermost regex. For example, this
code results in a crash, because `regex` is a `Regex<Substring>`
but the match tries to produce a `(Substring, number: Substring)`:

    let regex = Regex {
        ZeroOrMore(.whitespace)
        /:(?<number>\d+):/
        ZeroOrMore(.whitespace)
    }
    let match = try regex.wholeMatch(in: " :21: ")
    print(match!.output)

To fix this, we add a new `ignoreCapturesInTypedOutput` DSLTree node
to mark situations where the output type is discarded. This status
is propagated through the capture list into the match's storage,
which lets us produce the correct output type. Note that we can't just
drop the capture groups when building the compiled program because
(1) different parts of the regex might reference the capture group
and (2) all capture groups are available if a developer converts the
output to `AnyRegexOutput`.

    let anyOutput = AnyRegexOutput(match)
    // anyOutput[1] == "21"
    // anyOutput["number"] == Optional("21")

Fixes #625. rdar://104823356

Note: Linux seems to crash on different tests when the two customTest
overloads have `internal` visibility or are called. Switching one of the
functions to be generic over a RegexComponent works around the issue.

* Revert "Merge pull request #628 from apple/result_builder_changes_workaround"

This reverts commit 7e059b7, reversing
changes made to 3ca8b13.

* Use `some` syntax in variadics

This supports a type checker fix after the change in how result
builder closure parameters are type-checked.

* Type checker workaround: adjust test

* Further refactor to work around type checker regression

* Align availability macro with OS versions (#641)

* Speed up general character class matching (#642)

Short-circuit Character.isASCII checks inside built in character class matching.

Also, make benchmark try a few more times before giving up.

* Test for \s matching CRLF when scalar matching (#648)

* General ascii fast paths for character classes (#644)

General ASCII fast-paths for builtin character classes

* Remove the unsupported `anyScalar` case (#650)

We decided not to support the `anyScalar` character class, which would
match a single Unicode scalar regardless of matching mode. However,
its representation was still included in the various character class
types in the regex engine, leading to unreachable code and unclear
requirements when changing or adding new code. This change removes
that representation where possible.

The `DSLTree.Atom.CharacterClass` enum is left unchanged, since it
is marked `@_spi(RegexBuilder) public`. Any use of that enum case
is handled with a `fatalError("Unsupported")`, and it isn't produced
on any code path.

* Fix range-based quantification fast path (#653)

The fast path for quantification incorrectly discards the last save
position when the quantification used up all possible trips, which is
only possible with range-based quantifications (e.g. `{0,3}`). This
bug shows up when a range-based quantifier matches the maximum - 1
repetitions of the preceding pattern.

For example, the regex `/a{0,2}a/` should succeed as a full match any
of the strings "aa", "aaa", or "aaaa". However, the pattern fails
to match "aaa", since the save point allowing a single "a" to match
the first `a{0,2}` part of the regex is discarded.

This change only discards the last save position when advancing the
quantifier fails due to a failure to match, not maxing out the number
of trips.

* Add in ASCII fast-path for anyNonNewline (#654)

* Avoid long expression type checks (#657)

These changes remove several seconds of type-checking time from the
RegexBuilder test cases, bringing all expressions under 150ms (on
the tested computer).

* Processor cleanup (#655)

Clean up and refactor the processor

* Simplify instruction fetching

* Refactor metrics out, and void their storage in release builds

*Put operations onto String

* Fix `firstRange(of:)` search (#656)

Calls to `ranges(of:)` and `firstRange(of:)` with a string parameter
actually use two different string searching algorithms. `ranges(of:)`
uses the "z-searcher" algorithm, while `firstRange(of:)` uses a
two-way search. Since it's better to align on a single path for these
searches, the z-searcher has lower requirements, and the two-way
search implementation has a correctness bug, this change removes
the two-way search algorithm and uses z-search for `firstRange(of:)`.

The correctness bug in `firstRange(of:)` appears only when searching
for the second (or later) occurrence of a substring, which you have
to be fairly deliberate about. In the example below, the substring
at offsets `7..<12` is missed:

    let text = "ADACBADADACBADACB"
    //          =====  -----=====
    let pattern = "ADACB"
    let firstRange = text.firstRange(of: pattern)!
    // firstRange ~= 0..<5
    let secondRange = text[firstRange.upperBound...].firstRange(of: pattern)!
    // secondRange ~= 12..<17

This change also removes some unrelated, unused code in Split.swift,
in addition to removing an (unused) usage of `TwoWaySearcher`.

rdar://92794248

* Bug fix and hot path for quantified `.` (#658)

Bug fix in newline hot path, and apply hot path to quantified dot

* Run scalar-semantic benchmark variants (#659)

Run scalar semantic benchmarks

* Refactor operations to be on String (#664)

Finish refactoring logic onto String

* Provide unique generic method parameter names (#669)

This is getting warned on in the 5.9 compiler, will be an error
starting in Swift 6.

* Enable quantification optimizations for scalar semantics (#671)

*  Quantified scalar semantic matching

* Fix doc comment for trimPrefix and trimmingPrefix funcs (#673)

* Update availability for the 5.8 release (#680)

* Optimize search for start-anchored regexes (#682)

When a regex is anchored to the start of a subject, there's no need
to search throughout a string for the pattern when searching for the
first match: a prefix match is sufficient.

This adds a regex compilation-time check about whether a match can
only be found at the start of a subject, and then uses that to
choose whether to defer to `prefixMatch` from within `firstMatch`.

* Fix misuse of `XCTSkip()` (#685)

* Handle boundaries when matching in substrings (#675)

* Handle boundaries when matching in substrings

Some of our existing matching routines use the start/endIndex
of the input, which is basically never the right thing to do.

This change revises those checks to use the search bounds, by
either moving the boundary check out of the matching method, or
if the boundary is a part of what needs to be matched (e.g.
word boundaries have different behavior at the start/end than
in the middle of a string) the search bounds are passed into
the matching method.

Testing is currently handled by piggy-backing on the existing
match tests; we should add more tests to handle substring-
specific edge cases.

* Handle sub-character substring boundaries

This change passes the end boundary down into matching methods, and
uses it to find the actual character that is part of the input
substring, even if the substring's end boundary is in the middle of
a grapheme cluster.

Substrings cannot have sub-Unicode scalar boundaries as of Swift
5.7; we can remove a check for this when matching an individual
scalar.

* Overhaul quantification fast-path (#689)

Overhaul quantification save points and fast path logic, for significant wins in simplicity and performance.

* adopt the stdlib’s pattern for atomic lazy references

- avoids reliance on a pointer conversion

* pass a pointer instead of inout conversion

- this function is imported in a way that causes the compiler to not detect it as a C function

* Update Sources/_StringProcessing/Regex/Core.swift

comment spelling fix

* Adds SPI for a NSRE compatibility mode option (#698)

NSRegularExpression matches at the Unicode scalar level, but also
matches `\r\n` sequences with a single `.` when single-line mode is
enabled. This adds a `_nsreCompatibility` property that enables both
of those behaviors, and implements support for the special case
handling of `.`.

* Add ASCII fast-path ASCII character class matching (#690)

Uses quickASCIICharacter to speed up ASCII character class matching.

2x speedup for EmailLookahead_All and many, many others. 10% regression in AnchoredNotFound_First and related.

---------

Co-authored-by: Nate Cook <natecook@apple.com>
Co-authored-by: Butta <repo@butta.fastem.com>
Co-authored-by: Ole Begemann <ole@oleb.net>
Co-authored-by: Alex Martini <amartini@apple.com>
Co-authored-by: Alejandro Alonso <alejandro_alonso@apple.com>
Co-authored-by: David Ewing <dewing@apple.com>
Co-authored-by: Dave Ewing <96321608+DaveEwing@users.noreply.github.com>
Co-authored-by: Valeriy Van <github@w7software.com>
Co-authored-by: Jonathan Grynspan <grynspan@me.com>
Co-authored-by: Guillaume Lessard <guillaume.lessard@apple.com>
Co-authored-by: Guillaume Lessard <glessard@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants