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

Pass test_parse_big_integers on jruby #355

Closed
wants to merge 2 commits into from

Conversation

okazu-dm
Copy link

@okazu-dm okazu-dm commented Jun 2, 2018

Summary

I modified Generator.java and add INTEGER_HANDLER for passing test_parse_big_integers on jruby 9.2.0.0.
I think release of jruby 9.2.0.0 caused these failure. (I found this post http://jruby.org/2018/05/24/jruby-9-2-0-0)

Maybe this problem is mentioned in #336

note of implementation

The test failed because of Java::JavaLang::ClassCastException: org.jruby.RubyBignum cannot be cast to org.jruby.RubyFixnum.
I avoid this excpetion by adding INTEGER_HANDLER and use this handler instead of FIXNUM_HANDLER and BIGNUM_HANDLER.

@okazu-dm okazu-dm closed this Jun 2, 2018
@okazu-dm okazu-dm reopened this Jun 2, 2018
Aggregate FIXNUM_HANDLER and BIGNUM_HANDLER to INTEGER_HANDLER
@hsbt
Copy link
Collaborator

hsbt commented Oct 24, 2018

@headius Can you review this?

hsbt added a commit that referenced this pull request Oct 25, 2018
@kares
Copy link
Contributor

kares commented Nov 27, 2018

this will only work on JRuby 9.2 (Ruby 2.5) compat but will break under 9.1 (Ruby 2.3).
... so whether this is okay depends on whether the release would target ruby >= 2.5

@kares
Copy link
Contributor

kares commented Nov 28, 2018

setup an alternative fix, which works all the way back to JRuby 1.7 ... #361

@kares
Copy link
Contributor

kares commented Nov 28, 2018

just to be clear, this piece did not raise any failures
but Fixnum/Bignum objects did take the generic path with the return GENERIC_HANDLER; on JRuby < 9.2 ... which isn't ideal/intended

@marcandre
Copy link
Contributor

Commits should be squashed.

I can't set the reviewer to @headius (or maybe another JRuby user can review?)

@kares
Copy link
Contributor

kares commented Jul 6, 2020

Hey Marc, this PR is defunct due #361 being merged ... there's special handling for the internal RubyFixnum/RubyBignum objects used by JRuby (these won't surface as a warning to use Integer).

@okazu-dm
Copy link
Author

okazu-dm commented Jul 6, 2020

@kares Oh I found #361 fix this problem now. I close this PR.
Thanks for your kindness.

@okazu-dm okazu-dm closed this Jul 6, 2020
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

4 participants