List assignment with undef and re-used variable regression

Smylers commented 4 years ago

The return value of a list assignment involving both undef and a variable that appears on both sides of the assignment changed somewhere between perl 5.22 and 5.30.

This emits different output on v5.22.1 and v5.30.0:

my $part = 'zing';
my @both = (($part, undef) = (boo => $part));
say @both;

On 5.22, @both ends up containing ('boo', 'zing'), where as on 5.30 it contains ('boo', 'boo'). The 5.22 behaviour is what I'd logically expect.

The bug doesn't appear if undef is replaced with an (otherwise unused) variable:

my $part = 'zing';
my @both = (($part, (my $ignored)) = (boo => $part));
say @both;

Nor if a separate scalar variable is assigned to, rather than overwriting the one being evaluated:

my $part = 'zing';
my @both = (my ($another, undef) = (boo => $part));
say @both;

Nor if the variable is used as part of an expression rather than evaluated directly, such as by stringifying it:

my $part = 'zing';
my @both = (($part, undef) = (boo => "$part"));
say @both;

Wrapping it in a do block isn't sufficient, though; this still exhibits the bug:

my $part = 'zing';
my @both = (($part, undef) = (boo => do { $part }));
say @both;

For anybody wondering why I was doing anything as convoluted as this (or why it would crop up in real-world code), the context is an optional initial positional argument to a sub:

classmethod BUILDARGS
(
  $id_type,
  $id = (($id_type, undef) = (old_id => $id_type))[1],
)

A class used to have a constructor which took an ID:

my $record = SomeProject::Record->new(123);

Then a new ID system was introduced, and the class was altered to accept either an old or a new ID:

my $record = SomeProject::Record->new(old_id => 123);
my $record = SomeProject::Record->new(new_id => 789);

For backwards compatibility with existing uses of the class, a single argument to ->new should be interpreted as an old-style ID; effectively the equivalent of:

unshift @_, 'old_id' if @_ == 1;

It uses subroutine signatures (they happen to be Function::Parameters, but a quick check shows it's the same with Perl's signatures feature), so there's nowhere handy to put such @_ tweaking; and when only one argument is supplied, it has to be the second parameter that's optional, not the first.

So in the case where only one argument, an old-style ID, is provided:

$id_type first has the ID assigned to it.
$id has no argument, so its default expression is evaluated. This expression needs to return the ID that has already been assigned to $id_type, and also overwrite $id_type to contain 'old_id'.
Using list assignment allows for both setting $id_type and assigning its value elsewhere. We can't assign to $id in the list, because the value of the entire expression will be assigned to $id. So instead assign to undef (so the list has the correct number of items in it), then use (...)[1] to obtain the value from the list.

And this was working.

(A nicer syntax for specifying, ‘if this sub has an odd number of arguments then treat the first one as being named argument foo’ would of course also be nice. But since the change in behaviour doesn't involve signatures, that's a separate matter.)

Flags: category=core severity=medium

Site configuration information for perl 5.30.0:

Configured by Ubuntu at Fri Mar 6 21:15:57 UTC 2020.

Summary of my perl5 (revision 5 version 30 subversion 0) configuration:

