Skip to content

Commit

Permalink
3 new Convert API methods
Browse files Browse the repository at this point in the history
checkToInteger - Same as TypeConverter but it replaces it
integerAsInt - Works from knowing we have RubyInteger and bounds checks to
   make sure we really have an int
integerAsLong - Works from knowing we have RubyInteger and bounds checks to
   make sure we really have a long

Fallout from this is realizing we had two checkToInteger but whatever used
the more generic form which allows to pass in converter is no longer used
for anything but #to_int.  This PR deprecated both versions.

In refactoring to use these I noticed in two places we were not even checking
to see if the value was safely an int/long.  The integerAs* provides this and is
more direct/narrow in what it provides than num2int or num2long.
  • Loading branch information
enebo committed Jun 20, 2024
1 parent a51b85d commit 3c41bf2
Show file tree
Hide file tree
Showing 11 changed files with 96 additions and 31 deletions.
6 changes: 4 additions & 2 deletions core/src/main/java/org/jruby/RubyIO.java
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,8 @@
import static com.headius.backport9.buffer.Buffers.limitBuffer;
import static org.jruby.RubyEnumerator.enumeratorize;
import static org.jruby.anno.FrameField.LASTLINE;
import static org.jruby.api.Convert.checkToInteger;
import static org.jruby.api.Convert.integerAsInt;
import static org.jruby.api.Error.typeError;
import static org.jruby.runtime.ThreadContext.*;
import static org.jruby.runtime.Visibility.*;
Expand Down Expand Up @@ -1238,8 +1240,8 @@ public static IRubyObject sysopen(ThreadContext context, IRubyObject recv, IRuby

if (vmode.isNil())
oflags = OpenFlags.O_RDONLY.intValue();
else if (!(intmode = TypeConverter.checkToInteger(context, vmode)).isNil())
oflags = RubyNumeric.num2int(intmode);
else if (!(intmode = checkToInteger(context, vmode)).isNil())
oflags = integerAsInt(context, (RubyInteger) intmode);
else {
vmode = vmode.convertToString();
oflags = OpenFile.ioModestrOflags(runtime, vmode.toString());
Expand Down
7 changes: 4 additions & 3 deletions core/src/main/java/org/jruby/RubyInteger.java
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
import static org.jruby.RubyEnumerator.SizeFn;
import static org.jruby.RubyEnumerator.enumeratorizeWithSize;
import static org.jruby.api.Convert.castToInteger;
import static org.jruby.api.Convert.checkToInteger;
import static org.jruby.api.Error.typeError;
import static org.jruby.util.Numeric.f_gcd;
import static org.jruby.util.Numeric.f_lcm;
Expand Down Expand Up @@ -738,19 +739,19 @@ private static long op_mod_two(ThreadContext context, RubyInteger self) {

@JRubyMethod(name = "allbits?")
public IRubyObject allbits_p(ThreadContext context, IRubyObject other) {
IRubyObject mask = TypeConverter.checkToInteger(context, other);
IRubyObject mask = checkToInteger(context, other);
return ((RubyInteger) op_and(context, mask)).op_equal(context, mask);
}

@JRubyMethod(name = "anybits?")
public IRubyObject anybits_p(ThreadContext context, IRubyObject other) {
IRubyObject mask = TypeConverter.checkToInteger(context, other);
IRubyObject mask = checkToInteger(context, other);
return ((RubyInteger) op_and(context, mask)).zero_p(context).isTrue() ? context.fals : context.tru;
}

@JRubyMethod(name = "nobits?")
public IRubyObject nobits_p(ThreadContext context, IRubyObject other) {
IRubyObject mask = TypeConverter.checkToInteger(context, other);
IRubyObject mask = checkToInteger(context, other);
return ((RubyInteger) op_and(context, mask)).zero_p(context);
}

Expand Down
5 changes: 3 additions & 2 deletions core/src/main/java/org/jruby/RubyRandomBase.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import java.math.BigInteger;
import java.nio.ByteBuffer;

import static org.jruby.api.Convert.checkToInteger;
import static org.jruby.runtime.Visibility.PRIVATE;
import static org.jruby.util.TypeConverter.toFloat;

Expand Down Expand Up @@ -317,7 +318,7 @@ static IRubyObject randRandom(ThreadContext context, IRubyObject obj, RubyRandom

if (vmax.isNil()) return vmax;
if (!(vmax instanceof RubyFloat)) {
v = TypeConverter.checkToInteger(context, vmax);
v = checkToInteger(context, vmax);
if (!v.isNil()) return randInt(context, obj, rnd, (RubyInteger) v, true);
}
Ruby runtime = context.runtime;
Expand Down Expand Up @@ -463,7 +464,7 @@ private static void checkFloatValue(ThreadContext context, double x) {

private static IRubyObject checkMaxInt(ThreadContext context, IRubyObject vmax) {
if (!(vmax instanceof RubyFloat)) {
IRubyObject v = TypeConverter.checkToInteger(context, vmax);
IRubyObject v = checkToInteger(context, vmax);
if (v != context.nil) return v;
}
return null;
Expand Down
5 changes: 3 additions & 2 deletions core/src/main/java/org/jruby/RubyRange.java
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
import static org.jruby.RubyEnumerator.enumeratorizeWithSize;
import static org.jruby.RubyNumeric.dbl2num;
import static org.jruby.RubyNumeric.fix2long;
import static org.jruby.api.Convert.checkToInteger;
import static org.jruby.api.Error.typeError;
import static org.jruby.runtime.Helpers.hashEnd;
import static org.jruby.runtime.Helpers.hashStart;
Expand Down Expand Up @@ -887,8 +888,8 @@ private IRubyObject stepCommon(ThreadContext context, IRubyObject step, Block bl
b.uptoCommon(context, end.asString(), isExclusive, blockCallback);
}
} else if (begin instanceof RubyNumeric
|| !TypeConverter.checkToInteger(runtime, begin, "to_int").isNil()
|| !TypeConverter.checkToInteger(runtime, end, "to_int").isNil()) {
|| !checkToInteger(context, begin).isNil()
|| !checkToInteger(context, end).isNil()) {
numericStep(context, runtime, step, block);
} else {
IRubyObject tmp = begin.checkStringType();
Expand Down
5 changes: 3 additions & 2 deletions core/src/main/java/org/jruby/RubyRational.java
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
import org.jruby.util.Numeric;
import org.jruby.util.TypeConverter;

import static org.jruby.api.Convert.checkToInteger;
import static org.jruby.api.Error.typeError;
import static org.jruby.ast.util.ArgsUtil.hasExceptionOption;
import static org.jruby.runtime.Helpers.invokedynamic;
Expand Down Expand Up @@ -444,7 +445,7 @@ private static IRubyObject convertCommon(ThreadContext context, RubyClass clazz,
if (!raise && a1.isNil()) return a1;
} else if (a1 instanceof RubyObject && !a1.respondsTo("to_r")) {
try {
IRubyObject tmp = TypeConverter.checkToInteger(context, a1);
IRubyObject tmp = checkToInteger(context, a1);
if (!tmp.isNil()) {
a1 = tmp;
}
Expand All @@ -462,7 +463,7 @@ private static IRubyObject convertCommon(ThreadContext context, RubyClass clazz,
if (!raise && a2.isNil()) return a2;
} else if (!a2.isNil() & a2 instanceof RubyObject && !a2.respondsTo("to_r")) {
try {
IRubyObject tmp = TypeConverter.checkToInteger(context, a2);
IRubyObject tmp = checkToInteger(context, a2);
if (!tmp.isNil()) {
a2 = tmp;
}
Expand Down
8 changes: 6 additions & 2 deletions core/src/main/java/org/jruby/RubySignalException.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,11 @@
import org.jruby.runtime.Arity;
import org.jruby.runtime.Block;
import org.jruby.runtime.ThreadContext;

import static org.jruby.api.Convert.checkToInteger;
import static org.jruby.api.Convert.integerAsLong;
import static org.jruby.runtime.Visibility.PRIVATE;

import org.jruby.runtime.builtin.IRubyObject;
import org.jruby.util.TypeConverter;

Expand Down Expand Up @@ -69,7 +73,7 @@ public IRubyObject initialize(ThreadContext context, IRubyObject[] args, Block b
final Ruby runtime = context.runtime;
int argnum = 1;

IRubyObject sig = TypeConverter.checkToInteger(runtime, args[0], "to_int");
IRubyObject sig = checkToInteger(context, args[0]);

if (sig.isNil()) {
sig = args[0];
Expand All @@ -81,7 +85,7 @@ public IRubyObject initialize(ThreadContext context, IRubyObject[] args, Block b
long _signo;

if (argnum == 2) {
_signo = sig.convertToInteger().getLongValue();
_signo = integerAsLong(context, (RubyInteger) sig);
if (_signo < 0 || _signo > NSIG.longValue()) {
throw runtime.newArgumentError("invalid signal number (" + _signo + ")");
}
Expand Down
3 changes: 2 additions & 1 deletion core/src/main/java/org/jruby/RubyTime.java
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@
import java.util.regex.Pattern;

import static org.jruby.RubyComparable.invcmp;
import static org.jruby.api.Convert.checkToInteger;
import static org.jruby.api.Error.typeError;
import static org.jruby.runtime.Helpers.invokedynamic;
import static org.jruby.runtime.ThreadContext.hasKeywords;
Expand Down Expand Up @@ -354,7 +355,7 @@ private static RubyNumeric numExact(ThreadContext context, IRubyObject v) {
}
v = tmp; break;
}
if (!(tmp = TypeConverter.checkToInteger(context, v)).isNil()) {
if (!(tmp = checkToInteger(context, v)).isNil()) {
v = tmp; // return tmp;
}
else {
Expand Down
54 changes: 54 additions & 0 deletions core/src/main/java/org/jruby/api/Convert.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.jruby.api;

import org.jruby.Ruby;
import org.jruby.RubyArray;
import org.jruby.RubyBignum;
import org.jruby.RubyClass;
Expand All @@ -13,10 +14,14 @@
import org.jruby.RubyRange;
import org.jruby.RubyString;
import org.jruby.RubySymbol;
import org.jruby.runtime.JavaSites;
import org.jruby.runtime.ThreadContext;
import org.jruby.runtime.builtin.IRubyObject;

import static org.jruby.RubyBignum.big2long;
import static org.jruby.api.Error.typeError;
import static org.jruby.util.TypeConverter.convertToTypeWithCheck;
import static org.jruby.util.TypeConverter.sites;

public class Convert {
/**
Expand Down Expand Up @@ -281,4 +286,53 @@ public static RubySymbol castToSymbol(ThreadContext context, IRubyObject newValu
if (!(newValue instanceof RubySymbol)) throw typeError(context, newValue, "Symbol");
return (RubySymbol) newValue;
}

// FIXME: Create annotation @MRI so we can formalize these comments and provide a dictionary for embedders.
// MRI: rb_check_to_integer
/**
* Check whether the given object is an Integer or can be converted to an Integer using #to_int.
* @param context the current thread context
* @param obj the object to be converted
* @return the integer value or nil if the object or conversion is not an Integer.
*/
public static IRubyObject checkToInteger(ThreadContext context, IRubyObject obj) {
if (obj instanceof RubyFixnum) return obj;

JavaSites.TypeConverterSites sites = sites(context);

IRubyObject conv = convertToTypeWithCheck(context, obj, context.runtime.getInteger(), sites.to_int_checked);

return conv instanceof RubyInteger ? conv : context.nil;
}

/**
* Safely convert a Ruby Integer into a java int value. Raising if the value will not fit.
* @param context the current thread context
* @param value the RubyInteger to convert
* @return the int value
*/
public static int integerAsInt(ThreadContext context, RubyInteger value) {
long num = value.getLongValue();

if (((int) num) != num) {
throw context.runtime.newRangeError("integer " + num +
(num < Integer.MIN_VALUE ? " too small to convert to `int'" : " too big to convert to `int'"));
}

return (int) num;
}

/**
* Safely convert a Ruby Integer into a java int value. Raising if the value will not fit.
* @param context the current thread context
* @param value the RubyInteger to convert
* @return the int value
*/
public static long integerAsLong(ThreadContext context, RubyInteger value) {
if (value instanceof RubyBignum) {
return big2long((RubyBignum) value);
}

return value.getLongValue();
}
}
20 changes: 7 additions & 13 deletions core/src/main/java/org/jruby/ext/socket/Option.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import jnr.constants.platform.SocketOption;
import org.jruby.Ruby;
import org.jruby.RubyClass;
import org.jruby.RubyInteger;
import org.jruby.RubyNumeric;
import org.jruby.RubyObject;
import org.jruby.RubyString;
Expand All @@ -22,6 +23,8 @@
import java.nio.ByteBuffer;
import java.util.Locale;

import static org.jruby.api.Convert.checkToInteger;
import static org.jruby.api.Convert.integerAsInt;
import static org.jruby.api.Error.typeError;

public class Option extends RubyObject {
Expand Down Expand Up @@ -221,21 +224,12 @@ public IRubyObject bool(ThreadContext context) {
}

@JRubyMethod(meta = true)
public static IRubyObject linger(ThreadContext context, IRubyObject self, IRubyObject vonoff, IRubyObject vsecs) {
ProtocolFamily family = ProtocolFamily.PF_UNSPEC;
SocketLevel level = SocketLevel.SOL_SOCKET;
SocketOption option = SocketOption.SO_LINGER;
int coercedVonoff;

if (!TypeConverter.checkToInteger(context, vonoff).isNil()) {
coercedVonoff = vonoff.convertToInteger().getIntValue();
} else {
coercedVonoff = vonoff.isTrue() ? 1 : 0;
}

public static IRubyObject linger(ThreadContext context, IRubyObject self, IRubyObject vonoffArg, IRubyObject vsecs) {
IRubyObject vonoff = checkToInteger(context, vonoffArg);
int coercedVonoff = !vonoff.isNil() ? integerAsInt(context, (RubyInteger) vonoff) : (vonoffArg.isTrue() ? 1 : 0);
ByteList data = packLinger(coercedVonoff, vsecs.convertToInteger().getIntValue());

return new Option(context.getRuntime(), family, level, option, data);
return new Option(context.runtime, ProtocolFamily.PF_UNSPEC, SocketLevel.SOL_SOCKET, SocketOption.SO_LINGER, data);
}

@JRubyMethod
Expand Down
7 changes: 5 additions & 2 deletions core/src/main/java/org/jruby/util/TypeConverter.java
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import org.jruby.RubyNumeric;
import org.jruby.RubyString;
import org.jruby.RubySymbol;
import org.jruby.api.Convert;
import org.jruby.exceptions.RaiseException;
import org.jruby.java.proxies.JavaProxy;
import org.jruby.runtime.ClassIndex;
Expand Down Expand Up @@ -272,6 +273,7 @@ public static IRubyObject checkIntegerType(ThreadContext context, IRubyObject ob
}

// rb_check_to_integer
@Deprecated // Use Convert.checkToInteger
public static IRubyObject checkToInteger(ThreadContext context, IRubyObject obj) {
if (obj instanceof RubyFixnum) return obj;

Expand All @@ -283,8 +285,9 @@ public static IRubyObject checkToInteger(ThreadContext context, IRubyObject obj)
}

// rb_check_to_integer
@Deprecated // Use Convert.checkToInteger (and MRI uses not to_i so I do not think this signature is needed).
public static IRubyObject checkToInteger(Ruby runtime, IRubyObject obj, String method) {
if (method.equals("to_int")) return checkToInteger(runtime.getCurrentContext(), obj);
if (method.equals("to_int")) return Convert.checkToInteger(runtime.getCurrentContext(), obj);

if (obj instanceof RubyFixnum) return obj;

Expand Down Expand Up @@ -493,7 +496,7 @@ private static IRubyObject raiseIntegerBaseError(ThreadContext context, boolean
throw context.runtime.newArgumentError("base specified for non string value");
}

private static TypeConverterSites sites(ThreadContext context) {
public static TypeConverterSites sites(ThreadContext context) {
return context.sites.TypeConverter;
}

Expand Down
7 changes: 5 additions & 2 deletions core/src/main/java/org/jruby/util/io/EncodingUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.jruby.RubyFixnum;
import org.jruby.RubyHash;
import org.jruby.RubyIO;
import org.jruby.RubyInteger;
import org.jruby.RubyMethod;
import org.jruby.RubyNumeric;
import org.jruby.RubyProc;
Expand Down Expand Up @@ -56,6 +57,8 @@
import java.util.List;

import static org.jruby.RubyString.*;
import static org.jruby.api.Convert.checkToInteger;
import static org.jruby.api.Convert.integerAsInt;
import static org.jruby.api.Error.typeError;
import static org.jruby.util.StringSupport.CR_UNKNOWN;
import static org.jruby.util.StringSupport.searchNonAscii;
Expand Down Expand Up @@ -203,11 +206,11 @@ public static void extractModeEncoding(ThreadContext context,
fmode_p[0] = OpenFile.READABLE;
oflags_p[0] = ModeFlags.RDONLY;
} else {
intmode = TypeConverter.checkToInteger(context, vmode(vmodeAndVperm_p));
intmode = checkToInteger(context, vmode(vmodeAndVperm_p));

if (!intmode.isNil()) {
vmode(vmodeAndVperm_p, intmode);
oflags_p[0] = RubyNumeric.num2int(intmode);
oflags_p[0] = integerAsInt(context, (RubyInteger) intmode);
fmode_p[0] = ModeFlags.getOpenFileFlagsFor(oflags_p[0]);
} else {
String p = vmode(vmodeAndVperm_p).convertToString().asJavaString();
Expand Down

0 comments on commit 3c41bf2

Please sign in to comment.