diff --git a/MANIFEST b/MANIFEST index 712316de13..7c94ce9176 100644 --- a/MANIFEST +++ b/MANIFEST @@ -5878,6 +5878,7 @@ t/op/grep.t See if grep() and map() work t/op/groups.t See if $( works t/op/gv.t See if typeglobs work t/op/hash.t See if the complexity attackers are repelled +t/op/hash-clear-placeholders.t FIXME t/op/hash-rt85026.t See if hash iteration/deletion works t/op/hashassign.t See if hash assignments work t/op/hashwarn.t See if warnings for bad hash assignments work diff --git a/hv.c b/hv.c index bc2f24c177..9e6ba13362 100644 --- a/hv.c +++ b/hv.c @@ -1882,14 +1882,14 @@ Perl_hv_clear_placeholders(pTHX_ HV *hv) } static void -S_clear_placeholders(pTHX_ HV *hv, U32 items) +S_clear_placeholders(pTHX_ HV *hv, const U32 placeholders) { I32 i; + U32 to_find = placeholders; PERL_ARGS_ASSERT_CLEAR_PLACEHOLDERS; - if (items == 0) - return; + assert(to_find); i = HvMAX(hv); do { @@ -1909,12 +1909,10 @@ S_clear_placeholders(pTHX_ HV *hv, U32 items) hv_free_ent(hv, entry); } - if (--items == 0) { + if (--to_find == 0) { /* Finished. */ - I32 placeholders = HvPLACEHOLDERS_get(hv); HvTOTALKEYS(hv) -= (IV)placeholders; - /* HvUSEDKEYS expanded */ - if ((HvTOTALKEYS(hv) - placeholders) == 0) + if (HvTOTALKEYS(hv) == 0) HvHASKFLAGS_off(hv); HvPLACEHOLDERS_set(hv, 0); return; @@ -1925,7 +1923,7 @@ S_clear_placeholders(pTHX_ HV *hv, U32 items) } } while (--i >= 0); /* You can't get here, hence assertion should always fail. */ - assert (items == 0); + assert (to_find == 0); NOT_REACHED; /* NOTREACHED */ } @@ -3343,7 +3341,6 @@ Perl_refcounted_he_chain_2hv(pTHX_ const struct refcounted_he *chain, U32 flags) if (placeholders) { clear_placeholders(hv, placeholders); - HvTOTALKEYS(hv) -= placeholders; } /* We could check in the loop to see if we encounter any keys with key diff --git a/t/op/hash-clear-placeholders.t b/t/op/hash-clear-placeholders.t new file mode 100644 index 0000000000..7e2ca640d5 --- /dev/null +++ b/t/op/hash-clear-placeholders.t @@ -0,0 +1,56 @@ +#!/usr/bin/perl -w + +BEGIN { + chdir 't' if -d 't'; + require './test.pl'; + set_up_inc( '../lib' ); + skip_all_without_dynamic_extension("Devel::Peek"); + skip_all_without_dynamic_extension("Hash::Util"); +} + +use strict; +use Devel::Peek; +use Hash::Util qw(lock_keys_plus); + +my %hash = (chr 255 => 42, chr 256 => 6 * 9); +lock_keys_plus(%hash, "baz"); + +my $tempfile = tempfile(); + +local *OLDERR; +open(OLDERR, ">&STDERR") || die "Can't dup STDERR: $!"; +open(STDERR, ">", $tempfile) || + die "Could not open '$tempfile' for write: $^E"; + +my $sep = "=-=-=-=\n"; +Dump \%hash; +print STDERR $sep; +delete $hash{chr 255}; +Internals::hv_clear_placeholders(%hash); +Dump \%hash; +print STDERR $sep; +delete $hash{chr 256}; +Internals::hv_clear_placeholders(%hash); +Dump \%hash; + +open(STDERR, ">&OLDERR") || die "Can't dup OLDERR: $!"; +open(my $fh, "<", $tempfile) || + die "Could not open '$tempfile' for read: $^E"; +local $/; +my $got = <$fh>; + +my ($first, $second, $third) = split $sep, $got; + +like($first, qr/\bPERL_MAGIC_rhash\b/, 'first dump has rhash magic'); +like($second, qr/\bPERL_MAGIC_rhash\b/, 'second dump has rhash magic'); +like($third, qr/\bPERL_MAGIC_rhash\b/, 'third dump has rhash magic'); + +like($first, qr/\bHASKFLAGS\b/, 'first dump has HASHKFLAGS set'); +like($second, qr/\bHASKFLAGS\b/, 'second dump has HASHKFLAGS set'); +unlike($third, qr/\bHASKFLAGS\b/, 'third dump has HASHKFLAGS clear'); + +like($first, qr/\bMG_LEN = 1\b/, 'first dump has 1 placeholder'); +unlike($second, qr/\bMG_LEN\b/, 'second dump has 0 placeholders'); +unlike($third, qr/\bMG_LEN\b/, 'third dump has 0 placeholders'); + +done_testing();