Skip to content

Commit

Permalink
Test that mg_copy is called only when expected.
Browse files Browse the repository at this point in the history
mg_copy is called for most operations on tied hashes, but not for `keys`.
This is externally observable behaviour, and Variable::Magic on CPAN has
a regression test that expects the current behaviour.

However, until this commit we had no tests in core that verified that the
current behaviour has not changed. Hence this new test.

Add the `static` keyword that was missing on several related variables and
functions in APItest.xs
  • Loading branch information
nwc10 committed Aug 28, 2021
1 parent dff795e commit 2adefdb
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 3 deletions.
41 changes: 38 additions & 3 deletions ext/XS-APItest/APItest.xs
Expand Up @@ -105,7 +105,7 @@ typedef struct {

START_MY_CXT

int
static int
S_myset_set(pTHX_ SV* sv, MAGIC* mg)
{
SV *isv = (SV*)mg->mg_ptr;
Expand All @@ -115,9 +115,25 @@ S_myset_set(pTHX_ SV* sv, MAGIC* mg)
return 0;
}

MGVTBL vtbl_foo, vtbl_bar;
MGVTBL vtbl_myset = { 0, S_myset_set, 0, 0, 0, 0, 0, 0 };
static MGVTBL vtbl_foo, vtbl_bar;
static MGVTBL vtbl_myset = { 0, S_myset_set, 0, 0, 0, 0, 0, 0 };

static int
S_mycopy_copy(pTHX_ SV *sv, MAGIC* mg, SV *nsv, const char *name, I32 namlen) {
PERL_UNUSED_ARG(sv);
PERL_UNUSED_ARG(nsv);
PERL_UNUSED_ARG(name);
PERL_UNUSED_ARG(namlen);

/* Count that we were called to "copy".
There's actually no point in copying *this* magic onto nsv, as it's a
SCALAR, whereas mg_copy is only triggered for ARRAYs and HASHes.
It's not *exactly* generic. :-( */
++mg->mg_private;
return 0;
}

STATIC MGVTBL vtbl_mycopy = { 0, 0, 0, 0, 0, S_mycopy_copy, 0, 0 };

/* indirect functions to test the [pa]MY_CXT macros */

Expand Down Expand Up @@ -4664,6 +4680,25 @@ CODE:
sv_magicext(SvRV(rsv), NULL, PERL_MAGIC_ext, &vtbl_myset,
(const char *)thingy, 0);

void
sv_magic_mycopy(SV *rsv)
PREINIT:
MAGIC *mg;
CODE:
/* It's only actually useful to attach this to arrays and hashes. */
mg = sv_magicext(SvRV(rsv), NULL, PERL_MAGIC_ext, &vtbl_mycopy, NULL, 0);
mg->mg_flags = MGf_COPY;

SV *
sv_magic_mycopy_count(SV *rsv)
PREINIT:
MAGIC *mg;
CODE:
mg = mg_findext(SvRV(rsv), PERL_MAGIC_ext, &vtbl_mycopy);
RETVAL = mg ? newSViv(mg->mg_private) : &PL_sv_undef;
OUTPUT:
RETVAL


MODULE = XS::APItest PACKAGE = XS::APItest

Expand Down
34 changes: 34 additions & 0 deletions ext/XS-APItest/t/hash.t
Expand Up @@ -291,6 +291,40 @@ pass("hv_store works on the hint hash");
is_deeply(\@keys, [ sort keys %hash ], "check HeSVKEY_force()");
}

# Test that mg_copy is called when expected (and not called when not)
# No (other) tests in core will fail if the implementation of `keys %tied_hash`
# is (accidentally) changed to also call hv_iterval() and trigger mg_copy.
# However, this behaviour is visible, and tested by Variable::Magic on CPAN.

{
my %h;
my $obj = tie %h, 'Tie::StdHash';
sv_magic_mycopy(\%h);

is(sv_magic_mycopy_count(\%h), 0);

$h{perl} = "rules";

is(sv_magic_mycopy_count(\%h), 1);

is($h{perl}, "rules", "found key");

is(sv_magic_mycopy_count(\%h), 2);

# keys *doesn't* trigger copy magic, so the count is still 2
my @flat = keys %h;

is(sv_magic_mycopy_count(\%h), 2);

@flat = values %h;

is(sv_magic_mycopy_count(\%h), 3);

@flat = each %h;

is(sv_magic_mycopy_count(\%h), 4);
}

done_testing;
exit;

Expand Down

0 comments on commit 2adefdb

Please sign in to comment.