[ruby-core:25143] Is this an intentional change in 1.9?

[ruby-core:25143] Is this an intentional change in 1.9?

[Rick DeNatale]

This came up on the ruby lang forum.

This seems like a 1.9 bug. Is it?


---------- Forwarded message ----------
From: Aldric Giacomoni <aldric@trevoke.net>
Date: Wed, Aug 26, 2009 at 11:00 AM
Subject: ||= with 1.8 and 1.9 ?
To: ruby-talk ML <ruby-talk@ruby-lang.org>


A friend of mine on Twitter recently posted this tidbit of code:

class OrOrEquals
 def test
   @test
 end

 def test=(test)
   @test = test
   'not test'
 end
end

p (OrOrEquals.new.test = 'test')
# ruby 1.8 returns 'test'
# ruby 1.9 returns 'test'

p (OrOrEquals.new.test ||= 'test')
# ruby 1.8 returns 'test'
# ruby 1.9 returns 'not test'

It works as indicated. Is this -normal- behavior ?
--



-- 
Rick DeNatale

Blog: http://talklikeaduck.denhaven2.com/
Twitter: http://twitter.com/RickDeNatale
WWR: http://www.workingwithrails.com/person/9021-rick-denatale
LinkedIn: http://www.linkedin.com/in/rickdenatale

[ruby-core:25144] Re: Is this an intentional change in 1.9?

[Rick DeNatale]

On Wed, Aug 26, 2009 at 1:30 PM, Rick DeNatale<rick.denatale@gmail.com> wrote:
> This came up on the ruby lang forum.
>
> This seems like a 1.9 bug. Is it?
>
>
> ---------- Forwarded message ----------
> From: Aldric Giacomoni <aldric@trevoke.net>
> Date: Wed, Aug 26, 2009 at 11:00 AM
> Subject: ||= with 1.8 and 1.9 ?
> To: ruby-talk ML <ruby-talk@ruby-lang.org>
>
>
> A friend of mine on Twitter recently posted this tidbit of code:
>
> class OrOrEquals
>  def test
>    @test
>  end
>
>  def test=(test)
>    @test = test
>    'not test'
>  end
> end
>
> p (OrOrEquals.new.test = 'test')
> # ruby 1.8 returns 'test'
> # ruby 1.9 returns 'test'
>
> p (OrOrEquals.new.test ||= 'test')
> # ruby 1.8 returns 'test'
> # ruby 1.9 returns 'not test'
>
> It works as indicated. Is this -normal- behavior ?

And another test:

puts RUBY_VERSION

class OrOrEquals
  def test
    @test
  end

  def test=(test)
    @test = test
    'not test'
  end
end

o = OrOrEquals.new

direct = o.test = 'a test'
p direct
o.test = nil
with_oror = o.test ||= 'a test'
p with_oror
p o.test
o.test = nil

$ multiruby or_or_equals.rb
/opt/local/lib/ruby/site_ruby/1.8/rubygems/source_index.rb:144:
warning: /Users/rick/.gem/ruby/1.8/specifications: Permission denied

VERSION = 1.8.6-p368
CMD     = ~/.multiruby/install/1.8.6-p368/bin/ruby or_or_equals.rb

1.8.6
"a test"
"a test"
"a test"

RESULT = 0

VERSION = 1.8.7-p160
CMD     = ~/.multiruby/install/1.8.7-p160/bin/ruby or_or_equals.rb

1.8.7
"a test"
"a test"
"a test"

RESULT = 0

VERSION = 1.9.1-p0
CMD     = ~/.multiruby/install/1.9.1-p0/bin/ruby or_or_equals.rb

1.9.1
"a test"
"not test"
"a test"


I believe that the 1.8.x behavior is correct is it not?  The return
value of the setter should be discarded and the value of the rhs
expression should be used shouldn't it?
-- 
Rick DeNatale

Blog: http://talklikeaduck.denhaven2.com/
Twitter: http://twitter.com/RickDeNatale
WWR: http://www.workingwithrails.com/person/9021-rick-denatale
LinkedIn: http://www.linkedin.com/in/rickdenatale

