Why does Perl provide a misleading line number in this warning message?

Question

I have isolated a case where Perl provides a very misleading line number in a warning message. I tested the below in Strawberry Perl 5.16.3.

use strict;
use warnings;

my $choice = 0;

while ($choice == 0){

    #This is not numeric
    $choice = '5,6,7';

    if ($choice eq '-4'){
        print "This isn't going to happen\n";
    }
}

When you run this, you will get the warning message Argument "5,6,7" isn't numeric in numeric eq (==) at example.pl line 11. But line 11 corresponds to the line if ($choice eq '-4'){ which cannot possibly cause this warning message because it does not contain a numeric comparison.

It seems what's actually happening is that Perl advances to the next comparison, while ($choice == 0){, but the line counter used for the warning message does not advance.

What makes this particular case worse is that, since the "bad" comparison is the loop condition, it is actually far away from the provided line. In my (pre-simplification) script, it was hundreds of lines away from the provided line number.

Is this a bug or just an unfortunate limitation of the parser?

Answer 1

This isn't a solution so much as an avenue for for a solution if someone cared to work on it.

It would be expensive to store the location of each operator instance. As a compromise,

Recent work I have been doing in deparsing at runtime reduces the need to add additional locations in the tree.

For this example with Devel::Trepan::Deparse here is what you get:

(trepanpl): disasm
Package Main
------------
    #4: my $choice = 0;
        COP (0x195a0b0) dbstate 
        BINOP (0x195a110) sassign 
=>          SVOP (0x195a158) const  IV (0x1953f68) 0 
            OP (0x195a198) padsv [1] 
    #6: while ($choice == 0){
        COP (0x1b5b070) dbstate 
        BINOP (0x1b5b0d0) leaveloop 
            LOOP (0x1b5b118) enterloop 
            UNOP (0x1b5b178) null 
                LOGOP (0x1b5b1b8) and 
                    BINOP (0x195a850) eq <<< This is where the error is
                        OP (0x195a000) padsv [1] 
                        SVOP (0x195a898) const  IV (0x1953fb0) 0 
                    LISTOP (0x195a6e8) lineseq 
    #9:     $choice = '5,6,7';
            COP (0x2459530) dbstate 
            BINOP (0x2459590) sassign 
                SVOP (0x24595d8) const  PV (0x2da5ae0) "5,6,7" 
                OP (0x21e8618) padsv [1] 
