Bug #2545
Array#delete_if is borked if user calls 'break'
| Status: | Closed | Start date: | 01/02/2010 | |
|---|---|---|---|---|
| Priority: | Normal | Due date: | ||
| Assignee: |  Yukihiro Matsumoto |
% Done: | 100% |
|
| Category: | core | |||
| Target version: | - | |||
| ruby -v: | ruby 1.8.7 (2009-06-12 patchlevel 174) [x86_64-linux] |
Description
Array is corrupted if you break out of a delete_if { ... } loop. I would expect that the elements already marked as deleted would be deleted, and the remainder of the array would be unchanged.
>> a = [5,6,7,8,9,10]
=> [5, 6, 7, 8, 9, 10]
>> a.delete_if { |x| break if x > 8; x < 7 }
=> nil
>> a
=> [7, 8, 7, 8, 9, 10]
>> RUBY_VERSION
=> "1.8.7"
>> RUBY_PATCHLEVEL
=> 174
Associated revisions
* array.c (rb_ary_reject_bang, rb_ary_delete_if): rejected
elements should be removed. fixed [Bug #2545]
* array.c (rb_ary_reject_bang, rb_ary_delete_if): rejected
elements should be removed. fixed [Bug #2545]
* array.c (ary_reject_bang): should not remove elements which are
not yielded. [Bug #2545]
* array.c (ary_reject_bang): should not remove elements which are
not yielded. [Bug #2545]
History
Updated by Nobuyoshi Nakada about 2 years ago
Hi, At Sat, 2 Jan 2010 05:55:00 +0900, Brian Candler wrote in [ruby-core:27366]: > Array is corrupted if you break out of a delete_if { ... } > loop. I would expect that the elements already marked as > deleted would be deleted, and the remainder of the array > would be unchanged. The behavior would be an implementation detail, and should be undefined (or implementation defined), I guess. Index: array.c =================================================================== --- array.c (revision 26229) +++ array.c (working copy) @@ -2307,7 +2307,18 @@ rb_ary_reject_bang(VALUE ary) for (i1 = i2 = 0; i1 < RARRAY_LEN(ary); i1++) { VALUE v = RARRAY_PTR(ary)[i1]; - if (RTEST(rb_yield(v))) continue; if (i1 != i2) { + int state = 0; + if (RTEST(rb_protect(rb_yield, v, &state))) continue; rb_ary_store(ary, i2, v); + if (state) { + VALUE *ptr = RARRAY_PTR(ary); + long len = RARRAY_LEN(ary); + MEMCPY(ptr + i2 + 1, ptr + i1 + 1, VALUE, len - i1 - 1); + ARY_SET_LEN(ary, len - i1 + i2); + rb_jump_tag(state); + } + } + else { + if (RTEST(rb_yield(v))) continue; } i2++; -- Nobu Nakada
Updated by Shyouhei Urabe about 2 years ago
- Status changed from Open to Assigned
- Assignee set to Yukihiro Matsumoto
I don't think so. I'd also expect as the reporter did. Isn't it a bug? Assigning to matz because this can be a design issue.
Updated by Dave B 7 months ago
Just met this problem:
ruby 1.8.7 (2011-02-18 patchlevel 334) [i386-mingw32]
I'd also consider it a bug and that the ruby implementation should be hidden from the user. Once an element has been selected for deletion, at the end of this iteration, it should be expected to be gone. When using very large arrays, where the programmer knows of a shortcut (e.g. the rest of the array need not be considered), s/he should be encouraged to handle it with 'break'. In testing, I was left wondering whether 'delete_if' was non-destructive, because nothing had changed, and started looking for a bang! method.
To achieve the current behaviour, I only need an Array#dup above the loop.
Thanks to Nobu for working a patch.
daz
Updated by Nobuyoshi Nakada 7 months ago
- Status changed from Assigned to Closed
- % Done changed from 0 to 100
Updated by Nobuyoshi Nakada 7 months ago
- Project changed from Ruby 1.8 to ruby-trunk
- Category changed from core to core
- Target version deleted (
Ruby 1.8.7)
Updated by Tomoyuki Chikanaga 7 months ago
Hi,
According to test added by r32360,
a = [ 5, 6, 7, 8, 9, 10 ]
a.delete_if {|i| break i if i > 8; i < 7}
it results
a # => [7, 8]
But I feel it could be [7, 8, 9, 10] because block didn't *return true* for 9, 10.
Matz, How do you think about it?