This is a live mirror of the Perl 5 development currently hosted at https://github.com/perl/perl5
perl5.git
7 years agodounwind(): do a POPBLOCK for final cx frame.
David Mitchell [Thu, 24 Dec 2015 19:44:05 +0000 (19:44 +0000)]
dounwind(): do a POPBLOCK for final cx frame.

Previously dounwind() relied on the caller to a TOPBLOCK or POPBLOCK
following the call to dounwind(). It's debatable who should be
responsible. Arguably its more efficient for dounwind() not to do a
POPBLOCK, since the caller will probably immediately follow on with
POPFOO; POPBLOCK for the next context frame anyway.

Logically however, dounwind() should do this, and its not possible
for the caller to do so retrospectively, as context frame cxstack_ix + 1
may have been overwritten by the time dounwind returns.

Also, the changes in this branch mean that the old PL_tmps_floor is now
saved in the context struct rather than on the save stack, so code that
does C<dounwind(-1); LEAVE_SCOPE();> will no longer automatically
restore PL_tmps_floor. With thiis commit, it will.

The change to pp_return reflects that we now need to copy any return args
*before* donwind() is called, so that "return $1" will mg_get($1) while
the correct (inner) PL_curpm is still in scope.

7 years agopp_break(): don't use TOPBLOCK
David Mitchell [Mon, 21 Dec 2015 16:33:57 +0000 (16:33 +0000)]
pp_break(): don't use TOPBLOCK

It appears to be using TOPBLOCK purely for its effect of resetting
PL_stack_sp. Since the next op will be pp_leavegiven which will do a
POPBLOCK, the other actions of TOPBLOCK are redundant. So just set
PL_stack_sp directly.

7 years agotweak POPLOOP and CXt_LOOP_* order
David Mitchell [Mon, 21 Dec 2015 16:00:59 +0000 (16:00 +0000)]
tweak POPLOOP and CXt_LOOP_* order

Re-arrange the order of the CXt_LOOP_* to make it easier for compilers
to produce efficient code, and tweak POPLOOP so that the code for
freeing ary and cur is shared (as they occupy the same position in the
union).

7 years agofix CxFOREACH
David Mitchell [Mon, 21 Dec 2015 15:45:57 +0000 (15:45 +0000)]
fix CxFOREACH

It wasn't detecting CXt_LOOP_LAZYIV as a 'for' loop type. This only
affected 'given' and 'break' (which need to distinguish between being
in a 'given' block and being in a 'for' block).

7 years agoonly set CXp_FOR_DEF with CXp_FOR_GV
David Mitchell [Mon, 21 Dec 2015 13:32:13 +0000 (13:32 +0000)]
only set CXp_FOR_DEF with CXp_FOR_GV

C<for (...)> is a special-case of C<for $pkg_var (...)>, so CXp_FOR_DEF
should only be set when CXp_FOR_GV also is. So in pp_enteriter(), only
test for (PL_op->op_private & OPpITER_DEF) in the GV branch and assert
that it's otherwise unset. This is slightly more efficient in the non-GV
branches.

Also add some more commentary to the CXp_FOR_* flag definitions.

7 years agogive POP_SAVEARRAY() macro a cx arg
David Mitchell [Fri, 18 Dec 2015 22:09:58 +0000 (22:09 +0000)]
give POP_SAVEARRAY() macro a cx arg

rather than just assuming that there's a 'cx' var in scope

