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

no assignment to "my" variable #16655

Closed
p5pRT opened this issue Aug 11, 2018 · 8 comments
Closed

no assignment to "my" variable #16655

p5pRT opened this issue Aug 11, 2018 · 8 comments

Comments

@p5pRT
Copy link
Collaborator

@p5pRT p5pRT commented Aug 11, 2018

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

Searchable as RT133441$

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Aug 11, 2018

From wolf-dietrich_moeller@t-online.de

The following error is a regression in Perl 5.28.
No assignment to the "my" variable $m, but correct
assignment to $t.

my $m = 'TEST_my';
$t = 'TEST___';
$k = 'k';
($m = '??'.$k) .= ' m';
($t = '??'.$k) .= ' t';
print $m,"\n",$t,"\n";

output for Perl 5.26.2.1 (correct)

??k m
??k t

output for Perl 5.28.0.1 (no assignment in line 4 to $m)

TEST_my
??k t
Perl Info
-----------------------------------------------------------------
---
Flags:
category=core
severity=medium
---
Site configuration information for perl 5.28.0:

Configured by strawberry-perl at Sat Jun 23 11:55:17 2018.

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

Platform:
osname=MSWin32
osvers=10.0.17134.112
archname=MSWin32-x86-multi-thread-64int
uname='Win32 strawberry-perl 5.28.0.1 #1 Sat Jun 23 11:54:23 2018 i386'
config_args='undef'
hint=recommended
useposix=true
d_sigaction=undef
useithreads=define
usemultiplicity=define
use64bitint=define
use64bitall=undef
uselongdouble=undef
usemymalloc=n
default_inc_excludes_dot=define
bincompat5005=undef
Compiler:
cc='gcc'
ccflags =' -s -O2 -DWIN32 -D__USE_MINGW_ANSI_STDIO
-DPERL_TEXTMODE_SCRIPTS -DPERL_IMPLICIT_CONTEXT -DPERL_IMPLICIT_SYS
-DUSE_PERLIO -fwrapv -fno-strict-aliasing -mms-bitfields'
optimize='-s -O2'
cppflags='-DWIN32'
ccversion=''
gccversion='7.1.0'
gccosandvers=''
intsize=4
longsize=4
ptrsize=4
doublesize=8
byteorder=12345678
doublekind=3
d_longlong=define
longlongsize=8
d_longdbl=define
longdblsize=12
longdblkind=3
ivtype='long long'
ivsize=8
nvtype='double'
nvsize=8
Off_t='long long'
lseeksize=8
alignbytes=8
prototype=define
Linker and Libraries:
ld='g++'
ldflags ='-s -L"C:\Perl\perl\lib\CORE" -L"C:\Perl\c\lib"'
libpth=C:\Perl\c\lib C:\Perl\c\i686-w64-mingw32\lib
C:\Perl\c\lib\gcc\i686-w64-mingw32\7.1.0
libs= -lmoldname -lkernel32 -luser32 -lgdi32 -lwinspool -lcomdlg32
-ladvapi32 -lshell32 -lole32 -loleaut32 -lnetapi32 -luuid -lws2_32 -lmpr
-lwinmm -lversion -lodbc32 -lodbccp32 -lcomctl32
perllibs= -lmoldname -lkernel32 -luser32 -lgdi32 -lwinspool -lcomdlg32
-ladvapi32 -lshell32 -lole32 -loleaut32 -lnetapi32 -luuid -lws2_32 -lmpr
-lwinmm -lversion -lodbc32 -lodbccp32 -lcomctl32
libc=
so=dll
useshrplib=true
libperl=libperl528.a
gnulibc_version=''
Dynamic Linking:
dlsrc=dl_win32.xs
dlext=xs.dll
d_dlsymun=undef
ccdlflags=' '
cccdlflags=' '
lddlflags='-mdll -s -L"C:\Perl\perl\lib\CORE" -L"C:\Perl\c\lib"'


---
@INC for perl 5.28.0:
C:/Perl/perl/site/lib
C:/Perl/perl/vendor/lib
C:/Perl/perl/lib

---
Environment for perl 5.28.0:
HOME (unset)
LANG (unset)
LANGUAGE (unset)
LD_LIBRARY_PATH (unset)
LOGDIR (unset)
PATH=C:\Perl\c\bin;C:\Perl\perl\site\bin;C:\Perl\perl\bin ## some
entries deleted
PERL_BADLANG (unset)
SHELL (unset)
@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Aug 11, 2018

From @iabyn

On Sat, Aug 11, 2018 at 01​:11​:15PM -0700, Wolf-Dietrich Moeller (via RT) wrote​:

### test program ###
my $m = 'TEST_my';
$t = 'TEST___';
$k = 'k';
($m = '??'.$k) .= ' m';
($t = '??'.$k) .= ' t';
print $m,"\n",$t,"\n";
### end test program

### output for Perl 5.26.2.1 (correct)
??k m
??k t
### end

### output for Perl 5.28.0.1 (no assignment in line 4 to $m)
TEST_my
??k t
### end

