Forum: Ruby-core [Bug #1583] Time + String no Longer Raises TypeError?

Bug #1583: Time + String no Longer Raises TypeError?
http://redmine.ruby-lang.org/issues/show/1583

Author: Run Paint Run Run
Status: Open, Priority: Normal
ruby -v: ruby 1.9.2dev (2009-06-01 trunk 23614) [i686-linux

On 1.9.2 trunk `Time.now + 'glark'` succeeds, whereas on 1.9.1 and 1.8 
it raised a TypeError. Presumably this is a regression.

Hi,

At Sat, 6 Jun 2009 06:06:34 +0900,
Run Paint Run Run wrote in [ruby-core:23729]:
> On 1.9.2 trunk `Time.now + 'glark'` succeeds, whereas on
> 1.9.1 and 1.8 it raised a TypeError. Presumably this is a
> regression.

To get number from rhs, now calls to_r method on it.
I think distinct implicit coercion and explicit conversion for
Rational are needed, like as to_int and to_i for Integer.


Index: object.c
===================================================================
--- object.c  (revision 23638)
+++ object.c  (working copy)
@@ -2058,4 +2058,5 @@ rb_to_integer(VALUE val, const char *met

     if (FIXNUM_P(val)) return val;
+    if (TYPE(val) == T_BIGNUM) return val;
     v = convert_type(val, "Integer", method, Qtrue);
     if (!rb_obj_is_kind_of(v, rb_cInteger)) {
@@ -2073,4 +2074,5 @@ rb_check_to_integer(VALUE val, const cha

     if (FIXNUM_P(val)) return val;
+    if (TYPE(val) == T_BIGNUM) return val;
     v = convert_type(val, "Integer", method, Qfalse);
     if (!rb_obj_is_kind_of(v, rb_cInteger)) {
Index: rational.c
===================================================================
--- rational.c  (revision 23638)
+++ rational.c  (working copy)
@@ -1652,4 +1652,6 @@ Init_Rational(void)
     rb_define_method(rb_cInteger, "to_r", integer_to_r, 0);
     rb_define_method(rb_cFloat, "to_r", float_to_r, 0);
+    rb_define_method(rb_cInteger, "to_rat", integer_to_r, 0);
+    rb_define_method(rb_cFloat, "to_rat", float_to_r, 0);

     make_patterns();
Index: time.c
===================================================================
--- time.c  (revision 23638)
+++ time.c  (working copy)
@@ -199,9 +199,7 @@ num_exact(VALUE v)
       {
         VALUE tmp;
-        if (!NIL_P(tmp = rb_check_convert_type(v, T_RATIONAL, 
"Rational", "to_r")))
-            v = tmp;
-        else if (!NIL_P(tmp = rb_check_convert_type(v, T_FIXNUM, 
"Integer", "to_int")))
+        if (!NIL_P(tmp = rb_check_convert_type(v, T_RATIONAL, 
"Rational", "to_rat")))
             v = tmp;
-        else if (!NIL_P(tmp = rb_check_convert_type(v, T_BIGNUM, 
"Integer", "to_int")))
+        else if (!NIL_P(tmp = rb_check_to_integer(v, "to_int")))
             v = tmp;
         else {

Hi,

In message "Re: [ruby-core:23733] Re: [Bug #1583] Time + String no 
Longer Raises TypeError?"
    on Sun, 7 Jun 2009 02:05:23 +0900, Nobuyoshi Nakada 
<nobu@ruby-lang.org> writes:

|To get number from rhs, now calls to_r method on it.
|I think distinct implicit coercion and explicit conversion for
|Rational are needed, like as to_int and to_i for Integer.

And add to_flo as well?  I'd rather reject T_STRING, as 1.8.

diff --git a/time.c b/time.c
index 2775e57..e2131db 100644
--- a/time.c
+++ b/time.c
@@ -192,6 +192,7 @@ num_exact(VALUE v)
         break;
       }

+      case T_STRING:
       case T_NIL:
         goto typeerror;

Issue #1583 has been updated by Akira Tanaka.


I like to_rat.
----------------------------------------
http://redmine.ruby-lang.org/issues/show/1583

Akira Tanaka wrote:
> I like to_rat.
>
>   

Is that a new style of singing ... or dancing? ;)



Sorry :D

daz

Hi,

In message "Re: [ruby-core:23743] [Bug #1583] Time + String no Longer 
Raises TypeError?"
    on Sun, 7 Jun 2009 13:00:26 +0900, Akira Tanaka 
<redmine@ruby-lang.org> writes:

|I like to_rat.

#to_rat alone?  Or along with to_flo, to_comp, to_dec, etc?

              matz.

Issue #1583 has been updated by Akira Tanaka.


maybe.

rejecting T_STRING doesn't solve time + delegate-to-string.
----------------------------------------
http://redmine.ruby-lang.org/issues/show/1583

Excerpts from Yukihiro Matsumoto's message of Sun Jun 07 17:07:06 +0300 
2009:
> Hi,
> 
> In message "Re: [ruby-core:23743] [Bug #1583] Time + String no Longer Raises
> TypeError?"
>     on Sun, 7 Jun 2009 13:00:26 +0900, Akira Tanaka <redmine@ruby-lang.org> writes:
> 
> |I like to_rat.
> 
> #to_rat alone?  Or along with to_flo, to_comp, to_dec, etc?

Since some of these names will inevitably turn out to be
somewhat hard to decode at first glance, might it be an
option to consider establishing a convention different
from the #to_i => #to_int style and make it #to_i and
#as_i (or #to_int and #as_int) instead?

#to_<typeish> produces a representation of object, as #to_s.
#as_<typeish> "converts" object to type, as #to_str.

In my opinion, the symmetry would be helpful and the two
different prefixes help separate their responsibilities?
Of course, it is a departure from past semi-convention.


Regards,
        Eero

On 2009/06/08 0:27, Eero Saynatkari wrote:
>
> Since some of these names will inevitably turn out to be
> somewhat hard to decode at first glance, might it be an
> option to consider establishing a convention different
> from the #to_i =>  #to_int style and make it #to_i and
> #as_i (or #to_int and #as_int) instead?
>
> #to_<typeish>  produces a representation of object, as #to_s.
> #as_<typeish>  "converts" object to type, as #to_str.

Another opinion would be to use somewhat longer names where three
letters isn't helpful. When I saw to_rat first, I was first thinking of
a little rodent. #to_rational isn't that long, for the number of times
it needs to be used, I guess.

Regards,   Martin.

I'd like to second long explicit names. Maybe it's because I'm not
coming from perl, but there are already maybe abreviations that I find
questionable (elsif, Array#uniq, Time#min). Let's remember that "str"
and "int" are usual abbreviations (which you'll find on wikipedia, for
instance), while flo, rat, etc... are not.

I also like Eero's suggestion.

Issue #1583 has been updated by Akira Tanaka.


num_exact don't need to_flo and to_comp because it don't call to_f and 
to_c.

I don't know what to_dec is.
----------------------------------------
http://redmine.ruby-lang.org/issues/show/1583

Issue #1583 has been updated by Akira Tanaka.


The problem of rejecting T_STRING is that it cannot reject delegator.

For example,
  Time.now + SimpleDelegator.new("2")
should be rejected but the delegator is T_OBJECT, not T_STRING.

So we need a method distinguish a object is appropriate for the 
argument.

Unfortunately to_r doesn't work well because String#to_r exists.

So we need a more restricted method.

nobu's to_rat works well but matz don't accept, due to the name I guess.

Note that Time uses to_int and to_r for conversion.
So it only needs to_rat.
It don't need to_flo and other methods.

----------------------------------------
http://redmine.ruby-lang.org/issues/show/1583

Issue #1583 has been updated by Yui NARUSE.


What status is this?
----------------------------------------
http://redmine.ruby-lang.org/issues/show/1583

Issue #1583 has been updated by Marc-Andre Lafortune.

Category set to core
Status changed from Open to Closed

Solved in r25991.
----------------------------------------
http://redmine.ruby-lang.org/issues/show/1583