summaryrefslogtreecommitdiff
path: root/pp.c
diff options
context:
space:
mode:
authorDavid Mitchell <davem@iabyn.com>2016-03-20 17:12:13 +0000
committerDavid Mitchell <davem@iabyn.com>2016-03-24 13:32:47 +0000
commit1921e03146ca6022defa6af5267c4dd20c0ca699 (patch)
treea5b4c68425acce9ba1e07784f39151f98298d06e /pp.c
parentcc4d3128555c2fbf5af7fc75854461cd87502812 (diff)
downloadperl-1921e03146ca6022defa6af5267c4dd20c0ca699.tar.gz
stop lc() etc accidentally modifying in-place.
As an optimisation, [ul]c() and [ul]cfirst() sometimes modify their argument in-place rather than returning a modified copy. This should only be done when there is no possibility that the arg is going to be reused. However, this fails: use List::Util qw{ first }; my %hash = ( ASD => 1, ZXC => 2, QWE => 3, TYU => 4); print first { lc $_ eq 'qwe' } keys %hash; which prints "qwe" rather than "QWE". Bascally everything in perl that sets $_ or $a/$b and calls a code block or function, such as map, grep, for and, sort, either copies any PADTMPs, turns off SvTEMP, and/or bumps the reference count. List::Util doesn't do this, and it is likely that other CPAN modules which do "set $_ and call a block" don't either. This has been failing since 5.20.0: perl has been in-placing if the arg is (SvTEMP && RC==1 && !mg) (due to v5.19.7-112-g5cd5e2d). Make the optimisation critera stricter by always copying SvTEMPs. It still allows the optimisation if the arg is a PADTMP - I don't know whether this is unsafe too. Perhaps we can think of something better after 5.24?
Diffstat (limited to 'pp.c')
-rw-r--r--pp.c14
1 files changed, 3 insertions, 11 deletions
diff --git a/pp.c b/pp.c
index 4fcc57736f..4a2cde05e0 100644
--- a/pp.c
+++ b/pp.c
@@ -3875,10 +3875,7 @@ PP(pp_ucfirst)
/* We may be able to get away with changing only the first character, in
* place, but not if read-only, etc. Later we may discover more reasons to
* not convert in-place. */
- inplace = !SvREADONLY(source)
- && ( SvPADTMP(source)
- || ( SvTEMP(source) && !SvSMAGICAL(source)
- && SvREFCNT(source) == 1));
+ inplace = !SvREADONLY(source) && SvPADTMP(source);
/* First calculate what the changed first character should be. This affects
* whether we can just swap it out, leaving the rest of the string unchanged,
@@ -4118,9 +4115,7 @@ PP(pp_uc)
SvGETMAGIC(source);
- if ((SvPADTMP(source)
- ||
- (SvTEMP(source) && !SvSMAGICAL(source) && SvREFCNT(source) == 1))
+ if ( SvPADTMP(source)
&& !SvREADONLY(source) && SvPOK(source)
&& !DO_UTF8(source)
&& (
@@ -4377,10 +4372,7 @@ PP(pp_lc)
SvGETMAGIC(source);
- if ( ( SvPADTMP(source)
- || ( SvTEMP(source) && !SvSMAGICAL(source)
- && SvREFCNT(source) == 1 )
- )
+ if ( SvPADTMP(source)
&& !SvREADONLY(source) && SvPOK(source)
&& !DO_UTF8(source)) {