This is a live mirror of the Perl 5 development currently hosted at https://github.com/perl/perl5
perl5.git
5 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.

5 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.

5 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

5 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.

5 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

5 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.

5 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.

5 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);

5 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.

5 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.

5 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.

5 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.

5 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.

5 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.

5 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.

5 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.

5 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.

5 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

5 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.

5 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 :-)

5 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.

5 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.

5 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.

5 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.

5 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.

5 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.

5 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.

5 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.

5 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.)

5 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

5 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.

5 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.

5 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.

5 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.

5 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.

5 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.

5 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.

5 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.

5 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.

5 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.

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

5 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.

5 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.

5 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.

5 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.

5 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.

5 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.

5 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.

5 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.

5 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.

5 years agoPOPLOOP(): no need to mortalise current item
David Mitchell [Thu, 27 Aug 2015 12:40:20 +0000 (13:40 +0100)]
POPLOOP(): no need to mortalise current item

Now that pp_return() protects its return arg(s) against premature freeing
before calling dounwind(), there's no need for POPLOOP to mortalise the
current item.

5 years agoAdd itersave field to LOOP context struct
David Mitchell [Wed, 19 Aug 2015 11:33:24 +0000 (12:33 +0100)]
Add itersave field to LOOP context struct

When entering a for loop, rather than saving the original loop variable on
the save stack, save it in the context struct. Quicker and simpler.

Note that as of this commit, entering a for loop no longer saves anything
on the save stack.

5 years agoPOPLOOP: call LEAVE_SCOPE()
David Mitchell [Thu, 27 Aug 2015 11:13:58 +0000 (12:13 +0100)]
POPLOOP: call LEAVE_SCOPE()

in PUSHLOOP, save the current PL_savestack_ix, and in POPLOOP, do
    LEAVE_SCOPE(cx->blk_loop.old_savestack_ix);
This is in preparation for shortly not wrapping loops in ENTER/LEAVE.

5 years agopp_return(): reindent following previous commit
David Mitchell [Thu, 27 Aug 2015 11:02:27 +0000 (12:02 +0100)]
pp_return(): reindent following previous commit

whitespace-only change

5 years agopp_return(): handle dounwind() freeing args
David Mitchell [Tue, 25 Aug 2015 12:57:55 +0000 (13:57 +0100)]
pp_return(): handle dounwind() freeing args

Currently only POPSUB (and other sub-like contexts, such as POPEVAL) do a
LEAVE_SCOPE() as well as restoring things from the context struct. This
means that if pp_return() does a dounwind() to pop back to the next
SUB/EVAL/FORMAT context, LEAVE_SCOPE() won't get called, and any return
values aren't prematurely freed, e.g. in the following

    sub f {
        for (...) {
            my $x = 1;
            return $x;
        }
    }

POPLOOP() won't call LEAVE_SCOPE(), so $x doesn't get freed.

The next commit is about to change that: POPLOOP() will indeed call
LEAVE_SCOPE(), (and later commits may make other POPFOO() types do that
too). So in preparation, this commit makes pp_return() preserve any return
args before calling dounwind().

5 years agopp_leaveloop: use SVs_PADTMP|SVs_TEMP
David Mitchell [Wed, 26 Aug 2015 10:34:59 +0000 (11:34 +0100)]
pp_leaveloop: use SVs_PADTMP|SVs_TEMP

When calling leave_common(), most callers specify the "safe" return SVs
that don't need copying as being those with SVs_PADTMP or SVs_TEMP
set (leaveeval is a correct exception to this this).

pp_leaveloop() specified no flags, meaning that it always copies. This
appears to be just be an oversight rather than being for any good reason.

5 years agoS_leave_common(): simplify SVs_PADTMP handling
David Mitchell [Wed, 26 Aug 2015 08:53:09 +0000 (09:53 +0100)]
S_leave_common(): simplify SVs_PADTMP handling

Now that SVs_PADTMP is a simple flag again (at one point its meaning was
inferred from the state of two flags) it no longer needs special handling
in S_leave_common().

