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

Make behavior of Array#eql? more closely match MRI #1725

Merged
merged 2 commits into from Jun 14, 2014
Merged

Make behavior of Array#eql? more closely match MRI #1725

merged 2 commits into from Jun 14, 2014

Conversation

@alexdowad
Copy link
Contributor

@alexdowad alexdowad commented Jun 4, 2014

MRI's rb_ary_eql looks like this:

rb_ary_eql(VALUE ary1, VALUE ary2)
{
    if (ary1 == ary2) return Qtrue;
    if (!RB_TYPE_P(ary2, T_ARRAY)) return Qfalse;
    if (RARRAY_LEN(ary1) != RARRAY_LEN(ary2)) return Qfalse;
    return rb_exec_recursive_paired(recursive_eql, ary1, ary2, ary2);
}

Notice the RB_TYPE_P check. This will be true for instances of Array or a subclass of Array.

However, JRuby's Array#eql? returns true for objects which are not Arrays, as long as they implement #to_ary and as long as all their members are eql? to the corresponding members in the receiver Array.

This is contrary to the intent of #eql? -- it is supposed to be a stricter version of == which checks not just value but also type.

This patch should bring the behavior of JRuby a little bit closer in line with MRI.

@alexdowad
Copy link
Contributor Author

@alexdowad alexdowad commented Jun 5, 2014

Right now, there is no test in Rubyspec which verifies this behavior. I just issued a PR for one:

https://github.com/rubyspec/rubyspec/pull/276

@alexdowad
Copy link
Contributor Author

@alexdowad alexdowad commented Jun 6, 2014

The test which catches this bug has been merged into Rubyspec; therefore, if this PR is accepted, it will improve JRuby's Rubyspec pass rate.

enebo added a commit that referenced this pull request Jun 14, 2014
Make behavior of Array#eql? more closely match MRI
@enebo enebo merged commit fc287fa into jruby:master Jun 14, 2014
1 check failed
1 check failed
@enebo
continuous-integration/travis-ci The Travis CI build failed
Details
@enebo enebo added this to the JRuby 1.7.13 milestone Jun 14, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants