Always skip seen ops in our peep replacement

Sometimes, there are loops in the next chain as e.g. with infinite loops
like "for (;;) { ... }". We can't reuse the op_opt member because it's
almost always set after the first pass.

This fixes RT #64435.

diff --git a/MANIFEST b/MANIFEST
index c1af216dac6541a2c44ec6efa343da0bfdd790d0..8e98d27cf5a6c20f9ac51bdd9880627bac737b64 100644 (file)
--- a/MANIFEST
+++ b/MANIFEST
@@ -22,6 +22,7 @@ t/34-array-numerous.t
 t/40-scope.t
 t/41-padsv.t
 t/42-deparse.t
+t/43-peep.t
 t/50-threads.t
 t/51-threads-teardown.t
 t/91-pod.t

diff --git a/autovivification.xs b/autovivification.xs
index d07fd07f076645328255602c9ac4cf766fe3b82a..62162de1c331793406b464b65a8e342b160f2e62 100644 (file)
--- a/autovivification.xs
+++ b/autovivification.xs
@@ -107,8 +107,6 @@ typedef struct {
 
 #endif /* A_WORKAROUND_REQUIRE_PROPAGATION */
 
-#if !A_HAS_RPEEP
-
 #define PTABLE_NAME        ptable_seen
 #define PTABLE_VAL_FREE(V) NOOP
 
@@ -119,12 +117,6 @@ typedef struct {
 #define ptable_seen_clear(T)       ptable_seen_clear(aPTBLMS_ (T))
 #define ptable_seen_free(T)        ptable_seen_free(aPTBLMS_ (T))
 
-#endif /* !A_HAS_RPEEP */
-
-#define A_NEED_CXT ((A_THREADSAFE && A_WORKAROUND_REQUIRE_PROPAGATION) || !A_HAS_RPEEP)
-
-#if A_NEED_CXT
-
 #define MY_CXT_KEY __PACKAGE__ "::_guts" XS_VERSION
 
 typedef struct {
@@ -132,9 +124,7 @@ typedef struct {
  ptable *tbl;   /* It really is a ptable_hints */
  tTHX    owner;
 #endif /* A_THREADSAFE && A_WORKAROUND_REQUIRE_PROPAGATION */
-#if !A_HAS_RPEEP
  ptable *seen;  /* It really is a ptable_seen */
-#endif /* !A_HAS_RPEEP */
 } my_cxt_t;
 
 START_MY_CXT
@@ -190,15 +180,11 @@ STATIC void a_thread_cleanup(pTHX_ void *ud) {
 #if A_WORKAROUND_REQUIRE_PROPAGATION
  ptable_hints_free(MY_CXT.tbl);
 #endif /* A_WORKAROUND_REQUIRE_PROPAGATION */
-#if !A_HAS_RPEEP
  ptable_seen_free(MY_CXT.seen);
-#endif /* !A_HAS_RPEEP */
 }
 
 #endif /* A_THREADSAFE */
 
-#endif /* A_NEED_CXT */
-
 #if A_WORKAROUND_REQUIRE_PROPAGATION
 
 STATIC IV a_require_tag(pTHX) {
@@ -947,33 +933,19 @@ STATIC OP *a_ck_root(pTHX_ OP *o) {
 
 STATIC peep_t a_old_peep = 0; /* This is actually the rpeep past 5.13.5 */
 
-#if !A_HAS_RPEEP
-# define A_PEEP_REC_PROTO STATIC void a_peep_rec(pTHX_ OP *o, ptable *seen)
-#else /* !A_HAS_RPEEP */
-# define A_PEEP_REC_PROTO STATIC void a_peep_rec(pTHX_ OP *o)
-#endif /* A_HAS_RPEEP */
-
-A_PEEP_REC_PROTO;
-A_PEEP_REC_PROTO {
-#if !A_HAS_RPEEP
-# define a_peep_rec(O) a_peep_rec(aTHX_ (O), seen)
-#else /* !A_HAS_RPEEP */
-# define a_peep_rec(O) a_peep_rec(aTHX_ (O))
-#endif /* A_HAS_RPEEP */
- dA_MAP_THX;
-
-#if !A_HAS_RPEEP
- if (ptable_fetch(seen, o))
-  return;
-#endif
+STATIC void a_peep_rec(pTHX_ OP *o, ptable *seen);
 
+STATIC void a_peep_rec(pTHX_ OP *o, ptable *seen) {
+#define a_peep_rec(O) a_peep_rec(aTHX_ (O), seen)
  for (; o; o = o->op_next) {
+  dA_MAP_THX;
   const a_op_info *oi = NULL;
   UV flags = 0;
 
-#if !A_HAS_RPEEP
+  if (ptable_fetch(seen, o))
+   break;
   ptable_seen_store(seen, o, o);
-#endif
+
   switch (o->op_type) {
    case OP_PADSV:
     if (o->op_ppaddr != a_pp_deref) {
@@ -1058,15 +1030,14 @@ A_PEEP_REC_PROTO {
 }
 
 STATIC void a_peep(pTHX_ OP *o) {
-#if !A_HAS_RPEEP
  dMY_CXT;
  ptable *seen = MY_CXT.seen;
 
- ptable_seen_clear(seen);
-#endif /* !A_HAS_RPEEP */
-
  a_old_peep(aTHX_ o);
+
+ ptable_seen_clear(seen);
  a_peep_rec(o);
+ ptable_seen_clear(seen);
 }
 
 /* --- Interpreter setup/teardown ------------------------------------------ */
@@ -1083,17 +1054,13 @@ STATIC void a_teardown(pTHX_ void *root) {
   return;
 #endif
 
-#if A_NEED_CXT
  {
   dMY_CXT;
 # if A_THREADSAFE && A_WORKAROUND_REQUIRE_PROPAGATION
   ptable_hints_free(MY_CXT.tbl);
 # endif /* A_THREADSAFE && A_WORKAROUND_REQUIRE_PROPAGATION */
-# if !A_HAS_RPEEP
   ptable_seen_free(MY_CXT.seen);
-# endif /* !A_HAS_RPEEP */
  }
-#endif /* A_NEED_CXT */
 
  PL_check[OP_PADANY] = MEMBER_TO_FPTR(a_old_ck_padany);
  a_old_ck_padany     = 0;
@@ -1141,18 +1108,14 @@ STATIC void a_setup(pTHX) {
  if (a_initialized)
   return;
 
-#if A_NEED_CXT
  {
   MY_CXT_INIT;
 # if A_THREADSAFE && A_WORKAROUND_REQUIRE_PROPAGATION
   MY_CXT.tbl   = ptable_new();
   MY_CXT.owner = aTHX;
 # endif /* A_THREADSAFE && A_WORKAROUND_REQUIRE_PROPAGATION */
-# if !A_HAS_RPEEP
   MY_CXT.seen  = ptable_new();
-# endif /* !A_RPEEP */
  }
-#endif /* A_NEED_CXT */
 
  a_old_ck_padany     = PL_check[OP_PADANY];
  PL_check[OP_PADANY] = MEMBER_TO_FPTR(a_ck_padany);
@@ -1237,7 +1200,7 @@ BOOT:
  a_setup();
 }
 
-#if A_THREADSAFE && (A_WORKAROUND_REQUIRE_PROPAGATION || !A_HAS_RPEEP)
+#if A_THREADSAFE
 
 void
 CLONE(...)
@@ -1246,9 +1209,7 @@ PREINIT:
 #if A_WORKAROUND_REQUIRE_PROPAGATION
  ptable *t;
 #endif
-#if !A_HAS_RPEEP
  ptable *s;
-#endif
 PPCODE:
  {
   dMY_CXT;
@@ -1262,9 +1223,7 @@ PPCODE:
    a_ptable_clone_ud_deinit(ud);
   }
 #endif
-#if !A_HAS_RPEEP
   s = ptable_new();
-#endif
  }
  {
   MY_CXT_CLONE;
@@ -1272,14 +1231,12 @@ PPCODE:
   MY_CXT.tbl   = t;
   MY_CXT.owner = aTHX;
 #endif
-#if !A_HAS_RPEEP
   MY_CXT.seen  = s;
-#endif
  }
  reap(3, a_thread_cleanup, NULL);
  XSRETURN(0);
 
-#endif
+#endif /* A_THREADSAFE */
 
 SV *
 _tag(SV *hint)

diff --git a/t/43-peep.t b/t/43-peep.t
new file mode 100644 (file)
index 0000000..5050821
--- /dev/null
+++ b/t/43-peep.t
@@ -0,0 +1,198 @@
+#!perl -T
+
+use strict;
+use warnings;
+
+use Test::More tests => 11 + 6 * 3;
+
+{
+ my $desc = 'peephole optimization of conditionals';
+ my $x;
+
+ local $@;
+ my $code = eval <<' TESTCASE';
+  no autovivification;
+  sub {
+   if ($_[0]) {
+    my $z = $x->{a};
+    return 1;
+   } elsif ($_[1] || $_[2]) {
+    my $z = $x->{b};
+    return 2;
+   } elsif ($_[3] && $_[4]) {
+    my $z = $x->{c};
+    return 3;
+   } elsif ($_[5] ? $_[6] : 0) {
+    my $z = $x->{d};
+    return 4;
+   } else {
+    my $z = $x->{e};
+    return 5;
+   }
+   return 0;
+  }
+ TESTCASE
+ is $@, '', "$desc compiled fine";
+
+ my $ret = $code->(1);
+ is_deeply $x, undef, "$desc : first branch did not autovivify";
+ is      $ret, 1,     "$desc : first branch returned 1";
+
+ $ret = $code->(0, 1);
+ is_deeply $x, undef, "$desc : second branch did not autovivify";
+ is      $ret, 2,     "$desc : second branch returned 2";
+
+ $ret = $code->(0, 0, 0, 1, 1);
+ is_deeply $x, undef, "$desc : third branch did not autovivify";
+ is      $ret, 3,     "$desc : third branch returned 3";
+
+ $ret = $code->(0, 0, 0, 0, 0, 1, 1);
+ is_deeply $x, undef, "$desc : fourth branch did not autovivify";
+ is      $ret, 4,     "$desc : fourth branch returned 4";
+
+ $ret = $code->();
+ is_deeply $x, undef, "$desc : fifth branch did not autovivify";
+ is      $ret, 5,     "$desc : fifth branch returned 5";
+}
+
+{
+ my $desc = 'peephole optimization of C-style loops';
+ my $x;
+
+ local $@;
+ my $code = eval <<' TESTCASE';
+  no autovivification;
+  sub {
+   my $ret = 0;
+   for (
+     my ($z, $i) = ($x->[100], 0)
+    ;
+     do { my $z = $x->[200]; $i < 4 }
+    ;
+     do { my $z = $x->[300]; ++$i }
+   ) {
+    my $z = $x->[$i];
+    $ret += $i;
+   }
+   return $ret;
+  }
+ TESTCASE
+ is $@, '', "$desc compiled fine";
+
+ my $ret = $code->();
+ is_deeply $x, undef, "$desc did not autovivify";
+ is      $ret, 6,     "$desc returned 0+1+2+3";
+}
+
+{
+ my $desc = 'peephole optimization of range loops';
+ my $x;
+
+ local $@;
+ my $code = eval <<' TESTCASE';
+  no autovivification;
+  sub {
+   my $ret = 0;
+   for ((do { my $z = $x->[100]; 0 }) .. (do { my $z = $x->[200]; 3 })) {
+    my $z = $x->[$_];
+    $ret += $_;
+   }
+   return $ret;
+  }
+ TESTCASE
+ is $@, '', "$desc compiled fine";
+
+ my $ret = $code->();
+ is_deeply $x, undef, "$desc did not autovivify";
+ is      $ret, 6,     "$desc returned 0+1+2+3";
+}
+
+{
+ my $desc = 'peephole optimization of empty loops (RT #64435)';
+ my $x;
+
+ local $@;
+ my $code = eval <<' TESTCASE';
+  no autovivification;
+  sub {
+   my $ret = 0;
+   for (;;) {
+    ++$ret;
+    return $ret;
+   }
+   return $ret;
+  }
+ TESTCASE
+ is $@, '', "$desc compiled fine";
+
+ my $ret = $code->();
+ is_deeply $x, undef, "$desc did not autovivify";
+ is      $ret, 1,     "$desc returned 1";
+}
+
+{
+ my $desc = 'peephole optimization of map';
+ my $x;
+
+ local $@;
+ my $code = eval <<' TESTCASE';
+  no autovivification;
+  sub {
+   join ':', map {
+    my $z = $x->[$_];
+    "x${_}y"
+   } @_
+  }
+ TESTCASE
+ is $@, '', "$desc compiled fine";
+
+ my $ret = $code->(1, 2);
+ is_deeply $x, undef,     "$desc did not autovivify";
+ is      $ret, 'x1y:x2y', "$desc returned the right value";
+}
+
+{
+ my $desc = 'peephole optimization of grep';
+ my $x;
+
+ local $@;
+ my $code = eval <<' TESTCASE';
+  no autovivification;
+  sub {
+   join ':', grep {
+    my $z = $x->[$_];
+    $_ <= 3
+   } @_
+  }
+ TESTCASE
+ is $@, '', "$desc compiled fine";
+
+ my $ret = $code->(1 .. 5);
+ is_deeply $x, undef,   "$desc did not autovivify";
+ is      $ret, '1:2:3', "$desc returned the right value";
+}
+
+{
+ my $desc = 'peephole optimization of substitutions';
+ my $x;
+
+ local $@;
+ my $code = eval <<' TESTCASE';
+  no autovivification;
+  sub {
+   my $str = $_[0];
+   $str =~ s{
+    ([0-9])
+   }{
+    my $z = $x->[$1];
+    9 - $1;
+   }xge;
+   $str;
+  }
+ TESTCASE
+ is $@, '', "$desc compiled fine";
+
+ my $ret = $code->('0123456789');
+ is_deeply $x, undef,        "$desc did not autovivify";
+ is      $ret, '9876543210', "$desc returned the right value";
+}