The optimiser is misinterpreting the optree in the ($m = '??'.$k) .= ' m'
case. I'll look into it further on monday

--
O Unicef Clearasil!
Gibberish and Drivel!
  -- "Bored of the Rings"

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Aug 11, 2018

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

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Aug 29, 2018

From @iabyn

On Sat, Aug 11, 2018 at 11​:03​:34PM +0100, Dave Mitchell wrote​:

On Sat, Aug 11, 2018 at 01​:11​:15PM -0700, Wolf-Dietrich Moeller (via RT) wrote​:

### test program ###
my $m = 'TEST_my';
$t = 'TEST___';
$k = 'k';
($m = '??'.$k) .= ' m';
($t = '??'.$k) .= ' t';
print $m,"\n",$t,"\n";
### end test program

### output for Perl 5.26.2.1 (correct)
??k m
??k t
### end

### output for Perl 5.28.0.1 (no assignment in line 4 to $m)
TEST_my
??k t
### end

The optimiser is misinterpreting the optree in the ($m = '??'.$k) .= ' m'
case. I'll look into it further on monday

Now fixe din blead with the following commit. This is suitable for
backporting to 5.28.x.

commit 0fe04e1
Author​: David Mitchell <davem@​iabyn.com>
AuthorDate​: Wed Aug 29 14​:32​:24 2018 +0100
Commit​: David Mitchell <davem@​iabyn.com>
CommitDate​: Wed Aug 29 14​:32​:24 2018 +0100

  multiconcat​: mutator not seen in (lex = ...) .= ...
 
  RT #133441
 
  TL;DR​:
  (($lex = expr1.expr2) .= expr3) was being misinterpreted as
  (expr1 . expr2 . expr3) when the ($lex = expr1) subtree had had the
  assign op optimised away by the OPpTARGET_MY optimisation.
 
  Full details.
 
  S_maybe_multiconcat() looks for suitable chains of OP_CONCAT to convert
  into a single OP_MULTICONCAT.
 
  Part of the code needs to distinguish between (expr . expr) and
  (expr .= expr). This didn't used to be easy, as both are just OP_CONCAT
  ops, but with the OPf_STACKED set on the second one. But...
 
  perl also used to optimise ($a . $b . $c) into ($a . $b) .= $c, to
  reuse the padtmp returned by the $a.$b concat. This meant that an
  OP_CONCAT could have the OPf_STACKED flag on even when it was a '.'
  rather than a '.='.
 
  I disambiguated these cases by seeing whether the top op in the LHS
  expression had the OPf_MOD flag set too - if so, it implies '.='.
 
  This fails in the specific case where the LHS expression is a
  sub-expression which is assigned to a lexical variable, e.g.
 
  ($lex = $a+$b) .= $c.
 
  Initially the top node in the LHS expression above is OP_SASSIGN, with
  OPf_MOD set due to the enclosing '.='. Then the OPpTARGET_MY
  optimisation kicks in, and the ($lex = $a + $b) part of the optree is
  converted from
 
  sassign sKPRMS
  add[t4] sK
  padsv[a$] s
  padsv[$b] s
  padsv[$lex] s
 
  to
  add[$lex] sK/TARGMY
  padsv[a$] s
  padsv[$b] s
 
  which is all fine and dandy, except that the top node of that optree no
  longer has the OPf_MOD flag set, which trips up S_maybe_multiconcat into
  no longer spotting that the outer concat is a '.=' rather than a '.'.
 
  Whether the OPpTARGET_MY optimising code should copy the OPf_MOD from
  the being-removed sassign op to its successor is an issue I won't
  address here. But in the meantime, the good news is that for 5.28.0
  I added the OPpCONCAT_NESTED private flag, which is set whenever
  ($a . $b . $c) is optimised into ($a . $b) .= $c. This means that it's
  no longer necessary to inspect the OPf_MOD flag of the first child to
  disambiguate the two cases. So the fix is trivial.

--
Little fly, thy summer's play my thoughtless hand
has terminated with extreme prejudice.
  (with apologies to William Blake)

@p5pRT p5pRT added the Severity Low label Oct 19, 2019
@toddr
Copy link
Member

@toddr toddr commented Jan 31, 2020

This is fixed in 5.30. @jkeenan do we need to label this before closing it?

@jkeenan
Copy link
Contributor

@jkeenan jkeenan commented Jan 31, 2020

This is fixed in 5.30. @jkeenan do we need to label this before closing it?

I'm not sure what label you would want to apply? Can you clarify?

@toddr
Copy link
Member

@toddr toddr commented Jan 31, 2020

I was thinking we need to label it with a mile stone or something so we know where it was fixed?

@jkeenan
Copy link
Contributor

@jkeenan jkeenan commented Jan 31, 2020

@toddr toddr closed this Jan 31, 2020
@toddr toddr added this to the 5.30.0 milestone Jan 31, 2020
@toddr toddr added the affects-5.28 label Jan 31, 2020
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
3 participants
You can’t perform that action at this time.