Bug #11811: Chaining lazy enumerators causes duplicate ouput

• Status: Closed
• Assignee: nobu (Nobuyoshi Nakada)
• Target version: -
• ruby -v: ruby 2.3.0preview2 (2015-12-11 trunk 53028) [x86_64-darwin15]
• Backport: 2.0.0: DONTNEED, 2.1: DONTNEED, 2.2: DONTNEED

[cabeer (Chris Beer)]

In Ruby 2.3.0-preview2, I'm seeing a change in behavior using lazy enumerators with select/reject and the & operator:

irb(main):037:0> %w(1 2 3).lazy.reject(&:empty?).each { |x| puts x }
1
1
2
2
3
3

Note that the output is doubled. However, if I don't use the & shorthand, the output is as expected:

irb(main):038:0> %w(1 2 3).lazy.reject { |x| x.empty? }.each { |x| puts x }
1
2
3
=> nil

And in Ruby 2.2.3, both variants produced the same result:

irb(main):001:0> %w(1 2 3).lazy.reject(&:empty?).each { |x| puts x }
1
2
3
=> nil

[marcandre (Marc-Andre Lafortune)]

• Assignee set to nobu (Nobuyoshi Nakada)

Ouch.

Very odd. With p instead of puts, the output is not doubled.

[mame (Yusuke Endoh)]

Not only reject's bug. Lazy enumerator ignores &:method generally.

$ ./miniruby -e '

s = [1,2,3].lazy.map(&:nonexist)
p :foo
s.each {|x| p x; 100 + x }

'
:foo
1
101
2
102
3
103

Correct behavior:

$ ruby -ve '

s = [1,2,3].lazy.map(&:nonexist)
p :foo
s.each {|x| p x; 100 + x }

'
ruby 2.2.3p173 (2015-08-18 revision 51636) [x86_64-linux]
:foo
-e:5:in `each': undefined method `nonexist' for 1:Fixnum (NoMethodError)

Explicit .to_proc works correctly.

$ ./miniruby -e '

s = [1,2,3].lazy.map(&:nonexist.to_proc)
p :foo
s.each {|x| p x; 100 + x }

'
:foo
-e:5:in `each': undefined method `nonexist' for 1:Fixnum (NoMethodError)

So I guess this is caused by nobu's optimization.

--
Yusuke Endoh [e-mail]

[shugo (Shugo Maeda)]

• File vm_caller_setup_arg_block.diff vm_caller_setup_arg_block.diff added

Yusuke Endoh wrote:

So I guess this is caused by nobu's optimization.

The bug was introduced in r51995:

Author: nobu <nobu@b2dd03c8-39d4-4d8f-98ff-823fe69b080e>
Date:   Thu Oct 1 10:52:52 2015 +0000

    vm_args.c: fix marking symbol ifunc
    
    * vm_args.c (vm_caller_setup_arg_block): store new ifunc for
      symbol in control frame proc to be marked.
    
    * proc.c (proc_new), vm_insnhelper.c (vm_yield_with_cfunc):
      block->proc may be an ifunc now.

It seems to be fixed by the attached patch, but I'm not sure.
Could you check it, nobu?

[nobu (Nobuyoshi Nakada)]

I'm afraid that it may cause GC mark miss of dynamic symbols.

[shugo (Shugo Maeda)]

Nobuyoshi Nakada wrote:

I'm afraid that it may cause GC mark miss of dynamic symbols.

Ah, I see.

How about to apply the optimization only to static symbols?

        else if (STATIC_SYM_P(proc) && rb_method_basic_definition_p(rb_cSymbol, idTo_proc)) {
            calling->blockptr = RUBY_VM_GET_BLOCK_PTR_IN_CFP(reg_cfp);
            calling->blockptr->iseq = (rb_iseq_t *)proc;
            calling->blockptr->proc = 0;
        }

I guess dynamic symbols are rarely passed as block arguments.

[shugo (Shugo Maeda)]

Shugo Maeda wrote:

Nobuyoshi Nakada wrote:

I'm afraid that it may cause GC mark miss of dynamic symbols.

Ah, I see.

How about to apply the optimization only to static symbols?

Or the following patch also seems to fix the bug:

diff --git a/vm.c b/vm.c
index 087cba7..a03e068 100644
--- a/vm.c
+++ b/vm.c
@@ -567,6 +567,10 @@ vm_make_proc_from_block(rb_thread_t *th, rb_block_t *block, VALUE *procptr)
 	*procptr = block->proc = rb_vm_make_proc(th, block, rb_cProc);
 	return TRUE;
     }
+    else if (SYMBOL_P(block->proc)) {
+	*procptr = rb_sym_to_proc(block->proc);
+	return TRUE;
+    }
     else {
 	*procptr = block->proc;
 	return FALSE;

[nobu (Nobuyoshi Nakada)]

Great, that's it.
Please commit it.

[shugo (Shugo Maeda)]

• Status changed from Open to Closed

Applied in changeset r53152.

---

• vm.c (vm_make_proc_from_block): should convert a Symbol to a Proc.
  [ruby-core:72083] [Bug #11811]

[shugo (Shugo Maeda)]

Nobuyoshi Nakada wrote:

Great, that's it.
Please commit it.

Thanks for your confirmation.

I've committed it in r53152.
It's confusing that block->proc may not be a Proc, though....

[shugo (Shugo Maeda)]

• Backport changed from 2.0.0: UNKNOWN, 2.1: UNKNOWN, 2.2: UNKNOWN to 2.0.0: DONTNEED, 2.1: DONTNEED, 2.2: DONTNEED

[Hanmac (Hans Mackowiak)]

i don't know if thats the cause of it, but it seems that it did break this thing https://bugs.ruby-lang.org/issues/11830

[shugo (Shugo Maeda)]

Hans Mackowiak wrote:

i don't know if thats the cause of it, but it seems that it did break this thing https://bugs.ruby-lang.org/issues/11830

#11830 is not caused by r53152, but by the basically same reason of this issue (i.e., the fact that block->proc can be a Symbol now).
Thanks anyway.