From @epa

Created by @epa

Program​:

#!/usr/bin/perl
use warnings;
use strict;
our $x;
sub f {
  local ($x) = ();
  print '$x is ', defined $x ? '' : 'not ', "defined\n";
  local ($_) = ();
  print '$_ is ', defined $_ ? '' : 'not ', "defined\n";
}
'a' =~ /(.)/;
f() for $1;

Expected output​:

$x is not defined
$_ is not defined

Actual output​:

$x is not defined
$_ is defined

Comments​:

In a list assignment, shouldn't any 'missing' elements get assigned
undef, in all circumstances? Certainly there seems no reason to treat
localized $_ differently from localized $x.

This perlbug was built using Perl 5.00503 - Thu Aug 10 15:31:56 EDT 2000
It is being executed now by  Perl 5.008 - Sat Apr 12 10:55:50 BST 2003.

Site configuration information for perl 5.008:

Configured by tomc at Sat Apr 12 10:55:50 BST 2003.

Summary of my perl5 (revision 5.0 version 8 subversion 0) configuration:
  Platform:
    osname=linux, osvers=2.2.19-6.2.12.1rs, archname=i686-linux
    uname='linux budvar.future-i.net 2.2.19-6.2.12.1rs #1 sat nov 3 02:42:38 cst 2001 i686 unknown '
    config_args=''
    hint=recommended, useposix=true, d_sigaction=define
    usethreads=undef use5005threads=undef 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 -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -I/usr/include/gdbm',
    optimize='-O2',
    cppflags='-fno-strict-aliasing -I/usr/local/include -I/usr/include/gdbm'
    ccversion='', gccversion='egcs-2.91.66 19990314/Linux (egcs-1.1.2 release)', 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 =' -L/usr/local/lib'
    libpth=/usr/local/lib /lib /usr/lib
    libs=-lbind -lnsl -lgdbm -ldl -lm -lc -lcrypt -lutil
    perllibs=-lbind -lnsl -ldl -lm -lc -lcrypt -lutil
    libc=/lib/libc-2.2.4.so, so=so, useshrplib=false, libperl=libperl.a
    gnulibc_version='2.2.4'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-rdynamic'
    cccdlflags='-fpic', lddlflags='-shared -L/usr/local/lib'

Locally applied patches:
    


@INC for perl 5.008:
    /home/ed/lib/perl5/5.8.0/i686-linux
    /home/ed/lib/perl5/5.8.0
    /home/ed/lib/perl5/site_perl/5.8.0/i686-linux
    /home/ed/lib/perl5/site_perl/5.8.0
    /home/ed/lib/perl5/site_perl/5.8.0/i686-linux
    /home/ed/lib/perl5/site_perl/5.8.0/i386-linux
    /home/ed/lib/perl5/site_perl/5.8.0/i686-linux
    /home/ed/lib/perl5/site_perl/5.8.0
    /home/ed/lib/perl5/site_perl
    /usr/lib/perl5/5.8.0/i686-linux
    /usr/lib/perl5/5.8.0
    /usr/lib/perl5/site_perl/5.8.0/i686-linux
    /usr/lib/perl5/site_perl/5.8.0
    /usr/lib/perl5/site_perl/5.6.1
    /usr/lib/perl5/site_perl/5.005
    /usr/lib/perl5/site_perl
    .


Environment for perl 5.008:
    HOME=/home/ed
    LANG=en_US
    LANGUAGE (unset)
    LD_LIBRARY_PATH=/home/ed/lib:/home/ed/wine/inst/lib:/usr/local/lib
    LOGDIR (unset)
    PATH=/home/ed/bin:/home/ed/bin/i686-pc-linux-gnu:/home/ed/wine/inst/bin:/home/ed/lib/ant/bin:/bin:/usr/bin:/usr/local/maple/bin:/usr/local/bin:/usr/X11R6/bin:/usr/local/java/bin:/var/qmail/bin:/usr/local/pgsql-7.1.3/bin:/usr/local/ezmlm/bin:/opt/kde/bin:/usr/local/X11/bin:/usr/krb5/bin:/usr/athena/bin:/sbin:/usr/sbin:/opt/IBMJava2-13/bin:/usr/local/jdk1.2.2/bin:/usr/share/smlnj/bin:/usr/games:/usr/kerberos/bin:/usr/kerberos/sbin:/usr/libexec/openssh
    PERL5LIB=/home/ed/lib/perl5/5.8.0:/home/ed/lib/perl5/site_perl/5.8.0:/home/ed/lib/perl5/site_perl/5.8.0/i686-linux:/home/ed/lib/perl5/site_perl/5.8.0/i386-linux:/home/ed/lib/perl5/site_perl
    PERL_BADLANG (unset)
    SHELL=/bin/bash

