Perl / perl5

đŸȘ The Perl programming language
https://dev.perl.org/perl5/
Other
1.95k stars 554 forks source link

for loop doesn't update correct variable #15635

Open p5pRT opened 8 years ago

p5pRT commented 8 years ago

Migrated from rt.perl.org#129757 (status was 'open')

Searchable as RT129757$

p5pRT commented 8 years ago

From @mjdominus

Created by @mjdominus

The following program​:

  my $i;   sub announce { printf "Count %d\n"\, $i }

  for $i (1..3) {   announce();   }

It emits

  Count 0   Count 0   Count 0

But it should be

  Count 1   Count 2   Count 3

Also\, when correcting that bug\, please make sure this is also corrected​:

  for my $i (1..3) {   sub announce { printf "Count %d\n"\, $i }   announce();   }

It shows the same wrong behavior.

Perl Info ``` Flags: category=core severity=high Site configuration information for perl 5.18.2: Configured by Debian Project at Tue Mar 1 17:12:59 UTC 2016. Summary of my perl5 (revision 5 version 18 subversion 2) configuration: Platform: osname=linux, osvers=3.13.0-79-generic, archname=x86_64-linux-gnu-thread-multi uname='linux lgw01-37 3.13.0-79-generic #123-ubuntu smp fri feb 19 14:27:58 utc 2016 x86_64 x86_64 x86_64 gnulinux ' config_args='-Dusethreads -Duselargefiles -Dccflags=-DDEBIAN -D_FORTIFY_SOURCE=2 -g -O2 -fstack-protector --param=ssp-buffer-size=4 -Wformat -Werror=format-security -Dldflags= -Wl,-Bsymbolic-functions -Wl,-z,relro -Dlddlflags=-shared -Wl,-Bsymbolic-functions -Wl,-z,relro -Dcccdlflags=-fPIC -Darchname=x86_64-linux-gnu -Dprefix=/usr -Dprivlib=/usr/share/perl/5.18 -Darchlib=/usr/lib/perl/5.18 -Dvendorprefix=/usr -Dvendorlib=/usr/share/perl5 -Dvendorarch=/usr/lib/perl5 -Dsiteprefix=/usr/local -Dsitelib=/usr/local/share/perl/5.18.2 -Dsitearch=/usr/local/lib/perl/5.18.2 -Dman1dir=/usr/share/man/man1 -Dman3dir=/usr/share/man/man3 -Dsiteman1dir=/usr/local/man/man1 -Dsiteman3dir=/usr/local/man/man3 -Duse64bitint -Dman1ext=1 -Dman3ext=3perl -Dpager=/usr/bin/sensible-pager -Uafs -Ud_csh -Ud_ualarm -Uusesfio -Uusenm -Ui_libutil -Uversiononly -DDEBUGGING=-g -Doptimize=-O2 -Duseshrplib -Dlibperl=libperl.so.5.18.2 -des' hint=recommended, useposix=true, d_sigaction=define useithreads=define, usemultiplicity=define useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef use64bitint=define, use64bitall=define, uselongdouble=undef usemymalloc=n, bincompat5005=undef Compiler: cc='cc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fstack-protector -fno-strict-aliasing -pipe -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64', optimize='-O2 -g', cppflags='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fstack-protector -fno-strict-aliasing -pipe -I/usr/local/include' ccversion='', gccversion='4.8.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='cc', ldflags =' -fstack-protector -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=-lgdbm -lgdbm_compat -ldb -ldl -lm -lpthread -lc -lcrypt perllibs=-ldl -lm -lpthread -lc -lcrypt libc=, so=so, useshrplib=true, libperl=libperl.so.5.18.2 gnulibc_version='2.19' Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E' cccdlflags='-fPIC', lddlflags='-shared -L/usr/local/lib -fstack-protector' Locally applied patches: DEBPKG:debian/cpan_definstalldirs - Provide a sensible INSTALLDIRS default for modules installed from CPAN. DEBPKG:debian/db_file_ver - http://bugs.debian.org/340047 Remove overly restrictive DB_File version check. DEBPKG:debian/doc_info - Replace generic man(1) instructions with Debian-specific information. DEBPKG:debian/enc2xs_inc - http://bugs.debian.org/290336 Tweak enc2xs to follow symlinks and ignore missing @INC directories. DEBPKG:debian/errno_ver - http://bugs.debian.org/343351 Remove Errno version check due to upgrade problems with long-running processes. DEBPKG:debian/libperl_embed_doc - http://bugs.debian.org/186778 Note that libperl-dev package is required for embedded linking DEBPKG:fixes/respect_umask - Respect umask during installation DEBPKG:debian/writable_site_dirs - Set umask approproately for site install directories DEBPKG:debian/extutils_set_libperl_path - EU:MM: Set location of libperl.a to /usr/lib DEBPKG:debian/no_packlist_perllocal - Don't install .packlist or perllocal.pod for perl or vendor DEBPKG:debian/prefix_changes - Fiddle with *PREFIX and variables written to the makefile DEBPKG:debian/fakeroot - Postpone LD_LIBRARY_PATH evaluation to the binary targets. DEBPKG:debian/instmodsh_doc - Debian policy doesn't install .packlist files for core or vendor. DEBPKG:debian/ld_run_path - Remove standard libs from LD_RUN_PATH as per Debian policy. DEBPKG:debian/libnet_config_path - Set location of libnet.cfg to /etc/perl/Net as /usr may not be writable. DEBPKG:debian/mod_paths - Tweak @INC ordering for Debian DEBPKG:debian/module_build_man_extensions - http://bugs.debian.org/479460 Adjust Module::Build manual page extensions for the Debian Perl policy DEBPKG:debian/prune_libs - http://bugs.debian.org/128355 Prune the list of libraries wanted to what we actually need. DEBPKG:fixes/net_smtp_docs - [rt.cpan.org #36038] http://bugs.debian.org/100195 Document the Net::SMTP 'Port' option DEBPKG:debian/perlivp - http://bugs.debian.org/510895 Make perlivp skip include directories in /usr/local DEBPKG:debian/cpanplus_definstalldirs - http://bugs.debian.org/533707 Configure CPANPLUS to use the site directories by default. DEBPKG:debian/cpanplus_config_path - Save local versions of CPANPLUS::Config::System into /etc/perl. DEBPKG:debian/deprecate-with-apt - http://bugs.debian.org/702096 Point users to Debian packages of deprecated core modules DEBPKG:debian/squelch-locale-warnings - http://bugs.debian.org/508764 Squelch locale warnings in Debian package maintainer scripts DEBPKG:debian/skip-upstream-git-tests - Skip tests specific to the upstream Git repository DEBPKG:debian/patchlevel - http://bugs.debian.org/567489 List packaged patches for 5.18.2-2ubuntu1.1 in patchlevel.h DEBPKG:debian/skip-kfreebsd-crash - http://bugs.debian.org/628493 [perl #96272] Skip a crashing test case in t/op/threads.t on GNU/kFreeBSD DEBPKG:fixes/document_makemaker_ccflags - http://bugs.debian.org/628522 [rt.cpan.org #68613] Document that CCFLAGS should include $Config{ccflags} DEBPKG:debian/find_html2text - http://bugs.debian.org/640479 Configure CPAN::Distribution with correct name of html2text DEBPKG:debian/hurd_test_skip_stack - http://bugs.debian.org/650175 Disable failing GNU/Hurd tests dist/threads/t/stack.t DEBPKG:fixes/manpage_name_Test-Harness - http://bugs.debian.org/650451 [rt.cpan.org #73399] cpan/Test-Harness: add NAME headings in modules with POD DEBPKG:debian/makemaker-pasthru - http://bugs.debian.org/660195 [rt.cpan.org #28632] Make EU::MM pass LD through to recursive Makefile.PL invocations DEBPKG:debian/perl5db-x-terminal-emulator.patch - http://bugs.debian.org/668490 Invoke x-terminal-emulator rather than xterm in perl5db.pl DEBPKG:debian/cpan-missing-site-dirs - http://bugs.debian.org/688842 Fix CPAN::FirstTime defaults with nonexisting site dirs if a parent is writable DEBPKG:fixes/memoize_storable_nstore - [rt.cpan.org #77790] http://bugs.debian.org/587650 Memoize::Storable: respect 'nstore' option not respected DEBPKG:fixes/net_ftp_failed_command - [rt.cpan.org #37700] http://bugs.debian.org/491062 Net::FTP: cope gracefully with a failed command DEBPKG:fixes/perlbug-patchlist - [3541c11] http://bugs.debian.org/710842 [perl #118433] Make perlbug look up the list of local patches at run time DEBPKG:fixes/module_metadata_security_doc - [68cdd4b] CVE-2013-1437 documentation fix DEBPKG:fixes/module_metadata_taint_fix - [bff978f] http://bugs.debian.org/722210 [rt.cpan.org #88576] untaint version, if needed, in Module::Metadata DEBPKG:fixes/IPC-SysV-spelling - http://bugs.debian.org/730558 [rt.cpan.org #86736] Fix spelling of IPC_CREAT in IPC-SysV documentation DEBPKG:fixes/fix-undef-source - DEBPKG:fixes/CVE-2013-7422.patch - [PATCH] [perl #119505] Segfault from bad backreference DEBPKG:fixes/CVE-2014-4330.patch - [PATCH] don't recurse infinitely in Data::Dumper DEBPKG:fixes/CVE-2016-2381.patch - [PATCH 1/2] remove duplicate environment variables from environ @INC for perl 5.18.2: /etc/perl /usr/local/lib/perl/5.18.2 /usr/local/share/perl/5.18.2 /usr/lib/perl5 /usr/share/perl5 /usr/lib/perl/5.18 /usr/share/perl/5.18 /usr/local/lib/site_perl . Environment for perl 5.18.2: HOME=/home/mjd LANG=en_US.UTF-8 LANGUAGE (unset) LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/home/mjd/bin:/usr/local/nmh/bin:/home/mjd/misc/blog/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/var/qmail/bin PERL_BADLANG (unset) SHELL=/bin/bash ```
p5pRT commented 8 years ago

