Range#size missing from JRuby 1.7.8 in 2.0 mode #1252

Closed
wants to merge 25 commits into
from

Conversation

Projects
None yet
9 participants
@tedpennings
Contributor

tedpennings commented Dec 2, 2013

Range#size appears to be missing from the latest JRuby in Ruby 2.0.0 mode. Range#size was added to Ruby in 2.0.0. The other new Range methods added, like #bsearch, appear to be there -- only #range is missing.

irb(main):055:0> puts RUBY_VERSION
2.0.0
=> nil
irb(main):056:0> puts JRUBY_VERSION
1.7.8
=> nil
irb(main):057:0> (80..85).class
=> Range
irb(main):058:0> (80..85).size
NoMethodError: undefined method `size' for 80..85:Range
    from (irb):58:in `evaluate'
    from org/jruby/RubyKernel.java:1123:in `eval'
    from org/jruby/RubyKernel.java:1519:in `loop'
    from org/jruby/RubyKernel.java:1284:in `catch'
    from org/jruby/RubyKernel.java:1284:in `catch'
    from /Users/n0173592/.rvm/rubies/jruby-1.7.8/bin/irb:13:in `(root)'
irb(main):059:0> (80..85).first
=> 80
irb(main):060:0> (80..85).respond_to? :size
=> false

Enumerator#size appears to be defective and may be related:

# same session as above -- 1.7.8 in 2.0 mode
irb(main):061:0> [1,2,3,4]
=> [1, 2, 3, 4]
irb(main):062:0> [1,2,3,4].class
=> Array
irb(main):063:0> [1,2,3,4].each.size
=> nil   # should be 4
irb(main):064:0> [1,2,3,4].each.class
=> Enumerator
irb(main):065:0> [1,2,3,4].each.respond_to? :size
=> true

For comparison, here's the MRI 2.0 behavior:

2.0.0p247 :001 > RUBY_VERSION
 => "2.0.0" 
2.0.0p247 :002 > JRUBY_VERSION
NameError: uninitialized constant JRUBY_VERSION
    from (irb):2
    from /Users/n0173592/.rvm/rubies/ruby-2.0.0-p247/bin/irb:13:in `<main>'
2.0.0p247 :003 > (80..85).class
 => Range 
2.0.0p247 :004 > (80..85).size
 => 6 
2.0.0p247 :005 > (80..85).first
 => 80 
2.0.0p247 :006 > (80..85).respond_to? :size
 => true 
2.0.0p247 :007 > [1,2,3,4].each.size
 => 4 
2.0.0p247 :008 > [1,2,3,4].each.class
 => Enumerator 
2.0.0p247 :009 > [1,2,3,4].each.respond_to? :size
 => true 

I'd be happy to send in a PR for this if you could point me in the direction of where this should be implemented. It looks like the RubyRange.java and RubyEnumerator.java classes are probably the most obvious places, but I wanted to ask before just sending in a PR for that (+tests).

@tedpennings tedpennings reopened this Nov 21, 2013

@enebo

This comment has been minimized.

Show comment
Hide comment
@enebo

enebo Nov 21, 2013

Member

RubyRange.java seems like it is where a @jrubymethod for this should be made. Enumerable size in 2.0+ appears to be a funcall to a size method if it is available or nil.

Member

enebo commented Nov 21, 2013

RubyRange.java seems like it is where a @jrubymethod for this should be made. Enumerable size in 2.0+ appears to be a funcall to a size method if it is available or nil.

@tedpennings

This comment has been minimized.

Show comment
Hide comment
@tedpennings

tedpennings Nov 21, 2013

Contributor

Ok cool, thanks @enebo ! I'll take a stab at Range#size tonight. I'll give Enumerable some thought but it might be a little too in depth for me.

Contributor

tedpennings commented Nov 21, 2013

Ok cool, thanks @enebo ! I'll take a stab at Range#size tonight. I'll give Enumerable some thought but it might be a little too in depth for me.

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