5 years agofor my $x (...): $x is always stale
David Mitchell [Sun, 19 Jul 2015 09:51:38 +0000 (10:51 +0100)]
for my $x (...): $x is always stale

Remove the code in pp_enteriter that turns $x's staleness off and schedules
on the save stack for it to be turned back on at scope exit.

Since 'for my $x' works by making $x's pad slot temporarily point to
each item in turn, the "real" $x which lives in the pad outside of loop
time can never be accessed by the usual closure tricks (BEGIN, eval,
nested named subs etc) during a loop, so doesn't need to be marked as not
stale.

Skipping this step is one less thing to be pushed onto the save stack
on each for loop entry.

5 years agopp_last: use debugging LEAVE variant
David Mitchell [Fri, 17 Jul 2015 22:22:29 +0000 (23:22 +0100)]
pp_last: use debugging LEAVE variant

Make the two LEAVEs in pp_last() be instead

    LEAVE_with_name("loop2");
    LEAVE_with_name("loop1");

which is more self-documenting and will trap mistakes on DEBUGGING builds

5 years agoonly call leave_common in non-void context
David Mitchell [Fri, 17 Jul 2015 21:49:40 +0000 (22:49 +0100)]
only call leave_common in non-void context

A lot of the pp_leave* functions call S_leave_common() to process
any return args. Since many of these tend to be used in void context
(e.g. plain blocks {; ... }, for loops etc) tst gimme and only call
these function is non-void. For void context the function is mostly
a noop anyway; the only thing we're skipping is TAINT_NOT, which
doesn't matter in void context.

5 years agoCXt_EVAL: save savestack_ix and tmps_floor in CX
David Mitchell [Fri, 17 Jul 2015 21:23:51 +0000 (22:23 +0100)]
CXt_EVAL: save savestack_ix and tmps_floor in CX

In the various places that do PUSHEVAL (eval, require etc), eliminate

    ENTER; SAVETMPS

and instead save the old values of PL_savestack_ix and PL_tmps_floor
directly in the eval context frame, similarly to how subs have been
recently changed.

This is faster and cleaner.

5 years agomove SAVETMPS next to PUSHEVAL
David Mitchell [Fri, 17 Jul 2015 20:51:25 +0000 (21:51 +0100)]
move SAVETMPS next to PUSHEVAL

In the various places where PUSHEVAL() is used, a SAVETMPS appears
shortly before it. Move each occurrence of SAVETMPS to just after the
SAVETMPS, in preparation shortly for making PUSHEVAL responsible for
saving PL_tmps_floor.

In theory this could make certain temporary items created during
eval/require startup to be freed slightly later. I don't know whether
this is is an issue or not. No tests fail ;-)

5 years agocall_sv(), fold_const(): different CX pop test
David Mitchell [Fri, 17 Jul 2015 13:44:32 +0000 (14:44 +0100)]
call_sv(), fold_const(): different CX pop test

Perl_call_sv() and S_fold_constants() both have similar code that:
pushes an EVAL context, does a JMPENV_PUSH() and CALLRUNOPS(), then
optionally pops the EVAL context.

The optionally part depends on whether what's doing the dying
has already popped the context before long-jumping. Currently the decision
on whether to pop is based on whether the scope stack has already been
popped.

This commit changes that to whether the context stack has already been
popped, since shortly we're going to change eval contexts so that the old
savestack_ix is stored in the CX struct rather than on the scope stack.

I ran this code with some asserts that the two conditions were identical,
and nothing failed.

5 years agoCXt_FORMAT: save ss_ix and tmps_floor in CX struct
David Mitchell [Thu, 16 Jul 2015 13:44:06 +0000 (14:44 +0100)]
CXt_FORMAT: save ss_ix and tmps_floor in CX struct

To match the recent work in this branch on CXt_SUB, make CXt_FORMAT save
PL_savestack_ix and PL_tmps_floor in the context structure rather than
doing ENTER/SAVETMPS. It's more efficient and more consistent.

5 years agopp_entersub(): reduce life of padlist var
David Mitchell [Mon, 13 Jul 2015 15:31:20 +0000 (16:31 +0100)]
pp_entersub(): reduce life of padlist var