From @jkeenan

On Wed Sep 28 17​:50​:35 2016\, mjd@​plover.com wrote​:

This is a bug report for perl from mjd@​plover.com\, generated with the help of perlbug 1.39 running under perl 5.18.2.

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

The following program​:

my $i; sub announce { printf "Count %d\n"\, $i }

for $i (1..3) { announce(); }

It emits

Count 0 Count 0 Count 0

But it should be

Count 1 Count 2 Count 3

Also\, when correcting that bug\, please make sure this is also corrected​:

for my $i (1..3) { sub announce { printf "Count %d\n"\, $i } announce(); }

It shows the same wrong behavior.

Why\, in a similar circumstance\, does 'print' behave differently from 'printf'?

##### $ cat 129757-other.pl my $j; sub denounce { print "Count $j\n"; }

for $j (1..3) {   denounce(); }

$ perl 129757-other.pl Count Count Count #####

Thank you very much. -- James E Keenan (jkeenan@​cpan.org)

p5pRT commented 8 years ago

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

p5pRT commented 8 years ago

From Eirik-Berg.Hanssen@allverden.no

On Thu\, Sep 29\, 2016 at 3​:05 AM\, James E Keenan via RT \< perlbug-followup@​perl.org> wrote​:

Why\, in a similar circumstance\, does 'print' behave differently from 'printf'?

  It's not different\, is it?

  Well\, okay\, %d in the pattern to printf evaluates it in numeric context (getting 0)\, while double-quote interpolation evaluates it in string context (getting '')\, but in either case\, the variable is undefined (as you would see if running the snippets with warnings enabled).

