From d7ab38e86270ebb461e1e10c7be726e399e19c78 Mon Sep 17 00:00:00 2001 From: Father Chrysostomos Date: Sat, 14 Jul 2012 00:07:03 -0700 Subject: [perl #113710] Make __SUB__ work in sort block MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the peephole optimiser encounters a __SUB__, it looks to see whether the current sub is clonable. If it is not, it inlines the __SUB__ as a const op. This works most of the time. A forward declaration will cause the sub definition to reuse the existing stub. When that happens, the sub visible during compilation in PL_compcv is not the sub that the op tree will finally be attached to. But the peephole optimiser is called after that, with PL_compcv set to the other CV (what was a stub). ck_sort was calling the peephole optimiser on the sort block ahead of time. So this caused __SUB__ to point to the wrong subroutine. By removing the CALL_PEEP call from ck_sort and adding logic to the peephole optimiser itself to traverse the sort block (it is not in the usual op_next chain), this bug is eliminated. I modified the DEFER macro to work as a single statement. You don’t want to know how much time I spent debugging the bus errors that were occurring because if(foo) DEFER; didn’t do what I though. It turns out that grepstart and mapstart, which also use ck_sort, had their blocks go through the peephole optimiser twice, because grepwhile already has special-casing in the peephole optimiser. This also has the side-effect of making map and grep slightly more efficient, in that they no longer execute a scope op (which is just pp_null). By temporarily disconnecting the subtree before running the optimiser, ck_sort was hiding a possible optimisation (skipping the scope op). --- ext/B/t/f_map.t | 6 ------ ext/B/t/f_sort.t | 8 -------- 2 files changed, 14 deletions(-) (limited to 'ext') diff --git a/ext/B/t/f_map.t b/ext/B/t/f_map.t index 2fa2ec95eb..4506543411 100644 --- a/ext/B/t/f_map.t +++ b/ext/B/t/f_map.t @@ -247,7 +247,6 @@ checkOptree(note => q{}, # b <@> stringify[t5] sK/1 # c <$> const[IV 1] s # d <@> list lK -# - <@> scope lK # goto 7 # e <0> pushmark s # f <#> gv[*hash] s @@ -268,7 +267,6 @@ EOT_EOT # b <@> stringify[t3] sK/1 # c <$> const(IV 1) s # d <@> list lK -# - <@> scope lK # goto 7 # e <0> pushmark s # f <$> gv(*hash) s @@ -301,7 +299,6 @@ checkOptree(note => q{}, # b <@> stringify[t5] sK/1 # c <$> const[IV 1] s # d <@> list lKP -# - <@> scope lK # goto 7 # e <0> pushmark s # f <#> gv[*hash] s @@ -322,7 +319,6 @@ EOT_EOT # b <@> stringify[t3] sK/1 # c <$> const(IV 1) s # d <@> list lKP -# - <@> scope lK # goto 7 # e <0> pushmark s # f <$> gv(*hash) s @@ -354,7 +350,6 @@ checkOptree(note => q{}, # a <1> lc[t4] sK/1 # b <$> const[IV 1] s # c <@> list lK -# - <@> scope lK # goto 7 # d <0> pushmark s # e <#> gv[*hash] s @@ -374,7 +369,6 @@ EOT_EOT # a <1> lc[t2] sK/1 # b <$> const(IV 1) s # c <@> list lK -# - <@> scope lK # goto 7 # d <0> pushmark s # e <$> gv(*hash) s diff --git a/ext/B/t/f_sort.t b/ext/B/t/f_sort.t index 58a8cf2eed..f17976f11f 100644 --- a/ext/B/t/f_sort.t +++ b/ext/B/t/f_sort.t @@ -520,7 +520,6 @@ checkOptree(name => q{Compound sort/map Expression }, # o <1> rv2av[t4] sKR/1 # p <$> const[IV 0] s # q <2> aelem sK/2 -# - <@> scope lK # goto l # r <0> pushmark s # s <#> gv[*new] s @@ -555,7 +554,6 @@ EOT_EOT # o <1> rv2av[t2] sKR/1 # p <$> const(IV 0) s # q <2> aelem sK/2 -# - <@> scope lK # goto l # r <0> pushmark s # s <$> gv(*new) s @@ -790,7 +788,6 @@ checkOptree(note => q{}, # 9 <#> gvsv[*_] s # a <#> gvsv[*_] s # b <2> eq sK/2 -# - <@> scope sK # goto 8 # c <@> sort lK/NUM # d <0> pushmark s @@ -810,7 +807,6 @@ EOT_EOT # 9 <$> gvsv(*_) s # a <$> gvsv(*_) s # b <2> eq sK/2 -# - <@> scope sK # goto 8 # c <@> sort lK/NUM # d <0> pushmark s @@ -869,7 +865,6 @@ checkOptree(note => q{}, # 8 <#> gvsv[*_] s # 9 <#> gvsv[*_] s # a <2> eq sK/2 -# - <@> scope sK # goto 7 # b <@> sort K/NUM # c <1> leavesub[1 ref] K/REFC,1 @@ -884,7 +879,6 @@ EOT_EOT # 8 <$> gvsv(*_) s # 9 <$> gvsv(*_) s # a <2> eq sK/2 -# - <@> scope sK # goto 7 # b <@> sort K/NUM # c <1> leavesub[1 ref] K/REFC,1 @@ -942,7 +936,6 @@ checkOptree(note => q{}, # 8 <#> gvsv[*_] s # 9 <#> gvsv[*_] s # a <2> eq sK/2 -# - <@> scope sK # goto 7 # b <@> sort sK/NUM # c <#> gvsv[*s] s @@ -959,7 +952,6 @@ EOT_EOT # 8 <$> gvsv(*_) s # 9 <$> gvsv(*_) s # a <2> eq sK/2 -# - <@> scope sK # goto 7 # b <@> sort sK/NUM # c <$> gvsv(*s) s -- cgit v1.2.1