Implement Numeric#step.size
Gets Numeric#step.size doing its thing by putting the following pieces
in place:
* implement Numeric#intervalStepSize closely following the MRI
implementation (https://github.com/ruby/ruby/blob/09b02349c212cac6395e9a634e3d4610e9bbc48c/numeric.c#L1799)
* implement Numeric#floatStepSize which Numeric#intervalStepSize relies
on, again porting from MRI (https://github.com/ruby/ruby/blob/09b02349c212cac6395e9a634e3d4610e9bbc48c/numeric.c#L1748))
* refactor Numeric#floatStep19 to also use the new floatStepSize, which
not only avoids duplication, but also fixes some tests
* create Numeric#stepSize, and use it to enumeratorizeWithSize our steps

We'll be able to un-exclude test_size_for_step from test_enumerator.rb
once Range#size is implemented (jruby#1252).

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

Implement Numeric#step.size
Gets Numeric#step.size doing its thing by putting the following pieces
in place:
* implement Numeric#intervalStepSize closely following the MRI
implementation (https://github.com/ruby/ruby/blob/09b02349c212cac6395e9a634e3d4610e9bbc48c/numeric.c#L1799)
* implement Numeric#floatStepSize which Numeric#intervalStepSize relies
on, again porting from MRI (https://github.com/ruby/ruby/blob/09b02349c212cac6395e9a634e3d4610e9bbc48c/numeric.c#L1748))
* refactor Numeric#floatStep19 to also use the new floatStepSize, which
not only avoids duplication, but also fixes some tests
* create Numeric#stepSize, and use it to enumeratorizeWithSize our steps

We'll be able to un-exclude test_size_for_step from test_enumerator.rb
once Range#size is implemented (jruby#1252).
@dmarcotte

This comment has been minimized.

Show comment
Hide comment
@dmarcotte

dmarcotte Dec 1, 2013

Contributor

Hey @tedpennings, hopefully I'm not chiming in too late, but there's been a couple of recent changes that should help you fix this up:

  • #1281 added Numeric#intervalStepSize which you'll want to call in your Range#size (similar to how it's implemented in MRI)
  • #1039 added RubyEnumerator#enumeratorizeWithSize which you'll be able to use in Range#each instead of enumeratorize to fix the enum cases you note above

Then you'll be able to delete exclude :test_size, "needs investigation" from test/mri/excludes/TestRange.rb to get the tests for this in test/mri/ruby/test_range.rb running in CI.

Hope that helps! Let me know if you have any questions...

Contributor

dmarcotte commented Dec 1, 2013

Hey @tedpennings, hopefully I'm not chiming in too late, but there's been a couple of recent changes that should help you fix this up:

  • #1281 added Numeric#intervalStepSize which you'll want to call in your Range#size (similar to how it's implemented in MRI)
  • #1039 added RubyEnumerator#enumeratorizeWithSize which you'll be able to use in Range#each instead of enumeratorize to fix the enum cases you note above

Then you'll be able to delete exclude :test_size, "needs investigation" from test/mri/excludes/TestRange.rb to get the tests for this in test/mri/ruby/test_range.rb running in CI.

Hope that helps! Let me know if you have any questions...

@tedpennings

This comment has been minimized.

Show comment
Hide comment
@tedpennings

tedpennings Dec 2, 2013

Contributor

Thanks @dmarcotte ! I'm going to take a crack at this now. I'll update this issue once I've got something to show 😄

Contributor

tedpennings commented Dec 2, 2013

Thanks @dmarcotte ! I'm going to take a crack at this now. I'll update this issue once I've got something to show 😄

@tedpennings

This comment has been minimized.

Show comment
Hide comment
@tedpennings

tedpennings Dec 2, 2013

Contributor

How do the above two commits look? Is there anything that should be done in there to ensure that the @jrubymethod is only available in 2.0+ mode? (1.9 doesn't have a Range#size method)

I'll take another look at the Range#step and Range#each enumerator size problem tomorrow. I think that's a bit bigger. It might be worth a separate issue. I can create it if that would help.

Contributor

tedpennings commented Dec 2, 2013

How do the above two commits look? Is there anything that should be done in there to ensure that the @jrubymethod is only available in 2.0+ mode? (1.9 doesn't have a Range#size method)

I'll take another look at the Range#step and Range#each enumerator size problem tomorrow. I think that's a bit bigger. It might be worth a separate issue. I can create it if that would help.

@dmarcotte

This comment has been minimized.

Show comment
Hide comment
@dmarcotte

dmarcotte Dec 2, 2013

Contributor

Looking good @tedpennings! A few notes:

  • this should be checking instanceof RubyNumeric rather than instanceof RubyInteger
  • test_size won't pass until Range#each.size is fixed, so we should fix it here. You should be able replace enumeratorize(runtime, this, "each"); with enumeratorizeWithSize(context, this, "each", size(context)); to get this going (let me know if I'm missing something here!)
  • can you also rebase again from master when you make these changes? The tests happened to be in bad shape the last time you rebased, so Travis can't currently run on your changes

As for 2.0 vs. 1.9: the next major version of JRuby (the version being worked on in master) will only support a single Ruby version, so what you've got here is correct.

Finally, good call on step.size. That needs to be taken care of too, but it should be tackled separately, so yeah sending an issue for that would be great.

Hope that all makes sense! Thanks again, and feel free to let me know if you have more questions.

Contributor

dmarcotte commented Dec 2, 2013

Looking good @tedpennings! A few notes:

  • this should be checking instanceof RubyNumeric rather than instanceof RubyInteger
  • test_size won't pass until Range#each.size is fixed, so we should fix it here. You should be able replace enumeratorize(runtime, this, "each"); with enumeratorizeWithSize(context, this, "each", size(context)); to get this going (let me know if I'm missing something here!)
  • can you also rebase again from master when you make these changes? The tests happened to be in bad shape the last time you rebased, so Travis can't currently run on your changes

As for 2.0 vs. 1.9: the next major version of JRuby (the version being worked on in master) will only support a single Ruby version, so what you've got here is correct.

Finally, good call on step.size. That needs to be taken care of too, but it should be tackled separately, so yeah sending an issue for that would be great.

Hope that all makes sense! Thanks again, and feel free to let me know if you have more questions.

@tedpennings

This comment has been minimized.

Show comment
Hide comment
@tedpennings

tedpennings Dec 2, 2013

Contributor

Ok @dmarcotte - I'll rebase when I get home.

You're right re RubyNumeric instead of RubyInteger. I was thinking of the (quirky) MRI 2.0 behavior where it will return a size for a range of floats, and an Enumerable with size for each/step, but it won't return an array for them.

2.0.0p247 :003 > (1.5..5.5).size
 => 5 
2.0.0p247 :004 > (1.5...5.5).size
 => 4 
2.0.0p247 :005 > (1.5...5.5).each
 => #<Enumerator: 1.5...5.5:each> 
2.0.0p247 :006 > (1.5...5.5).each.size
 => 4 
2.0.0p247 :007 > (1.5...5.5).step
 => #<Enumerator: 1.5...5.5:step> 
2.0.0p247 :008 > (1.5...5.5).step.size
 => 4 
2.0.0p247 :009 > (1.5...5.5).to_a
TypeError: can't iterate from Float
    from (irb):9:in `each'
    from (irb):9:in `to_a'
    from (irb):9
    from /Users/ted/.rvm/rubies/ruby-2.0.0-p247/bin/irb:13:in `<main>'

Last night another test case came to mind that I think we need to consider. RubyNumeric/intervalStepSize doesn't handle RubyBignum. For a range size, that's actually a significant limitation, so RubyNumeric.intervalStepSize might need refactoring.

Here's the error you get with my present PR from Bignums:

 $ bin/jruby -S irb
irb(main):001:0> a=2**64
=> 18446744073709551616
irb(main):002:0> b=a**2 # 2^128
=> 340282366920938463463374607431768211456
irb(main):003:0> c=b**2  # 2^256
=> 115792089237316195423570985008687907853269984665640564039457584007913129639936
irb(main):004:0> c.class
=> Bignum
irb(main):005:0> (b..c)
=> 340282366920938463463374607431768211456..115792089237316195423570985008687907853269984665640564039457584007913129639936
irb(main):006:0> (b..c).size
Java::JavaLang::ClassCastException: org.jruby.RubyBignum cannot be cast to org.jruby.RubyFixnum
    from org.jruby.RubyNumeric.intervalStepSize(RubyNumeric.java:917)
    from org.jruby.RubyRange.size(RubyRange.java:676)
    from org.jruby.RubyRange$INVOKER$i$0$0$size.call(RubyRange$INVOKER$i$0$0$size.gen)
    from org.jruby.runtime.callsite.CachingCallSite.cacheAndCall(CachingCallSite.java:306)
    from org.jruby.runtime.callsite.CachingCallSite.call(CachingCallSite.java:136)
    from org.jruby.ast.CallNoArgNode.interpret(CallNoArgNode.java:60)
    from org.jruby.ast.NewlineNode.interpret(NewlineNode.java:105)
    from org.jruby.ast.RootNode.interpret(RootNode.java:129)
    from org.jruby.evaluator.ASTInterpreter.INTERPRET_EVAL(ASTInterpreter.java:95)
    from org.jruby.evaluator.ASTInterpreter.evalWithBinding(ASTInterpreter.java:179)
    from org.jruby.RubyKernel.evalCommon(RubyKernel.java:934)
    from org.jruby.RubyKernel.eval19(RubyKernel.java:898)
    from org.jruby.RubyKernel$INVOKER$s$0$3$eval19.call(RubyKernel$INVOKER$s$0$3$eval19.gen)
    from org.jruby.internal.runtime.methods.DynamicMethod.call(DynamicMethod.java:180)
    from org.jruby.runtime.callsite.CachingCallSite.call(CachingCallSite.java:70)
    from org.jruby.ast.FCallManyArgsNode.interpret(FCallManyArgsNode.java:60)
... 126 levels...
    from Users.n0173592.Developer.jruby.bin.jirb.load(/Users/n0173592/Developer/jruby/bin/jirb)
    from org.jruby.Ruby.runScript(Ruby.java:809)
    from org.jruby.Ruby.runScript(Ruby.java:802)
    from org.jruby.Ruby.runNormally(Ruby.java:671)
    from org.jruby.Ruby.runFromMain(Ruby.java:520)
    from org.jruby.Main.doRunFromMain(Main.java:395)
    from org.jruby.Main.internalRun(Main.java:290)
    from org.jruby.Main.run(Main.java:217)
    from org.jruby.Main.main(Main.java:197)
    from sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    from sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
    from sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    from java.lang.reflect.Method.invoke(Method.java:606)
    from org.flatland.drip.Main.invoke(Main.java:117)
    from org.flatland.drip.Main.start(Main.java:88)
    from org.flatland.drip.Main.main(Main.java:64)irb(main):007:0> 
Contributor

tedpennings commented Dec 2, 2013

Ok @dmarcotte - I'll rebase when I get home.

You're right re RubyNumeric instead of RubyInteger. I was thinking of the (quirky) MRI 2.0 behavior where it will return a size for a range of floats, and an Enumerable with size for each/step, but it won't return an array for them.

2.0.0p247 :003 > (1.5..5.5).size
 => 5 
2.0.0p247 :004 > (1.5...5.5).size
 => 4 
2.0.0p247 :005 > (1.5...5.5).each
 => #<Enumerator: 1.5...5.5:each> 
2.0.0p247 :006 > (1.5...5.5).each.size
 => 4 
2.0.0p247 :007 > (1.5...5.5).step
 => #<Enumerator: 1.5...5.5:step> 
2.0.0p247 :008 > (1.5...5.5).step.size
 => 4 
2.0.0p247 :009 > (1.5...5.5).to_a
TypeError: can't iterate from Float
    from (irb):9:in `each'
    from (irb):9:in `to_a'
    from (irb):9
    from /Users/ted/.rvm/rubies/ruby-2.0.0-p247/bin/irb:13:in `<main>'

Last night another test case came to mind that I think we need to consider. RubyNumeric/intervalStepSize doesn't handle RubyBignum. For a range size, that's actually a significant limitation, so RubyNumeric.intervalStepSize might need refactoring.

Here's the error you get with my present PR from Bignums:

 $ bin/jruby -S irb
irb(main):001:0> a=2**64
=> 18446744073709551616
irb(main):002:0> b=a**2 # 2^128
=> 340282366920938463463374607431768211456
irb(main):003:0> c=b**2  # 2^256
=> 115792089237316195423570985008687907853269984665640564039457584007913129639936
irb(main):004:0> c.class
=> Bignum
irb(main):005:0> (b..c)
=> 340282366920938463463374607431768211456..115792089237316195423570985008687907853269984665640564039457584007913129639936
irb(main):006:0> (b..c).size
Java::JavaLang::ClassCastException: org.jruby.RubyBignum cannot be cast to org.jruby.RubyFixnum
    from org.jruby.RubyNumeric.intervalStepSize(RubyNumeric.java:917)
    from org.jruby.RubyRange.size(RubyRange.java:676)
    from org.jruby.RubyRange$INVOKER$i$0$0$size.call(RubyRange$INVOKER$i$0$0$size.gen)
    from org.jruby.runtime.callsite.CachingCallSite.cacheAndCall(CachingCallSite.java:306)
    from org.jruby.runtime.callsite.CachingCallSite.call(CachingCallSite.java:136)
    from org.jruby.ast.CallNoArgNode.interpret(CallNoArgNode.java:60)
    from org.jruby.ast.NewlineNode.interpret(NewlineNode.java:105)
    from org.jruby.ast.RootNode.interpret(RootNode.java:129)
    from org.jruby.evaluator.ASTInterpreter.INTERPRET_EVAL(ASTInterpreter.java:95)
    from org.jruby.evaluator.ASTInterpreter.evalWithBinding(ASTInterpreter.java:179)
    from org.jruby.RubyKernel.evalCommon(RubyKernel.java:934)
    from org.jruby.RubyKernel.eval19(RubyKernel.java:898)
    from org.jruby.RubyKernel$INVOKER$s$0$3$eval19.call(RubyKernel$INVOKER$s$0$3$eval19.gen)
    from org.jruby.internal.runtime.methods.DynamicMethod.call(DynamicMethod.java:180)
    from org.jruby.runtime.callsite.CachingCallSite.call(CachingCallSite.java:70)
    from org.jruby.ast.FCallManyArgsNode.interpret(FCallManyArgsNode.java:60)
... 126 levels...
    from Users.n0173592.Developer.jruby.bin.jirb.load(/Users/n0173592/Developer/jruby/bin/jirb)
    from org.jruby.Ruby.runScript(Ruby.java:809)
    from org.jruby.Ruby.runScript(Ruby.java:802)
    from org.jruby.Ruby.runNormally(Ruby.java:671)
    from org.jruby.Ruby.runFromMain(Ruby.java:520)
    from org.jruby.Main.doRunFromMain(Main.java:395)
    from org.jruby.Main.internalRun(Main.java:290)
    from org.jruby.Main.run(Main.java:217)
    from org.jruby.Main.main(Main.java:197)
    from sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    from sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
    from sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    from java.lang.reflect.Method.invoke(Method.java:606)
    from org.flatland.drip.Main.invoke(Main.java:117)
    from org.flatland.drip.Main.start(Main.java:88)
    from org.flatland.drip.Main.main(Main.java:64)irb(main):007:0> 
@dmarcotte

This comment has been minimized.

Show comment
Hide comment
@dmarcotte

dmarcotte Dec 3, 2013

Contributor

Great catch on the class cast error @tedpennings! I'll get that fixed up very soon; I reckon you don't need to worry about it for your PR.

Contributor

dmarcotte commented Dec 3, 2013

Great catch on the class cast error @tedpennings! I'll get that fixed up very soon; I reckon you don't need to worry about it for your PR.

subbuss and others added some commits Dec 2, 2013

[IR] Some more fixes and FIXMEs to inlining code
* More progress inlining code on bench_method_dispatch.rb

* The next set of crashers in inlining requires fixes to the
  WrappedIRClosure operand to take an explicit self arg so that
  on inlining, the correct (original) self can be passed into
  the block rather than passing the self from the host scope
  into which code got inlined. TO BE DONE.
[IR] Extend WrappedIRClosure to include an explicit self operand
* This fix is required for proper inlining of scopes that contain
  blocks so that the self prior-to-inlining can be passed into
  the block after-inlining into another scope.

* With this fix, bench_method_dispatch.rb runs with -Xir.profile=true
  without crashing. Same for bench_fib_recursive, bench_red_black,
  shootout/meteor, shootout/mandelbrot, bench_richards,
  bench_neural_net, and bench_tictactoe.

* There are still known bugs in the inlining code (search for
  FIXMEs or 'This is buggy' comments in the relevant code), but the
  correctness has moved along far enough to enable profiling and
  inlining on a large enough set of programs without crashing or
  yielding incorrect results.
replaced JavaUtilLoggingLogger with StandardErrorLogger using the sam…
…e format. removed any import or usage of java.util.logging
no release repository in any artifact which get published on maven ce…
…ntral (surprised that oss.sonatype.org does not prevent that). that just produces a lot of traffic on that repository and if that repository is inaccessible the artifact can NOT be used anymore

[skip ci]
Always pass RootNode into runInterpreter
* This prevents IR interpreter from crashing since it always
  expects a RootNode.
* 2 more failing specs now green.
[IR] Begin/end block share scope of its lexical parent.
* Thus far, begin/end block were getting their own scope whereas
  they ought to share their parent's scope just like for-loops.
  This patch fixes it.

* One more spec now green. More debugging needed to figure out
  why the other BEGIN_spec is still failing.
@tedpennings

This comment has been minimized.

Show comment
Hide comment
@tedpennings

tedpennings Dec 5, 2013

Contributor

Hm I think I failed at git :( I'll create a new PR for this one change and you can close this as unmerged.

For future reference, how would I rebase to make this work but not have a million commits in this PR?

Contributor

tedpennings commented Dec 5, 2013

Hm I think I failed at git :( I'll create a new PR for this one change and you can close this as unmerged.

For future reference, how would I rebase to make this work but not have a million commits in this PR?

@dmarcotte

This comment has been minimized.

Show comment
Hide comment
@dmarcotte

dmarcotte Dec 5, 2013

Contributor

Hrm... looks like you were working on master in your fork, so I think when you rebased it and force-pushed, the commits you pulled in are now in a strict sense "different" from the ones already in jruby/master.

You should be able to avoid this by always working on a branch in your fork, and keeping its master up to date from jruby/master with git pull upstream master (this assumes you called your jruby remote "upsteam", i.e. you've done git remote add upstream git@github.com:jruby/jruby.git)

Note that since the rebase monkeyed with your master's history, to get back to a good state, the easiest thing may be to re-fork and apply your changes in a branch there.

Hope that solves it for you... as always, more questions are welcome.

Contributor

dmarcotte commented Dec 5, 2013

Hrm... looks like you were working on master in your fork, so I think when you rebased it and force-pushed, the commits you pulled in are now in a strict sense "different" from the ones already in jruby/master.

You should be able to avoid this by always working on a branch in your fork, and keeping its master up to date from jruby/master with git pull upstream master (this assumes you called your jruby remote "upsteam", i.e. you've done git remote add upstream git@github.com:jruby/jruby.git)

Note that since the rebase monkeyed with your master's history, to get back to a good state, the easiest thing may be to re-fork and apply your changes in a branch there.

Hope that solves it for you... as always, more questions are welcome.

@tedpennings

This comment has been minimized.

Show comment
Hide comment
@tedpennings

tedpennings Dec 5, 2013

Contributor

Thanks, doing that now. I'll create a new PR once it's done.

I think I'm going to add some rspec tests for different strange range conditions too, just so we have our bases covered re integer overflows and casts, e.g., (Fixnum..Bignum).size #=> Fixnum; (Fixnum..Bignum).size #=> Bignum; (Bignum..Bignum).size #=> Fixnum, etc

Contributor

tedpennings commented Dec 5, 2013

Thanks, doing that now. I'll create a new PR once it's done.

I think I'm going to add some rspec tests for different strange range conditions too, just so we have our bases covered re integer overflows and casts, e.g., (Fixnum..Bignum).size #=> Fixnum; (Fixnum..Bignum).size #=> Bignum; (Bignum..Bignum).size #=> Fixnum, etc

@dmarcotte

This comment has been minimized.

Show comment
Hide comment
@dmarcotte

dmarcotte Dec 5, 2013

Contributor

Sounds good.

The specs sound great too, but I'd say they should be sent to RubySpec rather than be added here, so you should be good to go with the PR as-is.

Let me know if you need anything else!

(Oh, and let me know if it'll be a while before you can send for whatever reason. No worries if that's the case! It's just that I'm putting off sending a change that would conflict with yours to avoid causing you churn, but I don't want to sit on it too long.)

Contributor

dmarcotte commented Dec 5, 2013

Sounds good.

The specs sound great too, but I'd say they should be sent to RubySpec rather than be added here, so you should be good to go with the PR as-is.

Let me know if you need anything else!

(Oh, and let me know if it'll be a while before you can send for whatever reason. No worries if that's the case! It's just that I'm putting off sending a change that would conflict with yours to avoid causing you churn, but I don't want to sit on it too long.)

@tedpennings

This comment has been minimized.

Show comment
Hide comment
@tedpennings

tedpennings Dec 5, 2013

Contributor

Ok all set, see #1305 . Thanks for all the help!

Contributor

tedpennings commented Dec 5, 2013

Ok all set, see #1305 . Thanks for all the help!

dmarcotte added a commit to dmarcotte/jruby that referenced this pull request Dec 6, 2013

@enebo enebo closed this in #1305 Dec 13, 2013

enebo added a commit that referenced this pull request Dec 13, 2013

@enebo enebo modified the milestones: JRuby 1.7.10, JRuby 1.7.11 Feb 24, 2014

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