[perl #118121] 'foreach' return value should be documented

[l . mai @ web . de]

# New Ticket Created by l....@web.de
# Please include the string: [perl #118121]
# in the subject line of all future correspondence about this issue.
# <URL: https://rt.perl.org:443/rt3/Ticket/Display.html?id=118121 >



This is a bug report for perl from l....@web.de,
generated with the help of perlbug 1.39 running under perl 5.16.3.


-----------------------------------------------------------------
[Please describe your issue here]

As far as I can see 'perldoc perlsyn' doesn't document the return value of
'foreach'. I think this should be changed, if only to say that you shouldn't
rely on the exact value.

Experiments suggest the behavior is as follows:

do { foreach (...) { ... } } evaluates to &PL_sv_no, unless you exit the loop
via 'last', in which case it evaluates to the empty list (== no return value).

(Personally I'd prefer if 'foreach' returned () always but changing that may
break existing code (?).)

What do you think about documenting the return value as "unspecified" and maybe
describing the current behavior?

[Please do not change anything below this line]
-----------------------------------------------------------------
---
Flags:
category=docs
severity=low
---
This perlbug was built using Perl 5.12.1 - Thu Jun 3 20:09:15 CEST 2010
It is being executed now by Perl 5.16.3 - Sun Mar 24 20:23:51 CET 2013.

Site configuration information for perl 5.16.3:

Configured by mauke at Sun Mar 24 20:23:51 CET 2013.

Summary of my perl5 (revision 5 version 16 subversion 3) configuration:

Platform:
osname=linux, osvers=3.5.7-gentoo, archname=i686-linux
uname='linux nora 3.5.7-gentoo #5 preempt sat jan 26 16:46:10 cet 2013 i686 amd athlon(tm) 64 processor 3200+ authenticamd gnulinux '
config_args=''
hint=previous, useposix=true, d_sigaction=define
useithreads=undef, usemultiplicity=undef
useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
use64bitint=undef, use64bitall=undef, uselongdouble=undef
usemymalloc=n, bincompat5005=undef
Compiler:
cc='cc', ccflags ='-fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
optimize='-O2 -flto',
cppflags='-fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64'
ccversion='', gccversion='4.8.0', gccosandvers=''
intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234
d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12
ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
alignbytes=4, prototype=define
Linker and Libraries:
ld='cc', ldflags ='-fstack-protector -L/usr/local/lib -O2 -flto'
libpth=/usr/local/lib /lib/../lib /usr/lib/../lib /lib /usr/lib
libs=-lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc -lgdbm_compat
perllibs=-lnsl -ldl -lm -lcrypt -lutil -lc
libc=/lib/libc-2.15.so, so=so, useshrplib=false, libperl=libperl.a
gnulibc_version='2.15'
Dynamic Linking:
dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
cccdlflags='-fPIC', lddlflags='-shared -O2 -flto -L/usr/local/lib -fstack-protector'

Locally applied patches:
SAVEARGV0 - disable magic open in <ARGV>

---
@INC for perl 5.16.3:
/home/mauke/usr/local/lib/perl5/site_perl/5.16.3/i686-linux
/home/mauke/usr/local/lib/perl5/site_perl/5.16.3
/home/mauke/usr/local/lib/perl5/5.16.3/i686-linux
/home/mauke/usr/local/lib/perl5/5.16.3
/home/mauke/usr/local/lib/perl5/site_perl/5.16.0/i686-linux
/home/mauke/usr/local/lib/perl5/site_perl/5.16.0
/home/mauke/usr/local/lib/perl5/site_perl
.

---
Environment for perl 5.16.3:
HOME=/home/mauke
LANG=en_US.UTF-8
LANGUAGE (unset)
LC_COLLATE=POSIX
LD_LIBRARY_PATH=/home/mauke/usr/local/lib
LOGDIR (unset)
PATH=/home/mauke/usr/perlbrew/bin:/home/mauke/usr/local/bin:/usr/local/bin:/usr/bin:/bin:/opt/bin:/usr/i686-pc-linux-gnu/gcc-bin/4.6.3:/opt/sun-jdk-1.4.2.13/bin:/opt/sun-jdk-1.4.2.13/jre/bin:/opt/sun-jdk-1.4.2.13/jre/javaws:/opt/dmd/bin:/usr/games/bin
PERLBREW_BASHRC_VERSION=0.43
PERLBREW_HOME=/home/mauke/.perlbrew
PERLBREW_PATH=/home/mauke/usr/perlbrew/bin
PERLBREW_ROOT=/home/mauke/usr/perlbrew
PERLBREW_VERSION=0.27
PERL_BADLANG (unset)
PERL_UNICODE=SAL
SHELL=/bin/bash

[Vincent Pit]

On 22/05/2013 11:33, l....@web.de (via RT) wrote:
> # New Ticket Created by l....@web.de
> # Please include the string: [perl #118121]
> # in the subject line of all future correspondence about this issue.
> # <URL: https://rt.perl.org:443/rt3/Ticket/Display.html?id=118121 >
>
>
>
> This is a bug report for perl from l....@web.de,
> generated with the help of perlbug 1.39 running under perl 5.16.3.
>
>
> -----------------------------------------------------------------
> [Please describe your issue here]
>
> As far as I can see 'perldoc perlsyn' doesn't document the return value of
> 'foreach'. I think this should be changed, if only to say that you shouldn't
> rely on the exact value.
>
> Experiments suggest the behavior is as follows:
>
> do { foreach (...) { ... } } evaluates to &PL_sv_no, unless you exit the loop
> via 'last', in which case it evaluates to the empty list (== no return value).

$ perl -wE'say for do { for (1) { foo(2, 3, last) } }'
2
3

Fixing this bug might be a good excuse to always make for return an
empty list.


Vincent

[James E Keenan via RT]

On Wed May 22 02:33:15 2013, mauke- wrote:
>
> This is a bug report for perl from l....@web.de,
> generated with the help of perlbug 1.39 running under perl 5.16.3.
>
>
> -----------------------------------------------------------------
> [Please describe your issue here]
>
> As far as I can see 'perldoc perlsyn' doesn't document the return
> value of
> 'foreach'. I think this should be changed, if only to say that you
> shouldn't
> rely on the exact value.
>
> Experiments suggest the behavior is as follows:
>
> do { foreach (...) { ... } } evaluates to &PL_sv_no, unless you exit
> the loop
> via 'last', in which case it evaluates to the empty list (== no return
> value).
>

> (Personally I'd prefer if 'foreach' returned () always but changing
> that may
> break existing code (?).)
>
> What do you think about documenting the return value as "unspecified"
> and maybe
> describing the current behavior?
>

-1

If the current behavior is undocumented, then the return value already
is unspecified.

More to the point, I have never in 13 years had to think about the
return value of a foreach loop -- specified or unspecified. So I have
never been tempted to rely on that value. If we were to start talking
about it now, somebody would, inevitably, start rely upon what we said.
And that poses a maintenance burden we do not need.

Thank you very much.
Jim Keenan


---
via perlbug: queue: perl5 status: open
https://rt.perl.org:443/rt3/Ticket/Display.html?id=118121

[Tony Cook via RT]

On Wed May 22 02:33:15 2013, mauke- wrote:
> As far as I can see 'perldoc perlsyn' doesn't document the return
> value of
> 'foreach'. I think this should be changed, if only to say that you
> shouldn't
> rely on the exact value.
>
> Experiments suggest the behavior is as follows:
>
> do { foreach (...) { ... } } evaluates to &PL_sv_no, unless you exit
> the loop
> via 'last', in which case it evaluates to the empty list (== no return
> value).
>
> (Personally I'd prefer if 'foreach' returned () always but changing
> that may
> break existing code (?).)
>
> What do you think about documenting the return value as "unspecified"
> and maybe
> describing the current behavior?

Documenting it as "Unspecified, and the current behaviour may change
without notice" would suite me better. If we document the current
behaviour then it isn't unspecified, and you know someone is going to
rely on it.

Though I wonder if we're better off with a "if we don't say how it
behaves we can change how it behaves in any major release", rather than
unspecifying every little piece of behaviour.

That might leave us with too much that's changable though.

Tony

[Nicholas Clark]

On Thu, May 23, 2013 at 05:30:31AM -0700, Tony Cook via RT wrote:
> On Wed May 22 02:33:15 2013, mauke- wrote:
> > As far as I can see 'perldoc perlsyn' doesn't document the return
> > value of
> > 'foreach'. I think this should be changed, if only to say that you
> > shouldn't
> > rely on the exact value.
> >
> > Experiments suggest the behavior is as follows:

> Documenting it as "Unspecified, and the current behaviour may change
> without notice" would suite me better. If we document the current
> behaviour then it isn't unspecified, and you know someone is going to
> rely on it.

I think I like this best.
Not just here, but generally.

> Though I wonder if we're better off with a "if we don't say how it
> behaves we can change how it behaves in any major release", rather than
> unspecifying every little piece of behaviour.

In theory that's nice (and it's probably worth doing), but in practice I
don't think that enough people would read the documentation and pay attention
to this. (Note how many modules continue to use deprecated features right up
until the point that those features warn. Including new code written after
those features were documented as deprecated)

