Skip to content

Commit

Permalink
Update logic for Numeric#step from MRI 2.5. Fixes #5078
Browse files Browse the repository at this point in the history 
This includes the following changes:

* Re-port Numeric#step logic and related functions.
* Re-port Numeric#step enumerator size logic.
* Modify ArgsUtil.extractKeywordArgs to use UNDEF instead of nil.
  • Loading branch information
@headius
headius committed Apr 12, 2018
1 parent 333a45e commit acf72b1
Show file tree
Hide file tree
Showing 6 changed files with 131 additions and 142 deletions.
4 changes: 2 additions & 2 deletions core/src/main/java/org/jruby/RubyArray.java
View file
Expand Up @@ -4163,7 +4163,7 @@ public IRubyObject shuffle_bang(ThreadContext context, IRubyObject[] args) {
IRubyObject hash = TypeConverter.checkHashType(context.runtime, args[args.length - 1]);
if (!hash.isNil()) {
IRubyObject[] rets = ArgsUtil.extractKeywordArgs(context, (RubyHash) hash, "random");
if (!rets[0].isNil()) randgen = rets[0];
if (rets[0] != UNDEF) randgen = rets[0];
}
}
int i = realLength;
Expand Down Expand Up @@ -4204,7 +4204,7 @@ public IRubyObject sample(ThreadContext context, IRubyObject[] args) {
IRubyObject hash = TypeConverter.checkHashType(context.runtime, args[args.length - 1]);
if (!hash.isNil()) {
IRubyObject[] rets = ArgsUtil.extractKeywordArgs(context, (RubyHash) hash, "random");
if (!rets[0].isNil()) randgen = rets[0];
if (rets[0] != UNDEF) randgen = rets[0];
args = ArraySupport.newCopy(args, args.length - 1);
}
}
Expand Down
222 changes: 126 additions & 96 deletions core/src/main/java/org/jruby/RubyNumeric.java
View file
Expand Up @@ -50,6 +50,7 @@
import org.jruby.runtime.ThreadContext;
import org.jruby.runtime.Visibility;
import org.jruby.runtime.builtin.IRubyObject;
import org.jruby.runtime.callsite.CachingCallSite;
import org.jruby.util.ByteList;
import org.jruby.util.ConvertBytes;
import org.jruby.util.ConvertDouble;
Expand Down Expand Up @@ -944,118 +945,136 @@ public IRubyObject step(ThreadContext context, IRubyObject[] args, Block block)
return enumeratorizeWithSize(context, this, "step", args, stepSizeFn(context, this, args));
}

IRubyObject[] scannedArgs = scanStepArgs(context, args);
return stepCommon(context, scannedArgs[0], scannedArgs[1], block);
IRubyObject[] newArgs = new IRubyObject[2];
boolean desc = scanStepArgs(context, args, newArgs);
return stepCommon(context, newArgs[0], newArgs[1], desc, block);
}

