Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

for loop doesn't update correct variable #15635

Open
p5pRT opened this issue Sep 29, 2016 · 8 comments
Open

for loop doesn't update correct variable #15635

p5pRT opened this issue Sep 29, 2016 · 8 comments

Comments

@p5pRT
Copy link

@p5pRT p5pRT commented Sep 29, 2016

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

Searchable as RT129757$

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Sep 29, 2016

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
Copy link
Author

@p5pRT p5pRT commented Sep 29, 2016

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
Copy link
Author

@p5pRT p5pRT commented Sep 29, 2016

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

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Sep 29, 2016

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
Copy link
Author

@p5pRT p5pRT commented Sep 29, 2016

From @rjbs

* James E Keenan via RT <perlbug-followup@​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
Copy link
Author

@p5pRT p5pRT commented Sep 29, 2016

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
Copy link
Author

@p5pRT p5pRT commented Sep 29, 2016

From @ap

* Mark-Jason Dominus <perlbug-followup@​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
Copy link
Author

@p5pRT p5pRT commented Sep 29, 2016

From @cpansprout

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

* Mark-Jason Dominus <perlbug-followup@​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 514e62e 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant