develooper Front page | perl.perl5.porters | Postings from March 2003

(?{ .. })'s and lexicals: A fix ([perl #9010], [perl #8817], [perl #8030], etc)

Thread Next
From:
Enache Adrian
Date:
March 15, 2003 21:55
Subject:
(?{ .. })'s and lexicals: A fix ([perl #9010], [perl #8817], [perl #8030], etc)
Message ID:
20030316060018.GA2596@ratsnest.hole
$ perl -le 'sub a { my $p; /(?{ print ++$p })/ } a, a, a' 
1
2
3

That should print '1' each time.
This seems to be a very old bug ( see the subject line ).

It happens because the (?{ .. }) block uses another pad than the block
it's in: so '$p' will have a refcount of 2 because it's used in two
different pads ( see S_pad_findlex() in pad.c - :752 ).

When the 'a' returns for the first time, the $p lexical will be
'abandonned' instead of 'cleared' - because its recount is >= 1.
( see scope.c:875 - & run the above script with -DXv ).
So the (?{ CODE }) will stick to the first instance of $p.

The PAD_SAVE_LOCAL, PAD_RESTORE_LOCAL macros at the EVAL: case
in S_regmatch() aren't at all useful: they don't free the lexicals
in the pad, they only switch pads.

The obvious solution is to let (?{ .. }) blocks use the pad of the
block they're in; that will also have the advantage of removing that
pad juggling and make things a little bit faster. 

This doesn't mean of course that
  { my $p=1; /(?{ my $p = 2 })/; print $p }
will work differently.
(?{ .. }) blocks will still be properly scoped.

The patch draft below passes all tests and works equally well
with precompiled && run-time compiled regexp's.

Any issues I missed ?

Note:
I think the PAD_SAVE_* macros should be removed completely: they're
flawed and anybody trying to use them will eventually fall in the
same trap.

Regards
Adi

--------------------------------------------------------------------
--- /arc/perl-current/regexec.c	2003-03-09 14:07:31.000000000 +0200
+++ regexec.c	2003-03-16 07:26:12.000000000 +0200
@@ -2832,13 +2832,11 @@ S_regmatch(pTHX_ regnode *prog)
 	    dSP;
 	    OP_4tree *oop = PL_op;
 	    COP *ocurcop = PL_curcop;
-	    PAD *old_comppad;
 	    SV *ret;
 	
 	    n = ARG(scan);
 	    PL_op = (OP_4tree*)PL_regdata->data[n];
 	    DEBUG_r( PerlIO_printf(Perl_debug_log, "  re_eval 0x%"UVxf"\n", PTR2UV(PL_op)) );
-	    PAD_SAVE_LOCAL(old_comppad, (PAD*)PL_regdata->data[n + 2]);
 	    PL_regendp[0] = PL_reg_magic->mg_len = locinput - PL_bostr;
 
 	    {
@@ -2854,7 +2852,6 @@ S_regmatch(pTHX_ regnode *prog)
 	    }
 
 	    PL_op = oop;
-	    PAD_RESTORE_LOCAL(old_comppad);
 	    PL_curcop = ocurcop;
 	    if (logical) {
 		if (logical == 2) {	/* Postponed subexpression. */
--- /arc/perl-current/pp_ctl.c	2003-03-05 01:37:58.000000000 +0200
+++ pp_ctl.c	2003-03-16 05:42:59.000000000 +0200
@@ -2810,10 +2815,10 @@ S_doeval(pTHX_ int gimme, OP** startop, 
 
     /* set up a scratch pad */
 
-    CvPADLIST(PL_compcv) = pad_new(padnew_SAVE);
-
-
-    SAVEMORTALIZESV(PL_compcv);	/* must remain until end of current statement */
+    if (!startop) {
+	CvPADLIST(PL_compcv) = pad_new(padnew_SAVE);
+	SAVEMORTALIZESV(PL_compcv);
+    }
 
     /* make sure we compile in the right package */
 


Thread Next


nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About