Skip Menu |
Report information
Id: 120386
Status: resolved
Priority: 0/
Queue: perl5

Owner: khw <khw [at] cpan.org>
Requestors: root [at] mail.nethype.de
Cc:
AdminCc:

Operating System: (no value)
PatchStatus: (no value)
Severity: (no value)
Type: (no value)
Perl Version: (no value)
Fixed In: (no value)



From: root [...] mail.nethype.de
Date: Sun, 27 Oct 2013 22:56:16 +0100
CC: perl-binary [...] plan9.de
Subject: av_len documentation
To: perlbug [...] perl.org
Download (untitled) / with headers
text/plain 4.7k
This is a bug report for perl from root@mail.nethype.de, generated with the help of perlbug 1.39 running under perl 5.18.1. ----------------------------------------------------------------- [Please describe your issue here] The av_len documentation for 5.18.1 says: Note that the return value is +1 what its name implies it returns; and hence differs in meaning from what the similarly named "sv_len" returns. That seems wrong. Shouldn't it be: Note that the return value +1 is what its name implies it returns Or maybe even less confusing: Note that, unlike the name implies, it returns the highest index in the array, so to get the size of the array you need to use C<av_len (av) + 1>. [Please do not change anything below this line] ----------------------------------------------------------------- --- Flags: category=core severity=low --- Site configuration information for perl 5.18.1: Configured by Marc Lehmann at Mon Oct 21 09:27:08 CEST 2013. Summary of my perl5 (revision 5 version 18 subversion 1) configuration: Platform: osname=linux, osvers=3.10-3-amd64, archname=x86_64-linux uname='linux cerebro 3.10-3-amd64 #1 smp debian 3.10.11-1 (2013-09-10) x86_64 gnulinux ' config_args='-Duselargefiles -Duse64bitint -Dusemymalloc=n -Dstatic_ext=Fcntl -Dcc=ccache gcc -Dccflags=-DPERL_DISABLE_PMC -DPERL_ARENA_SIZE=16376 -USITEARCH_EXP -USITELIB_EXP -UARCHLIB_EXP -D_GNU_SOURCE -I/opt/include -ggdb -gdwarf-2 -g3 -Doptimize=-O6 -fno-asynchronous-unwind-tables -fno-strict-aliasing -Dcccdlflags=-fPIC -Dldflags=-L/opt/perl/lib -L/opt/lib -Dlibs=-ldl -lm -lcrypt -Dprefix=/opt/perl -Dprivlib=/opt/perl/lib/perl5 -Darchlib=/opt/perl/lib/perl5 -Uusevendorprefix -Dsiteprefix=/opt/perl -Dsitelib=/opt/perl/lib/perl5 -Dsitearch=/opt/perl/lib/perl5 -Dsitebin=/opt/perl/bin -Dman1dir=/opt/perl/man/man1 -Dman3dir=/opt/perl/man/man3 -Dsiteman1dir=/opt/perl/man/man1 -Dsiteman3dir=/opt/perl/man/man3 -Dman1ext=1 -Dman3ext=3 -Dpager=/usr/bin/less -Uafs -Uusesfio -Uusenm -Uuseshrplib -Ud_dosuid -Dusethreads=undef -Duse5005threads=undef -Duseithreads=undef -Dusemultiplicity=undef -Demail=perl-binary@plan9.de -Dcf_email=perl-binary@plan9.de -Dcf_by=Marc Lehmann -Dlocincpth=/opt/perl/include /opt/include -Dmyhostname=localhost -Dmultiarch=undef -Dbin=/opt/perl/bin -Dxxxusedevel -DxxxDEBUGGING -Dxxxuse_debugging_perl -Dxxxuse_debugmalloc -dEs' hint=recommended, useposix=true, d_sigaction=define useithreads=undef, usemultiplicity=undef useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef use64bitint=define, use64bitall=define, uselongdouble=undef usemymalloc=n, bincompat5005=undef Compiler: cc='ccache gcc', ccflags ='-DPERL_DISABLE_PMC -DPERL_ARENA_SIZE=16376 -USITEARCH_EXP -USITELIB_EXP -UARCHLIB_EXP -D_GNU_SOURCE -I/opt/include -ggdb -gdwarf-2 -g3 -fno-strict-aliasing -pipe -I/opt/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64', optimize='-O6 -fno-asynchronous-unwind-tables -fno-strict-aliasing', cppflags='-DPERL_DISABLE_PMC -DPERL_ARENA_SIZE=16376 -USITEARCH_EXP -USITELIB_EXP -UARCHLIB_EXP -D_GNU_SOURCE -I/opt/include -ggdb -gdwarf-2 -g3 -fno-strict-aliasing -pipe -I/opt/include' ccversion='', gccversion='4.7.2', gccosandvers='' intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678 d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16 ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8 alignbytes=8, prototype=define Linker and Libraries: ld='ccache gcc', ldflags ='-L/opt/perl/lib -L/opt/lib -L/usr/local/lib' libpth=/usr/local/lib /lib/x86_64-linux-gnu /lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib /usr/lib libs=-ldl -lm -lcrypt perllibs=-ldl -lm -lcrypt libc=, so=so, useshrplib=false, libperl=libperl.a gnulibc_version='2.17' Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E' cccdlflags='-fPIC', lddlflags='-shared -O6 -fno-asynchronous-unwind-tables -fno-strict-aliasing -L/opt/perl/lib -L/opt/lib -L/usr/local/lib' Locally applied patches: --- @INC for perl 5.18.1: /root/src/sex /root/pserv/lib/perl5 /opt/perl/lib/perl5 /opt/perl/lib/perl5 . --- Environment for perl 5.18.1: HOME=/root LANG (unset) LANGUAGE (unset) LC_CTYPE=en_US.UTF-8 LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/root/s2:/root/s:/opt/bin:/opt/sbin:/bin:/sbin:/usr/bin:/usr/sbin:/usr/X11/bin:/usr/games:/usr/local/bin:/usr/local/sbin:/root/pserv:. PERL5LIB=/root/src/sex:/root/pserv/lib/perl5 PERL5_CPANPLUS_CONFIG=/root/.cpanplus/config PERLDB_OPTS=ornaments=0 PERL_ANYEVENT_DBI_TESTS=1 PERL_ANYEVENT_LOOP_TESTS=1 PERL_ANYEVENT_NET_TESTS=1 PERL_BADLANG (unset) PERL_UNICODE=E SHELL=/bin/bash
Subject: Re: [perl #120386] av_len documentation
To: perl5-porters [...] perl.org
Date: Mon, 28 Oct 2013 06:37:36 +0100
From: Steffen Mueller <smueller [...] cpan.org>
Download (untitled) / with headers
text/plain 307b
On 10/27/2013 10:56 PM, root@mail.nethype.de (via RT) wrote: Show quoted text
> Or maybe even less confusing: > > Note that, unlike the name implies, it returns the highest index in the array, so to get the size > of the array you need to use C<av_len (av) + 1>.
That sounds like and improvement to me. --Steffen
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.1k
On Sun Oct 27 14:56:41 2013, root@mail.nethype.de wrote: Show quoted text
> > This is a bug report for perl from root@mail.nethype.de, > generated with the help of perlbug 1.39 running under perl 5.18.1. > > > ----------------------------------------------------------------- > [Please describe your issue here] > > The av_len documentation for 5.18.1 says: > > Note that the return value is +1 what its name implies it returns; and > hence differs in meaning from what the similarly named "sv_len" > returns. > > That seems wrong. Shouldn't it be: > > Note that the return value +1 is what its name implies it returns > > Or maybe even less confusing: > > Note that, unlike the name implies, it returns the highest index in > the array, so to get the size > of the array you need to use C<av_len (av) + 1>. > > > [Please do not change anything below this line] > -----------------------------------------------------------------
Didn't KHW already fix this issue in these couple of commits earlier this year starting at http://perl5.git.perl.org/perl.git/23aa77bc9fa488ace3ef1089104e999c23821171 ? -- bulk88 ~ bulk88 at hotmail.com
Subject: Re: [perl #120386] av_len documentation
Date: Mon, 28 Oct 2013 09:08:51 +0100
To: bulk88 via RT <perlbug-followup [...] perl.org>
From: Marc Lehmann <schmorp [...] schmorp.de>
Download (untitled) / with headers
text/plain 915b
On Sun, Oct 27, 2013 at 10:48:07PM -0700, bulk88 via RT <perlbug-followup@perl.org> wrote: Show quoted text
> > Didn't KHW already fix this issue in these couple of commits earlier this year starting at http://perl5.git.perl.org/perl.git/23aa77bc9fa488ace3ef1089104e999c23821171 ?
If the idea is to deprecate it's use (which these changes kind of seem to want), why not simply document that, by explicitly asking not to use it in new code for example? Of course, for module authors it is always helpful to also have the information of which perl version introduced the alternative av_top_index and av_tindex. -- The choice of a Deliantra, the free code+content MORPG -----==- _GNU_ http://www.deliantra.net ----==-- _ generation ---==---(_)__ __ ____ __ Marc Lehmann --==---/ / _ \/ // /\ \/ / schmorp@schmorp.de -=====/_/_//_/\_,_/ /_/\_\
To: perl5-porters [...] perl.org
Subject: Re: [perl #120386] av_len documentation
From: Karl Williamson <public [...] khwilliamson.com>
Date: Tue, 29 Oct 2013 23:27:01 -0600
Download (untitled) / with headers
text/plain 1.6k
On 10/27/2013 03:56 PM, root@mail.nethype.de (via RT) wrote: Show quoted text
> # New Ticket Created by root@mail.nethype.de > # Please include the string: [perl #120386] > # in the subject line of all future correspondence about this issue. > # <URL: https://rt.perl.org/Ticket/Display.html?id=120386 > > > > > This is a bug report for perl from root@mail.nethype.de, > generated with the help of perlbug 1.39 running under perl 5.18.1. > > > ----------------------------------------------------------------- > [Please describe your issue here] > > The av_len documentation for 5.18.1 says: > > Note that the return value is +1 what its name implies it returns; and > hence differs in meaning from what the similarly named "sv_len" returns. > > That seems wrong. Shouldn't it be: > > Note that the return value +1 is what its name implies it returns > > Or maybe even less confusing: > > Note that, unlike the name implies, it returns the highest index in the array, so to get the size > of the array you need to use C<av_len (av) + 1>. > >
I wrote the original text. I realize that I do not have the talent to explain things easily, so I often look at what I've written and think of a better way to say it, or appreciate other people cleaning it up. But this time, I don't see anything wrong with the original. sv_len of a length 1 string will return 1. av_len of a length 1 array will return 0. I think it is worth pointing out that their behaviors do not correspond. In your first proposal, the word 'is' is missing, so it is not correct grammatically. I like your second proposal, but again, I think it useful to contrast this behavior with sv_len.
To: Marc Lehmann <schmorp [...] schmorp.de>, bulk88 via RT <perlbug-followup [...] perl.org>
Subject: Re: [perl #120386] av_len documentation
Date: Tue, 29 Oct 2013 23:31:10 -0600
From: Karl Williamson <public [...] khwilliamson.com>
Download (untitled) / with headers
text/plain 1.1k
On 10/28/2013 02:08 AM, Marc Lehmann wrote: Show quoted text
> On Sun, Oct 27, 2013 at 10:48:07PM -0700, bulk88 via RT <perlbug-followup@perl.org> wrote:
>> >> Didn't KHW already fix this issue in these couple of commits earlier this year starting at http://perl5.git.perl.org/perl.git/23aa77bc9fa488ace3ef1089104e999c23821171 ?
> > If the idea is to deprecate it's use (which these changes kind of seem > to want), why not simply document that, by explicitly asking not to use > it in new code for example? Of course, for module authors it is always > helpful to also have the information of which perl version introduced the > alternative av_top_index and av_tindex. >
I thought it wrong to deprecate this, because I imagine that this is used all over the place, and it seems like a lot of, when you get down to it, unnecessary work to change them all (though doing so might lead to bug fixes as people discover that the code operates based on the function's old name rather than what it actually does). I wrote a patch to remove all uses of av_len from the core, but I never pushed it, thinking it might be too controversial. I could rescue it from bit rot if people thought it were a good idea.
Date: Wed, 30 Oct 2013 09:43:14 +0100
From: Lukas Mai <plokinom [...] gmail.com>
To: perl5-porters [...] perl.org
Subject: Re: [perl #120386] av_len documentation
Download (untitled) / with headers
text/plain 2.6k
On 30.10.2013 06:27, Karl Williamson wrote: Show quoted text
> On 10/27/2013 03:56 PM, root@mail.nethype.de (via RT) wrote:
>> # New Ticket Created by root@mail.nethype.de >> # Please include the string: [perl #120386] >> # in the subject line of all future correspondence about this issue. >> # <URL: https://rt.perl.org/Ticket/Display.html?id=120386 > >> >> >> >> This is a bug report for perl from root@mail.nethype.de, >> generated with the help of perlbug 1.39 running under perl 5.18.1. >> >> >> ----------------------------------------------------------------- >> [Please describe your issue here] >> >> The av_len documentation for 5.18.1 says: >> >> Note that the return value is +1 what its name implies it returns; >> and >> hence differs in meaning from what the similarly named "sv_len" >> returns. >> >> That seems wrong. Shouldn't it be: >> >> Note that the return value +1 is what its name implies it returns >> >> Or maybe even less confusing: >> >> Note that, unlike the name implies, it returns the highest index >> in the array, so to get the size >> of the array you need to use C<av_len (av) + 1>. >> >>
> > I wrote the original text. I realize that I do not have the talent to > explain things easily, so I often look at what I've written and think of > a better way to say it, or appreciate other people cleaning it up. But > this time, I don't see anything wrong with the original. sv_len of a > length 1 string will return 1. av_len of a length 1 array will return > 0. I think it is worth pointing out that their behaviors do not > correspond.
"Note that the return value is +1 what its name implies it returns." Its name implies that it returns 5 for a length 5 array. The documentation says its return value R is that plus one; i.e. R = 5 + 1, thus R = 6. That's the bug. Show quoted text
> In your first proposal, the word 'is' is missing, so it is not correct > grammatically.
No, it's not. "Note that the return value +1 is what its name implies it returns." -------------------------------^^ (I hope you have a fixed-width font) Its name implies it returns 5 for a length 5 array. The sentence says that the actual return value R, plus one, equals that; i.e. R + 1 == 5. By algebra we get R = 4, so the sentence is correct but weirdly worded. Show quoted text
> I like your second proposal, but again, I think it useful to contrast > this behavior with sv_len.
Sure, but that seems orthogonal to the problem at hand. How about this addition? Note that, unlike the name implies, it returns the highest index in the array, so to get the size of the array you need to use C<av_len(av) + Show quoted text
1>. This is unlike C<sv_len>, which returns what you would expect.
-- Lukas Mai <plokinom@gmail.com>
RT-Send-CC: perl5-porters [...] perl.org
On Tue Oct 29 22:27:41 2013, public@khwilliamson.com wrote: Show quoted text
> > I wrote the original text. I realize that I do not have the talent to > explain things easily, so I often look at what I've written and think > of > a better way to say it, or appreciate other people cleaning it up. > But > this time, I don't see anything wrong with the original. sv_len of a > length 1 string will return 1. av_len of a length 1 array will return > 0. I think it is worth pointing out that their behaviors do not > correspond. > > In your first proposal, the word 'is' is missing, so it is not correct > grammatically. > > I like your second proposal, but again, I think it useful to contrast > this behavior with sv_len.
Since av_len is av_top_index, they should share the same docs. I like the "The Perl equivalent for this is C<$#myarray>." in av_top_index and I'd like to see that in av_len. $# (or directly what the C function means in Perl) is the fastest way to realize what the C function does. The opposite of that is C<scalar(@myarray)>, which is what av_len does NOT do. -- bulk88 ~ bulk88 at hotmail.com
Subject: Re: [perl #120386] av_len documentation
To: bulk88 via RT <perlbug-followup [...] perl.org>
From: Marc Lehmann <schmorp [...] schmorp.de>
Date: Wed, 30 Oct 2013 10:48:04 +0100
Download (untitled) / with headers
text/plain 1.2k
On Tue, Oct 29, 2013 at 11:31:10PM -0600, Karl Williamson <public@khwilliamson.com> wrote: Show quoted text
> I thought it wrong to deprecate this, because I imagine that this is > used all over the place, and it seems like a lot of, when you get > down to it, unnecessary work to change them all (though doing so > might lead to bug fixes as people discover that the code operates > based on the function's old name rather than what it actually does).
It's only my opinion, but introducing two extra names without deprecating (not: removing!) the original one does more harm than good. There are now three av_len functions, and if av_len isn't deprecated, I will never use the longer names. The longer names cause more work to type and more work to think, because each time I encounter an av_top_index I have to think "ah, that's av_len, I hope". (I read a lot more code than I write, and I think thats quite typical for a coder). I agree that if av_len isn't deprecated, all three should share the same documentation. -- The choice of a Deliantra, the free code+content MORPG -----==- _GNU_ http://www.deliantra.net ----==-- _ generation ---==---(_)__ __ ____ __ Marc Lehmann --==---/ / _ \/ // /\ \/ / schmorp@schmorp.de -=====/_/_//_/\_,_/ /_/\_\
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.3k
On Wed Oct 30 02:01:24 2013, bulk88 wrote: Show quoted text
> On Tue Oct 29 22:27:41 2013, public@khwilliamson.com wrote:
> > > > I wrote the original text. I realize that I do not have the talent > > to > > explain things easily, so I often look at what I've written and think > > of > > a better way to say it, or appreciate other people cleaning it up. > > But > > this time, I don't see anything wrong with the original. sv_len of a > > length 1 string will return 1. av_len of a length 1 array will > > return > > 0. I think it is worth pointing out that their behaviors do not > > correspond. > > > > In your first proposal, the word 'is' is missing, so it is not > > correct > > grammatically. > > > > I like your second proposal, but again, I think it useful to contrast > > this behavior with sv_len.
> > Since av_len is av_top_index, they should share the same docs. I like > the "The Perl equivalent for this is C<$#myarray>." in av_top_index > and I'd like to see that in av_len. $# (or directly what the C > function means in Perl) is the fastest way to realize what the C > function does. The opposite of that is C<scalar(@myarray)>, which is > what av_len does NOT do.
I pushed commit b985ae615210d15bfa67bddb2118de1c02c21935, attached, that I believe incorporates the suggested changes. I've taken this ticket, and if I don't hear otherwise, will close it after a month -- Karl Williamson
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 817b
On Sat May 31 18:19:20 2014, khw wrote: Show quoted text
> On Wed Oct 30 02:01:24 2013, bulk88 wrote:
> > > > Since av_len is av_top_index, they should share the same docs. I like > > the "The Perl equivalent for this is C<$#myarray>." in av_top_index > > and I'd like to see that in av_len. $# (or directly what the C > > function means in Perl) is the fastest way to realize what the C > > function does. The opposite of that is C<scalar(@myarray)>, which is > > what av_len does NOT do.
> > I pushed commit b985ae615210d15bfa67bddb2118de1c02c21935, attached, > that I believe incorporates the suggested changes. I've taken this > ticket, and if I don't hear otherwise, will close it after a month
The commit above doesnt mention C<$#myarray> at all. Also nothing was attached to your RT post. -- bulk88 ~ bulk88 at hotmail.com
From: Karl Williamson <public [...] khwilliamson.com>
CC: perl5-porters [...] perl.org
Subject: Re: [perl #120386] av_len documentation
To: perlbug-followup [...] perl.org
Date: Sun, 01 Jun 2014 12:00:30 -0600
Download (untitled) / with headers
text/plain 1.4k
On 06/01/2014 01:23 AM, bulk88 via RT wrote: Show quoted text
> On Sat May 31 18:19:20 2014, khw wrote:
>> On Wed Oct 30 02:01:24 2013, bulk88 wrote:
>>> >>> Since av_len is av_top_index, they should share the same docs. I like >>> the "The Perl equivalent for this is C<$#myarray>." in av_top_index >>> and I'd like to see that in av_len. $# (or directly what the C >>> function means in Perl) is the fastest way to realize what the C >>> function does. The opposite of that is C<scalar(@myarray)>, which is >>> what av_len does NOT do.
>> >> I pushed commit b985ae615210d15bfa67bddb2118de1c02c21935, attached, >> that I believe incorporates the suggested changes. I've taken this >> ticket, and if I don't hear otherwise, will close it after a month
> > The commit above doesnt mention C<$#myarray> at all. Also nothing was attached to your RT post. >
Sorry, I forgot to attach, but it turns out that it wasn't so helpful anyway. It's better to look at the final pod. People wanted the documentation to be in terms of just one of the synonyms, and so I chose what I think is the preferred long form, and created links to it from the other synonyms. The current patch just clarified the entry for av_len(). The long form, av_top_index(), had already been changed to incorporate your suggestion, and is in v5.20 this way: "The Perl equivalent for this is C<$#myarray>." I would entertain a patch to add something like this text to the av_len entry, if you desire.
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 134b
I still would entertain a patch as mentioned above. But I don't think there is any need to keep this ticket open -- Karl Williamson


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

For issues related to this RT instance (aka "perlbug"), please contact perlbug-admin at perl.org