Skip to content

Commit c1c8af3

Browse files
committed
S_fold_constants: remove early SvREADONLY(sv) to allow SvIsCOW(sv)
Standard CONST PVs have the IsCOW flag set, meaning that COW can be used when assigning the CONST to a variable, rather than making a copy of the buffer. CONST PVs arising from constant folding have been lacking this flag, leading to unnecessary copying of PV buffers. This seems to have occurred because a common branch in S_fold_constants marks SVs as READONLY before the new CONST OP is created. When the OP is created, the Perl_ck_svconst() check function is called - this is the same as when a standard CONST OP is created. If the SV is not already marked as READONLY, the check function will try to set IsCOW if it is safe to do so, then in either case will make sure that the READONLY flag is set. This commit therefore removes the SvREADONLY(sv) statement from S_fold_constants(), allowing Perl_ck_svconst() to set the IsCOW and READONLY flags itself. Minor test updates are also included.
1 parent 81c6106 commit c1c8af3

File tree

3 files changed

+8
-4
lines changed

3 files changed

+8
-4
lines changed

ext/Devel-Peek/t/Peek.t

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -534,7 +534,7 @@ do_test('string with Unicode',
534534
. '"\\\0 \[UTF8 "\\\x\{100\}\\\x\{0\}\\\x\{200\}"\]
535535
CUR = 5
536536
LEN = \\d+
537-
COW_REFCNT = 1 # $] < 5.019007
537+
COW_REFCNT = 1 # $] < 5.019007 || $] >=5.041000
538538
');
539539

540540
do_test('reference to hash containing Unicode',
@@ -558,7 +558,7 @@ do_test('reference to hash containing Unicode',
558558
PV = $ADDR "' . $cp200_bytes . '"\\\0 \[UTF8 "\\\x\{200\}"\]
559559
CUR = 2
560560
LEN = \\d+
561-
COW_REFCNT = 1 # $] < 5.019007
561+
COW_REFCNT = 1 # $] < 5.019007 || $] >=5.041000
562562
', '',
563563
$] >= 5.015
564564
? undef

op.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5081,7 +5081,9 @@ S_fold_constants(pTHX_ OP *const o)
50815081
SvPADTMP_off(sv);
50825082
else if (!SvIMMORTAL(sv)) {
50835083
SvPADTMP_on(sv);
5084-
SvREADONLY_on(sv);
5084+
/* Do not set SvREADONLY(sv) here. newSVOP will call
5085+
* Perl_ck_svconst, which will do it. Setting it early
5086+
* here prevents Perl_ck_svconst from setting SvIsCOW(sv).*/
50855087
}
50865088
newop = newSVOP(OP_CONST, 0, MUTABLE_SV(sv));
50875089
if (!is_stringify) newop->op_folded = 1;

t/op/undef.t

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,9 @@ SKIP: {
165165
my $out = runperl(stderr => 1,
166166
progs => [ split /\n/, <<'EOS' ]);
167167
require Devel::Peek;
168-
my $f = q(x) x 40; $f = undef;
168+
my $f = q(x) x 40;
169+
chop $f; # Make sure that the PV buffer is not COWed
170+
$f = undef;
169171
Devel::Peek::Dump($f);
170172
undef $f;
171173
Devel::Peek::Dump($f);

0 commit comments

Comments
 (0)