From root@mail.nethype.de

Created by root@mail.nethype.de

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>.

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

From @tsee

On 10/27/2013 10​:56 PM, root@​mail.nethype.de (via RT) wrote​:

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

The RT System itself - Status changed from 'new' to 'open'

From @bulk88

On Sun Oct 27 14​:56​:41 2013, root@​mail.nethype.de wrote​:

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

From schmorp@schmorp.de

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.

--
  The choice of a Deliantra, the free code+content MORPG
  -----==- _GNU_ http​://www.deliantra.net
  ----==-- _ generation
  ---==---(_)__ __ ____ __ Marc Lehmann
  --==---/ / _ \/ // /\ \/ / schmorp@​schmorp.de
  -=====/_/_//_/\_,_/ /_/\_\

From @khwilliamson

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-archive.perl.org/perl5/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.

From @khwilliamson

On 10/28/2013 02​:08 AM, Marc Lehmann wrote​:

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.

From @mauke

On 30.10.2013 06​:27, Karl Williamson wrote​:

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-archive.perl.org/perl5/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.

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.

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) +
1>. This is unlike C<sv_len>, which returns what you would expect.

--
Lukas Mai <plokinom@​gmail.com>

From @bulk88

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.

--
bulk88 ~ bulk88 at hotmail.com

From schmorp@schmorp.de

On Tue, Oct 29, 2013 at 11​:31​:10PM -0600, Karl Williamson <public@​khwilliamson.com> wrote​:

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
  -=====/_/_//_/\_,_/ /_/\_\

From @khwilliamson

On Wed Oct 30 02​:01​:24 2013, bulk88 wrote​:

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 b985ae6, 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

From @bulk88

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 b985ae6, 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 @khwilliamson

On 06/01/2014 01​:23 AM, bulk88 via RT wrote​:

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 b985ae6, 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.

From @khwilliamson

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

@khwilliamson - Status changed from 'open' to 'resolved'