Whilst it does make the documentation even bigger to explicitly state
"unspecified" everywhere it is, I think it's better than the status quo,
where it's not obvious from the documentation whether it's in omission in
the documentation, or actually unspecified.

> That might leave us with too much that's changable though.

I think that if we stuck to the view that "everything that isn't documented
is changeable" and then did so, sufficient of the useful code on CPAN would
break that nearly no-one would actually upgrade. Which would render the whole
point of the changes moot. After all, if no-one uses the changed version,
what was the point of doing it?

Nicholas Clark

[Steffen Mueller]

On 05/24/2013 11:01 AM, Nicholas Clark wrote:
> On Thu, May 23, 2013 at 05:30:31AM -0700, Tony Cook via RT wrote:
>> On Wed May 22 02:33:15 2013, mauke- wrote:
>>> As far as I can see 'perldoc perlsyn' doesn't document the return
>>> value of
>>> 'foreach'. I think this should be changed, if only to say that you
>>> shouldn't
>>> rely on the exact value.
>>>
>>> Experiments suggest the behavior is as follows:
>
>> Documenting it as "Unspecified, and the current behaviour may change
>> without notice" would suite me better. If we document the current
>> behaviour then it isn't unspecified, and you know someone is going to
>> rely on it.
>
> I think I like this best.
> Not just here, but generally.

