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

[3.2 compatibility] Fix array slice with arithmetic sequence for Ruby v3.2 #2981

Conversation

itarato
Copy link
Collaborator

@itarato itarato commented Apr 5, 2023

#3039

Ruby 3.2 is applying slice with an Enumerator::ArithmeticSequence argument slightly differently: for negative steps and exclusive-ending range the start/end is adjusted in this PR.

Related Ruby core issue: https://bugs.ruby-lang.org/issues/18247

Specs: 7d5bad8#diff-5359533868e5029f9a20bc73cd77a856a8557d94d2726006cd218b14e3c87c4eR787-R881

@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Apr 5, 2023
@andrykonchin
Copy link
Member

andrykonchin commented Apr 5, 2023

Any specs covered this new behaviour would be appreciated.

Also could you please point at description of this change (CRuby bug tracker issue, PR or commit in the CRuby reopsitory) - I haven't found any mentioning in the release notes/news? Could it be this one https://bugs.ruby-lang.org/issues/18247?

@itarato
Copy link
Collaborator Author

itarato commented Apr 5, 2023

Any specs covered this new behaviour would be appreciated.

This PR is fixing broken tests for v3.2: 7d5bad8#diff-5359533868e5029f9a20bc73cd77a856a8557d94d2726006cd218b14e3c87c4eR787-R881 These already exist.

Also could you please point at description of this change (CRuby bug tracker issue, PR or commit in the CRuby reopsitory) - I haven't found any mentioning in the release notes/news? Could it be this one https://bugs.ruby-lang.org/issues/18247?

Correct. That's the thread that led to the extra specs.

Anything else should be added?

@andrykonchin

Copy link
Member

@andrykonchin andrykonchin left a comment

Choose a reason for hiding this comment

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

Thank you!

@andrykonchin
Copy link
Member

andrykonchin commented Apr 6, 2023

Anything else should be added?

There are some additional steps that are supposed to be done in such PRs:

I've done them already for the existing PRs (this one and #2980) so there is nothing to do. The PRs will be merged when pass our internal CI.

@andrykonchin andrykonchin added the in-ci The PR is being tested in CI. Do not push new commits. label Apr 6, 2023
@eregon eregon force-pushed the feature/PA-array-slice-with-negative-step-on-version-3.2 branch from f5d3183 to f84bb0d Compare April 19, 2023 10:39
@graalvmbot graalvmbot merged commit d24019b into oracle:master Apr 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

5 participants