[ruby-core:25147] Re: Is this an intentional change in 1.9?

[Yukihiro Matsumoto]

Hi,

In message "Re: [ruby-core:25143] Is this an intentional change in 1.9?"
    on Thu, 27 Aug 2009 02:30:51 +0900, Rick DeNatale <rick.denatale@gmail.com> writes:

|This seems like a 1.9 bug. Is it?

I vaguely remember ko1 explained me about the change due to a YARV
internal issue.  We have to wait ko1 to determine it's bug or not.

							matz.

[ruby-core:25149] Re: Is this an intentional change in 1.9?

[Rick DeNatale]

On Wed, Aug 26, 2009 at 7:25 PM, Yukihiro Matsumoto<matz@ruby-lang.org> wrote:
> Hi,
>
> In message "Re: [ruby-core:25143] Is this an intentional change in 1.9?"
>    on Thu, 27 Aug 2009 02:30:51 +0900, Rick DeNatale <rick.denatale@gmail.com> writes:
>
> |This seems like a 1.9 bug. Is it?
>
> I vaguely remember ko1 explained me about the change due to a YARV
> internal issue.  We have to wait ko1 to determine it's bug or not.

I think it's a code generation issue, and also that it's important not
to change the semantics of a.x ||= value

First here's what's being generated for an instruction sequence:

$ ruby1.9 -e'puts VM::InstructionSequence.compile("obj=Object.new;a =
obj.x ||= 2").disassemble'
== disasm: <ISeq:<compiled>@<compiled>>=================================
local table (size: 3, argc: 0 [opts: 0, rest: -1, post: 0, block: -1] s1)
[ 3] obj        [ 2] a
0000 getinlinecache   <ic>, 7                                         (   1)
0003 getconstant      :Object
0005 setinlinecache   0
0007 send             :new, 0, nil, 0, <ic>
0013 setlocal         obj
0015 getlocal         obj
0017 dup
0018 send             :x, 0, nil, 0, <ic>
0024 dup
0025 branchif         38
0027 pop
0028 putobject        2
0030 send             :x=, 1, nil, 0, <ic>
0036 jump             40
0038 swap
0039 pop
0040 dup
0041 setlocal         a
0043 leave

In the case where obj.x returns a truthy value this code is clearly
evaluating to the return value of the setter and not the rhs.

