diff options
author | Father Chrysostomos <sprout@cpan.org> | 2011-11-05 18:25:42 -0700 |
---|---|---|
committer | Father Chrysostomos <sprout@cpan.org> | 2011-11-05 18:25:42 -0700 |
commit | 41aa816fb45bd52c294789c95d121e737480711f (patch) | |
tree | 21ef3aa46bdbf9bab866549b4f8736dfd455035a /t/op/each.t | |
parent | bfaf5b52ed4d07654faa7d3a6fb83d363c7110da (diff) | |
download | perl-41aa816fb45bd52c294789c95d121e737480711f.tar.gz |
each() should not leave RITER set on empty hash
Commit 900ac0519e (5.11.0) sped up keys() on an empty hash by modify-
ing the iteration code not to loop through the buckets looking for an
entry if the number of keys is 0. Interestingly, it had no visible
affect on keys(), but it *did* have one on each(). Resetting the ite-
rator’s current bucket number (RITER) used to be done inside that loop
in hv_iternext. keys() always begins by resetting the iterator, so it
was unaffected. But each %empty will leave the iterator as-is. It
will be set on an empty hash if the last element was deleted while an
iterator was active. This has buggy side-effects.
$h{1} = 2;
each %h; # returns (1, 2)
delete $h{1};
each %h; # returns false; should reset iterator
$h{1}=2;
print each %h, "\n"; # prints nothing
Commit 3b37eb248 (5.15.0), which changed the way S_hfreeentries works.
(S_hfreentries is called by all operators that empty hashes, such as
%h=() and undef %h.) Now S_hfreentries does nothing if the hash is
empty. That change on its own should have been harmless, but the
result was that even %h=() won’t reset RITER after each() has put
things in an inconsistent state. This caused test failures in
Text::Tabulate.
So the solution, of course, is to complete the change made by
900ac0519e and reset the iterator properly in hv_iternext if the
hash is empty.
Diffstat (limited to 't/op/each.t')
-rw-r--r-- | t/op/each.t | 16 |
1 files changed, 15 insertions, 1 deletions
diff --git a/t/op/each.t b/t/op/each.t index d9e15422b2..d12d678ea5 100644 --- a/t/op/each.t +++ b/t/op/each.t @@ -6,7 +6,7 @@ BEGIN { require './test.pl'; } -plan tests => 56; +plan tests => 57; $h{'abc'} = 'ABC'; $h{'def'} = 'DEF'; @@ -255,3 +255,17 @@ for my $k (qw(each keys values)) { } ::is($c, 1, "single key now freed"); } + +{ + # Make sure each() does not leave the iterator in an inconsistent state + # (RITER set to >= 0, with EITER null) if the active iterator is + # deleted, leaving the hash apparently empty. + my %h; + $h{1} = 2; + each %h; + delete $h{1}; + each %h; + $h{1}=2; + is join ("-", each %h), '1-2', + 'each on apparently empty hash does not leave RITER set'; +} |