[perl #33962] readline returns one too many lines
Matt Diephouse
# Please include the string: [perl #33962]
# in the subject line of all future correspondence about this issue.
# <URL: https://rt.perl.org/rt3/Ticket/Display.html?id=33962 >
The readline opcode returns too many lines. Compare the following pir
with `wc -l`.
.sub main @MAIN
.param pmc args
.local pmc file
$S0 = args[1]
file = open $S0, "<"
$I0 = 0
LOOP:
unless file goto END
$S0 = readline file
$I0 += 1
goto LOOP
END:
print $I0
print "\n"
end
.end
harmony:~/Projects/parrot ezekiel$ parrot lines.pir README
154
harmony:~/Projects/parrot ezekiel$ wc -l README
153 README
--
matt diephouse
http://matt.diephouse.com
Leopold Toetsch
> The readline opcode returns too many lines. Compare the following pir
> with `wc -l`.
> unless file goto END
> $S0 = readline file
It needs (currently) a test for an empty string:
unless $S0 goto END
I didn't look at the code, if EOF is set correctly.
leo
Matt Diephouse
> Matt Diephouse <parrotbug...@parrotcode.org> wrote:
>
> > The readline opcode returns too many lines. Compare the following pir
> > with `wc -l`.
>
> > unless file goto END
> > $S0 = readline file
>
> It needs (currently) a test for an empty string:
>
> unless $S0 goto END
I already use that workaround in some of my code, but that sorta
defeats the purpose of testing the filehandle itself. The filehandle
should become false after it returns the last line of the file - not
the last line plus an empty string.
Leopold Toetsch
Looked again at that stuff. You are testing for EOF before the readline,
which implies that EOF should be set along with returning data. This
seems to be wrong.
So the correct way seems to be:
unless file goto END
LOOP:
$S0 = readline file
unless file goto END
$I0 += 1
goto LOOP
BTW: if you replace "$S0 = readline file" with "$S0 = read file, 1" in
your original example you get again the amount of chars + 1. So it's
consistent.
leo
Allison Randal via RT
> Matt Diephouse <mdd...@gmail.com> wrote:
> > I already use that workaround in some of my code, but that sorta
> > defeats the purpose of testing the filehandle itself. The filehandle
> > should become false after it returns the last line of the file - not
> > the last line plus an empty string.
>
> Looked again at that stuff. You are testing for EOF before the readline,
> which implies that EOF should be set along with returning data. This
> seems to be wrong.
>
> So the correct way seems to be:
>
> unless file goto END
> LOOP:
> $S0 = readline file
> unless file goto END
> $I0 += 1
> goto LOOP
Leo is right. Compare the equivalent Perl 5 code:
my $file;
open $file, @ARGV[0] or die "can't open file";
my $count = 0;
while ($file) {
my $string = readline $file;
print "Count is $count: $string";
$count++;
last if $string eq "";
}
print "\nTotal count: $count\n";
which prints out:
Count is 0: line 1
Count is 1: line 2
Count is 2: line 3
Count is 3:
Total count: 4
There's no way the filehandle can know that it has reached the last line
of the file before it reads the last line of the file. The extra "line"
isn't a problem with readline, it's simply a result of the loop control
structure you've chosen. Perl 5's
while (<FILEHANDLE>) {...}
construct isn't a simple while loop on a file handle, it's syntactic
sugar for a filehandle iterator. Which reminds me that we need to either
make sure that the default Iterator object can be applied to ParrotIO
objects, or provide one that can (or perhaps an Iterator role). So, I'm
leaving this ticket open and attached to the I/O PDD ticket.
Allison