Platform: osname=linux osvers=4.19.0 archname=x86_64-linux-gnu-thread-multi uname='linux localhost 4.19.0 #1 smp debian 4.19.0 x86_64 gnulinux ' config_args='-Dmksymlinks -Dusethreads -Duselargefiles -Dcc=x86_64-linux-gnu-gcc -Dcpp=x86_64-linux-gnu-cpp -Dld=x86_64-linux-gnu-gcc -Dccflags=-DDEBIAN -Wdate-time -D_FORTIFY_SOURCE=2 -g -O2 -fdebug-prefix-map=/build/perl-hOEDTI/perl-5.30.0=. -fstack-protector-strong -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.30 -Darchlib=/usr/lib/x86_64-linux-gnu/perl/5.30 -Dvendorprefix=/usr -Dvendorlib=/usr/share/perl5 -Dvendorarch=/usr/lib/x86_64-linux-gnu/perl5/5.30 -Dsiteprefix=/usr/local -Dsitelib=/usr/local/share/perl/5.30.0 -Dsitearch=/usr/local/lib/x86_64-linux-gnu/perl/5.30.0 -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 -Ui_xlocale -Uversiononly -DDEBUGGING=-g -Doptimize=-O2 -dEs -Duseshrplib -Dlibperl=libperl.so.5.30.0' hint=recommended useposix=true d_sigaction=define useithreads=define usemultiplicity=define use64bitint=define use64bitall=define uselongdouble=undef usemymalloc=n default_inc_excludes_dot=define bincompat5005=undef Compiler: cc='x86_64-linux-gnu-gcc' ccflags ='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fwrapv -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 -fwrapv -fno-strict-aliasing -pipe -I/usr/local/include' ccversion='' gccversion='9.2.1 20200304' gccosandvers='' intsize=4 longsize=8 ptrsize=8 doublesize=8 byteorder=12345678 doublekind=3 d_longlong=define longlongsize=8 d_longdbl=define longdblsize=16 longdblkind=3 ivtype='long' ivsize=8 nvtype='double' nvsize=8 Off_t='off_t' lseeksize=8 alignbytes=8 prototype=define Linker and Libraries: ld='x86_64-linux-gnu-gcc' ldflags =' -fstack-protector-strong -L/usr/local/lib' libpth=/usr/local/lib /usr/lib/gcc/x86_64-linux-gnu/9/include-fixed /usr/include/x86_64-linux-gnu /usr/lib /lib/x86_64-linux-gnu /lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib libs=-lgdbm -lgdbm_compat -ldb -ldl -lm -lpthread -lc -lcrypt perllibs=-ldl -lm -lpthread -lc -lcrypt libc=libc-2.31.so so=so useshrplib=true libperl=libperl.so.5.30 gnulibc_version='2.31' Dynamic Linking: dlsrc=dl_dlopen.xs dlext=so d_dlsymun=undef ccdlflags='-Wl,-E' cccdlflags='-fPIC' lddlflags='-shared -L/usr/local/lib -fstack-protector-strong'

