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

String range object incorrectly tries to convert to int #4021

Closed
rovf opened this Issue Jul 19, 2016 · 4 comments

Comments

Projects
None yet
2 participants
@rovf

rovf commented Jul 19, 2016

jruby 1.7.25 (1.9.3p551) 2016-04-13 867cb81 on Java HotSpot(TM) 64-Bit Server VM 1.7.0_79-b15 +jit [Windows 7-amd64]

Consider the following programs:

mystr='2202702806'; (mystr..mystr).member?(mystr)

Or alternatively:

("2202702806".."2202702806").to_a

These are string ranges, and it works in MRI Ruby (the first example should return true, and the second one should returne a one-element array). In JRuby, I get an error:

RangeError: integer 2202702806 too big to convert to `int'

It seems that JRuby tries to convert the range bounds into Java int values!

@rovf

This comment has been minimized.

Show comment
Hide comment
@rovf

rovf Jul 19, 2016

I just noticed that the same problem occurs on

jruby 9.0.4.0 (2.2.2) 2015-11-12 b9fb7aa Java HotSpot(TM) 64-Bit Server VM 24.79-b02 on 1.7.0_79-b15 +jit [Windows 7-amd64]

as well.

rovf commented Jul 19, 2016

I just noticed that the same problem occurs on

jruby 9.0.4.0 (2.2.2) 2015-11-12 b9fb7aa Java HotSpot(TM) 64-Bit Server VM 24.79-b02 on 1.7.0_79-b15 +jit [Windows 7-amd64]

as well.

@kares

This comment has been minimized.

Show comment
Hide comment
@kares

kares Jul 19, 2016

Member

present in latest 9K (9.1.2.0) as well ... maybe the index is too big when to_a converting to Array

Member

kares commented Jul 19, 2016

present in latest 9K (9.1.2.0) as well ... maybe the index is too big when to_a converting to Array

@rovf

This comment has been minimized.

Show comment
Hide comment
@rovf

rovf Jul 19, 2016

On Tue, Jul 19, 2016, at 13:33, Karol Bucek wrote:

present in latest 9K (9.1.2.0) as well ... maybe the index is too big
when to_a converting to Array

I don't think so, for two reason: First, the array would contain only 1
element in the example which I have provided; second, the error does
also occur if we don't use to_a. See my other example.

I didn't look at the Ruby code, but from the behaviour, a wild guess
would be, that JRuby became a victim of its own optimization:

Ranges of strings are defined (according to the docs) in terms of the
succ method and the <=> operator. Both work fine with the string given
in my example, so I don't think there is an error in this part. I don't
see any reason, why JRuby wants to convert the start or end of the range
into a Java-int. I would be surprised, if this was done for
optimization, for Range#member? can be done easily on strings too (one
doesn't gain that much when holding the values as int).

Ronald

rovf commented Jul 19, 2016

On Tue, Jul 19, 2016, at 13:33, Karol Bucek wrote:

present in latest 9K (9.1.2.0) as well ... maybe the index is too big
when to_a converting to Array

I don't think so, for two reason: First, the array would contain only 1
element in the example which I have provided; second, the error does
also occur if we don't use to_a. See my other example.

I didn't look at the Ruby code, but from the behaviour, a wild guess
would be, that JRuby became a victim of its own optimization:

Ranges of strings are defined (according to the docs) in terms of the
succ method and the <=> operator. Both work fine with the string given
in my example, so I don't think there is an error in this part. I don't
see any reason, why JRuby wants to convert the start or end of the range
into a Java-int. I would be surprised, if this was done for
optimization, for Range#member? can be done easily on strings too (one
doesn't gain that much when holding the values as int).

Ronald

@kares kares added this to the JRuby 1.7.26 milestone Jul 19, 2016

@kares kares closed this in 9c1abe7 Jul 19, 2016

@rovf

This comment has been minimized.

Show comment
Hide comment
@rovf

rovf Jul 20, 2016

On Tue, Jul 19, 2016, at 17:53, Karol Bucek wrote:

Closed #4021 via 9c1abe7.

Thank you for fixing this so quickly. Just a personal question: Why is
it necessary to convert a string to an integer, when the string consists
of digits only? For example

('1234567'..'1234569').member?('12345678')

causes a conversion to int, while the seemingly similar

('1234567A'..'1234569Z').member?('12345678G')

works fine. Why is it that not all strings are treated equal in a Range?

Ronald

rovf commented Jul 20, 2016

On Tue, Jul 19, 2016, at 17:53, Karol Bucek wrote:

Closed #4021 via 9c1abe7.

Thank you for fixing this so quickly. Just a personal question: Why is
it necessary to convert a string to an integer, when the string consists
of digits only? For example

('1234567'..'1234569').member?('12345678')

causes a conversion to int, while the seemingly similar

('1234567A'..'1234569Z').member?('12345678G')

works fine. Why is it that not all strings are treated equal in a Range?

Ronald

kares added a commit that referenced this issue Jul 20, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment