Project

General

Profile

Actions

Bug #18998

closed

Kernel#Integer does not convert SimpleDelegator object expectly

Added by taichi730 (Taichi Ishitani) over 1 year ago. Updated over 1 year ago.

Status:
Closed
Assignee:
-
Target version:
-
ruby -v:
ruby 3.1.2p20 (2022-04-12 revision 4491bb740a) [x86_64-linux]
[ruby-core:109848]

Description

Kernel#Integer method doens not convert a SimpleDelegator object of which value is a String.
This is an sample code.

require 'delegate'
p Integer(SimpleDelegator.new('0x10'))

I expect Kernel#Integer to convert the input value as a String and the expected returned value is 16.
Hoever the actual returned value is 0 so it's seemed that #to_i method is just called.

taichi@LAPTOP-TVTKLNFD:temp
$ cat test.rb
require 'delegate'
p Integer(SimpleDelegator.new('0x10'))

taichi@LAPTOP-TVTKLNFD:temp
$ ruby test.rb
0

Which is the correct behavior?

Actions #1

Updated by taichi730 (Taichi Ishitani) over 1 year ago

  • Subject changed from Kernel#Integer does not convert SimpleDelegator expectly to Kernel#Integer does not convert SimpleDelegator object expectly

Updated by byroot (Jean Boussier) over 1 year ago

So this behavior isn't documented, so it's hard to say wether it's correct:

Integer(arg, base=0, exception: true) → integer or nil
Converts arg to an Integer. Numeric types are converted directly (with floating point numbers being truncated). base (0, or between 2 and 36) is a base for integer string representation. If arg is a String, when base is omitted or equals zero, radix indicators (0, 0b, and 0x) are honored. In any case, strings should consist only of one or more digits, except for that a sign, one underscore between two digits, and leading/trailing spaces are optional. This behavior is different from that of String#to_i. Non string values will be converted by first trying to_int, then to_i.

Passing nil raises a TypeError, while passing a String that does not conform with numeric representation raises an ArgumentError. This behavior can be altered by passing exception: false, in this case a not convertible value will return nil.

However the intent of the implementation is fairly clear:

static VALUE
rb_convert_to_integer(VALUE val, int base, int raise_exception)
{
    VALUE tmp;
    // snip
    if (RB_FLOAT_TYPE_P(val)) {
        double f = RFLOAT_VALUE(val);
        if (!raise_exception && !isfinite(f)) return Qnil;
        if (FIXABLE(f)) return LONG2FIX((long)f);
        return rb_dbl2big(f);
    }
    else if (RB_INTEGER_TYPE_P(val)) {
        return val;
    }
    else if (RB_TYPE_P(val, T_STRING)) {
        return rb_str_convert_to_inum(val, base, TRUE, raise_exception);
    }
    else if (NIL_P(val)) {
        if (!raise_exception) return Qnil;
        rb_raise(rb_eTypeError, "can't convert nil into Integer");
    }

    tmp = rb_protect(rb_check_to_int, val, NULL);
    if (RB_INTEGER_TYPE_P(tmp)) return tmp;
    rb_set_errinfo(Qnil);

    if (!raise_exception) {
        VALUE result = rb_protect(rb_check_to_i, val, NULL);
        rb_set_errinfo(Qnil);
        return result;
    }

    return rb_to_integer(val, "to_i", idTo_i);
}

If passed an object that is neither a direct string nor a native numeric, Integer first tries to invoke to_int, then fallback to invoke to_i.

I suppose a case could be made to first try to call to_str.

Updated by taichi730 (Taichi Ishitani) over 1 year ago

Thank you for your reply.
I understood that there is no documented specification and the current implementation.

How should Integer method behave for this case?
My thought is that Integer method should convert a Delegator object like when a String is given.
Because of this, I override Integer method like below in my project.

module Kernel
  alias_method :__orignal_Integer, :Integer

  def Integer(arg, base = 0, exception: true)
    arg = arg.__getobj__ if arg.is_a?(::Delegator)
    __orignal_Integer(arg, base, exception: exception)
  end
end

Updated by byroot (Jean Boussier) over 1 year ago

My thought is that Integer method should convert a Delegator object like when a String is given.

I don't think it would be advisable for Kernel#Integer to know about Delegator. Delegator is just a class from the stdlib, you could implement the same yourself and that wouldn't solve your problem.

I think based on the spec lined up above, it would be best if you defined #to_int on your delegator. But I don't know your full use case, so not sure if it's relevant.

Either way I don't think there is really a bug here, aside from maybe the method needed more documentation.

A change of behavior would fall under a feature request, and a proper spec would need to be drafted. The only possible change I could see would be to try to call to_str between to_int and to_i, but that could be a breaking change, so a hard sell.

Updated by taichi730 (Taichi Ishitani) over 1 year ago

it would be best if you defined #to_int on your delegator

I've tried this approach but it was failed because base and exception arguments cannot be passed to #to_i method on my delegator.
Therefore, I've overridden Kernel#Integer method.

Actions #6

Updated by nobu (Nobuyoshi Nakada) over 1 year ago

  • Status changed from Open to Closed

Applied in changeset git|7563604fb868d87057733f52d780d841fc1ab6bb.


[Bug #18998] Honor #to_str next to #to_int in Kernel#Integer

Actions #7

Updated by nobu (Nobuyoshi Nakada) over 1 year ago

  • Backport changed from 2.7: UNKNOWN, 3.0: UNKNOWN, 3.1: UNKNOWN to 2.7: REQUIRED, 3.0: REQUIRED, 3.1: REQUIRED
Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0