Front page | perl.perl5.porters |
Postings from April 2018
On Fri, Apr 20, 2018 at 09:19:32AM -0700, Aaron Crane wrote: > Commit 7c114860c0fa8ade5e00a4b609d2fbd11d5a494c introduced an > optimisation in pp_iter(). Before the optimisation, pp_iter() pushed > either &PL_SV_yes or &PL_sv_no to the stack, and returned the op_next > in the obvious way. > > The optimisation takes advantage of the fact that the op_next of an > OP_ITER always points to an OP_AND node, so pp_iter() now directly > jumps to either the op_next or the op_other of the OP_AND as > appropriate. The commit message also says this: > > It's possible that some weird optree-munging XS module may break this > assumption. For now I've just added asserts that the next op is OP_AND > with an op_ppaddr of Perl_pp_and; if that assertion fails, it may be > necessary to convert pp_iter()s' asserts into conditional statements. > > However, Devel::Cover does change the op_ppaddr of the ops it can see, > so the assertions on op_ppaddr were being tripped when Devel::Cover > was run under a -DDEBUGGING Perl. But even if the asserts didn't trip, > skipping the OP_AND nodes would prevent Devel::Cover from determining > branch coverage in the way that it wants. > > The attached patch converts the asserts into conditional statements, > as outlined in the commit message above, and undoes the optimisation > when the op_ppaddr doesn't match. > > With this change, tests pass for me on a DEBUGGING Perl (and allow the > Devel::Cover tests to pass also). Tests also pass when the > optimisation is fully disabled (so I have some confidence that my > deoptimisation is working correctly). However, I'd be grateful for > further review of the change — Dave, I'm hoping you're well placed to > do that. > > I believe a fix for this issue should be applied despite the state of > the freeze, so I will add this ticket to the 5.28 blockers. Looks good to me. I agree it should go in 5.28. Also, is it possible for you supply a test patch for Devel::Cover so that such breakage gets spotted in future? -- But Pity stayed his hand. "It's a pity I've run out of bullets", he thought. -- "Bored of the Rings"Thread Previous | Thread Next