... 
(trepanpl): deparse 0x195a850
binary operator ==, eq    
while ($choice == 0)  {
       ------------

And if you put in other offsets you get other choices. For example

(trepanpl): deparse 0x195a000
not my, padsv 
while ($choice == 0)  {
       -------
(trepanpl): deparse 0x21e8618
not my, padsv 
$choice = '5,6,7';
-------

Notice that you not only get the text of the line but also where in the line there was a problem. If the code had been

while ($choice == 0 && $i_feel_pretty == 3) 

the deparsing would indicate which of the two conditions had the problem, even though they are both on the same line.

So what's needed is to get to OP address when there is a warning or an error and then call B::DeparseTree to report the location and context.

Currently Perl provides $SIG{__WARN__} and $SIG{__DIE__}, but the value of PL_op is not available at the time the handler is called, even if the handler is an XS sub. PL_op gets saved, then set to a faked up OP_ENTERSUBa for the purposes of calling the handler.

However there is hope that addressing this can be done quicker than fixing this bug which has existed for 16 years, and that I doubt will be addressed anytime soon. See perl #133239.

Finally I'll say that right now B::DeparseTree doesn't go back to 5.16 and the code in general needs more love than I can give it by myself.

Answer 2

Apparently, starting with Perl 5.008, the interpreter began omitting an opcode that let the interpreter know the current line number.

Using the minimalist script:

while ($choice == 0) {
    $choice = '5,6,7';
}

We get this output from Perl 5.6 and B::Concise:

$ perl506 -MO=Concise badwarnline.pl
f  <@> leave[t1] vKP/REFC ->(end)
1     <0> enter ->2
2     <;> nextstate(main 2 badwarnline.pl:1) v ->3
e     <2> leaveloop vK/2 ->f
3        <{> enterloop(next->8 last->e redo->4) v ->a
-        <1> null vK/1 ->e
d           <|> and(other->4) vK/1 ->e
c              <2> eq sK/2 ->d
-                 <1> ex-rv2sv sK/1 ->b
a                    <$> gvsv(*choice) s ->b
b                 <$> const(IV 0) s ->c
-              <@> lineseq vKP ->-
4                 <;> nextstate(main 1 badwarnline.pl:2) v ->5
7                 <2> sassign vKS/2 ->8
5                    <$> const(PV "5,6,7") s ->6
-                    <1> ex-rv2sv sKRM*/1 ->7
6                       <$> gvsv(*choice) s ->7
8                 <0> unstack v ->9
9                 <;> nextstate(main 2 badwarnline.pl:1) v ->a

and this output from Perl v5.008:

$ perl508 -MO=Concise badwarnline.pl
e  <@> leave[1 ref] vKP/REFC ->(end)
1     <0> enter ->2
2     <;> nextstate(main 2 badwarnline.pl:1) v ->3
d     <2> leaveloop vK/2 ->e
3        <{> enterloop(next->8 last->d redo->4) v ->9
-        <1> null vK/1 ->d
c           <|> and(other->4) vK/1 ->d
b              <2> eq sK/2 ->c
-                 <1> ex-rv2sv sK/1 ->a
9                    <#> gvsv[*choice] s ->a
a                 <$> const[IV 0] s ->b
-              <@> lineseq vKP ->-
4                 <;> nextstate(main 1 badwarnline.pl:2) v ->5
7                 <2> sassign vKS/2 ->8
5                    <$> const[PV "5,6,7"] s ->6
-                    <1> ex-rv2sv sKRM*/1 ->7
6                       <#> gvsv[*choice] s ->7
8                 <0> unstack v ->9

The main difference between these outputs is the last line produced by 5.006:

9                 <;> nextstate(main 2 badwarnline.pl:1) v ->a

which is omitted in the 5.008 output.

Answer 3

It would be expensive to store the location of each operator instance. As a compromise, Perl only tracks the location of statements. It does so by adding a location-setting opcode at the start of every statement. The if statement is the last statement to be started before $choice == 0 is performed, so the warning is reported as coming from that line.

$ perl -MO=Concise,-exec a.pl
1  <0> enter
2  <;> nextstate(main 3 a.pl:4) v:*,&,{,x*,x&,x$,$
3  <$> const[IV 0] s
4  <0> padsv[$choice:3,12] sRM*/LVINTRO
5  <2> sassign vKS/2
6  <;> nextstate(main 4 a.pl:6) v:*,&,{,x*,x&,x$,$           <--- Location set to line 6
7  <{> enterloop(next->k last->p redo->8) v                  <--- Start of while loop
l  <0> padsv[$choice:3,12] s                                 <--- $choice == 0
m  <$> const[IV 0] s
n  <2> eq sK/2
o  <|> and(other->8) vK/1
8      <;> nextstate(main 6 a.pl:9) v:*,&,x*,x&,x$,$
9      <$> const[PV "5,6,7"] s
a      <0> padsv[$choice:3,12] sRM*
b      <2> sassign vKS/2
c      <;> nextstate(main 6 a.pl:11) v:*,&,x*,x&,x$,$        <--- Location set to line 11
d      <0> padsv[$choice:3,12] s                             <--- Start of if statement
e      <$> const[PV "-4"] s
f      <2> seq sK/2
g      <|> and(other->h) vK/1
h          <0> pushmark s
i          <$> const[PV "This isn't going to happen\n"] s
j          <@> print vK
k      <0> unstack v
           goto l                                            <--- Jump back to loop expr
p  <2> leaveloop vKP/2
q  <@> leave[1 ref] vKP/REFC
a.pl syntax OK

This is a known limitation. I don't know why they don't simply put a nextstate op in the loop expression.