Skip to content
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

regcomp.c - make sure conditional match (GROUPP) is not broken by CURLYM #20654

Merged
merged 1 commit into from Jan 6, 2023

Conversation

demerphq
Copy link
Collaborator

@demerphq demerphq commented Dec 29, 2022

This disables the CURLY->CURLYM optimization when the contents contains a GROUPP.

This PR also includes a prior commit which fixes this a different, less efficient way, and then corrects that to a better way afterwards.

See #7754

@demerphq
Copy link
Collaborator Author

CURLYM fakes an OPEN/CLOSE paren, and was deferring updating its close paren data until after it finished looping. This meant that tests against the capture buffer would not work. Changed it to close the parens each time. An alternative solution would be to exclude cases containing conditional logic from the CURLYX->CURLYM conversion, just like EVAL is excluded.

We have to close the paren immediately after each time we
match A, or conditional patterns will break.

An alternative and possibly more efficient solution would be to block
CURLYX -> CURLYM conversion when the inside contains a conditional check
just like we do with EVAL.

Fixes #7754 aka #7754
@demerphq demerphq changed the title regcomp.c - make sure CURLYM closes the capture buffer after each match regcomp.c - make sure conditional match (GROUPP) is not broken by CURLYM Dec 31, 2022
@demerphq
Copy link
Collaborator Author

demerphq commented Jan 4, 2023

I will merge this tomorrow unless there are objections.

@khwilliamson
Copy link
Contributor

I've never understood this area of the code very well, but it looks plausible to me

@demerphq demerphq merged commit d6a7f64 into blead Jan 6, 2023
@demerphq demerphq deleted the yves/re_curlym_close_capture branch January 6, 2023 08:47
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.

None yet

2 participants