From 3c41bf22aaa0aa7315019038c737f57461ed7c83 Mon Sep 17 00:00:00 2001 From: "Thomas E. Enebo" Date: Thu, 20 Jun 2024 11:34:31 -0400 Subject: [PATCH] 3 new Convert API methods 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. --- core/src/main/java/org/jruby/RubyIO.java | 6 ++- core/src/main/java/org/jruby/RubyInteger.java | 7 +-- .../main/java/org/jruby/RubyRandomBase.java | 5 +- core/src/main/java/org/jruby/RubyRange.java | 5 +- .../src/main/java/org/jruby/RubyRational.java | 5 +- .../java/org/jruby/RubySignalException.java | 8 ++- core/src/main/java/org/jruby/RubyTime.java | 3 +- core/src/main/java/org/jruby/api/Convert.java | 54 +++++++++++++++++++ .../java/org/jruby/ext/socket/Option.java | 20 +++---- .../java/org/jruby/util/TypeConverter.java | 7 ++- .../java/org/jruby/util/io/EncodingUtils.java | 7 ++- 11 files changed, 96 insertions(+), 31 deletions(-) diff --git a/core/src/main/java/org/jruby/RubyIO.java b/core/src/main/java/org/jruby/RubyIO.java index 90c7f19538b..dcc0493b5b4 100644 --- a/core/src/main/java/org/jruby/RubyIO.java +++ b/core/src/main/java/org/jruby/RubyIO.java @@ -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.*; @@ -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()); diff --git a/core/src/main/java/org/jruby/RubyInteger.java b/core/src/main/java/org/jruby/RubyInteger.java index 14a6e8dc169..d6c62b3bdf7 100644 --- a/core/src/main/java/org/jruby/RubyInteger.java +++ b/core/src/main/java/org/jruby/RubyInteger.java @@ -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; @@ -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); } diff --git a/core/src/main/java/org/jruby/RubyRandomBase.java b/core/src/main/java/org/jruby/RubyRandomBase.java index b013d07807e..f82ae2c6283 100644 --- a/core/src/main/java/org/jruby/RubyRandomBase.java +++ b/core/src/main/java/org/jruby/RubyRandomBase.java @@ -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; @@ -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; @@ -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; diff --git a/core/src/main/java/org/jruby/RubyRange.java b/core/src/main/java/org/jruby/RubyRange.java index 3813fc1d940..c7ba2f076bc 100644 --- a/core/src/main/java/org/jruby/RubyRange.java +++ b/core/src/main/java/org/jruby/RubyRange.java @@ -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; @@ -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(); diff --git a/core/src/main/java/org/jruby/RubyRational.java b/core/src/main/java/org/jruby/RubyRational.java index 7834a89163f..042cb9f4b66 100644 --- a/core/src/main/java/org/jruby/RubyRational.java +++ b/core/src/main/java/org/jruby/RubyRational.java @@ -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; @@ -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; } @@ -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; } diff --git a/core/src/main/java/org/jruby/RubySignalException.java b/core/src/main/java/org/jruby/RubySignalException.java index fa9ed8f6e7a..4410b9651a5 100644 --- a/core/src/main/java/org/jruby/RubySignalException.java +++ b/core/src/main/java/org/jruby/RubySignalException.java @@ -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; @@ -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]; @@ -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 + ")"); } diff --git a/core/src/main/java/org/jruby/RubyTime.java b/core/src/main/java/org/jruby/RubyTime.java index 0ae1b61eaba..572182c0307 100644 --- a/core/src/main/java/org/jruby/RubyTime.java +++ b/core/src/main/java/org/jruby/RubyTime.java @@ -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; @@ -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 { diff --git a/core/src/main/java/org/jruby/api/Convert.java b/core/src/main/java/org/jruby/api/Convert.java index b4a5cb9a04d..d91e53effb4 100644 --- a/core/src/main/java/org/jruby/api/Convert.java +++ b/core/src/main/java/org/jruby/api/Convert.java @@ -1,5 +1,6 @@ package org.jruby.api; +import org.jruby.Ruby; import org.jruby.RubyArray; import org.jruby.RubyBignum; import org.jruby.RubyClass; @@ -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 { /** @@ -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(); + } } diff --git a/core/src/main/java/org/jruby/ext/socket/Option.java b/core/src/main/java/org/jruby/ext/socket/Option.java index 89033786ec0..4f9f2f27c65 100644 --- a/core/src/main/java/org/jruby/ext/socket/Option.java +++ b/core/src/main/java/org/jruby/ext/socket/Option.java @@ -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; @@ -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 { @@ -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 diff --git a/core/src/main/java/org/jruby/util/TypeConverter.java b/core/src/main/java/org/jruby/util/TypeConverter.java index ef00efab374..851915c4067 100644 --- a/core/src/main/java/org/jruby/util/TypeConverter.java +++ b/core/src/main/java/org/jruby/util/TypeConverter.java @@ -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; @@ -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; @@ -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; @@ -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; } diff --git a/core/src/main/java/org/jruby/util/io/EncodingUtils.java b/core/src/main/java/org/jruby/util/io/EncodingUtils.java index 78269a9472a..ee5b1f23946 100644 --- a/core/src/main/java/org/jruby/util/io/EncodingUtils.java +++ b/core/src/main/java/org/jruby/util/io/EncodingUtils.java @@ -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; @@ -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; @@ -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();