only set it when it's needed

5 years agopp_entersub: unroll some CvFLAGS(cv) tests
David Mitchell [Mon, 13 Jul 2015 15:25:36 +0000 (16:25 +0100)]
pp_entersub: unroll some CvFLAGS(cv) tests

Do a single bit comparison rather than two conditionals. Slightly faster.

5 years agopp_entersub(): reduce scope of gimme
David Mitchell [Mon, 13 Jul 2015 15:18:55 +0000 (16:18 +0100)]
pp_entersub(): reduce scope of gimme

Only calculate the gimme var where its needed.

5 years agopp_entersub(): don't prematurely calc hasargs
David Mitchell [Mon, 13 Jul 2015 12:11:01 +0000 (13:11 +0100)]
pp_entersub(): don't prematurely calc hasargs

Currently at the top of pp_entersub() is:

    const bool hasargs = (PL_op->op_flags & OPf_STACKED) != 0;

Defer testing this flag and/or saving it to a local var until
its actually needed. This is a micro-optimisation.

5 years agorevamp pp_entersub()'s CV locating code
David Mitchell [Mon, 13 Jul 2015 11:48:17 +0000 (12:48 +0100)]
revamp pp_entersub()'s CV locating code

The big switch statement at the top of pp_entersub() is intended
to extract the CV from the passed argument, which might be a CV, a GV,
a ref to a CV, etc etc.

In 5.22.0, an optimisation was added to stashes such that if an entry
contained only a CV, then a ref to the CV was stored in the stash, rather
than a GV. This means that in the common case for a plain sub call, sv is
now a ref to a CV rather than a GV. So update the code to:

1) remove the special-casing of GVs;
2) add special casing for the sv being a non-magical, non-overloaded RV
   pointing to a CV;
3) add special casing for the sv being a CV (true on method calls);
3) move the rare "sv == &PL_sv_yes" test further down the condition chain;
4) generally rearrange the switch cases so that common things fall though
   and uncommon things do a goto;
5) sprinkle more LIKELY() pixie dust.

Part of the intention of special-casing is to avoid doing an indirect
branch by avoiding the switch statement completely.

5 years agotweak POPSUB()
David Mitchell [Mon, 13 Jul 2015 10:48:04 +0000 (11:48 +0100)]
tweak POPSUB()

re-order things slightly in POPSUB() so that values aren't being
read into local vars before they're needed (so the compiler doesn't
have to write the values back out to the local stack frame if it runs out
of spare registers).

Also, re-order the fields in struct block_sub so that they roughly
correspond with the order in which they are accessed by POPSUB. This is on
the voodoo theory that if the struct straddles a cache line, we may
trigger a prefetch of the second line, so the extra data will be ready for
us when we need it.

5 years agoEliminate ENTER/LEAVE from sub calls
David Mitchell [Sat, 11 Jul 2015 21:13:51 +0000 (22:13 +0100)]
Eliminate ENTER/LEAVE from sub calls

Every sub call is wrapped in an ENTER/LEAVE pair, which uses the next
free slot on the scope stack to save and then restore the old value of
PL_savestack_ix. Instead, store the old value in a new field in the
context structure, old_savestack_ix. This is quicker and simpler.

Not that we keep the ENTER/LEAVE for XS sub calls, as they don't push a
context frame, and so have nowhere else to remember PL_savestack_ix.

As a side-effect, this commit fixes a TODO test in t/op/sub.t,
which was related to dying while popping a context, then re-popping that
context. For the second pop, the scopestack has since been overwritten
and so too much was getting popped from the savestack. Since we no longer
use the scopestack, it's no longer an issue.

5 years agopp_hot.c: skip unnecessary test
David Mitchell [Sat, 11 Jul 2015 15:43:25 +0000 (16:43 +0100)]
pp_hot.c: skip unnecessary test

This condition:

    !CvROOT(cv) && !CvXSUB(cv)

can be simplified because those two cv fields are actually
the same slot in a union.