From @epa

A slightly better test case​:

#!/usr/bin/perl
use warnings;
use strict;
our $x = 'x';
sub f {
  local ($x) = ();
  print '$x is ', defined $x ? '' : 'not ', "defined\n";
  local ($_) = ();
  print '$_ is ', defined $_ ? '' : 'not ', "defined\n";
}
'a' =~ /(.)/;
f() for $1;

Output as before.

From ben.goldberg@hotpop.com

"Ed Avis (via RT)" wrote​:

Program​:

#!/usr/bin/perl
use warnings;
use strict;
our $x;
sub f {
local ($x) = ();
print '$x is ', defined $x ? '' : 'not ', "defined\n";
local ($_) = ();
print '$_ is ', defined $_ ? '' : 'not ', "defined\n";
}
'a' =~ /(.)/;
f() for $1;

Expected output​:

$x is not defined
$_ is not defined

Actually, I would expect​:

  $x is not defined
  Modification of a read-only value attempted at foo.pl line 8.

Similar to how this​:

  C​:\> perl -e "'a' =~ /(1)/; $_ = 2 for $1"

Produces​:

  Modification of a read-only value attempted at -e line 1.

Actual output​:

$x is not defined
$_ is defined

I'd consider this a bug. If you do local ($_) = (), then it should
either alter $_, or else die due to "Modification of a read-only
value..."

(Well, unless $_ is an alias to a tied value, in which case it should
fetch the old value, store the new value, and at scope-exit, store back
the old value.)

Comments​:

In a list assignment, shouldn't any 'missing' elements get assigned
undef, in all circumstances? Certainly there seems no reason to treat
localized $_ differently from localized $x.

*Assigned* undef, certainly.

Whether that assign operation should succeed depends on what's being
assigned to.

Anyway... your example code could be reduced to​:

  perl -le "'a' =~ /(.)/; local ($1) = (); print $1"

Which prints "a", instead of dieing with "Modification..." as I would
expect.

--
$a=24;split//,240513;s/\B/ => /for@​@​=qw(ac ab bc ba cb ca
);{push(@​b,$a),($a-=6)^=1 for 2..$a/6x--$|;print "$@​[$a%6
]\n";((6<=($a-=6))?$a+=$_[$a%6]-$a%6​:($a=pop @​b))&&redo;}

From @epa

On 28 Jul 2003, Benjamin Goldberg wrote​:

our $x;
sub f {
local ($x) = ();
print '$x is ', defined $x ? '' : 'not ', "defined\n";
local ($_) = ();
print '$_ is ', defined $_ ? '' : 'not ', "defined\n";
}
'a' =~ /(.)/;
f() for $1;

Actually, I would expect​:

$x is not defined
Modification of a read-only value attempted at foo.pl line 8.

That would make _some_ sense - certainly better than the current
behaviour - but it doesn't really seem right. Why shouldn't f() be
able to localize $_ and use it in a list assignment if it wants to?

I know that 'local' is really for saving and restoring the value, not
making a whole new copy of the variable, but still it seems a bit
wrong that

  local ($_) = ();

could work some of the time and break some of the time, depending on
what $_ was holding before. It should either always work or always
break, and not suffer from spooky action at a distance depending on
what some unrelated piece of code happened to have set $_ to before.

Otherwise one would have to conclude that the above line of code is
unusable in any subroutine, since you can never know what the caller
might have set $_ to before calling your routine. And it's not
reasonable to add bizarre preconditions about the value of $_ to
every subroutine you write.

Similar to how this​:

C​:\> perl -e "'a' =~ /(1)/; $_ = 2 for $1"

Produces​:

Modification of a read-only value attempted at -e line 1.

I think that's fair enough, $1 is supposed to be read-only. And you
have not said 'local $_' to indicate your intention that changes to $_
within the local block should not affect code that runs after the
block finishes.

I'd consider this a bug. If you do local ($_) = (), then it should
either alter $_, or else die due to "Modification of a read-only
value..."

Yes, better, but still not great. If it's unpredictable which of the
two it will do, then that line would be unsafe to use in any
subroutine (unless you are prepared to tell all the callers of that
routine to be careful what they have in $_ before calling it).

Anyway... your example code could be reduced to​:

perl -le "'a' =~ /(.)/; local ($1) = (); print $1"

Which prints "a", instead of dieing with "Modification..." as I
would expect.

