Fix an assignment to a lexical variable in multiconcatenation expressions
This commit is contained in:
parent
8401a631d8
commit
e7c58147e0
|
@ -0,0 +1,111 @@
|
||||||
|
From 0fe04e1dc741a43190e79a985fb0cec0493ebfe9 Mon Sep 17 00:00:00 2001
|
||||||
|
From: David Mitchell <davem@iabyn.com>
|
||||||
|
Date: Wed, 29 Aug 2018 14:32:24 +0100
|
||||||
|
Subject: [PATCH] multiconcat: mutator not seen in (lex = ...) .= ...
|
||||||
|
MIME-Version: 1.0
|
||||||
|
Content-Type: text/plain; charset=UTF-8
|
||||||
|
Content-Transfer-Encoding: 8bit
|
||||||
|
|
||||||
|
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.
|
||||||
|
|
||||||
|
Signed-off-by: Petr Písař <ppisar@redhat.com>
|
||||||
|
---
|
||||||
|
op.c | 1 -
|
||||||
|
t/opbasic/concat.t | 10 +++++++++-
|
||||||
|
2 files changed, 9 insertions(+), 2 deletions(-)
|
||||||
|
|
||||||
|
diff --git a/op.c b/op.c
|
||||||
|
index ddeb484b64..d0dcffbecb 100644
|
||||||
|
--- a/op.c
|
||||||
|
+++ b/op.c
|
||||||
|
@@ -2722,7 +2722,6 @@ S_maybe_multiconcat(pTHX_ OP *o)
|
||||||
|
}
|
||||||
|
else if ( topop->op_type == OP_CONCAT
|
||||||
|
&& (topop->op_flags & OPf_STACKED)
|
||||||
|
- && (cUNOPo->op_first->op_flags & OPf_MOD)
|
||||||
|
&& (!(topop->op_private & OPpCONCAT_NESTED))
|
||||||
|
)
|
||||||
|
{
|
||||||
|
diff --git a/t/opbasic/concat.t b/t/opbasic/concat.t
|
||||||
|
index 9ce9722f5c..4b73b22c1c 100644
|
||||||
|
--- a/t/opbasic/concat.t
|
||||||
|
+++ b/t/opbasic/concat.t
|
||||||
|
@@ -39,7 +39,7 @@ sub is {
|
||||||
|
return $ok;
|
||||||
|
}
|
||||||
|
|
||||||
|
-print "1..253\n";
|
||||||
|
+print "1..254\n";
|
||||||
|
|
||||||
|
($a, $b, $c) = qw(foo bar);
|
||||||
|
|
||||||
|
@@ -853,3 +853,11 @@ package RT132595 {
|
||||||
|
my $res = $a.$t.$a.$t;
|
||||||
|
::is($res, "b1c1b1c2", "RT #132595");
|
||||||
|
}
|
||||||
|
+
|
||||||
|
+# RT #133441
|
||||||
|
+# multiconcat wasn't seeing a mutator as a mutator
|
||||||
|
+{
|
||||||
|
+ my ($a, $b) = qw(a b);
|
||||||
|
+ ($a = 'A'.$b) .= 'c';
|
||||||
|
+ is($a, "Abc", "RT #133441");
|
||||||
|
+}
|
||||||
|
--
|
||||||
|
2.14.4
|
||||||
|
|
|
@ -198,6 +198,10 @@ Patch28: perl-5.28.0-Fix-script-run-bug-1-followed-by-Thai-digit.patch
|
||||||
# in upstream after 5.29.1
|
# in upstream after 5.29.1
|
||||||
Patch29: perl-5.29.1-Update-Time-Piece-to-CPAN-version-1.33.patch
|
Patch29: perl-5.29.1-Update-Time-Piece-to-CPAN-version-1.33.patch
|
||||||
|
|
||||||
|
# Fix an assignment to a lexical variable in multiconcatenation expressions,
|
||||||
|
# RT#133441, in upstream after 5.29.2
|
||||||
|
Patch30: perl-5.29.2-multiconcat-mutator-not-seen-in-lex.patch
|
||||||
|
|
||||||
# Link XS modules to libperl.so with EU::CBuilder on Linux, bug #960048
|
# Link XS modules to libperl.so with EU::CBuilder on Linux, bug #960048
|
||||||
Patch200: perl-5.16.3-Link-XS-modules-to-libperl.so-with-EU-CBuilder-on-Li.patch
|
Patch200: perl-5.16.3-Link-XS-modules-to-libperl.so-with-EU-CBuilder-on-Li.patch
|
||||||
|
|
||||||
|
@ -2778,6 +2782,7 @@ Perl extension for Version Objects
|
||||||
%patch27 -p1
|
%patch27 -p1
|
||||||
%patch28 -p1
|
%patch28 -p1
|
||||||
%patch29 -p1
|
%patch29 -p1
|
||||||
|
%patch30 -p1
|
||||||
%patch200 -p1
|
%patch200 -p1
|
||||||
%patch201 -p1
|
%patch201 -p1
|
||||||
|
|
||||||
|
@ -2814,6 +2819,7 @@ perl -x patchlevel.h \
|
||||||
'Fedora Patch27: Fix a time race in Time-HiRes/t/itimer.t test' \
|
'Fedora Patch27: Fix a time race in Time-HiRes/t/itimer.t test' \
|
||||||
'Fedora Patch28: Fix matching an ASCII digit followed by a non-ASCII digit using a script run' \
|
'Fedora Patch28: Fix matching an ASCII digit followed by a non-ASCII digit using a script run' \
|
||||||
'Fedora Patch29: Fix Time::Piece to handle objects in overloaded methods correctly' \
|
'Fedora Patch29: Fix Time::Piece to handle objects in overloaded methods correctly' \
|
||||||
|
'Fedora Patch30: Fix an assignment to a lexical variable in multiconcatenation expressions (RT#133441)' \
|
||||||
'Fedora Patch200: Link XS modules to libperl.so with EU::CBuilder on Linux' \
|
'Fedora Patch200: Link XS modules to libperl.so with EU::CBuilder on Linux' \
|
||||||
'Fedora Patch201: Link XS modules to libperl.so with EU::MM on Linux' \
|
'Fedora Patch201: Link XS modules to libperl.so with EU::MM on Linux' \
|
||||||
%{nil}
|
%{nil}
|
||||||
|
@ -5108,6 +5114,8 @@ popd
|
||||||
- Fix a time race in Time-HiRes/t/itimer.t test
|
- Fix a time race in Time-HiRes/t/itimer.t test
|
||||||
- Fix matching an ASCII digit followed by a non-ASCII digit using a script run
|
- Fix matching an ASCII digit followed by a non-ASCII digit using a script run
|
||||||
- Fix Time::Piece to handle objects in overloaded methods correctly
|
- Fix Time::Piece to handle objects in overloaded methods correctly
|
||||||
|
- Fix an assignment to a lexical variable in multiconcatenation expressions
|
||||||
|
(RT#133441)
|
||||||
|
|
||||||
* Wed Aug 01 2018 Petr Pisar <ppisar@redhat.com> - 4:5.28.0-420
|
* Wed Aug 01 2018 Petr Pisar <ppisar@redhat.com> - 4:5.28.0-420
|
||||||
- Fix a file descriptor leak in in-place edits (RT#133314)
|
- Fix a file descriptor leak in in-place edits (RT#133314)
|
||||||
|
|
Loading…
Reference in New Issue