5 years agopp_entersub(): simplify autoload logic
David Mitchell [Sat, 11 Jul 2015 14:37:11 +0000 (15:37 +0100)]
pp_entersub(): simplify autoload logic

eliminate a label and goto by by just setting cv to null on failure,
and have a single "if (!cv) croak()" test at the end of the loop.

5 years agopp_entersub(): eliminate a label
David Mitchell [Sat, 11 Jul 2015 14:30:38 +0000 (15:30 +0100)]
pp_entersub(): eliminate a label

replace:

  retry:
    if (A) die;
    if (B) {
        ...;
        goto retry;
    }

with

    while (B) {
        ...;
    }

    if (A) die;

it's functionally equivalent except that the A test is now only
tried after we have been successful at B. This is ok, because
A is testing for an uncloned closure prototype, while B is looking
for a CV stub which needs autoloading,and it doesn't rally matter
which we test for first.

5 years agoadd old_tmpsfloor field to CXt_SUB context frame
David Mitchell [Sat, 11 Jul 2015 13:42:56 +0000 (14:42 +0100)]
add old_tmpsfloor field to CXt_SUB context frame

Rather than saving and restoring PL_tmps_floor on subroutine entry/exit
by using SAVETMPS and the save stack, store the old value directly
in the context struct and make PUSHSUB/POPSUB handle the saving and
restoring.

5 years agoPUSH_MULTICALL: move SAVETMPS later
David Mitchell [Sat, 11 Jul 2015 13:16:29 +0000 (14:16 +0100)]
PUSH_MULTICALL: move SAVETMPS later

Move the SAVETMPS to just after the PUSHSUB. This should make no
functional difference, because nothing should have affected the TMPS stack
between the old and the new code positions.

5 years agopp_dbstate: do SAVETMPS etc in both branches
David Mitchell [Sat, 11 Jul 2015 13:04:59 +0000 (14:04 +0100)]
pp_dbstate: do SAVETMPS etc in both branches

pp_dbstate() does an XS or non-XS subroutine call to &DB::DB;
move the SAVETMPS from before the (if CvISXSUB(cv)) test to the head of
both branches. This duplication of code is currently a noop, but will
shortly allow us to handle things differently in the two branches.

5 years agopp_sort: move SAVETMPS later
David Mitchell [Sat, 11 Jul 2015 12:58:47 +0000 (13:58 +0100)]
pp_sort: move SAVETMPS later

Move the SAVETMPS to just after the PUSHSUB. This should make no
functional difference, because nothing should have affected the TMPS stack
between the old and the new code positions.

For the OPf_SPECIAL/ CXt_NULL case, which doesn't do a PUSHSUB,
do another SAVETMPS there too.

5 years agopp_goto: do SAVETMPS etc in XS and non-XS branches
David Mitchell [Sat, 11 Jul 2015 12:45:01 +0000 (13:45 +0100)]
pp_goto: do SAVETMPS etc in XS and non-XS branches

Move a SAVETMPS;SAVEFREESV(cv); from before the "if (CvISXSUB(cv))" test
to the head of both branches. This duplication of code is currently
a NOOP, but will shortly allow us to handle things differently in the two
branches.

5 years agopp_entersub(): move SAVETMPS next to PUSHSUB
David Mitchell [Sat, 11 Jul 2015 12:32:03 +0000 (13:32 +0100)]
pp_entersub(): move SAVETMPS next to PUSHSUB

It it intended that eventually PUSHSUB() will do the SAVETMPS itself,
so move the SAVETMPS up so that it's next to the PUSHSUB.
There should be nothing between those two points that use the TEMPS stack,
so this commit should have no functional effect.

5 years agopp_entersub(): remove an unnecessary condition
David Mitchell [Sat, 11 Jul 2015 12:10:00 +0000 (13:10 +0100)]
pp_entersub(): remove an unnecessary condition

