Preserve undef identity in const ops
authorFather Chrysostomos <sprout@cpan.org>
Fri, 13 Sep 2013 01:26:30 +0000 (18:26 -0700)
committerFather Chrysostomos <sprout@cpan.org>
Fri, 13 Sep 2013 07:49:38 +0000 (00:49 -0700)
This fixes the one remaining issue in ticket #105906.

Under ithreads, the constants are transferred to the pad at compile
time.  Since the pads are AVs, and since AVs used &PL_sv_undef to rep-
resent unused slots, &PL_sv_undef could not be stored in the pad, and
so was copied.

That meant the same constant would produce a different scalar at each
call site:

$  ./perl -Ilib -le 'BEGIN { $::{z} = \undef }  for(z,z) { print \$_ }'
SCALAR(0x7fecb38062e0)
SCALAR(0x7fecb3806100)

Commit ce0d59f changed the way AVs work, so now we *can* store
&PL_sv_undef in an AV meaningfully, and there is no longer any need
for this copying hack.

op.c
t/op/undef.t

diff --git a/op.c b/op.c
index 9be7bbc..ae3739e 100644 (file)
--- a/op.c
+++ b/op.c
@@ -1754,24 +1754,10 @@ S_finalize_op(pTHX_ OP* o)
         * for reference counts, sv_upgrade() etc. */
        if (cSVOPo->op_sv) {
            const PADOFFSET ix = pad_alloc(OP_CONST, SVf_READONLY);
-           if (o->op_type != OP_METHOD_NAMED
-               && cSVOPo->op_sv == &PL_sv_undef) {
-               /* PL_sv_undef is hack - it's unsafe to store it in the
-                  AV that is the pad, because av_fetch treats values of
-                  PL_sv_undef as a "free" AV entry and will merrily
-                  replace them with a new SV, causing pad_alloc to think
-                  that this pad slot is free. (When, clearly, it is not)
-               */
-               SvOK_off(PAD_SVl(ix));
-               SvPADTMP_on(PAD_SVl(ix));
-               SvREADONLY_on(PAD_SVl(ix));
-           }
-           else {
-               SvREFCNT_dec(PAD_SVl(ix));
-               PAD_SETSV(ix, cSVOPo->op_sv);
-               /* XXX I don't know how this isn't readonly already. */
-               if (!SvIsCOW(PAD_SVl(ix))) SvREADONLY_on(PAD_SVl(ix));
-           }
+           SvREFCNT_dec(PAD_SVl(ix));
+           PAD_SETSV(ix, cSVOPo->op_sv);
+           /* XXX I don't know how this isn't readonly already. */
+           if (!SvIsCOW(PAD_SVl(ix))) SvREADONLY_on(PAD_SVl(ix));
            cSVOPo->op_sv = NULL;
            o->op_targ = ix;
        }
index eafa6db..d7d78c0 100644 (file)
@@ -10,7 +10,7 @@ use strict;
 
 use vars qw(@ary %ary %hash);
 
-plan 85;
+plan 86;
 
 ok !defined($a);
 
@@ -168,6 +168,13 @@ like $@, qr/^Modification of a read/;
     is $k, undef, 'each undef at end';
 }
 
+# part of #105906: inlined undef constant getting copied
+BEGIN { $::{z} = \undef }
+for (z,z) {
+    push @_, \$_;
+}
+is $_[0], $_[1], 'undef constants preserve identity';
+
 # this will segfault if it fails
 
 sub PVBM () { 'foo' }