From: David Mitchell <davem@iabyn.com>
Date: Thu, 27 Aug 2015 15:03:42 +0000 (+0100)
Subject: add CXp_FOR_PAD, CXp_FOR_GV flags
X-Git-Tag: v5.23.8~154^2~141
X-Git-Url: https://perl5.git.perl.org/perl5.git/commitdiff_plain/9af0df0b57d248c9c26f1464e1f1c18f24c642c2?hp=4ad63d708f405909d048160f3eaf32590132e3cb

add CXp_FOR_PAD, CXp_FOR_GV flags

These indicate that the loop is a for loop, with a lexical or package
iterator variable.

These flags make various tests simpler and more efficient.

This commit also fixes a bug introduced a couple of commits ago;
I was assuming that in POPLOOP, itersave being non-NULL indicated a
'for $lex' or 'for $pkg', but something like

    local *x;
    for $x (..) {}

leaves the loop with itersave set to NULL. Instead we can use the new
flags to indicate that itersave is valid.
---

diff --git a/cop.h b/cop.h
index 08b52f2..c708cb7 100644
--- a/cop.h
+++ b/cop.h
@@ -781,14 +781,12 @@ struct block_loop {
 #endif
 };
 
-#define CxITERVAR(c)							\
-        (CxPADLOOP(c)     						\
-            ? (c)->blk_loop.itervar_u.svp        			\
-            : (c)->blk_loop.itervar_u.svp       			\
-                ? isGV((c)->blk_loop.itervar_u.gv)			\
-                    ? &GvSV((c)->blk_loop.itervar_u.gv)			\
-                    : (SV **)&(c)->blk_loop.itervar_u.gv		\
-                : (SV**)NULL)
+#define CxITERVAR(c)                                    \
+        (CxPADLOOP(c)                                   \
+            ? (c)->blk_loop.itervar_u.svp               \
+            : ((c)->cx_type & CXp_FOR_GV)               \
+                ? &GvSV((c)->blk_loop.itervar_u.gv)     \
+                : (SV **)&(c)->blk_loop.itervar_u.gv)
 
 #define CxLABEL(c)	(0 + CopLABEL((c)->blk_oldcop))
 #define CxLABEL_len(c,len)	(0 + CopLABEL_len((c)->blk_oldcop, len))
@@ -832,10 +830,10 @@ struct block_loop {
 	}								\
 	else if (CxTYPE(cx) == CXt_LOOP_FOR)  				\
 	    SvREFCNT_dec(cx->blk_loop.state_u.ary.ary);                 \
-        if (cx->blk_loop.itersave) {                                    \
+        if (cx->cx_type & (CXp_FOR_PAD|CXp_FOR_GV)) {                   \
             SV **svp = (cx)->blk_loop.itervar_u.svp;                    \
             SV *cursv;                                                  \
-            if (isGV((GV*)svp)) {                                       \
+            if ((cx->cx_type & CXp_FOR_GV)) {                           \
                 svp = &GvSV((GV*)svp);                                  \
                 cursv = *svp;                                           \
                 *svp = cx->blk_loop.itersave;                           \
@@ -845,7 +843,7 @@ struct block_loop {
                 *svp = cx->blk_loop.itersave;                           \
             }                                                           \
             SvREFCNT_dec(cursv);                                        \
-        }                                                               \
+        }
 
 /* given/when context */
 struct block_givwhen {
@@ -1038,7 +1036,9 @@ struct context {
 /* private flags for CXt_LOOP */
 #define CXp_FOR_DEF	0x10	/* foreach using $_ */
 #define CXp_FOR_LVREF	0x20	/* foreach using \$var */
-#define CxPADLOOP(c)	((c)->blk_loop.my_op->op_targ)
+#define CXp_FOR_GV	0x40	/* foreach using package var */
+#define CXp_FOR_PAD	0x80	/* foreach using lexical var */
+#define CxPADLOOP(c)	((c)->cx_type & CXp_FOR_PAD)
 
 /* private flags for CXt_SUBST */
 #define CXp_ONCE	0x10	/* What was sbu_once in struct subst */
diff --git a/pp_ctl.c b/pp_ctl.c
index 89aaf3a..7b3589d 100644
--- a/pp_ctl.c
+++ b/pp_ctl.c
@@ -2135,6 +2135,7 @@ PP(pp_enteriter)
 	    SvPADSTALE_on(itersave);
 	}
         SvREFCNT_inc_simple_void_NN(itersave);
+	cxtype |= CXp_FOR_PAD;
     }
     else if (LIKELY(isGV(TOPs))) {		/* symbol table variable */
 	GV * const gv = MUTABLE_GV(POPs);
@@ -2142,6 +2143,7 @@ PP(pp_enteriter)
 	itervarp = (void *)gv;
         itersave = *svp;
 	*svp = newSV(0);
+	cxtype |= CXp_FOR_GV;
     }
     else {
 	SV * const sv = POPs;
diff --git a/t/op/for.t b/t/op/for.t
index 922e3da..65a7464 100644
--- a/t/op/for.t
+++ b/t/op/for.t
@@ -5,7 +5,7 @@ BEGIN {
     require "./test.pl";
 }
 
-plan(110);
+plan(111);
 
 # A lot of tests to check that reversed for works.
 
@@ -609,3 +609,14 @@ sub fscope {
 }
 
 is(fscope(), 1, 'return via loop in sub');
+
+# make sure a NULL GvSV is restored at the end of the loop
+
+{
+    local $foo = "boo";
+    {
+        local *foo;
+        for $foo (1,2) {}
+        ok(!defined $foo, "NULL GvSV");
+    }
+}