The code that scans the args list making mortal copies is protected by an

    if (LIKELY(hasargs)) {

However, a sub called as "&foo;" (which is how hasargs is determined)
won't have pushed anything on the stack, so the first part of that code
block, "while (svp < SP)", will drop out immediately.

So skip doing the hasargs test.

NB - it's possible in theory for XS code to call call_sv() jn such a way
as to both have !hasargs but with stuff on the stack; in this worst case,
we just waste a bit of time making some mortal copies of those args.

5 years agopp_entersub(): mortal-copy args earlier
David Mitchell [Sat, 11 Jul 2015 11:43:43 +0000 (12:43 +0100)]
pp_entersub(): mortal-copy args earlier

Move the code earlier which makes a mortal copy of any PADTMP args.
This should make no functional difference, but will shortly allow
us to move the SAVETMPS earlier too (while still coming after the
mortal copying).

5 years agopp_leavesub: simplify recursion test
David Mitchell [Sat, 11 Jul 2015 10:49:13 +0000 (11:49 +0100)]
pp_leavesub: simplify recursion test

Part of pp_leavesub() tests whether we're in recursion by looking
at CvDEPTH(cx->blk_sub.cv). Instead, just directly check
cx->blk_sub.olddepth, which is equivalent (allowing for an offset of 1)
but faster.

5 years agoclear_defarray(): clear @_ if possible
David Mitchell [Sat, 11 Jul 2015 10:05:55 +0000 (11:05 +0100)]
clear_defarray(): clear @_ if possible

clear_defarray() is called during sub exit to clean up @_ in the
less common case where @_ has somehow gotten reified.

At the moment it just frees the old @_, then creates a new AV and
sticks it in pad[0].

This commit changes it so that for the reasonably common case of a reified
@_ where it's not magical and only has a reference count of 1, just call
av_clear() on it, rather than freeing and recreating.

Typical code that will benefit from this change is be something like:

    sub f { push @_, ...; ... }

which causes the AV to be reified, but not to become otherwise visible.

5 years agopp_goto(): use clear_defarray()
David Mitchell [Sat, 11 Jul 2015 09:52:49 +0000 (10:52 +0100)]
pp_goto(): use clear_defarray()

In pp_goto(), use the newly-added clear_defarray() function
to the clear the old @_. This ensures that POPSUB() and pp_goto()
do the same thing. Note that clear_defarray() is actually better than
the old pp_goto() code, in that it pre-sizes the newly-created array
to match the size of the array being abandoned.

5 years agoadd Perl_clear_defarray()
David Mitchell [Sat, 11 Jul 2015 09:40:23 +0000 (10:40 +0100)]
add Perl_clear_defarray()

This function implements the less commonly used branch in the POPSUB()
macro that clears @_ in place, or abandons it and creates a new array
in pad slot 0 of the function (the common branch is where @_ hasn't been
reified, and so can be clered simply by setting fill to -1).

By moving this out to a separate function we can avoid repeating the same
code everywhere the POPSUB macro is used; but since its only used
in the less frequent cases, the extra overall of a function call doesn't
matter.

It has a currently unused arg, 'abandon', which will be used shortly.

5 years agoavoid leaking @_ in goto
David Mitchell [Sat, 11 Jul 2015 09:06:39 +0000 (10:06 +0100)]
avoid leaking @_ in goto

pp_goto temporarily bumps the reference count of @_ while doing
LEAVE_SCOPE(), to stop it getting prematurelly freed. If something
dies during the save stack unwinding, it will leak.
Instead, make it mortal.

5 years agogoto.t: add freeing CV test
David Mitchell [Thu, 9 Jul 2015 08:22:38 +0000 (09:22 +0100)]
goto.t: add freeing CV test

This code SEGVed in a cpan/ module while I was messing with pp_goto.
Add it to t/op/goto.t so that it can SEGV there instead.

5 years agopp_goto: do the DIEing before the POPing
David Mitchell [Sun, 5 Jul 2015 09:21:14 +0000 (10:21 +0100)]
pp_goto: do the DIEing before the POPing

Rearrange the beginning of the 'goto &func' part of pp_goto so that it
does most of its error checks (e.g. "Can't goto subroutine from an eval")
before it starts doing "return-ish" stuff. This makes the code slightly
cleaner, especially as it doesn't have to undo the SvREFCNT_inc(cv) guard
in every die branch.

5 years agoeliminate a goto in pp_goto (!)
David Mitchell [Sun, 5 Jul 2015 09:08:25 +0000 (10:08 +0100)]
eliminate a goto in pp_goto (!)

Replace a C-level goto with a while loop: logically equivalent,
but doesn't use a goto. (I know, I know, this is in pp_goto, but even
so...)

