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.

Sign up for GitHub

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

Jump to bottom

Fix an issue where Regexp.union was improperly negotiating the result encoding #3296

Merged
merged 3 commits into from
Oct 19, 2023
Merged

Fix an issue where Regexp.union was improperly negotiating the result encoding #3296

merged 3 commits into from
Oct 19, 2023

Conversation

nirvdrum
Copy link
Collaborator

@nirvdrum nirvdrum commented Oct 17, 2023 •
edited by eregon
Loading

As part of this fix we were able to remove the non-standard Regexp.convert.

Looking at this code, there's a lot more we could do to make it compilation friendly. I think we could implement QuoteNode without delegating to ClassicRegexp.quote19, but I'm going to save that for future work in the interest of getting #3287 fixed.

/cc @simonlevasseur

Fixes #3287

@nirvdrum @simonlevasseur
Fix an issue where Regexp.union was improperly negotiating the resu…
49ae17c 
…lt encoding.

As part of this fix we were able to remove the non-standard `Regexp.convert`.

Co-authored-by: Simon LeVasseur <[email protected]>
@oracle-contributor-agreement
Copy link

Thank you for your pull request and welcome to our community! To contribute, please sign the Oracle Contributor Agreement (OCA).
The following contributors of this PR have not signed the OCA:

To sign the OCA, please create an Oracle account and sign the OCA in Oracle's Contributor Agreement Application.

When signing the OCA, please provide your GitHub username. After signing the OCA and getting an OCA approval from Oracle, this PR will be automatically updated.

If you are an Oracle employee, please make sure that you are a member of the main Oracle GitHub organization, and your membership in this organization is public.

@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Required At least one contributor does not have an approved Oracle Contributor Agreement. label Oct 17, 2023
@nirvdrum nirvdrum added shopify OCA Required At least one contributor does not have an approved Oracle Contributor Agreement. and removed OCA Required At least one contributor does not have an approved Oracle Contributor Agreement. labels Oct 17, 2023
eregon
eregon approved these changes Oct 18, 2023
View reviewed changes
src/main/ruby/truffleruby/core/regexp.rb Outdated
res = enc
else
res = Primitive.encoding_compatible?(enc, res)
raise ArgumentError, "incompatible encodings: #{enc} and #{res}" unless res
Copy link
Member

Choose a reason for hiding this comment

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

res in the exception message will always be nil here, we need to save the value in a local variable. I'll do it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch. Thanks. I had originally written it using reduce and missed this when I restructured the code.

@eregon eregon added the in-ci The PR is being tested in CI. Do not push new commits. label Oct 18, 2023
@oracle-contributor-agreement
Copy link

Thank you for signing the OCA.

@oracle-contributor-agreement oracle-contributor-agreement bot added OCA Verified All contributors have signed the Oracle Contributor Agreement. and removed OCA Required At least one contributor does not have an approved Oracle Contributor Agreement. labels Oct 18, 2023
@eregon eregon merged commit 6642143 into oracle:master Oct 19, 2023
@eregon eregon mentioned this pull request Nov 13, 2023
Closed
graalvmbot pushed a commit that referenced this pull request Dec 7, 2023
@eregon
[GR-19220] Fix an issue where Regexp.union was improperly negotiating…
152a876 
… the result encoding (#3296)

PullRequest: truffleruby/4034
(cherry picked from commit 6642143)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eregon eregon eregon approved these changes

Assignees
No one assigned
Labels
in-ci The PR is being tested in CI. Do not push new commits. OCA Verified All contributors have signed the Oracle Contributor Agreement. shopify
Projects
None yet
Milestone
23.1.2 Release (January 16, 2024)
Development

Successfully merging this pull request may close these issues.

net-imap 0.4.0 Regexp Error
2 participants
@nirvdrum @eregon