Locally applied patches: DEBPKG:debian/cpan_definstalldirs - Provide a sensible INSTALLDIRS default for modules installed from CPAN. DEBPKG:debian/db_file_ver - https://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 - https://bugs.debian.org/290336 Tweak enc2xs to follow symlinks and ignore missing @INC directories. DEBPKG:debian/errno_ver - https://bugs.debian.org/343351 Remove Errno version check due to upgrade problems with long-running processes. DEBPKG:debian/libperl_embed_doc - https://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 under /usr/lib DEBPKG:debian/no_packlist_perllocal - Don't install .packlist or perllocal.pod for perl or vendor 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/perlivp - https://bugs.debian.org/510895 Make perlivp skip include directories in /usr/local DEBPKG:debian/squelch-locale-warnings - https://bugs.debian.org/508764 Squelch locale warnings in Debian package maintainer scripts DEBPKG:debian/patchlevel - https://bugs.debian.org/567489 List packaged patches for 5.30.0-9build1 in patchlevel.h DEBPKG:fixes/document_makemaker_ccflags - https://bugs.debian.org/628522 [rt.cpan.org #68613] Document that CCFLAGS should include $Config{ccflags} DEBPKG:debian/find_html2text - https://bugs.debian.org/640479 Configure CPAN::Distribution with correct name of html2text DEBPKG:debian/perl5db-x-terminal-emulator.patch - https://bugs.debian.org/668490 Invoke x-terminal-emulator rather than xterm in perl5db.pl DEBPKG:debian/cpan-missing-site-dirs - https://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] https://bugs.debian.org/587650 Memoize::Storable: respect 'nstore' option not respected DEBPKG:debian/makemaker-pasthru - https://bugs.debian.org/758471 Pass LD settings through to subdirectories DEBPKG:debian/makemaker-manext - https://bugs.debian.org/247370 Make EU::MakeMaker honour MANnEXT settings in generated manpage headers DEBPKG:debian/kfreebsd-softupdates - https://bugs.debian.org/796798 Work around Debian Bug#796798 DEBPKG:fixes/autodie-scope - https://bugs.debian.org/798096 Fix a scoping issue with "no autodie" and the "system" sub DEBPKG:fixes/memoize-pod - [rt.cpan.org #89441] Fix POD errors in Memoize DEBPKG:debian/hurd-softupdates - https://bugs.debian.org/822735 Fix t/op/stat.t failures on hurd DEBPKG:fixes/math_complex_doc_great_circle - https://bugs.debian.org/697567 [rt.cpan.org #114104] Math::Trig: clarify definition of great_circle_midpoint DEBPKG:fixes/math_complex_doc_see_also - https://bugs.debian.org/697568 [rt.cpan.org #114105] Math::Trig: add missing SEE ALSO DEBPKG:fixes/math_complex_doc_angle_units - https://bugs.debian.org/731505 [rt.cpan.org #114106] Math::Trig: document angle units DEBPKG:fixes/cpan_web_link - https://bugs.debian.org/367291 CPAN: Add link to main CPAN web site DEBPKG:debian/hppa_op_optimize_workaround - https://bugs.debian.org/838613 Temporarily lower the optimization of op.c on hppa due to gcc-6 problems DEBPKG:debian/installman-utf8 - https://bugs.debian.org/840211 Generate man pages with UTF-8 characters DEBPKG:fixes/getopt-long-4 - https://bugs.debian.org/864544 [rt.cpan.org #122068] Fix issue #122068. DEBPKG:debian/hppa_opmini_optimize_workaround - https://bugs.debian.org/869122 Lower the optimization level of opmini.c on hppa DEBPKG:debian/sh4_op_optimize_workaround - https://bugs.debian.org/869373 Also lower the optimization level of op.c and opmini.c on sh4 DEBPKG:debian/perldoc-pager - https://bugs.debian.org/870340 [rt.cpan.org #120229] Fix perldoc terminal escapes when sensible-pager is less DEBPKG:debian/prune_libs - https://bugs.debian.org/128355 Prune the list of libraries wanted to what we actually need. DEBPKG:debian/mod_paths - Tweak @INC ordering for Debian DEBPKG:debian/configure-regen - https://bugs.debian.org/762638 Regenerate Configure et al. after probe unit changes DEBPKG:debian/deprecate-with-apt - https://bugs.debian.org/747628 Point users to Debian packages of deprecated core modules DEBPKG:debian/disable-stack-check - https://bugs.debian.org/902779 [perl #133327] Disable debugperl stack extension checks for binary compatibility with perl DEBPKG:fixes/eumm-usrmerge - https://bugs.debian.org/913637 Avoid mangling /bin non-perl shebangs on merged-/usr systems DEBPKG:debian/perlbug-editor - https://bugs.debian.org/922609 Use "editor" as the default perlbug editor, as per Debian policy DEBPKG:fixes/gid-parsing - [79e302e] https://bugs.debian.org/941985 [perl #134169] (perl #134169) mg.c reset endptr after use

@INC for perl 5.30.0: /etc/perl /usr/local/lib/x86_64-linux-gnu/perl/5.30.0 /usr/local/share/perl/5.30.0 /usr/lib/x86_64-linux-gnu/perl5/5.30 /usr/share/perl5 /usr/lib/x86_64-linux-gnu/perl/5.30 /usr/share/perl/5.30 /usr/local/lib/site_perl /usr/lib/x86_64-linux-gnu/perl-base

Environment for perl 5.30.0: HOME=/home/smylers LANG=en_GB.utf8 LANGUAGE=en_GB:en LC_COLLATE=C LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/home/smylers/bin:/usr/local/sbin:/usr/local/bin:/snap/bin:/sbin:/bin:/usr/sbin:/usr/bin:/usr/X11R6/bin:/usr/games PERL_BADLANG (unset) PERL_CPANM_OPT=--sudo --prompt SHELL=/bin/bash

Smylers commented 4 years ago

Script demonstrating the issue: list_assignment_demo.txt

On perl v5.30.0 it emits:

booboo
boozing
boozing
boozing
booboo

On v5.22.1 there aren't any booboos.

xenu commented 4 years ago

The behavior changed in commit 808ce55782035154ec42358256dbec2e226977fe:

commit 808ce55782035154ec42358256dbec2e226977fe
Author: David Mitchell <davem@iabyn.com>
Date:   Thu Aug 13 17:00:32 2015 +0100

    Optimise 1 arg in list assign

    Avoid setting common scalar flags in these cases:

    ($x) = (...);
    (...) = ($x);

xenu commented 4 years ago

CC @iabyn

iabyn commented 4 years ago

now fixed in blead by v5.33.0-235-g5b354d2a8a

Perl / perl5

List assignment with undef and re-used variable regression #17816

Flags: category=core severity=medium