This is a live mirror of the Perl 5 development currently hosted at https://github.com/perl/perl5
Remove !IS_PADGV assertions
authorFather Chrysostomos <sprout@cpan.org>
Thu, 18 Sep 2014 05:10:47 +0000 (22:10 -0700)
committerFather Chrysostomos <sprout@cpan.org>
Thu, 18 Sep 2014 06:39:30 +0000 (23:39 -0700)
Commit 60779a30f stopped setting PADTMP on GVs in pads,
and changed many instances of if(SvPADTMP && !IS_PADGV) to
if(SvPADTMP){assert(!IS_PADGV)...}.

This PADTMP was leftover from the original ithreads implementation
that marked these as PADTMP under the assumption that anything in a
pad had to be marked PADMY or PADTMP.

Since we don’t want these GVs copied the way operator targets are
(PADTMP indicates the latter), we needed this !IS_PADGV check all over
the place.

60779a30f was actually right in removing the flag and the !IS_PADGV
check, because it should be possible for XS modules to create ops that
return copiable GVs.  But the assertions prevent that from working.

More importantly (to me at least), this IS_PADGV doesn’t quite make
sense and I am trying to eliminate it.

BTW, you have to be doubly naughty, but it is possible to fail these
assertions:

$ ./perl -Ilib -e 'BEGIN { sub { $::{foo} = \@_; constant::_make_const(@_) }->(*bar) } \ foo'
Assertion failed: (!IS_PADGV(sv)), function S_refto, file pp.c, line 576.
Abort trap: 6

pp.c
pp_ctl.c
pp_hot.c
pp_sort.c
regexec.c

diff --git a/pp.c b/pp.c
index 547731f..0e92254 100644 (file)
--- a/pp.c
+++ b/pp.c
@@ -573,7 +573,6 @@ S_refto(pTHX_ SV *sv)
        SvREFCNT_inc_void_NN(sv);
     }
     else if (SvPADTMP(sv)) {
-        assert(!IS_PADGV(sv));
         sv = newSVsv(sv);
     }
     else {
@@ -1709,7 +1708,6 @@ PP(pp_repeat)
 #else
                 if (*SP) {
                    if (mod && SvPADTMP(*SP)) {
-                       assert(!IS_PADGV(*SP));
                        *SP = sv_mortalcopy(*SP);
                    }
                   SvTEMP_off((*SP));
@@ -4958,7 +4956,6 @@ PP(pp_lslice)
            if (!(*lelem = firstrelem[ix]))
                *lelem = &PL_sv_undef;
            else if (mod && SvPADTMP(*lelem)) {
-                assert(!IS_PADGV(*lelem));
                *lelem = firstrelem[ix] = sv_mortalcopy(*lelem);
             }
        }
index 4ceca53..95ba848 100644 (file)
--- a/pp_ctl.c
+++ b/pp_ctl.c
@@ -927,7 +927,6 @@ PP(pp_grepstart)
 
     src = PL_stack_base[*PL_markstack_ptr];
     if (SvPADTMP(src)) {
-        assert(!IS_PADGV(src));
        src = PL_stack_base[*PL_markstack_ptr] = sv_mortalcopy(src);
        PL_tmps_floor++;
     }
@@ -1080,7 +1079,6 @@ PP(pp_mapwhile)
        /* set $_ to the new source item */
        src = PL_stack_base[PL_markstack_ptr[-1]];
        if (SvPADTMP(src)) {
-            assert(!IS_PADGV(src));
             src = sv_mortalcopy(src);
         }
        SvTEMP_off(src);
index 2624a71..97f24d8 100644 (file)
--- a/pp_hot.c
+++ b/pp_hot.c
@@ -1917,7 +1917,6 @@ PP(pp_iter)
                 Perl_croak(aTHX_ "Use of freed value in iteration");
             }
             if (SvPADTMP(sv)) {
-                assert(!IS_PADGV(sv));
                 sv = newSVsv(sv);
             }
             else {
@@ -2435,7 +2434,6 @@ PP(pp_grepwhile)
 
        src = PL_stack_base[*PL_markstack_ptr];
        if (SvPADTMP(src)) {
-            assert(!IS_PADGV(src));
            src = PL_stack_base[*PL_markstack_ptr] = sv_mortalcopy(src);
            PL_tmps_floor++;
        }
@@ -2697,7 +2695,6 @@ try_autoload:
                if (*MARK)
                {
                    if (SvPADTMP(*MARK)) {
-                        assert(!IS_PADGV(*MARK));
                        *MARK = sv_mortalcopy(*MARK);
                     }
                    SvTEMP_off(*MARK);
@@ -2766,7 +2763,6 @@ try_autoload:
            while (items--) {
                mark++;
                if (*mark && SvPADTMP(*mark)) {
-                    assert(!IS_PADGV(*mark));
                    *mark = sv_mortalcopy(*mark);
                 }
            }
index f75ecd9..9213621 100644 (file)
--- a/pp_sort.c
+++ b/pp_sort.c
@@ -1604,7 +1604,6 @@ PP(pp_sort)
     for (i=max; i > 0 ; i--) {
        if ((*p1 = *p2++)) {                    /* Weed out nulls. */
            if (copytmps && SvPADTMP(*p1)) {
-                assert(!IS_PADGV(*p1));
                *p1 = sv_mortalcopy(*p1);
             }
            SvTEMP_off(*p1);
index 54ba2ce..9e5872c 100644 (file)
--- a/regexec.c
+++ b/regexec.c
@@ -2637,7 +2637,6 @@ Perl_regexec_flags(pTHX_ REGEXP * const rx, char *stringarg, char *strend,
            magic belonging to this SV.
            Not newSVsv, either, as it does not COW.
         */
-        assert(!IS_PADGV(sv));
         reginfo->sv = newSV(0);
         SvSetSV_nosteal(reginfo->sv, sv);
         SAVEFREESV(reginfo->sv);