pp_enteriter: don't create new SV for GvSV slot

Currently in

    for $pkg_var (...) {}

pp_enteriter() saves the current SV that's in the GvSV  slot, then
replaces it with a newSV(0). Instead, leave it untouched. In the range
cases:

    for $pkg_var (1..10) {}
    for $pkg_var ('a'..'z') {}

a new SV will be created anyway in the first call to pp_iter(), since each
time round the loop it checks whether the refcount of the iterator vars is
greater than 1 and if so abandons it.

For the list case, e.g.

    for $pkg_var (1,3,5) {}
    for $pkg_var (@foo) {}

each call to pp_iter updates the GvSV slot to point to the current var,
so sticking a null SV in there initially, and freeing it the end is just
pointless extra work.

The only slight proviso is when the GvSV slot is initially NULL. In this
rare case we still create the SV, as it will be needed for the range
cases.

diff --git a/pp_ctl.c b/pp_ctl.c
index 7b3589d..1ba7800 100644 (file)
--- a/pp_ctl.c
+++ b/pp_ctl.c
@@ -2142,7 +2142,10 @@ PP(pp_enteriter)
        SV** svp = &GvSV(gv);
        itervarp = (void *)gv;
         itersave = *svp;
-       *svp = newSV(0);
+        if (LIKELY(itersave))
+            SvREFCNT_inc_simple_void_NN(itersave);
+        else
+            *svp = newSV(0);
        cxtype |= CXp_FOR_GV;
     }
     else {

diff --git a/t/op/for.t b/t/op/for.t
index 65a7464..ef60f4d 100644 (file)
--- a/t/op/for.t
+++ b/t/op/for.t
@@ -5,7 +5,7 @@ BEGIN {
     require "./test.pl";
 }
 
-plan(111);
+plan(112);
 
 # A lot of tests to check that reversed for works.
 
@@ -620,3 +620,14 @@ is(fscope(), 1, 'return via loop in sub');
         ok(!defined $foo, "NULL GvSV");
     }
 }
+
+# make sure storing an int in a NULL GvSV is ok
+
+{
+    local $foo = "boo";
+    {
+        local *foo;
+        for $foo (1..2) {}
+        ok(!defined $foo, "NULL GvSV int iterator");
+    }
+}