I think that something like this would work  (I made the branch target
symbolic so I wouldn't need to count bytes.

     getinlinecache   <ic>, 7                                         (   1)
     getconstant      :Object
     setinlinecache   0
     send             :new, 0, nil, 0, <ic>
     setlocal         obj
     getlocal         obj
     dup
     send             :x, 0, nil, 0, <ic>
     dup
     branchif         skip
     pop
     putobject        2
# inserted instruction
     dupn             2           #duplicate receiver of x= and rhs
value stack now has obj 2 obj 2
# end inserted instruction
     send             :x=, 1, nil, 0, <ic>
# inserted instructions
     pop                           # discard result of send, stack now has obj 2
     swap                         # swap stack how has 2 obj
     pop                           # discard receiver of x= stack now
has rhs (2)
# end of inserted instructions
     jump             set
skip swap
set  pop
     dup
     setlocal         a
     leave

I'm assuming here that dup n 2 changes the top of the stack (stack
grows to the right so the rightmost item is the top

value 1 value 2

to

value 1 value 2 value 1 value 2

-- 
Rick DeNatale

Blog: http://talklikeaduck.denhaven2.com/
Twitter: http://twitter.com/RickDeNatale
WWR: http://www.workingwithrails.com/person/9021-rick-denatale
LinkedIn: http://www.linkedin.com/in/rickdenatale

[ruby-core:25170] Re: Is this an intentional change in 1.9?

[Yusuke ENDOH]

Hi,

2009/8/27 Rick DeNatale <rick.denatale@gmail.com>:
> On Wed, Aug 26, 2009 at 7:25 PM, Yukihiro Matsumoto<matz@ruby-lang.org> wrote:
>> Hi,
>>
>> In message "Re: [ruby-core:25143] Is this an intentional change in 1.9?"
>>    on Thu, 27 Aug 2009 02:30:51 +0900, Rick DeNatale <rick.denatale@gmail.com> writes:
>>
>> |This seems like a 1.9 bug. Is it?
>>
>> I vaguely remember ko1 explained me about the change due to a YARV
>> internal issue.  We have to wait ko1 to determine it's bug or not.
>
> I think it's a code generation issue, and also that it's important not
> to change the semantics of a.x ||= value


I agree.  I expected that `a.x ||= value' is equal to `a.x || a.x = value',
but 1.9 cheated on me.


  a = Object.new
  def a.foo; nil; end
  def a.foo=(x); :boo; end

  p(a.foo ||= :foo)        #=> :boo
  p(a.foo || a.foo = :foo) #=> :foo


Here is a patch.  When a.x returns nil or false, three more instructions
are executed than current implementation.  I have not measured actual
impact of speed.


Index: compile.c
===================================================================
--- compile.c	(revision 24684)
+++ compile.c	(working copy)
@@ -3842,22 +3842,26 @@
 	  if lcfin  # r o
 	  pop       # r
 	  eval v    # r v
-	  send a=   # v
-	  jump lfin # v
+	  swap      # v r
+	  topn 1    # v r v
+	  send a=   # v ?
+	  jump lfin # v ?

 	  lcfin:      # r o
 	  swap      # o r
+
+	  lfin:       # o ?
 	  pop       # o

-	  lfin:       # v
-
 	  # and
 	  dup       # r o o
 	  unless lcfin
 	  pop       # r
 	  eval v    # r v
-	  send a=   # v
-	  jump lfin # v
+	  swap      # v r
+	  topn 1    # v r v
+	  send a=   # v ?
+	  jump lfin # v ?

 	  # others
 	  eval v    # r o v
@@ -3881,15 +3885,17 @@
 	    }
 	    ADD_INSN(ret, nd_line(node), pop);
 	    COMPILE(ret, "NODE_OP_ASGN2 val", node->nd_value);
+	    ADD_INSN(ret, nd_line(node), swap);
+	    ADD_INSN1(ret, nd_line(node), topn, INT2FIX(1));
 	    ADD_SEND(ret, nd_line(node), ID2SYM(node->nd_next->nd_aid),
 		     INT2FIX(1));
 	    ADD_INSNL(ret, nd_line(node), jump, lfin);

 	    ADD_LABEL(ret, lcfin);
 	    ADD_INSN(ret, nd_line(node), swap);
-	    ADD_INSN(ret, nd_line(node), pop);

 	    ADD_LABEL(ret, lfin);
