Front page | perl.perl5.porters |
Postings from January 2014
On Mon, Jan 27, 2014 at 8:18 AM, Dave Mitchell <davem@iabyn.com> wrote: > It looks okay to me. One thing: I'd like more detailed comments at the top > of the code that describes exactly what sequence of ops it matches, and > how it transforms them, e.g. Thanks for the review! If you don't mind, I'll steal yours verbatim. They look great to me. > Also, you don't appear to op_free() nextstate2. I'll add this in. > >> There's two differences here: >> >> The ranges of the padops [$x:1,4 ...] vs [$x1,2 ...], though I'm not >> sure that matters. > > Shouldn't do. That's just the range of cops for which the lexical is in > scope, so that for example in > > my $x; eval '$x'; my $x; eval '$x' > > each eval sees a different $x. > >> The flag 'M' (OPf_MOD) on vanilla blead's padrange doesn't show up >> on my patched version. >> I'm not sure if I should be setting this here, how to detect when to >> set it, etc... > > The M on the vanilla padrange is inherited from the M on the pushmark, > when that latter op is converted into the padrange. I think it's harmless > (neither pp_pushmark nor pp_padrange use it), but it might be worth seeing > where it gets set on a pushmark and under which circumstances, and > duplicate that behaviour if possible. Hrm, I'll see if I can figure something out. Thanks for the explanations. I have one other concern, which is how I'm generating the listop. It seems very hacky, and I only figured out through lots of trial, error, and debugging. If someone could scrutinize that closely I'd appreciate it. -- Matthew Horsfall (alh)Thread Previous | Thread Next