Skip to content

Commit

Permalink
Hash-Util-FieldHash: fixup test on PERL_RC_STACK
Browse files Browse the repository at this point in the history
Hash::Util::FieldHash is an implementation of "inside out" objects,
where field values are stored in per-field hashes whose keys are the
numeric addresses of the objects, and whose values represent the value
of that field for a particular object.

The hashes are magic, in that they don't hold a reference to each
object, but automatically delete the relevant entry from the hash when
the object is freed. This is achieved internally with weak references
and a bunch of user magic attached to the hash and the object.

A test in 02_function.t checks that such a link in the magic hash to an
object is removed after the real object is freed. This test looked
something like:

    $h{[]} = 123;
    is(keys %h, 0);

which relied on the temp anon array being freed almost immediately and
triggering the hash delete. Now it turns out that OP_ANONLIST (and its
later optimisation, OP_EMPTYAVHV) return an SvTEMP RV. In the code
above, this means that [] gets freed at the start of the next statement
(by the freetmps() in pp_nextstate()), and all works as expected.

However, when pp_emptyavhv() is unwrapped in the next commit, on
PERL_RC_STACK builds it will skip mortalising the return result, and
just rely on the ref count from the stack to keep the RV alive as long
as needed. By "needed", this is only while the helem op is being
executed, and the temp array is thus freed at the end of the helem,
rather than at the end of the statement. This triggers the entry in the
hash being auto-deleted earlier, so when the next op, the scalar assign,
executes, it triggers a warning:

    Useless assignment to a temporary at t/02_function.t line 121.

After a lot of faffing about trying to work out what was going on, I
concluded that the intent of the test could just as well be done by
creating a real array reference variable and to just undef it directly
after the hash assign.

(I.e. it took some time to convince myself that there wasn't a bug in
pp_emptyavhv, pp_sassign nor FieldHash.xs, and that just the test needed
fixing.)
  • Loading branch information
iabyn committed Nov 16, 2023
1 parent 8b7d699 commit 25d6d0c
Showing 1 changed file with 5 additions and 2 deletions.
7 changes: 5 additions & 2 deletions ext/Hash-Util-FieldHash/t/02_function.t
Expand Up @@ -118,8 +118,11 @@ my @test_types = qw(SCALAR ARRAY HASH GLOB);
{
my %h;
Hash::Util::FieldHash::_fieldhash \ %h, $fieldhash_mode;
$h{ []} = 123;
is( keys %h, 0, "blip");
my $ar = [];
$h{$ar} = 123;
is( keys %h, 1, "blip");
undef $ar;
is( keys %h, 0, "blop");
}

for my $preload ( [], [ map {}, 1 .. 3] ) {
Expand Down

0 comments on commit 25d6d0c

Please sign in to comment.