Eirik

p5pRT commented 8 years ago

From @rjbs

* James E Keenan via RT \perlbug\-followup@&#8203;perl\.org [2016-09-28T21​:05​:02]

Why\, in a similar circumstance\, does 'print' behave differently from 'printf'?

  ~$ perl -e 'my $x = undef; printf "%d\n"\, $x'   0   ~$ perl -e 'my $x = undef; print "$x\n"'

The %d format numifies undef.

  ~$ perl -Mwarnings -e 'my $x = undef; printf "%d\n"\, $x'   Use of uninitialized value $x in printf at -e line 1.   0   ~$ perl -Mwarnings -e 'my $x = undef; print "$x\n"'   Use of uninitialized value $x in concatenation (.) or string at -e line 1.

-- rjbs

p5pRT commented 8 years ago

From @petdance

# A self-contained test that shows the correct behavior and doesn't get into print/printf.

require "t/test.pl";

my $i = 2112;

ok( $i == 2112\, '$i starts at 2112' );

my $n; for $i (1..3) {   ++$n;   ok( $i == $n\, '$i is ' . $n . ' inside of the for loop' );   somesub(); }

ok( $i == 2112\, '$i should go back to 2112 outside of the loop' );

exit 0;

sub somesub {   ok( $i == $n\, '$i is ' . $n . ' in the subroutine' ); }

