Bug #11811
closedChaining lazy enumerators causes duplicate ouput
Description
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
Files
Updated by marcandre (Marc-Andre Lafortune) over 8 years ago
- Assignee set to nobu (Nobuyoshi Nakada)
Ouch.
Very odd. With p
instead of puts
, the output is not doubled.
Updated by mame (Yusuke Endoh) over 8 years ago
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 mame@ruby-lang.org
Updated by shugo (Shugo Maeda) over 8 years ago
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?
Updated by nobu (Nobuyoshi Nakada) over 8 years ago
I'm afraid that it may cause GC mark miss of dynamic symbols.
Updated by shugo (Shugo Maeda) over 8 years ago
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.
Updated by shugo (Shugo Maeda) over 8 years ago
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;
Updated by nobu (Nobuyoshi Nakada) over 8 years ago
Great, that's it.
Please commit it.
Updated by shugo (Shugo Maeda) over 8 years ago
- 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]
Updated by shugo (Shugo Maeda) over 8 years ago
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....
Updated by shugo (Shugo Maeda) over 8 years ago
- Backport changed from 2.0.0: UNKNOWN, 2.1: UNKNOWN, 2.2: UNKNOWN to 2.0.0: DONTNEED, 2.1: DONTNEED, 2.2: DONTNEED
Updated by Hanmac (Hans Mackowiak) over 8 years ago
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
Updated by shugo (Shugo Maeda) over 8 years ago
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.