develooper Front page | perl.perl5.porters | Postings from August 2001

RE: [PATCH] Add a nextstate into empty blocks

Thread Previous | Thread Next
From:
Ilmari Karonen
Date:
August 19, 2001 09:27
Subject:
RE: [PATCH] Add a nextstate into empty blocks
Message ID:
Pine.SOL.3.96.1010819163840.8384B-100000@simpukka

On Sun, 19 Aug 2001, Arthur Bergman wrote:
> > 
> > The following patch fixes this by inserting a nextstate op into empty
> > blocks.  No harmful effects are observed, although two test cases
> > (op/sub_lval:31 and lib/warnings:255) fail because of minor changes in
> > error messages.  (The former is IMHO an improvement.  The latter is a
> > mystery to me, but doesn't seem a particularly serious issue.)
> 
> Could you please post the new error messages and why you want this
> behaviour to change?

Actually, I got the lib/warnings test failure fixed -- it turns out
calling newSTATEOP() inside end_block() was wiping out PL_copline, which
had just been set in perly.c.  I ended up saving and restoring the line
number, and also took a belt-and-suspenders approach by adding a sanity
check for the value of PL_copline.  Amended patch below.


As for the op/sub_lval test, it expects

  sub lv0 : lvalue { }
  lv0 = (2,3);

to die with "Can't return a readonly value from lvalue subroutine".  It
now dies with "Empty array returned from lvalue subroutine in scalar
context".  I looked into the code that generates this (pp_leavesublv)
but it's getting a bit over my head.  

In any case, the test suite is a mess and some of the code looks a bit
dubious too, from what I could understand of it.  Someone who knows more
about lvalue subs Really Ought To(TM) look into it and tidy it up some.

Meanwhile, I'd suggest just changing the test.


> What does the backword compatibility police say?

I was rather expecting to break some obscure test case relying on the
IMHO broken stack handling, but no such thing happens.  The original
behavior is undocumented, and apparently anyone who has stumbled onto it
has found it too confusing to actually use.  (For details on just how
confusing, see the comp.lang.perl.misc thread on this.)


> This behavour seems to go away when the debugger is turned on, seems
> like a optimization issue?

No, I don't really think so.  I'm not that familiar with the debugger,
but since the bug my patch fixes goes away when _anything_, even a stub
or just a label, is placed in the sub, I'd guess that the debugger is
introducing something into the block that is enough to make it non-empty
in this sense.


Here's the second version of the patch -- this one gives correct line
numbers in redefinition warnings:

--- op.c.orig	Sun Aug 19 02:23:46 2001
+++ op.c	Sun Aug 19 19:35:30 2001
@@ -2146,7 +2146,10 @@
 Perl_block_end(pTHX_ I32 floor, OP *seq)
 {
     int needblockscope = PL_hints & HINT_BLOCK_SCOPE;
-    OP* retval = scalarseq(seq);
+    line_t copline = PL_copline;
+    /* there should be a nextstate in every block */
+    OP* retval = seq ? scalarseq(seq) : newSTATEOP(0, Nullch, seq);
+    PL_copline = copline;  /* XXX newSTATEOP may reset PL_copline */
     LEAVE_SCOPE(floor);
     PL_pad_reset_pending = FALSE;
     PL_compiling.op_private = PL_hints;
@@ -4687,7 +4690,8 @@
 			&& (!const_sv || sv_cmp(cv_const_sv(cv), const_sv))))
 		{
 		    line_t oldline = CopLINE(PL_curcop);
-		    CopLINE_set(PL_curcop, PL_copline);
+		    if (PL_copline != NOLINE)
+			CopLINE_set(PL_curcop, PL_copline);
 		    Perl_warner(aTHX_ WARN_REDEFINE,
 			CvCONST(cv) ? "Constant subroutine %s redefined"
 				    : "Subroutine %s redefined", name);
@@ -5160,8 +5164,8 @@
     if ((cv = GvFORM(gv))) {
 	if (ckWARN(WARN_REDEFINE)) {
 	    line_t oldline = CopLINE(PL_curcop);
-
-	    CopLINE_set(PL_curcop, PL_copline);
+	    if (PL_copline != NOLINE)
+		CopLINE_set(PL_curcop, PL_copline);
 	    Perl_warner(aTHX_ WARN_REDEFINE, "Format %s redefined",name);
 	    CopLINE_set(PL_curcop, oldline);
 	}


-- 
# Ilmari Karonen -- http://www.sci.fi/~iltzu/
y/n/\n/,s/\d+/$"x$&/eg,print for qw'n4|9|21|3|n1\2||2|(_-<2_|4_`1|3\3_1\2_|3\3
-_)2_|n\__/\_,_|___/\__|2\__,_|_|1_|\___/\__|_|1_|\___|_|nn4_1\9|3|14|n4__/1-_
)2_|1|5\3_`1|2_|1!1/2-_)2_|n3_|1\___|_|2_|2_|1_|\__,_|\__|_i_\\\___|_|1)n45/n'



Thread Previous | 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