/** num_step_scan_args
*
/**
* MRI: num_step_scan_args
*/
private IRubyObject[] scanStepArgs(ThreadContext context, IRubyObject[] args) {
private boolean scanStepArgs(ThreadContext context, IRubyObject[] args, IRubyObject[] newArgs) {
Ruby runtime = context.runtime;
IRubyObject to = context.nil;
IRubyObject step = runtime.newFixnum(1);

if (args.length >= 1) to = args[0];
if (args.length >= 2) step = args[1];
int argc;

// TODO: Fold kwargs into the @JRubyMethod decorator
IRubyObject[] kwargs = ArgsUtil.extractKeywordArgs(context, args, validStepArgs);
IRubyObject hash;
boolean desc;
IRubyObject to = context.nil, step = context.nil;

if (kwargs != null) {
to = kwargs[0];
step = kwargs[1];

if(!to.isNil() && args.length > 1) {
throw runtime.newArgumentError("to is given twice");
hash = ArgsUtil.getOptionsArg(runtime, args);
if (hash.isNil()) {
argc = args.length;
} else {
argc = args.length - 1;
}
switch (argc) {
case 2:
newArgs[1] = step = args[1];
case 1:
newArgs[0] = to = args[0];
}
if (!hash.isNil()) {
IRubyObject[] values = ArgsUtil.extractKeywordArgs(context, (RubyHash) hash, validStepArgs);
if (values[0] != UNDEF) {
if (argc > 0) throw runtime.newArgumentError("to is given twice");
newArgs[0] = to = values[0];
}
if(!step.isNil() && args.length > 2) {
throw runtime.newArgumentError("step is given twice");
if (values[1] != UNDEF) {
if (argc > 1) throw runtime.newArgumentError("step is given twice");
newArgs[1] = step = values[1];
}
} else {
if (step.isNil()) {
/* compatibility */
if (argc > 1 && step.isNil()) {
throw runtime.newTypeError("step must be numeric");
}
if (RubyBasicObject.equalInternal(context, step, RubyFixnum.zero(runtime))) {
if (step.op_equal(context, RubyFixnum.zero(runtime)).isTrue()) {
throw runtime.newArgumentError("step can't be 0");
}
}

if (step.isNil()) {
step = RubyFixnum.one(runtime);
newArgs[1] = step = RubyFixnum.one(runtime);
}

boolean desc = numStepNegative(runtime, context, step);
desc = numStepNegative(context, runtime, step);
if (to.isNil()) {
to = desc ?
RubyFloat.newFloat(runtime, Double.NEGATIVE_INFINITY) :
RubyFloat.newFloat(runtime, Double.POSITIVE_INFINITY);
newArgs[0] = to = desc ? dbl2num(runtime, Double.NEGATIVE_INFINITY) : dbl2num(runtime, Double.POSITIVE_INFINITY);
}

return new IRubyObject[] {to, step};
return desc;
}

private boolean numStepNegative(Ruby runtime, ThreadContext context, IRubyObject num) {
// MRI: num_step_negative_p
private static boolean numStepNegative(ThreadContext context, Ruby runtime, IRubyObject num) {
CachingCallSite cmp = sites(context).op_lt;
RubyFixnum zero = RubyFixnum.zero(runtime);
IRubyObject r;

if (num instanceof RubyFixnum) {
if (sites(context).op_lt.isBuiltin(runtime.getInteger())) {
if (cmp.retrieveCache(runtime.getInteger()).method.isBuiltin()) {
return ((RubyFixnum) num).getLongValue() < 0;
}
}
else if (num instanceof RubyBignum) {
if (sites(context).op_lt.isBuiltin(runtime.getInteger())) {
} else if (num instanceof RubyBignum) {
if (cmp.retrieveCache(runtime.getInteger()).method.isBuiltin()) {
return ((RubyBignum) num).isNegative(context).isTrue();
}
}

return !stepCompareWithZero(context, num).isTrue();
}

private IRubyObject stepCompareWithZero(ThreadContext context, IRubyObject num) {
IRubyObject zero = RubyFixnum.zero(context.runtime);
return Helpers.invokeChecked(context, num, sites(context).op_gt_checked, zero);
r = num.getMetaClass().finvokeChecked(context, num, sites(context).op_gt_checked, zero);
if (r == null) {
((RubyNumeric) num).coerceFailed(context, zero);
}
return !r.isTrue();
}

private IRubyObject stepCommon(ThreadContext context, IRubyObject to, IRubyObject step, Block block) {
private IRubyObject stepCommon(ThreadContext context, IRubyObject to, IRubyObject step, boolean desc, Block block) {
Ruby runtime = context.runtime;
if (this instanceof RubyFixnum && to instanceof RubyFixnum && step instanceof RubyFixnum) {
fixnumStep(context, runtime, (RubyFixnum) this,
this.getLongValue(),
((RubyFixnum)to).getLongValue(),
((RubyFixnum)step).getLongValue(),
block);

boolean inf;

if (step.op_equal(context, RubyFixnum.zero(runtime)).isTrue()) {
inf = true;
} else if (to instanceof RubyFloat) {
double f = ((RubyFloat) to).getDoubleValue();
inf = Double.isInfinite(f) && (f <= -0.0 ? desc : !desc);
} else {
inf = false;
}

if (this instanceof RubyFixnum && (inf || to instanceof RubyFixnum) && step instanceof RubyFixnum) {
fixnumStep(context, runtime,
(RubyFixnum) this,
to,
(RubyFixnum) step,
inf,
desc,
block);
} else if (this instanceof RubyFloat || to instanceof RubyFloat || step instanceof RubyFloat) {
floatStep(context, runtime, this, to, step, false, block);
} else {
duckStep(context, runtime, this, to, step, block);
duckStep(context, this, to, step, inf, desc, block);
}
return this;
}

private static void fixnumStep(ThreadContext context, Ruby runtime, RubyFixnum fromObj, long from, long to, long step, Block block) {
private static void fixnumStep(ThreadContext context, Ruby runtime, RubyFixnum from, IRubyObject to, RubyFixnum step, boolean inf, boolean desc, Block block) {
long i = from.getLongValue();
long diff = step.getLongValue();

// We must avoid integer overflows in "i += step".
if (step == 0) {
for (;;) {
block.yield(context, fromObj);
}
} else if (step > 0) {
long tov = Long.MAX_VALUE - step;
if (to < tov) tov = to;
long i;
for (i = from; i <= tov; i += step) {
block.yield(context, RubyFixnum.newFixnum(runtime, i));
}
if (i <= to) {
if (inf) {
for (;; i += diff) {
block.yield(context, RubyFixnum.newFixnum(runtime, i));
}
} else {
long tov = Long.MIN_VALUE - step;
if (to > tov) tov = to;
long i;
for (i = from; i >= tov; i += step) {
block.yield(context, RubyFixnum.newFixnum(runtime, i));
}
if (i >= to) {
block.yield(context, RubyFixnum.newFixnum(runtime, i));
long end = ((RubyFixnum) to).getLongValue();

if (desc) {
for (; i >= end && i < Long.MAX_VALUE; i += diff) {
block.yield(context, RubyFixnum.newFixnum(runtime, i));
}
} else {
for (; i <= end && i > Long.MIN_VALUE; i += diff) {
block.yield(context, RubyFixnum.newFixnum(runtime, i));
}
}
}
}
Expand All @@ -1066,56 +1085,61 @@ static void floatStep(ThreadContext context, Ruby runtime, IRubyObject from, IRu
double unit = num2dbl(step);

double n = floatStepSize(beg, end, unit, excl);
long i;

if (Double.isInfinite(unit)) {
/* if unit is infinity, i*unit+beg is NaN */
if (n != 0) block.yield(context, from);
} else if (unit == 0) {
while(true) {
block.yield(context, from);
IRubyObject val = from;
for (;;) {
block.yield(context, val);
}
} else {
for (long i = 0; i < n; i++){
double d = i * unit + beg;
if (unit >= 0 ? end < d : d < end) {
d = end;
}
for (i=0; i<n; i++) {
double d = i*unit+beg;
if (unit >= 0 ? end < d : d < end) d = end;
block.yield(context, RubyFloat.newFloat(runtime, d));
}
}
}

private static void duckStep(ThreadContext context, Ruby runtime, IRubyObject from, IRubyObject to, IRubyObject step, Block block) {
private static void duckStep(ThreadContext context, IRubyObject from, IRubyObject to, IRubyObject step, boolean inf, boolean desc, Block block) {
IRubyObject i = from;

CallSite cmpSite = sites(context).op_gt.call(context, step, step, RubyFixnum.newFixnum(context.runtime, 0)).isTrue() ? sites(context).op_gt : sites(context).op_lt;
if(sites(context).op_equals.call(context, step, step, RubyFixnum.newFixnum(context.runtime, 0)).isTrue()) {
cmpSite = sites(context).op_equals;
}
if (inf) {
for (;; i = sites(context).op_plus.call(context, i, i, step)) {
block.yield(context, i);
}
} else {
CallSite cmpSite = desc ? sites(context).op_lt : sites(context).op_gt;

while (true) {
if (cmpSite.call(context, i, i, to).isTrue()) break;
block.yield(context, i);
i = sites(context).op_plus.call(context, i, i, step);
for (; !cmpSite.call(context, i, i, to).isTrue(); i = sites(context).op_plus.call(context, i, i, step)) {
block.yield(context, i);
}
}
}

public static RubyNumeric intervalStepSize(ThreadContext context, IRubyObject from, IRubyObject to, IRubyObject step, boolean excludeLast) {
// MRI: ruby_num_interval_step_size
public static RubyNumeric intervalStepSize(ThreadContext context, IRubyObject from, IRubyObject to, IRubyObject step, boolean excl) {
Ruby runtime = context.runtime;

if (from instanceof RubyFixnum && to instanceof RubyFixnum && step instanceof RubyFixnum) {
long diff = ((RubyFixnum) step).getLongValue();
long delta, diff;

diff = ((RubyFixnum) step).getLongValue();
if (diff == 0) {
return RubyFloat.newFloat(runtime, Double.POSITIVE_INFINITY);
}
long toLong = ((RubyFixnum) to).getLongValue();
long fromLong = ((RubyFixnum) from).getLongValue();
long delta = toLong - fromLong;
delta = toLong - fromLong;
if (!Helpers.subtractionOverflowed(toLong, fromLong, delta)) {
if (diff < 0) {
diff = -diff;
delta = -delta;
}
if (excludeLast) {
if (excl) {
delta--;
}
if (delta < 0) {
Expand All @@ -1132,12 +1156,14 @@ public static RubyNumeric intervalStepSize(ThreadContext context, IRubyObject fr
}
// fall through to duck-typed logic
} else if (from instanceof RubyFloat || to instanceof RubyFloat || step instanceof RubyFloat) {
double n = floatStepSize(from.convertToFloat().getDoubleValue(), to.convertToFloat().getDoubleValue(), step.convertToFloat().getDoubleValue(), excludeLast);
double n = floatStepSize(from.convertToFloat().getDoubleValue(), to.convertToFloat().getDoubleValue(), step.convertToFloat().getDoubleValue(), excl);

if (Double.isInfinite(n)) {
return runtime.newFloat(n);
} else {
} else if (posFixable(n)) {
return runtime.newFloat(n).convertToInteger();
} else {
return RubyBignum.newBignorm(runtime, n);
}
}

Expand All @@ -1164,18 +1190,20 @@ public static RubyNumeric intervalStepSize(ThreadContext context, IRubyObject fr
IRubyObject deltaObj = sites.op_minus.call(context, to, to, from);
IRubyObject result = sites.div.call(context, deltaObj, deltaObj, step);
IRubyObject timesPlus = sites.op_plus.call(context, from, from, sites.op_times.call(context, result, result, step));
if (!excludeLast || cmpSite.call(context, timesPlus, timesPlus, to).isTrue()) {
if (!excl || cmpSite.call(context, timesPlus, timesPlus, to).isTrue()) {
result = sites.op_plus.call(context, result, result, RubyFixnum.newFixnum(runtime, 1));
}
return (RubyNumeric) result;
}

private SizeFn stepSizeFn(final ThreadContext context, final IRubyObject from, final IRubyObject[] args) {
return new SizeFn() {
// MRI: num_step_size
@Override
public IRubyObject size(IRubyObject[] args) {
IRubyObject[] scannedArgs = scanStepArgs(context, args);
return intervalStepSize(context, from, scannedArgs[0], scannedArgs[1], false);
IRubyObject[] newArgs = new IRubyObject[2];
scanStepArgs(context, args, newArgs);
return intervalStepSize(context, from, newArgs[0], newArgs[1], false);
}
};
}
Expand All @@ -1184,6 +1212,8 @@ public IRubyObject size(IRubyObject[] args) {
* Returns the number of unit-sized steps between the given beg and end.
*
* NOTE: the returned value is either Double.POSITIVE_INFINITY, or a rounded value appropriate to be cast to a long
*
* MRI: ruby_float_step_size
*/
public static double floatStepSize(double beg, double end, double unit, boolean excludeLast) {
double n = (end - beg)/unit;
Expand Down
3 changes: 2 additions & 1 deletion core/src/main/java/org/jruby/ast/util/ArgsUtil.java
View file
Expand Up @@ -34,6 +34,7 @@

import org.jruby.Ruby;
import org.jruby.RubyArray;
import org.jruby.RubyBasicObject;
import org.jruby.RubyHash;
import org.jruby.RubySymbol;
import org.jruby.runtime.ThreadContext;
Expand Down Expand Up @@ -119,7 +120,7 @@ public static IRubyObject[] extractKeywordArgs(ThreadContext context, RubyHash o
if (options.containsKey(keySym)) {
ret[index] = options.fastARef(keySym);
} else {
ret[index] = context.nil;
ret[index] = RubyBasicObject.UNDEF;
}
index++;
validKeySet.add(keySym);
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/org/jruby/runtime/JavaSites.java
View file
Expand Up @@ -172,7 +172,7 @@ public static class NumericSites {
public final CallSite op_times = new FunctionalCachingCallSite("*");
public final CallSite op_mod = new FunctionalCachingCallSite("%");
public final CachingCallSite op_lt = new FunctionalCachingCallSite("<");
public final CallSite op_gt = new FunctionalCachingCallSite(">");
public final CachingCallSite op_gt = new FunctionalCachingCallSite(">");
public final CheckedSites op_lt_checked = new CheckedSites("<");
public final CheckedSites op_gt_checked = new CheckedSites(">");
public final CallSite op_uminus = new FunctionalCachingCallSite("-@");
Expand Down
1 change: 0 additions & 1 deletion spec/tags/ruby/core/numeric/modulo_tags.txt
View file

This file was deleted.

0 comments on commit acf72b1

Please sign in to comment.