5 years agopp_goto: a couple of cosmetic changes
David Mitchell [Fri, 3 Jul 2015 10:40:01 +0000 (11:40 +0100)]
pp_goto: a couple of cosmetic changes

had a lable outdented 2, not 4 chars, and move a comment describing
what a branch does from before to after the 'if'.

5 years agomake POP_SAVEARRAY() safe
David Mitchell [Fri, 3 Jul 2015 10:14:30 +0000 (11:14 +0100)]
make POP_SAVEARRAY() safe

POP_SAVEARRAY() frees the current @_ and restores it to the callers value.
As currently defined, it frees the old AV then updates GvAV(PL_defgv). If
the free old the old AV triggers a destructor for example, then in
theory something bad could happen.

I couldn't actually find a simple failure case, but I suspect that
something like

    sub f { *_ = bless [], 'Foo' }

where Foo::DESTROY is an XS sub that messes with GvAV(PL_defgv), could do
it.

Anyway to play safe, this commit updates the GvAV() slot and *then* frees
the old AV. This our normal modus operandi in other places anyway.

The equivalent code in pp_goto already does this the safe way. This commit
also updates pp_goto to use the (now equivalent) POP_SAVEARRAY() macro.

5 years agot/perf/benchmarks: add a few sub and goto tests
David Mitchell [Thu, 2 Jul 2015 09:14:14 +0000 (10:14 +0100)]
t/perf/benchmarks: add a few sub and goto tests

5 years agopp_entersub: skip resetting @_
David Mitchell [Wed, 1 Jul 2015 19:19:23 +0000 (20:19 +0100)]
pp_entersub: skip resetting @_

Whenever we leave a sub by whatever means (pp_leavesub, pp_return,
pp_goto, die etc) it is the responsibility of the code that pops
the SUB context to clean up the private @_ of that sub (i.e pad[0])
so that it's empty and not real.

There's some code in pp_entersub that duplicates this check. I believe
this check is unnecessary and so this commit removes it and replaces it
with an assert. It was added by 221373f04 in 1999 to fix the issue
described in

    Subject: [ID 19991015.010] [BUG 5.005_62 Assertation failed:
            "pp_ctl.c" line 2443]
    Message-Id: <19991016024500.A32541@athens.aocn.com>

There were two fixes applied for this issue; the other was
0253cb4. I think the second commit actually fixed the issue and the
first fix was a red herring.

If my newly-added assert fails, it implies that something needs fixing in
the context-popping code, rather than in pp_entersub.

In fact the new assert showed up one issue: the special-case handling
of @_ for &CORE::undef in pp_coreargs wasn't emptying the array,
so I added a CLEAR_ARGARRAY().

5 years agoimprove @_ commentary in pp_goto
David Mitchell [Wed, 1 Jul 2015 15:50:53 +0000 (16:50 +0100)]
improve @_ commentary in pp_goto

Following on from the previous commit which removed cx.argarray,
remove 'argarray' from the comments about @_, and at the same time
generally overhaul those comments; in particular, explaining how
it's a two part process of donating the current @_ to the new sub.

5 years agoeliminate the argarray field from the CX struct
David Mitchell [Wed, 1 Jul 2015 12:37:32 +0000 (13:37 +0100)]
eliminate the argarray field from the CX struct

At the end of a normal sub call there are often 3 AVs of interest
associated with @_; these are:

    cx->blk_sub.savearray  the caller's @_
    GvAV(PL_defgv)         the current sub's current @_
    cx->blk_sub.argarray   the current sub's original @_

Note that those last two can differ: if for example the sub does

    local *_ = [];

Each sub's arg array is created and stored in pad slot 0 at the time
the sub is created. When a sub is called, pp_entersub() does

    cx->blk_sub.argarray = PL_curpad[0]