p5pRT commented 8 years ago

From @ap

* Mark-Jason Dominus \perlbug\-followup@&#8203;perl\.org [2016-09-29 03​:00]​:

The following program​:

my $i; sub announce { printf "Count %d\n"\, $i }

for $i (1..3) { announce(); }

It emits

Count 0 Count 0 Count 0

But it should be

Count 1 Count 2 Count 3

Maybe. But that might affect this​:

  $ perl -E 'my @​i; for my $i (5..8) { push @​i\, sub { say $i } }; $_->() for @​i'   5   6   7   8

That behaviour must not break under any circumstances.

To make that work\, in each iteration\, foreach rebinds the name $i to the scalar for that iteration​:

  perl -E 'my @​i = (5..8); for my $i (@​i) { say \$i eq \$i[$a++] }'   1   1   1   1

It seems to me that this binding is necessarily lexical to the loop block scope.

Meanwhile the `announce` sub has already closed over $i at compile time.

I don’t know that it’s possible to change either of these facts. Not to mention doing it in such a way that closing over the loop variable at runtime would remain unaffected.

The straightforward way to get your “correct” output would be to change foreach to do local()-style value shadowing with the same scalar. Which would break a huge amount of code\, and I think it would generally be a worse default\, even though one consequence of the current behaviour is the case you ran into here.

You can always use while() when you need it.

Also\, when correcting that bug\, please make sure this is also corrected​:

for my $i (1..3) { sub announce { printf "Count %d\n"\, $i } announce(); }

It shows the same wrong behavior.

That sub is compiled just once despite its placement inside the loop and it binds $i at compile time.

Arguably perl ought to warn about this\, the same way that it warns when you make the same mistake but you write it this way​:

  sub do_announce {   my $i = shift;   sub announce { printf "Count %d\n"\, $i }   announce();   }

That throws the infamous “will not stay shared” warning. But you would not want this to start warning​:

  {   my $count = 0;   sub announce { printf "Count %d\n"\, $count }   }

I don’t know how hard it would be to distinguish sure-fire warn-worthy situations from benign ones here.

Regards\, -- Aristotle Pagaltzis // \<http​://plasmasturm.org/>

p5pRT commented 8 years ago

From @cpansprout

On Wed Sep 28 21​:01​:34 2016\, aristotle wrote​:

* Mark-Jason Dominus \perlbug\-followup@&#8203;perl\.org [2016-09-29 03​:00]​:

Also\, when correcting that bug\, please make sure this is also corrected​:

for my $i (1..3) { sub announce { printf "Count %d\n"\, $i } announce(); }

It shows the same wrong behavior.

That sub is compiled just once despite its placement inside the loop and it binds $i at compile time.

This is similar to the problem that plagues lexical aliasing. The notes I added in 514e62e37b8 may also be relevant to package subs\, but I do not have time to think it through now\, or in the foreseeable future.

--

Father Chrysostomos