Change 33727: Integrate:

Change 33727 by nicholas@entropy on 2008/04/22 16:07:01

	Integrate:
	[ 33726]
	Integrate:
	[ 33709]
	Add "eliminate incorrect line numbers in warnings" to perltodo, and
	a pair of TODO tests for the desired behaviour.
	
	[ 33710]
	Fix the line-number-in-elsif longstanding bug.
	This patch does two things :
	- newSTATEOP now nullifies the state op it
	  just created if OPf_SPECIAL is passed to it
	  in flags
	- the parser now inserts a nullified stateop
	  in the expression block of an elsif
	
	[ 33724]
	Note that Rafael did the key elsif() line number TODO, and we're not
	certain that solving the general case is worth it.

Affected files ...

... //depot/maint-5.8/perl/op.c#229 integrate
... //depot/maint-5.8/perl/perly.c#11 edit
... //depot/maint-5.8/perl/perly.y#11 integrate
... //depot/maint-5.8/perl/pod/perltodo.pod#43 integrate
... //depot/maint-5.8/perl/vms/perly_c.vms#7 edit

Differences ...

==== //depot/maint-5.8/perl/op.c#229 (text) ====
Index: perl/op.c
--- perl/op.c#228~33449~	2008-03-06 06:14:52.000000000 -0800
+++ perl/op.c	2008-04-22 09:07:01.000000000 -0700
@@ -3712,6 +3712,8 @@
 	}
     }
 
+    if (flags & OPf_SPECIAL)
+	op_null((OP*)cop);
     return prepend_elem(OP_LINESEQ, (OP*)cop, o);
 }
 

==== //depot/maint-5.8/perl/perly.c#11 (text) ====
Index: perl/perly.c
--- perl/perly.c#10~28154~	2006-05-10 11:31:34.000000000 -0700
+++ perl/perly.c	2008-04-22 09:07:01.000000000 -0700
@@ -1771,7 +1771,7 @@
 case 23:
 #line 229 "perly.y"
 { PL_copline = (line_t)yyvsp[-5].ival;
-			    yyval.opval = newCONDOP(0, yyvsp[-3].opval, scope(yyvsp[-1].opval), yyvsp[0].opval);
+			    yyval.opval = newCONDOP(0, newSTATEOP(OPf_SPECIAL,NULL,yyvsp[-3].opval), scope(yyvsp[-1].opval), yyvsp[0].opval);
 			    PL_hints |= HINT_BLOCK_SCOPE; }
 break;
 case 24:

==== //depot/maint-5.8/perl/perly.y#11 (text) ====
Index: perl/perly.y
--- perl/perly.y#10~28154~	2006-05-10 11:31:34.000000000 -0700
+++ perl/perly.y	2008-04-22 09:07:01.000000000 -0700
@@ -227,7 +227,7 @@
 			{ ($2)->op_flags |= OPf_PARENS; $$ = scope($2); }
 	|	ELSIF '(' mexpr ')' mblock else
 			{ PL_copline = (line_t)$1;
-			    $$ = newCONDOP(0, $3, scope($5), $6);
+			    $$ = newCONDOP(0, newSTATEOP(OPf_SPECIAL,NULL,$3), scope($5), $6);
 			    PL_hints |= HINT_BLOCK_SCOPE; }
 	;
 

==== //depot/maint-5.8/perl/pod/perltodo.pod#43 (text) ====
Index: perl/pod/perltodo.pod
--- perl/pod/perltodo.pod#42~33576~	2008-03-26 09:58:17.000000000 -0700
+++ perl/pod/perltodo.pod	2008-04-22 09:07:01.000000000 -0700
@@ -884,6 +884,65 @@
 hash key scalars. Under ithreads, something is undoing this work. See
 See http://www.xray.mpe.mpg.de/mailing-lists/perl5-porters/2007-09/msg00793.html
 
+=head2 eliminate incorrect line numbers in warnings
+
+This code
+
+    use warnings;
+    my $undef;
+    
+    if ($undef == 3) {
+    } elsif ($undef == 0) {
+    }
+
+used to produce this output:
+
+    Use of uninitialized value in numeric eq (==) at wrong.pl line 4.
+    Use of uninitialized value in numeric eq (==) at wrong.pl line 4.
+
+where the line of the second warning was misreported - it should be line 5.
+Rafael fixed this - the problem arose because there was no nextstate OP
+between the execution of the C<if> and the C<elsif>, hence C<PL_curcop> still
+reports that the currently executing line is line 4. The solution was to inject
+a nextstate OPs for each C<elsif>, although it turned out that the nextstate
+OP needed to be a nulled OP, rather than a live nextstate OP, else other line
+numbers became misreported. (Jenga!)
+
+The problem is more general than C<elsif> (although the C<elsif> case is the
+most common and the most confusing). Ideally this code
+
+    use warnings;
+    my $undef;
+    
+    my $a = $undef + 1;
+    my $b
+      = $undef
+      + 1;
+
+would produce this output
+
+    Use of uninitialized value $undef in addition (+) at wrong.pl line 4.
+    Use of uninitialized value $undef in addition (+) at wrong.pl line 7.
+
+(rather than lines 4 and 5), but this would seem to require every OP to carry
+(at least) line number information.
+
+What might work is to have an optional line number in memory just before the
+BASEOP structure, with a flag bit in the op to say whether it's present.
+Initially during compile every OP would carry its line number. Then add a late
+pass to the optimiser (potentially combined with L</repack the optree>) which
+looks at the two ops on every edge of the graph of the execution path. If
+the line number changes, flags the destination OP with this information.
+Once all paths are traced, replace every op with the flag with a
+nextstate-light op (that just updates C<PL_curcop>), which in turn then passes
+control on to the true op. All ops would then be replaced by variants that
+do not store the line number. (Which, logically, why it would work best in
+conjunction with L</repack the optree>, as that is already copying/reallocating
+all the OPs)
+
+(Although I should note that we're not certain that doing this for the general
+case is worth it)
+
 =head1 Big projects
 
 Tasks that will get your name mentioned in the description of the "Highlights

==== //depot/maint-5.8/perl/vms/perly_c.vms#7 (text) ====
Index: perl/vms/perly_c.vms
--- perl/vms/perly_c.vms#6~28154~	2006-05-10 11:31:34.000000000 -0700
+++ perl/vms/perly_c.vms	2008-04-22 09:07:01.000000000 -0700
@@ -1781,7 +1781,7 @@
 case 23:
 #line 229 "perly.y"
 { PL_copline = (line_t)yyvsp[-5].ival;
-			    yyval.opval = newCONDOP(0, yyvsp[-3].opval, scope(yyvsp[-1].opval), yyvsp[0].opval);
+			    yyval.opval = newCONDOP(0, newSTATEOP(OPf_SPECIAL,NULL,yyvsp[-3].opval), scope(yyvsp[-1].opval), yyvsp[0].opval);
 			    PL_hints |= HINT_BLOCK_SCOPE; }
 break;
 case 24:
End of Patch.

• Change 33727: Integrate: by Nicholas Clark