-
Notifications
You must be signed in to change notification settings - Fork 245
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Enable some repetitions for \A
and \Z
#5349
Enable some repetitions for \A
and \Z
#5349
Conversation
Signed-off-by: Navin Kumar <navink@nvidia.com>
Signed-off-by: Navin Kumar <navink@nvidia.com>
Signed-off-by: Navin Kumar <navink@nvidia.com>
…able_cap_a_z_repetitions
…able_cap_a_z_repetitions
Signed-off-by: Navin Kumar <navink@nvidia.com>
Signed-off-by: Navin Kumar <navink@nvidia.com>
build |
case (RegexEscaped('A'), '+') | | ||
(RegexSequence(ListBuffer(RegexEscaped('A'))), '+') => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This pattern is repeated a few times. I wonder if it is worth introducing a utility function that can simply expressions to remove redundant list buffers?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I left one suggestion but not critical for this PR.
Fixes #4800 (which was also partially fixed by PR#5319 when
\Z
was finally re-enabled).This enables using
\A
and\Z
in some repetition sequences to add more full support for those escape sequences in regular expressions on the GPU. This enables:+
near\A
or\Z
{n}
or{n,}
or{n,m}
wheren > 0
NOTE:
\A*
and\A{...}
can be transpiled to\A?
; however cuDF does not yet support\A?
, so this*
and?
and{0,}
will all still fallback to CPU.