Front page | perl.perl5.porters |
Postings from July 2021
(resurrecting an old thread:
https://www.nntp.perl.org/group/perl.perl5.porters/2020/07/msg258096.html
)
I've finally (pardon the pun) got around to resuming my branch on this.
Since the original review, we've renamed the CPAN module to "defer"
because that seems to match a bunch of other languages. It's also been
implemented and tried out as a CPAN module:
https://metacpan.org/pod/Syntax::Keyword::Defer
The branch for core perl now lives at:
https://github.com/leonerd/perl5/tree/defer
On Mon, 27 Jul 2020 11:53:52 +0100
Dave Mitchell <davem@iabyn.com> wrote:
> On Tue, Jul 21, 2020 at 11:47:55PM +0100, Paul "LeoNerd" Evans wrote:
> > It is now ready for general comment and review; I'll make a PR for
> > it soon.
>
> Some random comments:
>
> * using a PVOP - this is bad, and you should feel bad!!!! :-)
>
> I'd suggest using a LOGOP instead as a somewhat less torturing
> approach. If I understand correctly, the two OP pointers you want to
> hang off the OP_PUSHFINALLY are the root op and start op of the block.
>
> making logop->op_first point to the root op of the block is "normal"
> and should make everything work just fine: for example at the moment
> 'perl -DX' and 'perl -MO=Concise' don't appear to display the body of
> the block. Similarly, any B-type code will then automatically see the
> block, e.g. if scanning the op tree for some reason.
>
> The logop->op_other field is a generic pointer to some other op which
> is usually in some way an alternative to op_next under some
> circumstances; using this to store the block start is a slight
> stretch, but will generally work well;
Yup. That's now all done, based on following the same pattern that
worked for Syntax::Keyword::Defer, and List::Keywords.
> you just need to ensure that
> OP_PUSHFINALLY is treated the same as OP_AND etc in places like
> Perl_rpeep so that code within the block gets optimised.
>
> I'd also suggest adding a test to t/perf/opcount.t that with code like
> FINALLY { $a[0] } there is 1 aelemfast op present, proving that the
> code within the blocjk has been optimised.
Done.
> Similarly test that the block is processed by Perl_finalize_optree
> too, e.g. do eval '{ FINALLY { use strict; foo }' and check that you
> get 'Bareword "foo" not allowed'. Even if it works at the moment, it
> might not in future.
Done
> * why not just "use feature 'finally'" rather than 'finally_block'?
This is now renamed to `defer` anyway.
> * I think pp_pushfinally should be in pp_ctl.c rather than pp.c as
> it's related to flow control - its certainly where I would first look
> for it.
Moved.
> * your branch doesn't current compile under threads.
Huh, most most weird. Apparently you can't use croak() in pp_ctl.c when
under -Dusethreads, but you can without it. Highly strange.
In any case, now fixed.
> * I like the name LEAVE - why was it rejected?
(See above on renaming thoughts)
> * docs: the opening line:
>
> A block prefixed by the FINALLY modifier provides a section of
> code which runs at a later time in execution.
>
> I would change that to
>
> ... at a later time during scope exit.
>
> to more immediately emphasise what type of beast it is.
Done
> * docs: I would put explicit { }'s around your code examples to
> emphasise that things run on scope exit, as opposed to .. what? ..
> file exit??? etc.
Good idea.
--
Paul "LeoNerd" Evans
leonerd@leonerd.org.uk | https://metacpan.org/author/PEVANS
http://www.leonerd.org.uk/ | https://www.tindie.com/stores/leonerd/
Thread Next