7 years agoclarify code comment in pp_goto(()
David Mitchell [Fri, 18 Dec 2015 22:07:25 +0000 (22:07 +0000)]
clarify code comment in pp_goto(()

7 years agoAPItest.xs: fixup clone_with_stack()
David Mitchell [Fri, 18 Dec 2015 21:33:10 +0000 (21:33 +0000)]
APItest.xs: fixup clone_with_stack()

Make the unwinding and freeing of the old interpreter up to date
with the latest handling of PL_scopestack.

7 years agofixup cx_dup()
David Mitchell [Fri, 18 Dec 2015 20:18:13 +0000 (20:18 +0000)]
fixup cx_dup()

By visual inspection, tweak bits of the context stack duplicating code
to reflect recent changes in this branch, especially new fields, or
whether fields hold a reference count or not.

Since I don't use Windows, this has largely been done blind.

7 years agopp_iter: optimise integer setting in for (1..10) {}
David Mitchell [Fri, 18 Dec 2015 16:36:54 +0000 (16:36 +0000)]
pp_iter: optimise integer setting in for (1..10) {}

If the target SV is a simple SVt_IV, which it is likely to be,
just update it directly rather than calling sv_setiv()

7 years agofix *_ = "" for 0 .. 1;
David Mitchell [Fri, 18 Dec 2015 15:09:42 +0000 (15:09 +0000)]
fix *_ = "" for 0 .. 1;

RT #123994

pp_iter couldn't handle GvSv(gv) being NULL.

In that ticket, Tony Cook suggested two possible fixes. First,
always instantiate the GvSV slot at the start of pp_iter by using
GvSVn rather than GvSV in the CxITERVAR() macro;

Second, test for it being null within the two 'range' branches,
(for(1..9), for('a'..'z')), and if so create it. One advantage of doing
it there is that there's already code for (re)creating the SV if the
reference count is != 1. It also means that the list and array cases
(for(@a), for(1,3,5)) which always put the next iterated SV into the
pad/GvSV slot don't waste time creating and then immediately discarding an
SV if GvSV was NULL.

I went for the second fix.

It also means that's there's no longer any need in pp_enteriter to
initially poulate GvSV is it was null, as this will be detected during
the first pp_iter() anyway.

7 years agopp_enteriter: use efficient SvREFCNT_inc variant
David Mitchell [Fri, 18 Dec 2015 12:34:37 +0000 (12:34 +0000)]
pp_enteriter: use efficient SvREFCNT_inc variant

replace a couple of SvREFCNT_inc() with SvREFCNT_inc_simple_void_NN().

Also add a couple of code comments.

7 years agoprovide some basic documentation for Perl_dounwind
David Mitchell [Thu, 17 Dec 2015 17:50:52 +0000 (17:50 +0000)]
provide some basic documentation for Perl_dounwind

7 years agoTOPBLOCK: make comment clear its used by goto too
David Mitchell [Thu, 17 Dec 2015 17:34:36 +0000 (17:34 +0000)]
TOPBLOCK: make comment clear its used by goto too

7 years agoPOPSUB_ARGS: move a code comment to the right line
David Mitchell [Thu, 17 Dec 2015 17:30:48 +0000 (17:30 +0000)]
POPSUB_ARGS: move a code comment to the right line

7 years agopp_ctl.c: s/newsp/oldsp/g
David Mitchell [Thu, 17 Dec 2015 12:37:17 +0000 (12:37 +0000)]
pp_ctl.c: s/newsp/oldsp/g

There are various pp_leavefoo() functions that have similar code along the
lines of

    SV **newsp;
    newsp = PL_stack_base + cx->blk_oldsp;

Rename all these vars to 'oldsp' to reduce the cognitive dissonance.
The name 'newsp' was a hangover from when POPBLOCK used to implicitly
set a var called 'newsp.

7 years agoreplace leave_common() with leave_adjust_stacks()
David Mitchell [Thu, 17 Dec 2015 12:13:09 +0000 (12:13 +0000)]
replace leave_common() with leave_adjust_stacks()

Make the remaining callers of S_leave_common() use leave_adjust_stacks()
instead, then delete this static function.

This brings the benefits of freeing TEMPS on all scope exists that
has already been introduced on sub exits; uses the optimised code for
creating mortal copies; and finally unifies all the different 'process
return args on scope exit' implementations into single function.

7 years agomake pp_return() use leave_adjust_stacks()
David Mitchell [Wed, 16 Dec 2015 14:52:22 +0000 (14:52 +0000)]
make pp_return() use leave_adjust_stacks()

It was using S_leave_common(), but that's shortly to be removed.  It also
required adding an extra arg to leave_adjust_stacks() to indicate where to
shift the return args to. This will also be needed for when we replace the
remaining uses of  S_leave_common() with leave_adjust_stacks().

7 years agomake pp_leavesublv use S_leavesub_adjust_stacks()
David Mitchell [Wed, 16 Dec 2015 12:30:01 +0000 (12:30 +0000)]
make pp_leavesublv use S_leavesub_adjust_stacks()

Currently S_leavesub_adjust_stacks() is just used by pp_leavesub.
Rename it to Perl_leave_adjust_stacks(), extend its functionality
slightly, then make pp_leavesublv() use it too.

This means that lvalue sub exit gains the benefit of FREETMPS being done,
and (where mortal copying needs doing) the optimised copying code.
It also means there is now one less version of the "process args on scope
exit" code.

pp_leavesublv() still does a scan of its return args looking for things to
croak() on, but leaves everything else to leave_adjust_stacks().

leave_adjust_stacks() is intended shortly to be used in place of
S_leave_common() too, thus unifying all args-on-scope-exit code.

The changes to leave_adjust_stacks() in this commit (apart from the
renaming and doc changes) are:
* a new arg to indicate what condition to use to decide whether to
  pass or copy the arg;
* a new branch to mortalise and ref count bump an arg

7 years agopp_leavesublv(): document PL_sv_undef exception
David Mitchell [Mon, 14 Dec 2015 12:10:45 +0000 (12:10 +0000)]
pp_leavesublv(): document PL_sv_undef exception

7 years agopp_leavesublv(): croak on *all* PADTMPs
David Mitchell [Mon, 14 Dec 2015 11:55:14 +0000 (11:55 +0000)]
pp_leavesublv(): croak on *all* PADTMPs

pp_leavesublv() generally croaks on returned PADTMPs when called in lvalue
context. However, an exception was made in scalar context if the PADTMP
had set magic. This was to allow for things like

    sub :lvalue { $tied{foo} }
and
    sub :lvalue { substr($foo,1,2) }

However, it was later found that in places like pp_substr(), when
returning a PVLV, it should return a new mortal rather than upgrading
its pad target to PVLV, because the PVLV holds a reference to $foo which
then gets delayed being freed indefinitely.

Since places like pp_susbtr() no longer return lvalue PADTMPs, there's
no longer any need to make a special exception in pp_leavesublv().

I've added an assertion to the effect that PADTMPs don't have set
container magic, but I've added the assert to pp_leavesub() rather than
pp_leavesublv(), since the former is much likely to be used in many weird
situations and edge cases that would quickly flush out any false
assumptions.

If this assumption is wrong and the exception needs to be re-instated in
pp_leavesublv(), then presumably it needs adding to the ARRAY context
branch too - I'm assuming that its previous absence there was an oversight;
i.e.

    sub foo :lvalue { $tied{foo}, substr($foo,1,2) }
    foo() = (1,2);

should work too.

7 years agopp_leavesub(): call FREETMPS and optimise
David Mitchell [Sun, 8 Nov 2015 15:16:09 +0000 (15:16 +0000)]
pp_leavesub(): call FREETMPS and optimise

Currently pp_leavesub() doesn't call FREETMPS. Presumably this is due to
the danger of freeing temps that need to exist beyond the return, such
as the mortal copies we make of return args, or return args that are
already temps.

The down side of this is that temps aren't freed until the next nextstate
is executed following the function call. If the function isn't near a
statement boundary, then it may be a while before temps are freed; e.g.
in f(g()), when g() returns, its temps continue to exist during the call
to f(). For recursive subs it gets worse, although there is a specific
hack already in pp_leavesub() that says in scalar context if CvDEPTH > 1,
then temporarily RC++ the single return value then do a FREETMPS. This
can in theory leak if something dies during the FREETMPS.

This commit provides a more general solution. During the course of
processing (usually mortal copying) the return args, it divides the
current temps stack frame into two halves, with the temps that need
keeping migrating to the bottom half. It then does a FREETMPS equivalent
only of the top half.

This is actually more efficient than it sounds; but in addition, the code
has been heavily optimised: in particular the call to sv_mortalcopy()
has been unrolled and inlined, and within that code common cases (such as
IV, RV) handled directly, and the mortal stack is only checked/extended
once, rather than for every arg.

Also, the arg adjust / freetmps code has been moved out of pp_leavesub()
and into a separate static function.

7 years agomove SET_SVANY_FOR_BODYLESS_IV() from sv.c to sv.h
David Mitchell [Fri, 4 Dec 2015 15:50:54 +0000 (15:50 +0000)]
move SET_SVANY_FOR_BODYLESS_IV() from sv.c to sv.h

Ditto for SET_SVANY_FOR_BODYLESS_NV.

The IV variant will shortly be needed in pp_hot.c, so make it
available outside sv.c. For now, protect with #ifdef PERL_CORE,
so as little as possible is changed by this commit.

7 years agoPerl_runops_debug(): do FREETMPS
David Mitchell [Mon, 16 Nov 2015 14:40:04 +0000 (14:40 +0000)]
Perl_runops_debug(): do FREETMPS

In runops_debug() wrap the optional printing of next op, arg stack
etc with ENTER/SAVETMPS, FREETMPS/LEAVE - so that temporaries created by
the dump output are promptly freed, and thus don't alter the tmps stack.

(I'm trying to debug some tmps stack corruption, and running with -Dst
made the problem go away).

7 years agooptimise sv_setsv_flags()
David Mitchell [Mon, 9 Nov 2015 21:11:58 +0000 (21:11 +0000)]
optimise sv_setsv_flags()

This commit does two things.

First, it streamlines and re-orders some of the initial tests,
such as 'is sstr already freed?'.

Second, it looks for a reasonably common case where both sstr and dstr are
SVt_NULL/SVt_IV. This covers undef, int and refs, where the SV hasn't
previously been used for other things (like strings).

With just SVt_NULL/SVt_IV, we know that the SV won't have a real body, and
won't need one and can be very quick.

The check for SVt_NULL/SVt_IV is a single compare-and-branch, so
has a minimal effect on users of sv_setsv_flags() that have more complex
types.

7 years agointrvar.h: document PL_tmps_max
David Mitchell [Sun, 8 Nov 2015 20:08:20 +0000 (20:08 +0000)]
intrvar.h: document PL_tmps_max

Its name implies that it's the top allocated element; in fact it's top+1.

7 years agopp_leavesub: reset SP in void context
David Mitchell [Sun, 8 Nov 2015 15:05:01 +0000 (15:05 +0000)]
pp_leavesub: reset SP in void context

In void context, pp_leavesub() doesn't bother resetting SP to the
base; either as an efficiency measure, or as an oversight (the other
pp_leavefoo functions do reset).

This is mostly harmless, as being void context, it's likely to immediately
execute pp_nextstate or similar, which will reset the stack anyway.
However with attributes, something like

    my @foo :attr = ()

causes attributes::import() to be called in void context, and whatever
dross it leaves on the stack becomes part of the assign, i.e. that assign
becomes the equivalent of:

    my (@foo, dross) = ()

which again is fairly harmless, if slightly inefficient.

However, the next commit should make pp_leavesub() call FRETMPS,
which means that 'dross' may now include freed SVs, which will make
pp_aassign choke.

This commit also requires ext/XS-APItest/t/call.t to be fixed.
Some tests in there (added years ago by myself) test the statu quo; that
is, it expects that calling call_sv(G_VOID) will leave return args on the
stack. Now it doesn't.

7 years agoPerl_free_tmps(): don't test for PL_sv_undef
David Mitchell [Sun, 8 Nov 2015 19:19:02 +0000 (19:19 +0000)]
Perl_free_tmps(): don't test for PL_sv_undef

Each sv being popped off the tmps stack is compared against PL_sv_undef,
and if so, the SvTEMP_off(sv); SvREFCNT_dec_NN(sv); is skipped.

This condition was added by 8aacddc1 back in 2001, as part of a commit
to add support for hash placeholders. Initially it used &PL_sv_undef in
HE value slots to indicate a placeholder. In a later commit, that was
changed to &PL_sv_placeholder, but the test in free_tmps() continued
to test against &PL_sv_undef.

Im not sure what the original intent was, but I strongly suspect that
whatever purpose it may have originally served, it no longer served that
after the switch to PL_sv_placeholder.

On a purely pragmatic note, I can't see any harm in untemping PL_sv_undef;
at worst its ref count will occasionally reach 0, and the special code in
sv_free2() will reset its count to SvREFCNT_IMMORTAL. And no tests
fail....

7 years agorestore PL_tmps_floor on exit
David Mitchell [Wed, 28 Oct 2015 08:28:56 +0000 (08:28 +0000)]
restore PL_tmps_floor on exit

A couple of places were expecting dounwind(-1); LEAVE_SCOPE(0);
to restore PL_tmps_floor, since its old value was saved on the savestack.
Since that's now stored in the context struct instead, do
a POPBLOCK(&cxstack[0]).

This problem only showed up on 'make test' rather than 'make
test_harness', since only the former sets PERL_DESTRUCT_LEVEL.

7 years agoextend magic copy test to all scope exit types
David Mitchell [Wed, 21 Oct 2015 18:10:34 +0000 (19:10 +0100)]
extend magic copy test to all scope exit types

Commit v5.15.6-387-g6f48390 forced leavesub to copy returned items
if they were get-magical. Normally rvalue subs  are supposed to return a
copy of their return args, but that copy is sometimes skipped if leavesub
thinks the side-effects will never be visible. Tied elements was an
example where the implementation leaked.

However, this applies equally well to other leave types, such as
do { ....}, so test for get magic in those too.

7 years agotest that pp_leavesub copies returned PADTMPs.
David Mitchell [Wed, 21 Oct 2015 16:54:29 +0000 (17:54 +0100)]
test that pp_leavesub copies returned PADTMPs.

Although pp_leavesub() has in fact always done this, it was never actually
tested for.

7 years agoAlways copy return values when exiting scope
David Mitchell [Wed, 21 Oct 2015 12:10:44 +0000 (13:10 +0100)]
Always copy return values when exiting scope

v5.14.0-642-g3ed94dc fixed certain instances where returning from a sub
incorrectly returned the actual value rather than a copy, e.g.

    sub f { delete $foo{bar} }

This was because if the value(s) being returned had SvTEMP set, copying
was skipped. That commit added an extra condition to the skip test,
SvREFCNT(sv) == 1.

However, this applies equally well to other scope exits, for example

    do { ...; delete $foo{bar} }

So this commits adds the RC==1 test to S_leave_common() too, which handles
all the non-sub scope exits. As well as adding a test to do.t, it adds an
additional test to sub.t, since the original tests, although they
*detected* a non-copied return, didn't actually demonstrate a case where
it was actually harmful.

Note that S_leave_common() also sometimes skips on PADTMPs as well as
TEMPs, so this commit as a side-effect also makes it copy PADTMPs unless
their RC ==1. But since their RC should in fact almost always be 1 anyway,
in practice it makes no difference.

7 years agoop/sub.t: fix ticket number in comment
David Mitchell [Wed, 21 Oct 2015 11:28:35 +0000 (12:28 +0100)]
op/sub.t: fix ticket number in comment

Took me a while to track down that non-existent ticket!

7 years agomake TOPBLOCK(cx) not set cx
David Mitchell [Wed, 21 Oct 2015 08:47:21 +0000 (09:47 +0100)]
make TOPBLOCK(cx) not set cx

i.e. replace

    TOPBLOCK(cx);

with

     cx = CX_CUR();
     TOPBLOCK(cx);

This is part of general trend of separating out the cx/cxstack_ix
setting/manipulation from the save/restore actions

7 years agorename DEBUG_CX() to CX_DEBUG()
David Mitchell [Wed, 21 Oct 2015 08:41:52 +0000 (09:41 +0100)]
rename DEBUG_CX() to CX_DEBUG()

For consistency with the new naming convention for context-stack macros.
Also, give it a cx arg rather than using CX_CUR()

7 years agoadd CX_CUR() macro
David Mitchell [Wed, 21 Oct 2015 08:28:52 +0000 (09:28 +0100)]
add CX_CUR() macro

This is simply

    #define CX_CUR() (&cxstack[cxstack_ix])

7 years agomake POPSUBST re-entrant safe
David Mitchell [Mon, 19 Oct 2015 16:21:58 +0000 (17:21 +0100)]
make POPSUBST re-entrant safe

7 years agomake POPGIVEN re-entrant safe
David Mitchell [Mon, 19 Oct 2015 16:14:58 +0000 (17:14 +0100)]
make POPGIVEN re-entrant safe

7 years agomake POPSUB re-entrant safe
David Mitchell [Mon, 19 Oct 2015 16:04:02 +0000 (17:04 +0100)]
make POPSUB re-entrant safe

7 years agomake POPEVAL safe against re-entrancy
David Mitchell [Mon, 19 Oct 2015 15:46:06 +0000 (16:46 +0100)]
make POPEVAL safe against re-entrancy

7 years agomake POPSUB and POPFORMAT re-entrant safe
David Mitchell [Mon, 19 Oct 2015 15:12:33 +0000 (16:12 +0100)]
make POPSUB and POPFORMAT re-entrant safe

In code like

    sub DESTROY { exit }
    sub f { push @_, bless [] }
    f()

While leaving f, POPSUB will attempt to clear @_. This triggers the
destructor, which calls exit, which unwinds the context stack, calling
POPSUB again on thr same context stack entry.

Thus we need to to be careful that POPSUB can be called multiple times
on the same CX stack entry.

The general way of doing this is to NULL or update pointers *before*
calling SvREFCNT_dec() on the old value.

This commit also removes the old CxPOPSUB_DONE mechanism which was a poor
bandaid added at the last minute to 5.22 to try and achieve the basics
of what this commit does comprehensively (hopefully). Also, CxPOPSUB_DONE
was mainly a protection against re-entrantly calling LEAVE_SCOPE(). Since
that's now done before POPSUB rather than in the middle of it, it has
become reentrant-safe by default.

7 years agoreorder 'struct block' fields.
David Mitchell [Sat, 17 Oct 2015 14:22:05 +0000 (15:22 +0100)]
reorder 'struct block' fields.

On 64-bit builds, there was a 32-bit hole near the beginning. Shuffle this
towards the middle of the struct (just before the blk_u union).

Unfortunately this (moved) hole can't be filled by having an I32 at the
start of each union member, since struct block_eval is already the largest
and has no 32-bit fields.

Still, moving further on than some of the hotter fields can't hurt.

7 years agomove and rename cx_old_savestack_ix
David Mitchell [Sat, 17 Oct 2015 13:52:50 +0000 (14:52 +0100)]
move and rename cx_old_savestack_ix

Earlier on in this branch I temporarily added cx_old_savestack_ix
as the first element in the context struct. This commit moves it down
32 bits so it now follows type/gimme/u16 in the sbu and blku unions.

This means that type is now back as the first item in the struct.

I've also renamed it blku_oldsaveix to be more in keeping with similar
field names.

7 years agosimplify two conditions in pp_iter:
David Mitchell [Sat, 17 Oct 2015 12:24:30 +0000 (13:24 +0100)]
simplify two conditions in pp_iter:

    -        if (UNLIKELY(SvMAGICAL(av) || AvREIFY(av))) {
    +        if (UNLIKELY(SvRMAGICAL(av))) {

This condition is deciding whether to call av_fetch or cheat and do
sv = AvARRAY(av)[ix].

The magic test is too broad: av_fetch() only handles the specially on
RMG, not all magic.

The AvREIFY condition was originally added by 64138690 in 2001. It was
an attempt to paper over the cracks if function args are deleted, then
iterate over @_, which now contains freed elements. Since pp_iter now
includes a 'freed sv' check itself, there's no need to call into av_fetch
which has a similar check.

This makes a slight functional change - previously if @_ contained a freed
element, av_fetch would silently return NULL; now instead, pp_iter
croaks with "Use of freed value in iteration", the same as it does with
all other cases involving freed elements being iterated over.

7 years agoPUSHLOOP_PLAIN: don't set unused fields
David Mitchell [Sat, 17 Oct 2015 12:15:06 +0000 (13:15 +0100)]
PUSHLOOP_PLAIN: don't set unused fields

Currently PUSHLOOP_PLAIN sets a whole unch of fields that it isn't going
to ue to NULL or 0, e.g. blk_loop.itersave.

Skip this and leave them wild. Make cx_dump() not try to dump them for
CXt_LOOP_PLAIN

7 years agoEliminate cx->blk_loop.resetsp
David Mitchell [Sat, 17 Oct 2015 11:56:15 +0000 (12:56 +0100)]
Eliminate cx->blk_loop.resetsp

Of all the loop types, only CXt_LOOP_LIST, i.e. for (1,2,3) {},
needs to keep anything on the stack, i.e. for all the others,
cx->blk_loop.resetsp can always equal cx->blk_oldsp.
For CXt_LOOP_LIST, the same value as resetsp is stored as
blk_loop.state_u.stack.basesp.

So we can eliminate the resetsp field. This is good as the loop substruct
is the largest (ITHREADS) or joint largest with eval (non-threaded).

Also, we now don't have to store that value for non-CXt_LOOP_LIST loops.
The downside is we now have to choose between basesp and oldsp in
pp_leaveloop and pp_last, based on CX type.

7 years agopp_iter(): optimise stack handling
David Mitchell [Sat, 17 Oct 2015 09:53:10 +0000 (10:53 +0100)]
pp_iter(): optimise stack handling

Make pp_enteriter() do EXTEND(SP,1); then there's no need for pp_iter() to
check for space to push PL_sv_yes/no each time round the loop.

Since the only stack manipulation in pp_iter is now just pushing a boolean
at the end, remove dSP etc and just directly push to PL_stack_sp at the
end.

7 years agosplit CXt_LOOP_FOR into CXt_LOOP_LIST,CXt_LOOP_ARY
David Mitchell [Fri, 16 Oct 2015 23:18:44 +0000 (00:18 +0100)]
split CXt_LOOP_FOR into CXt_LOOP_LIST,CXt_LOOP_ARY

Create a new context type so that "for (1,2,3)" and "for (@ary)"
are now two separate types.

For the list type, we store the index of the base stack element in the
state union rather than having an array pointer. Currently this is just
the same as blk_resetsp, but this will shortly allow us to eliminate the
resetsp field from the struct block_loop - which is currently the largest
sub-struct within the block union.

Having two separate types also allows the two cases to be handled directly
in the main switch in the hot pp_iter code, rather than having extra
conditionals.

7 years agobenchmarks: add some 'for' array iterating
David Mitchell [Fri, 16 Oct 2015 15:20:47 +0000 (16:20 +0100)]
benchmarks: add some 'for' array iterating

7 years agostop S_undo_inc_then_croak() doing CX_POP
David Mitchell [Fri, 16 Oct 2015 13:32:26 +0000 (14:32 +0100)]
stop S_undo_inc_then_croak() doing CX_POP

I added this static function several commits commits ago but in hindsight
I don't like that a cx is passed to it and it does a CX_POP(cx). This is
too much like action at a distance. Instead make the caller save
cx->blk_eval.old_namesv and do the CX_POP itself, then pass namesv as an
arg.

7 years agopp_leaveeval: reset stack in VOID context
David Mitchell [Fri, 16 Oct 2015 12:31:57 +0000 (13:31 +0100)]
pp_leaveeval: reset stack in VOID context

    $ perl -Dst -e'eval"1"'

Gives:
    ....
    ((eval 1):1) leaveeval
        =>  (FREED)

Change it so that like all the other pp_leavefoo() functions, it does

    if (gimme == G_VOID)
        PL_stack_sp = newsp;

I can't think of any (non-debugging) perl-level badness the old behaviour
can be shown to demonstrate, but best not to have freed values left
dangling.

This also allows pp_leaveeval() to (like the other pp_leavefoo functions)
avoid doing a dSP with the associated unnecessary PUTBACKs and SPAGAINs.

Finally, the code to detect a false require is moved to earlier in the
function where it's in the same place as the rest of the stack arg
processing code.

7 years agorename S_doeval() to S_doeval_compile()
David Mitchell [Fri, 16 Oct 2015 12:07:23 +0000 (13:07 +0100)]
rename S_doeval() to S_doeval_compile()

This makes it a bit more obvious what niche in the "eval" ecosystem
that it occupies.

7 years agoS_doeval(): tidy up comments
David Mitchell [Fri, 16 Oct 2015 11:19:44 +0000 (12:19 +0100)]
S_doeval(): tidy up comments

Also, replace

    if (...) {
        ...
        return;
    }
    else
        B

with

    if (...) {
        ...
        return;
    }
    B

7 years agoremove obsolete panic from die_unwind()
David Mitchell [Fri, 16 Oct 2015 11:00:46 +0000 (12:00 +0100)]
remove obsolete panic from die_unwind()

It panics if the context it has just popped back to isn't a CXt_EVAL.
Since we have just called dopoptoeval() which can only pop to an eval, and
since we assert that its an eval, this seems such an unlikely "this can
can never happen" that it doesn't really seem worth testing for.

It was added in perl5.000, and may have made slightly more sense then, as
there used to be a POPBLOCK just before it.

7 years agopp_return: avoid potential CX stack realloc prob
David Mitchell [Fri, 16 Oct 2015 10:33:43 +0000 (11:33 +0100)]
pp_return: avoid potential CX stack realloc prob

7 years agoPerl_die_unwind(): remove unneeded assert
David Mitchell [Fri, 16 Oct 2015 10:25:42 +0000 (11:25 +0100)]
Perl_die_unwind(): remove unneeded assert

Now that the order of things has changed, PL_curcop will always have just
been restored at this point. So there's no longer any needs to assert that
its the correct value.

7 years agoadd S_undo_inc_then_croak()
David Mitchell [Thu, 15 Oct 2015 23:03:00 +0000 (00:03 +0100)]
add S_undo_inc_then_croak()

Consolidate into a single static function, code in 3 functions that all
'undo' an added %INC entry on failure, then croak.

7 years agoPOPEVAL: don't set optype
David Mitchell [Thu, 15 Oct 2015 21:27:03 +0000 (22:27 +0100)]
POPEVAL: don't set optype

don't quietly set the local var optype; instead explicitly set it
in the couple of places its actually needed. This allows us to
get rid of a bunch of PERL_UNUSED_VAR(optype);

7 years agoadd CX_POP(cx) macro: glorified cxstack_ix--
David Mitchell [Thu, 15 Oct 2015 16:46:31 +0000 (17:46 +0100)]
add CX_POP(cx) macro: glorified cxstack_ix--

but with extra checking goodness on debugging builds.

7 years agosimplify CHANGE_MULTICALL_FLAGS
David Mitchell [Thu, 15 Oct 2015 15:59:19 +0000 (16:59 +0100)]
simplify CHANGE_MULTICALL_FLAGS

But using POPSUB_COMMON(), we achieve commonality. This incurs the extra
overhead of restoring PL_comppad/PL_curpad, which is compensated for by
not having to temporarily protect PL_comppad in the following PUSHSUB.

7 years agoremove redundant croak code in pp_leavesublv
David Mitchell [Thu, 15 Oct 2015 15:03:34 +0000 (16:03 +0100)]
remove redundant croak code in pp_leavesublv

Just before a code path that croaks, it does a POPSUB etc.
Since this will be done by the unwind following the croak anyway, it's
redundant.

7 years agofactor common code into POPSUB_ARGS()
David Mitchell [Thu, 15 Oct 2015 14:50:08 +0000 (15:50 +0100)]
factor common code into POPSUB_ARGS()

Some code that is common to POPSUB() and pp_coreargs, extract out
into a common macro. (There's some almost identical code in pp_goto,
so add a note there).

This makes pp_coreargs slightly less efficient, since it now tests for
AvREAL(av), when that should never be true. But it's not exactly hot code.

7 years agopp_coreargs: rationalise @_ code
David Mitchell [Thu, 15 Oct 2015 14:39:42 +0000 (15:39 +0100)]
pp_coreargs: rationalise @_ code

Make the code in pp_coreargs that cleans up @_ more like the code in
POPSUB, so that the two can eventually be extracted out into a common
macro.

7 years agosplit some common POPSUB code into a macro
David Mitchell [Thu, 15 Oct 2015 13:58:35 +0000 (14:58 +0100)]
split some common POPSUB code into a macro

POP_MULTICALL duplicates some of the code in POPSUB, so extract it
out into a common macro.

7 years agomake POPSUBST not cxstack_ix--
David Mitchell [Thu, 15 Oct 2015 13:36:59 +0000 (14:36 +0100)]
make POPSUBST not cxstack_ix--

For consistency with with the new POPBLOCK, make POPSUBST(cx) no longer
set c nor decrement cxstack_ix. This also makes dounwind more consistent.

7 years agodowinwind(): move common CX_LEAVE_SCOPE() outside
David Mitchell [Thu, 15 Oct 2015 13:28:28 +0000 (14:28 +0100)]
dowinwind(): move common CX_LEAVE_SCOPE() outside

Now the every branch in the switch does CX_LEAVE_SCOPE(cx), hoist it
above the satrt of the switch.

7 years agodounwind: CX_LEAVE_SCOPE for CXt_SUBST too
David Mitchell [Thu, 15 Oct 2015 13:26:21 +0000 (14:26 +0100)]
dounwind: CX_LEAVE_SCOPE for CXt_SUBST too

This was the only branch that didn't clear up the context stack as
part of a context stack unwind.

7 years agomove blku_old_savestack_ix to base of cxt struct
David Mitchell [Thu, 15 Oct 2015 13:15:57 +0000 (14:15 +0100)]
move blku_old_savestack_ix to base of cxt struct

The previous commit moved the "old savestack" field of substitution
contexts into the shared root of the context union. This commit does the
same for all other context types

7 years agomove sbu_oldsave into base of CX struct
David Mitchell [Thu, 15 Oct 2015 13:04:09 +0000 (14:04 +0100)]
move sbu_oldsave into base of CX struct

the context struct is a union of two basic types: the substitution context
and everything else.

This commit moves the sbu_oldsave field from the substitution context
into the base of the context struct, so that the next commit will allow
the same field (and macros) to be used by *all* context types.

7 years agomove CX_LEAVE_SCOPE outside the POPFOO's
David Mitchell [Thu, 15 Oct 2015 12:48:25 +0000 (13:48 +0100)]
move CX_LEAVE_SCOPE outside the POPFOO's

Currently every POPFOO macro has CX_LEAVE_SCOPE(cx) as its first action.
Since this is common code and not specific to a particular POPFOO type,
remove it from each POPFOO macro and make each caller of POPFOO explicitly
call CX_LEAVE_SCOPE() instead.

This should make no functional difference (but will help if you're
single-stepping the code in a debugger :-)

7 years agodo PL_tmps_floor save in PUSHBLOCK
David Mitchell [Thu, 15 Oct 2015 10:47:07 +0000 (11:47 +0100)]
do PL_tmps_floor save in PUSHBLOCK

Currently every individual PUSHFOO type does

    cx->cx_u.cx_blk.blku_old_tmpsfloor = PL_tmps_floor;
    PL_tmps_floor = PL_tmps_ix;

Move all these into PUSHBLOCK instead, which usually immediately precedes
the PUSHFOO.

7 years agodo PL_tmps_floor restore in POPBLOCK
David Mitchell [Thu, 15 Oct 2015 10:34:41 +0000 (11:34 +0100)]
do PL_tmps_floor restore in POPBLOCK

Currently every individual POPFOO type does

    PL_tmps_floor = cx->cx_u.cx_blk.blku_old_tmpsfloor

as its last action.

Move all these into POPBLOCK instead, which always immediately follows
the  POPFOO.

7 years agosort(!) out CXt_NULL and CXp_MULTICALL
David Mitchell [Thu, 15 Oct 2015 10:16:14 +0000 (11:16 +0100)]
sort(!) out CXt_NULL and CXp_MULTICALL

A sort BLOCK is done using a CXt_NULL context type. Currently it has
the CXp_MULTICALL flag set. Remove this flag so that CXp_MULTICALL is
only set on CXt_SUB contexts.

Also add code comments in various places explainging that CXt_NULL is
likely to a sort BLOCK, and fix the comments in pp_return which said
a particular code path was only taken by sort BLOCK; it's also taken
be (?{...}) too.

7 years agopp_sort: add missing CX_LEAVE_SCOPE()
David Mitchell [Thu, 15 Oct 2015 07:52:18 +0000 (08:52 +0100)]
pp_sort: add missing CX_LEAVE_SCOPE()

The branch that does POPSUB has an implicit CX_LEAVE_SCOPE; the
other branch didn't; do add one for consistency. It's fairly harmless,
as pp_sort shortly afterwards does a LEAVE() anyway.

Also add an assertion to POPBLOCK that the save stack has been popped;
this is how I discovered the sort issue in the first place.

7 years agoremove newpm param from POPBLOCK() macro.
David Mitchell [Mon, 12 Oct 2015 14:33:56 +0000 (15:33 +0100)]
remove newpm param from POPBLOCK() macro.

Since all core usage of POPBLOCK now immediately restores PL_curpm,
there's no need to copy the old value to a temp var specified by the
caller; just make POPBLOCK itself restore PL_curpm in the same way that
it restores all the other PL_ vars.

7 years agoreverse the order of POPBLOCK; POPFOO
David Mitchell [Mon, 12 Oct 2015 13:56:35 +0000 (14:56 +0100)]
reverse the order of POPBLOCK; POPFOO

Currently most pp_leavefoo subs have something along the lines of

    POPBLOCK(cx);
    POPFOO(cx);

where POPBLOCK does cxstack_ix-- and sets cx to point to the top CX stack
entry. It then restores a bunch of PL_ vars saved in the CX struct.

Then POPFOO does any type-specific restoration, e.g. POPSUB decrements the
ref count of the cv that was just executed.

However, this is logically the wrong order. When we *enter* a scope, we do

    PUSHBLOCK;
    PUSHFOO;

so undoing the PUSHBLOCK should be the last thing we do.  As it happens,
it doesn't really make any difference to the running, which is why we've
never fixed it before.

Reordering it has two advantages.

First, it allows the steps for scope exit to be the exact logical reverse
of scope exit, which makes understanding what's going on and debugging
easier.

It allows us to make the code cleaner.

This commit also removes the cxstack_ix-- and setting cx steps from
POPBLOCK; now we already expect cx to be set (which it usually already is)
and we do the cxstack_ix-- ourselves. This also means we can remove a
whole bunch of cxstack_ix++'s that were added immediately after the
POPBLOCK in order to prevent the context being inadvertently overwritten
before we've finished using it.

So in full,

    POPBLOCK(cx);
    POPFOO(cx);

is now implemented as:

    cx = &cxstack[cxstack_ix];
    ... other stuff done with cx ...
    POPFOO(cx);
    POPBLOCK(cx);
    cxstack_ix--;

Finally, this commit also tweaks PL_curcop in pp_leaveeval, since
otherwise PL_curcop could temporarily be NULL when debugging code is
called in the presence of 'use re Debug'. It also stops the debugging code
crashing if PL_curcop is still NULL.

7 years agomake PL_tmps_floor restore consistent
David Mitchell [Mon, 12 Oct 2015 11:02:05 +0000 (12:02 +0100)]
make PL_tmps_floor restore consistent

On scope exit this is done:

    PL_tmps_floor = cx->cx_u.cx_blk.blku_old_tmpsfloor;

This is mostly contained within the POPFOO macros; for those,
make it consistently the last entry in the macro. POPEVAL
didn't include this line; move it into the macro and out of the
various places where it's done explicitly.

This will allow us to move it into a revamped POPBLOCK macro shortly.

7 years agosimplify S_leave_common() and callers
David Mitchell [Sun, 11 Oct 2015 14:58:06 +0000 (15:58 +0100)]
simplify S_leave_common() and callers

Currently one of the args to S_leave_common() is supposed to be the
current stack pointer; it returns an updated sp. Instead make it get/set
PL_stack_sp directly.

e.g. in the caller, replace

    dSP;
    SP = S_leave_common(..., SP, ...);
    PUTBACK;

with
    S_leave_common(..., ...);

and in S_leave_common(), make it initially get PL_stack_sp, and before
returning, update PL_stack_sp.

7 years agoeliminate weird gimme calc in pp_leave()
David Mitchell [Sat, 10 Oct 2015 22:41:15 +0000 (23:41 +0100)]
eliminate weird gimme calc in pp_leave()

Some code in pp_leave that's been around in one form or another since
5.000, calculates the value of gimme in a complicated way, rather than
just using the saved value in the current context, like all the other
pp_leave* functions do.

I think this code is redundant; running the test suite with an assert that
the extra calculation doesn't change anything, doesn't produce any failures.
So out it goes.

(In particular. cxstack_ix can never be < 0 here.)

7 years agoeliminate LEAVESUB() macro
David Mitchell [Sat, 10 Oct 2015 22:22:30 +0000 (23:22 +0100)]
eliminate LEAVESUB() macro

Many years ago, POPSUB was split into two parts, with a final cleanup
being done by the LEAVESUB() macro. After the previous commit, LEAVESUB
now always immediately follows POPSUB, so roll its action into the
last line of POPSUB and eliminate it.

This also allows us to remove the 'sv' parameter from POPSUB(), which
was needed purely to communicate to LEAVESUB() which var to process

7 years agomake LEAVESUB() always immediately follow POPSUB()
David Mitchell [Sat, 10 Oct 2015 22:10:23 +0000 (23:10 +0100)]
make LEAVESUB() always immediately follow POPSUB()

Many years ago, POPSUB was split into two parts, with a final cleanup
being done by the LEAVESUB() macro. This is no longer needed;
shuffle LEAVESUB up a bit in various places so that it always immediately
follows POPSUB. Then the next commit will eliminate it altogether.

7 years agomove CX_LEAVE_SCOPE into POPEVAL
David Mitchell [Sat, 10 Oct 2015 22:01:55 +0000 (23:01 +0100)]
move CX_LEAVE_SCOPE into POPEVAL

All the other POPFOO types have CX_LEAVE_SCOPE as their first action.
Add it as the first action of POPEVAL too, and remove all the explicit
calls to CX_LEAVE_SCOPE dotted around near POPEVALs.

Note that this causes a slight change in behaviour in dounwind:
the CX_LEAVE_SCOPE is now done as the first action of POPEVAL there
rather than being done afterwards. This is now consistence will all other
uses of POPEVAL.

7 years agoadd CX_LEAVE_SCOPE(cx) macro
David Mitchell [Sat, 10 Oct 2015 21:49:46 +0000 (22:49 +0100)]
add CX_LEAVE_SCOPE(cx) macro

which is shorthand for

    LEAVE_SCOPE(cx->cx_u.cx_blk.blku_old_savestack_ix)

This is the start of a new breed of macros designed to manipulate the
conetect stack, that have a CX_ prefix, that will eventually superede
the (PUSH/POP)(BLOCK/SUB/ETC) system.

7 years agocall LEAVE_SCOPE() before POPEVAL()
David Mitchell [Sat, 10 Oct 2015 21:38:54 +0000 (22:38 +0100)]
call LEAVE_SCOPE() before POPEVAL()

All the other POPFOO() types have

    LEAVE_SCOPE(cx->cx_u.cx_blk.blku_old_savestack_ix);

as their first action.

POPEVAL doesn't include this. Instead, each place that does POPEVAL()
currently does a LEAVE_SCOPE() sometime shortly afterwards.
This commit moves all those LEAVE_SCOPE()s to just before each POPEVAL()
to make the behaviour like all the other context types.

This is the logically correct order: process all the savestack items
accumulated during the eval before popping the eval itself.

7 years agoPOPBLOCK: don't set newsp and gimme
David Mitchell [Sat, 10 Oct 2015 21:23:19 +0000 (22:23 +0100)]
POPBLOCK: don't set newsp and gimme

This macro used to set these two vars as a side-effect.
Since we now usually access those values before we call POPBLOCK,
it's wasteful to set them again.

7 years agomove POPBLOCK after arg stack munging
David Mitchell [Sat, 10 Oct 2015 15:31:28 +0000 (16:31 +0100)]
move POPBLOCK after arg stack munging

In various places where scope is left (pp_leave, pp_leavesub etc)
the perl argument stack is processed: a combination of shifting return
args down and mortalising or copying them. This is typically done in
S_leave_common() (although pp_leavesub(lv) roll their own).

Normally POPBLOCK() is called just before this, which
a) initialises cx;
b) sets newsp, gimme etc which are needed by S_leave_common()
c) restores a bunch of PL_ vars from the saved values in cx.

Logically, (c) should be the last thing done in the various pp_leave*
functions, as saving the vars is the first thing done in the various
pp_enter* functions.

This commit moves POPBLOCK after S_leave_common() as a first step towards
migrating it towards the end of the various functions.

It shouldn't make any functional difference, as the values of the various
restored vars aren't used by the arg munging code.

7 years agoadd PUSH/POPBASICBLK macros.
David Mitchell [Sat, 10 Oct 2015 14:39:53 +0000 (15:39 +0100)]
add PUSH/POPBASICBLK macros.

Most contexts types have a pair, e.g. PUSHSUB/POPSUB for CXt_SUB.
Add them for a basic block too, CXt_BLOCK, i.e. pp_enter/pp_leave.
It makes things slightly more regular ahead of future refactoring.

7 years agopp_leavewhen(): skip POPWHEN()
David Mitchell [Mon, 5 Oct 2015 15:59:05 +0000 (16:59 +0100)]
pp_leavewhen(): skip POPWHEN()

pp_leavewhen at some point will do an unconditional dounwind()
to pop the WHEN, BLOCK, and any other intermediate contexts, till it
returns to the enclosing FOR or GIVEN context. Thus there is no need to do
an initial POPBLOCK()/POPWHEN(), as this will be done by dounwind.
We just need to grab a couple of values from the top (GIVEN) context
to go arg stack processing correctly.

7 years agopp_leavewhen: simply for() handling
David Mitchell [Mon, 5 Oct 2015 15:23:07 +0000 (16:23 +0100)]
pp_leavewhen: simply for() handling

when returning control to an enclosing for loop, the next op to execute
(i.e. my_op->op_nextop) will be an OP_UNSTACK, which handles popping stuff
of stacks and doing PERL_ASYNC_CHECK(); so no need to do it here too.

7 years agorename S_dopoptogiven() to S_dopoptogivenfor()
David Mitchell [Mon, 5 Oct 2015 14:50:18 +0000 (15:50 +0100)]
rename S_dopoptogiven() to S_dopoptogivenfor()

Since it searches the context stack for the next GIVEN *or* FOR LOOP
context, make the name better express its purpose.

7 years agoadd loop benchmark tests
David Mitchell [Mon, 5 Oct 2015 13:48:35 +0000 (14:48 +0100)]
add loop benchmark tests

7 years agoMake remaining context types avoid ENTER/LEAVE
David Mitchell [Fri, 28 Aug 2015 15:11:50 +0000 (16:11 +0100)]
Make remaining context types avoid ENTER/LEAVE

Previous commits have made the 'subbish' context types like SUB, EVAL
use the context stack to save the old values of PL_savestack_ix and
PL_tmps_floor rather than using ENTER/SAVETMPS/LEAVE; extend this
to the remaining contexts types; specifically loops, bare blocks and
given/when.

Note that this means that during context stack unwinding (e.g. in
dounwind()), LEAVE_SCOPE() is called for every context popped, whereas
before it was only done for each subbish context type.

This makes the ordering of the actions in scope unwinding match much more
closely the reverse order in which things were done during scope pushing;
for example entering and leaving a for loop will now do:

    PUSHLOOP_FOR();
    ...
    ... code within loop which pushes stuff on the savestack
    ...
    LEAVE_SCOPE(); /* pops the stuff from above */
    POPLOOP();

The main reason for this commit (apart from increased consistency)
is that its a lot faster.

7 years agoConsistently call leave_common() before POPFOO
David Mitchell [Mon, 5 Oct 2015 09:57:09 +0000 (10:57 +0100)]
Consistently call leave_common() before POPFOO

In some places the code does

    POPBLOCK(); POPFOO(); leave_common();

while in other places it does

    POPBLOCK(); leave_common(); POPFOO();

Make it do the latter consistently. This is because the next commit
will make *all* context types do a LEAVE_SCOPE(), not just the 'subish'
ones as now. This means that any context type that can return args needs
to preserve its args before doing the LEAVE_SCOPE()/POPFOO(), not just the
sub ones.

For example this lexical var is declared within a LOOP_PLAIN:

    sub foo {
        {
            my $x = 1;
            $x;
        }
    }

where the return value of the loop block ($x) is also the return value of
the sub. So let leave_common() make a mortal copy of $x before POPLOOP
frees the lexicals in the inner scope.

Similar comments apply to calling leave_common() before calling
dounwind().

Note that where the LEAVE_SCOPE() is located it not yet consistent;
sometimes it's part of the POPFOO macro, sometimes it's not, and so has to
be done explicitly just before calling POPBAR.

7 years agopp_given: avoid using savestack for old var
David Mitchell [Fri, 2 Oct 2015 16:28:00 +0000 (17:28 +0100)]
pp_given: avoid using savestack for old var

Add a new field, defsv_save, to struct block_givwhen, and use this
to save the previous $_ in 'when(expr)' rather than saving it on the save
stack.

Also add POPWHEN and POPGIVEN macros. The former is a no-op for now.

7 years agocontext: move couple of fields into block struct
David Mitchell [Fri, 28 Aug 2015 14:37:23 +0000 (15:37 +0100)]
context: move couple of fields into block struct

A few of the block_foo substructs within the context struct have the
same two fields, old_savestack_ix and old_tmpsfloor. Move these into the
basic block struct so they can be used by all blockish context types.

This is basically a big renaming exercise and should make no functional
difference.

7 years agopp_enteriter, POPLOOP: simplify some code
David Mitchell [Fri, 28 Aug 2015 07:37:42 +0000 (08:37 +0100)]
pp_enteriter, POPLOOP: simplify some code

merge some common code from branch arms and tidy up. Shouldn't make
any functional difference.

7 years agopp_enteriter: don't create new SV for GvSV slot
David Mitchell [Fri, 28 Aug 2015 07:21:05 +0000 (08:21 +0100)]
pp_enteriter: don't create new SV for GvSV slot

Currently in

    for $pkg_var (...) {}

pp_enteriter() saves the current SV that's in the GvSV  slot, then
replaces it with a newSV(0). Instead, leave it untouched. In the range
cases:

    for $pkg_var (1..10) {}
    for $pkg_var ('a'..'z') {}

a new SV will be created anyway in the first call to pp_iter(), since each
time round the loop it checks whether the refcount of the iterator vars is
greater than 1 and if so abandons it.

For the list case, e.g.

    for $pkg_var (1,3,5) {}
    for $pkg_var (@foo) {}

each call to pp_iter updates the GvSV slot to point to the current var,
so sticking a null SV in there initially, and freeing it the end is just
pointless extra work.

The only slight proviso is when the GvSV slot is initially NULL. In this
rare case we still create the SV, as it will be needed for the range
cases.

7 years agoadd CXp_FOR_PAD, CXp_FOR_GV flags
David Mitchell [Thu, 27 Aug 2015 15:03:42 +0000 (16:03 +0100)]
add CXp_FOR_PAD, CXp_FOR_GV flags

These indicate that the loop is a for loop, with a lexical or package
iterator variable.

These flags make various tests simpler and more efficient.

This commit also fixes a bug introduced a couple of commits ago;
I was assuming that in POPLOOP, itersave being non-NULL indicated a
'for $lex' or 'for $pkg', but something like

    local *x;
    for $x (..) {}

leaves the loop with itersave set to NULL. Instead we can use the new
flags to indicate that itersave is valid.

7 years agopp_enteriter: tidy itervar-setting code
David Mitchell [Thu, 27 Aug 2015 13:45:13 +0000 (14:45 +0100)]
pp_enteriter: tidy itervar-setting code

rename itervar to itervarp since it's more like an SV** than an SV*,
and set itersave earlier so that it can be used directly rather than
repeatedly dereffing itervarp.

Shouldn't be any functional change.

7 years agofor loops: don't refcount bump orig var
David Mitchell [Thu, 27 Aug 2015 12:43:49 +0000 (13:43 +0100)]
for loops: don't refcount bump orig var

In something like

    for $pkg_var (...)

pp_enteriter() bumps the reference count or $pkg_var before making
itersave point to it. POPLOOP later decrements this ref count.

This bump is unnecessary; since we are effectively transferring ownership
(and thus ref count contribution) of the $pkg_var SV from the GvSV slot of
*pkg_var to the itersave slot of the context struct, the overall ref count
of the var should remain unchanged.

So skip the bump and later undo. This should make no functional difference;
it's just more efficient.