Hmm. When you put it in those terms it seems more reasonable that the
modification should fail. But then, $1 is clearly marked out as a
read-only variable. $_ is not, and it's very surprising for an
assignment to $_ to fail. Doubly so when marked as 'local'.

If localizing $_ is not sufficient to make sure you can safely assign
to it, what is?

--
Ed Avis <ed@​membled.com>

From @rgs

Ed Avis (via RT) wrote​:

In a list assignment, shouldn't any 'missing' elements get assigned
undef, in all circumstances? Certainly there seems no reason to treat
localized $_ differently from localized $x.

In fact this bug can be reduced to :

  'not ok' =~ /(.+)/;
  for ($1) { local $_ = "ok"; print $_; }

-->prints "not ok".

in other words the localization of $_ is silently ignored when it's
aliased to $1. That's bad. It's there at least since 5.6.0.

From @rgs

Rafael Garcia-Suarez wrote​:

In fact this bug can be reduced to :

'not ok' =~ /\(\.\+\)/;
for \($1\) \{ local $\_ = "ok"; print $\_; \}

-->prints "not ok".

in other words the localization of $_ is silently ignored when it's
aliased to $1. That's bad. It's there at least since 5.6.0.

This patch appears to fix the bug, and, (a bit suprisingly), doesn't
break any regression test. It propagates the SvREADONLY flag of magical
SVs that are localized. With it, the above code prints "Modification of
a read-only value attempted".

--- scope.c (revision 1484)
+++ scope.c (working copy)
@​@​ -214,7 +214,7 @​@​ S_save_scalar_at(pTHX_ SV **sptr)
  PL_tainted = oldtainted;
  }
  SvMAGIC(sv) = SvMAGIC(osv);
- SvFLAGS(sv) |= SvMAGICAL(osv);
+ SvFLAGS(sv) |= SvMAGICAL(osv) | SvREADONLY(osv);
  /* XXX SvMAGIC() is *shared* between osv and sv. This can
  * lead to coredumps when both SVs are destroyed without one
  * of their SvMAGIC() slots being NULLed. */
End.

Anyway this is a suboptimal fix ; I think that perl should be able
to do the right thing and localize $_ properly. But localization of
magic isn't a field I'm comfortable in ;- so I will probably apply this
patch with a regression test if nobody speaks up in the next days.

