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

Fix any_spec #1234

Merged
merged 1 commit into from Nov 20, 2013

Conversation

Projects
None yet
2 participants
@dmarcotte
Copy link
Contributor

dmarcotte commented Nov 17, 2013

Use the appropriate arity to get Enumerable#any to handle its args correctly. Note that we need to switch from JavaInternalBlockBody to a BlockCallback since JavaInternalBlockBody does not properly respect its arity in 1_7.

Note: this does not need to be merged into master. any_spec was fixed for master in #1202

Fix any_spec
Use the appropriate arity to get Enumerable#any to handle its args
correctly.  Note that we need to switch from JavaInternalBlockBody to a
BlockCallback since JavaInternalBlockBody does not properly respect its
arity in 1_7.
@@ -4025,7 +4025,6 @@ private IRubyObject all_pBlockless(ThreadContext context) {
}

public IRubyObject any_p(ThreadContext context, Block block) {
if (!isBuiltin("each")) return RubyEnumerable.any_pCommon(context, this, block);

This comment has been minimized.

Copy link
@dmarcotte

dmarcotte Nov 17, 2013

Author Contributor

Please let me know if my thinking is wrong on this line... since this is an instance method on RubyArray, which has a native each, this is always true and so can totally be deleted. Correct?

Let me know if I'm missing something... if we do want to call into RubyEnumerable here, I can restore this line (though it will need a switch on Ruby version to decide what arity the any block should use)

This comment has been minimized.

Copy link
@enebo

enebo Nov 20, 2013

Member

I am trying to think of Enumerable is included into another core class which defines each. Although in that case it might also have an instance of any_p defined. I am ok with removing this.

This comment has been minimized.

Copy link
@dmarcotte

dmarcotte Nov 26, 2013

Author Contributor

Interesting follow-up to this @enebo. Looking into #1265, I figured out what this line was good for: we don't want to directly consult the array elements for #any? if #each has bee overridden.

Restored the line in #1271 (though note I left out the RubyVersion switch since it's not used in the other places we're doing this. This probably leaves things subtly broken in 1.8, but in a pretty narrow use case: only affects multi-args yields on arrays which have #each overridden. Will have a look at it, and probably send another pull addressing it)

@dmarcotte

This comment has been minimized.

Copy link
Contributor Author

dmarcotte commented Nov 19, 2013

Travis note: the failures here are not related to this change. See a clean run of this branch here.

enebo added a commit that referenced this pull request Nov 20, 2013

@enebo enebo merged commit 7139a09 into jruby:jruby-1_7 Nov 20, 2013

1 check failed

default The Travis CI build failed
Details

dmarcotte added a commit to dmarcotte/jruby that referenced this pull request Nov 26, 2013

Fix arrays with overridden each
Fix a stack overflow in Array#all (jruby#1265)
and restore the correct behavior for Array#any (the needed isBuiltn
check was incorrectly removed in jruby#1234)

Also add a spec to ensure this remains stable going forward.

dmarcotte added a commit to dmarcotte/jruby that referenced this pull request Nov 26, 2013

Fix arrays with overridden each
Fix a stack overflow in Array#all (jruby#1265)
and restore the correct behavior for Array#any (the needed isBuiltn
check was incorrectly removed in jruby#1234)

Also add a spec to ensure this remains stable going forward.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.