Agreed both on this count and in general!

>> Though I wonder if we're better off with a "if we don't say how it
>> behaves we can change how it behaves in any major release", rather than
>> unspecifying every little piece of behaviour.
>
> In theory that's nice (and it's probably worth doing), but in practice I
> don't think that enough people would read the documentation and pay attention
> to this. (Note how many modules continue to use deprecated features right up
> until the point that those features warn. Including new code written after
> those features were documented as deprecated)
>
> Whilst it does make the documentation even bigger to explicitly state
> "unspecified" everywhere it is, I think it's better than the status quo,
> where it's not obvious from the documentation whether it's in omission in
> the documentation, or actually unspecified.

I don't think a concerted effort to add these things all over the place
is worth it. Using the cases where people actually stumble to add notes
where they matter is an entirely different matter, though.

>> That might leave us with too much that's changable though.
>
> I think that if we stuck to the view that "everything that isn't documented
> is changeable" and then did so, sufficient of the useful code on CPAN would
> break that nearly no-one would actually upgrade. Which would render the whole
> point of the changes moot. After all, if no-one uses the changed version,
> what was the point of doing it?

I think you're being a bit pessimistic here. Such changes need to be
made at times and most of the time, fixing the few bits of code that
fail because of such changes isn't a huge deal. On a more pointed note,
if not overdone (Python 3 level of breakage) this is actually a
convenient way of externalizing cost from the small group of people
capable and willing to do work on the core to the much larger group of
people capable and willing to do small fixes to CPAN modules.

--Steffen

[Ricardo SIGNES via RT]

I have documented the undocumented nature of the return value as 739ba95.

--
rjbs