(PS. I wouldn't trust this patch for 5.8.1 at this point.)

From @chipdude

According to Rafael Garcia-Suarez​:

Anyway this is a suboptimal fix ; I think that perl should be able
to do the right thing and localize $_ properly.

I believe that it *is* localizing $_ properly (modulo the error
message which you have so helpfully fixed; thank you).

Magical values are always localized in magical fashion (i.e. paying
attention to what magic they have). Otherwise how could these work​:

  { local $/; <slurp> }
  local $SIG{INT} = ...
  local $ENV{TERM} = ...

When $_ is aliased to $1, C<local $_> should do the same level of
magic as C<local $1>. Since the latter fails, so should the former.
--
Chip Salzenberg - a.k.a. - <chip@​pobox.com>
"I wanted to play hopscotch with the impenetrable mystery of existence,
  but he stepped in a wormhole and had to go in early." // MST3K

From @rgs

Chip Salzenberg wrote​:

According to Rafael Garcia-Suarez​:

Anyway this is a suboptimal fix ; I think that perl should be able
to do the right thing and localize $_ properly.

I believe that it *is* localizing $_ properly (modulo the error
message which you have so helpfully fixed; thank you).

Magical values are always localized in magical fashion (i.e. paying
attention to what magic they have). Otherwise how could these work​:

{ local $/; <slurp> }
local $SIG{INT} = ...
local $ENV{TERM} = ...

When $_ is aliased to $1, C<local $_> should do the same level of
magic as C<local $1>. Since the latter fails, so should the former.

Makes sense, since I remarked that my patch fixes also :

$ perl5.8.0 -wle 'local $1=1;print $1'
Use of uninitialized value in print at -e line 1.

$ bleadperl -e 'local $1=1'
Modification of a read-only value attempted at -e line 1.

('bleadperl' being an alias to my patched perl)

From @hvds

Rafael Garcia-Suarez <rgarciasuarez@​free.fr> wrote​:
:Chip Salzenberg wrote​:
:> According to Rafael Garcia-Suarez​:
:> > Anyway this is a suboptimal fix ; I think that perl should be able
:> > to do the right thing and localize $_ properly.
:>
:> I believe that it *is* localizing $_ properly (modulo the error
:> message which you have so helpfully fixed; thank you).
:>
:> Magical values are always localized in magical fashion (i.e. paying
:> attention to what magic they have). Otherwise how could these work​:
:>
:> { local $/; <slurp> }
:> local $SIG{INT} = ...
:> local $ENV{TERM} = ...
:>
:> When $_ is aliased to $1, C<local $_> should do the same level of
:> magic as C<local $1>. Since the latter fails, so should the former.
:
:Makes sense, since I remarked that my patch fixes also :
:
:$ perl5.8.0 -wle 'local $1=1;print $1'
:Use of uninitialized value in print at -e line 1.
:
:$ bleadperl -e 'local $1=1'
:Modification of a read-only value attempted at -e line 1.
:
:('bleadperl' being an alias to my patched perl)

Hmm, is SvREADONLY ever set on user-domain non-magical items? I wonder
if there is a danger that the patch will catch more than it should.

ugo

From ctriv@dyndns.org

On Mon, 4 Aug 2003 hv@​crypt.org wrote​:

Hmm, is SvREADONLY ever set on user-domain non-magical items? I wonder
if there is a danger that the patch will catch more than it should.

Readonly​::XS on CPAN does exactly that.

--
Chris Reinhardt
ctriv@​dyndns.org
Systems Architect
Dynamic DNS Network Services
http​://www.dyndns.org/

From @chipdude

According to Hugo van der Sanden​:

Rafael Garcia-Suarez <rgarciasuarez@​free.fr> wrote​:
:Chip Salzenberg wrote​:
:> Magical values are always localized in magical fashion (i.e. paying
:> attention to what magic they have). Otherwise how could these work​:
:> { local $/; <slurp> }
:> local $SIG{INT} = ...
:> local $ENV{TERM} = ...
:
:$ bleadperl -e 'local $1=1'
:Modification of a read-only value attempted at -e line 1.

Hmm, is SvREADONLY ever set on user-domain non-magical items? I wonder
if there is a danger that the patch will catch more than it should.

Any attempt to localize a user-set-readonly value should produce the
same results as localizing $1. So the patch is correct almost by
definition.
--
Chip Salzenberg - a.k.a. - <chip@​pobox.com>
"I wanted to play hopscotch with the impenetrable mystery of existence,
  but he stepped in a wormhole and had to go in early." // MST3K

From @rgs

hv@​crypt.org wrote​:

:Makes sense, since I remarked that my patch fixes also :
:
:$ perl5.8.0 -wle 'local $1=1;print $1'
:Use of uninitialized value in print at -e line 1.
:
:$ bleadperl -e 'local $1=1'
:Modification of a read-only value attempted at -e line 1.
:
:('bleadperl' being an alias to my patched perl)

Hmm, is SvREADONLY ever set on user-domain non-magical items? I wonder
if there is a danger that the patch will catch more than it should.

That should be safe, the line I changed is included in an if() statement :

  if (SvTYPE(osv) >= SVt_PVMG && SvMAGIC(osv) && SvTYPE(osv) != SVt_PVGV) {

And with my patch :

$ bleadperl -MScalar​::Util=readonly -wle 'for (1) { print readonly $_; local $_ = 2; print ; print readonly $_ }'
8388608
2
0

From @rgs

Chip Salzenberg wrote​:

Any attempt to localize a user-set-readonly value should produce the
same results as localizing $1. So the patch is correct almost by
definition.

Er, if you want that C<for(1){local $_=2}> croaks, another patch is needed.
And this will break the Backward Compatibility (for no good reason IMHO?).
My patch applies only to magic SVs, because it makes sense to link the
SvREADONLY flag with the absence of a 'set' magic method.

From @chipdude

According to Rafael Garcia-Suarez​:

That should be safe, the line I changed is included in an if() statement :
if (SvTYPE(osv) >= SVt_PVMG && SvMAGIC(osv) && SvTYPE(osv) != SVt_PVGV) {

Oh, yeah. Darn, I missed that possibility. Glad the patch works.
--
Chip Salzenberg - a.k.a. - <chip@​pobox.com>
"I wanted to play hopscotch with the impenetrable mystery of existence,
  but he stepped in a wormhole and had to go in early." // MST3K

From @rgs

Chip Salzenberg wrote​:

According to Rafael Garcia-Suarez​:

That should be safe, the line I changed is included in an if() statement :
if (SvTYPE(osv) >= SVt_PVMG && SvMAGIC(osv) && SvTYPE(osv) != SVt_PVGV) {

Oh, yeah. Darn, I missed that possibility. Glad the patch works.

After popular approval, I thanks-applied my patch as #20479. I even
provided regression tests (after having asked for them politely).

@rgs - Status changed from 'new' to 'resolved'