The argarray field is used in two main places:

* at subroutine exit, to clear/abandon the sub's original @_;
* in pp_caller, to set @DB::args to the caller(n)'s args.

In the first case, the last few commits have arranged things so that at
that point, PL_curpad always points to the current pad of the sub being
exited from. So we can just use PL_curpad[0] rather than
cx->blk_sub.argarray.

In the second case (which is not performance critical), we can just use
    cx->blk_sub.cv
and
    cx->blk_sub.olddepth+1
to find the pad of that call frame's CV.

So we can eliminate the argarray field. This saves a write during
a sub call, and potentially reduces the size of the context struct.

It also makes the code marginally less confusing: there are now 3 arrays
pointed to from 3 places, rather than 3 arrays pointed to from 4 places.

The asserts added in the previous commit confirmed that using argarray
is the same as using PL_curpad[0]; They also confirmed that the current
PL_curpad matches the current CV at the current depth. Those latter
asserts are kept for now.

5 years agoassert that it's safe to remove CX.argarray field
David Mitchell [Wed, 1 Jul 2015 11:21:06 +0000 (12:21 +0100)]
assert that it's safe to remove CX.argarray field

This commit adds assertions that demonstrate that the next commit is safe.
See the description of that commit for details.

5 years agoUnwind save stack in sync with POPEVAL
David Mitchell [Wed, 1 Jul 2015 08:57:51 +0000 (09:57 +0100)]
Unwind save stack in sync with POPEVAL

During normal exit from an eval, both POPEVAL and LEAVE are done,
meaning that the context stack and save stack are unwound in lockstep.

However, during an exception, dounwind() does the POPEVAL but not the
LEAVE, leaving them temporarily out of step.

About a 2 years ago, with v5.19.3-139-g2537512, subs and formats were
changed so that the save stack popping was done in sync in dounwind.

This commit extends that to evals too.

I'm doing this principally so that PL_compad will always be restored at
the correct moment, which will shortly allow me to eliminate the argarray
field from the context struct.

NB: unlike POPSUB and POPFORMAT, I've added the LEAVE_SCOPE to dounwind
rather than directly adding to the POPEVAL macro - this is because the
various places that use POPEVAL typically use it earlier than the
comparable places with POPSUB, which means they aren't ready to pop the
save stack yet.

NNB: The LEAVE_SCOPE can't be extended to all context types in dounwind(),
only to sub-ish types - specifically the types that can be 'return'ed from.
This is because if you 'return' from a sub or eval, pp_return will
unwind all contexts down to the next sub or eval (discarding all the loops
etc that it escaping out of), but the args it is returning shouldn't
be prematurely freed.

5 years agopp_goto: skip restoring PL_comppad
David Mitchell [Wed, 1 Jul 2015 06:56:39 +0000 (07:56 +0100)]
pp_goto: skip restoring PL_comppad

Now that PL_comppad is saved in the context struct rather than on the save
stack, we can better control when and how it is restored. In the case of
pp_goto, there's no need to restore the caller's PL_comppad in the non-XS
case, since we will be immediately setting it to the new function's pad.

AS well as being slightly more efficient, this avoids restoring PL_comppad
too early, where if we subsequently croak we re-process the CXt_SUB block
in dounwind, but this time with the wrong pad.

In particular, by having always PL_curpad[0] == cx.argarray at sub exit
time, we can shortly eliminate the argarray field.

5 years agoundef *_; goto &f: update cx.argarray
David Mitchell [Tue, 30 Jun 2015 07:03:43 +0000 (08:03 +0100)]
undef *_; goto &f: update cx.argarray

In something like

    sub f { goto &g }

normally g's pad[0] is updated to hold the passed-across @_, and the
context block's argarray field is updated to point to g's @_ too.

However in the case of

    sub f { undef *_; goto &g }

cx.argarray isn't being updated. This is probably harmless (I couldn't
come up with a test case that fails), but for consistency, update it too.

This is mainly so that over the next few commits, this condition will come
to apply consistently:

    cx.argarray == PL_curpad[0]

and so the argarray field can be eliminated.