+	    ADD_INSN(ret, nd_line(node), pop);
 	}
 	else {
 	    COMPILE(ret, "NODE_OP_ASGN2 val", node->nd_value);

-- 
Yusuke ENDOH <mame@tsg.ne.jp>

[ruby-core:25553] Re: Is this an intentional change in 1.9?

[Rick DeNatale]

This discussion seems to have died out.  Is this something which
should/will be fixed.  Should I open a bug ticket?

On Thu, Aug 27, 2009 at 12:53 PM, Yusuke ENDOH <mame@tsg.ne.jp> wrote:
> Hi,
>
> 2009/8/27 Rick DeNatale <rick.denatale@gmail.com>:
>> On Wed, Aug 26, 2009 at 7:25 PM, Yukihiro Matsumoto<matz@ruby-lang.org> wrote:
>>> Hi,
>>>
>>> In message "Re: [ruby-core:25143] Is this an intentional change in 1.9?"
>>>    on Thu, 27 Aug 2009 02:30:51 +0900, Rick DeNatale <rick.denatale@gmail.com> writes:
>>>
>>> |This seems like a 1.9 bug. Is it?
>>>
>>> I vaguely remember ko1 explained me about the change due to a YARV
>>> internal issue.  We have to wait ko1 to determine it's bug or not.
>>
>> I think it's a code generation issue, and also that it's important not
>> to change the semantics of a.x ||= value
>
>
> I agree.  I expected that `a.x ||= value' is equal to `a.x || a.x = value',
> but 1.9 cheated on me.
>
>
>  a = Object.new
>  def a.foo; nil; end
>  def a.foo=(x); :boo; end
>
>  p(a.foo ||= :foo)        #=> :boo
>  p(a.foo || a.foo = :foo) #=> :foo
>
>
> Here is a patch.  When a.x returns nil or false, three more instructions
> are executed than current implementation.  I have not measured actual
> impact of speed.
>
>
> Index: compile.c
> ===================================================================
> --- compile.c   (revision 24684)
> +++ compile.c   (working copy)
> @@ -3842,22 +3842,26 @@
>          if lcfin  # r o
>          pop       # r
>          eval v    # r v
> -         send a=   # v
> -         jump lfin # v
> +         swap      # v r
> +         topn 1    # v r v
> +         send a=   # v ?
> +         jump lfin # v ?
>
>          lcfin:      # r o
>          swap      # o r
> +
> +         lfin:       # o ?
>          pop       # o
>
> -         lfin:       # v
> -
>          # and
>          dup       # r o o
>          unless lcfin
>          pop       # r
>          eval v    # r v
> -         send a=   # v
> -         jump lfin # v
> +         swap      # v r
> +         topn 1    # v r v
> +         send a=   # v ?
> +         jump lfin # v ?
>
>          # others
>          eval v    # r o v
> @@ -3881,15 +3885,17 @@
>            }
>            ADD_INSN(ret, nd_line(node), pop);
>            COMPILE(ret, "NODE_OP_ASGN2 val", node->nd_value);
> +           ADD_INSN(ret, nd_line(node), swap);
> +           ADD_INSN1(ret, nd_line(node), topn, INT2FIX(1));
>            ADD_SEND(ret, nd_line(node), ID2SYM(node->nd_next->nd_aid),
>                     INT2FIX(1));
>            ADD_INSNL(ret, nd_line(node), jump, lfin);
>
>            ADD_LABEL(ret, lcfin);
>            ADD_INSN(ret, nd_line(node), swap);
> -           ADD_INSN(ret, nd_line(node), pop);
>
>            ADD_LABEL(ret, lfin);
> +           ADD_INSN(ret, nd_line(node), pop);
>        }
>        else {
>            COMPILE(ret, "NODE_OP_ASGN2 val", node->nd_value);
>
> --
> Yusuke ENDOH <mame@tsg.ne.jp>
>
>



-- 
Rick DeNatale

Blog: http://talklikeaduck.denhaven2.com/
Twitter: http://twitter.com/RickDeNatale
WWR: http://www.workingwithrails.com/person/9021-rick-denatale
LinkedIn: http://www.linkedin.com/in/rickdenatale

[ruby-core:25560] Re: Is this an intentional change in 1.9?

[Nobuyoshi Nakada]

Hi,

At Mon, 14 Sep 2009 05:44:15 +0900,
Rick DeNatale wrote in [ruby-core:25553]:
> This discussion seems to have died out.  Is this something which
> should/will be fixed.  Should I open a bug ticket?

Already fixed.  See #1996 and #2050.

-- 
Nobu Nakada

[ruby-core:25561] Re: Is this an intentional change in 1.9?

[Rick DeNatale]

Domo arrigato Nakada-san!

On Sun, Sep 13, 2009 at 8:53 PM, Nobuyoshi Nakada <nobu@ruby-lang.org> wrote:
> Hi,
>
> At Mon, 14 Sep 2009 05:44:15 +0900,
> Rick DeNatale wrote in [ruby-core:25553]:
>> This discussion seems to have died out.  Is this something which
>> should/will be fixed.  Should I open a bug ticket?
>
> Already fixed.  See #1996 and #2050.
>
> --
> Nobu Nakada
>
>



-- 
Rick DeNatale

Blog: http://talklikeaduck.denhaven2.com/
Twitter: http://twitter.com/RickDeNatale
WWR: http://www.workingwithrails.com/person/9021-rick-denatale
LinkedIn: http://www.linkedin.com/in/rickdenatale