In perl.git, the branch smoke-me/padconst has been created
<http://perl5.git.perl.org/perl.git/commitdiff/e65ec16eff2f80a45912353eee14cde71a2bb6ac?hp=0000000000000000000000000000000000000000>
at e65ec16eff2f80a45912353eee14cde71a2bb6ac (commit)
- Log -----------------------------------------------------------------
commit e65ec16eff2f80a45912353eee14cde71a2bb6ac
Author: Father Chrysostomos <[email protected]>
Date: Thu Jul 25 00:32:49 2013 -0700
Fix up IO::Compress tests
They were expecting \"abc" to return a constant value, but constant
folding no longer causes \ to return a read-only scalar.
M cpan/IO-Compress/t/01misc.t
M cpan/IO-Compress/t/compress/oneshot.pl
commit 26894d310fc5650827ac3c9ba3073234c1dbe0fc
Author: Father Chrysostomos <[email protected]>
Date: Wed Jul 24 11:00:08 2013 -0700
Increase $XS::APItest::VERSION to 0.55
M ext/XS-APItest/APItest.pm
commit 1e0fd57605d3f0b1d807188949759e5f9f40c4c4
Author: Father Chrysostomos <[email protected]>
Date: Wed Jul 24 08:39:01 2013 -0700
Stop op freeing from interfering with sub(){42} mutability
The problem is that when an OP_CONST is freed, it calls pad_swipe,
which clears the PADTMP flag when removing the SV from the pad. Since
PADTMP no longer necessarily means âin a padâ, we shouldnât turn
this flag off.
M pad.c
M t/op/sub.t
commit ef21b30f136fcd7720822bd6963d23c9aa69c41a
Author: Father Chrysostomos <[email protected]>
Date: Wed Jul 24 08:35:46 2013 -0700
fresh_perl.t: Make the test for #3066 more explicit
This test was added to make sure that constants donât become undefined
as a result of being shared between ops.
What was tested, though, was a side-effect, and not the actual bug
itself.
This behaviour has changed (sub(){42} now returns a mutable val-
ue), so this test needs to change, too. It was only passing under
ithreads, and only as the result of another bug, which the next com-
mit will fix.
M t/run/fresh_perl.t
commit 8e28940a245b4ad0dff713650c6e06c2fa4be408
Author: Father Chrysostomos <[email protected]>
Date: Tue Jul 2 19:53:05 2013 -0700
Make overloaded constants always read-only
They were not read-only if the constant handler returned a shared
hash key scalar.
This was brought up in bug #109744.
M lib/overload.t
M op.c
commit c68035540f9889cf2cc4f28863de64fa65cf4632
Author: Father Chrysostomos <[email protected]>
Date: Tue Jul 2 18:29:43 2013 -0700
Donât check IS_PADCONST in pad.c:pad_alloc
Since recent commits have given constants &PL_sv_no names, this check
is redundant, since any slots for constants will have been skipped
over by the sv != &PL_sv_undef check just above.
M pad.c
commit 58eb978761abd8f0d63c15e9ca4cda623cc127ef
Author: Father Chrysostomos <[email protected]>
Date: Tue Jul 2 18:25:48 2013 -0700
pad.c: Donât copy shared hash key targets when cloning
When creating a new thread, donât treat shared hash key targets as
constants. (See the previous commit for explanation.)
This should cause no change in behaviour, because the new target will
not be in use, and its next use will immediately overwrite its value.
It just saves having to copy a string that will be overwritten.
M pad.c
commit a446b6f10f5d86931969926f658ae8f7998b5a90
Author: Father Chrysostomos <[email protected]>
Date: Tue Jul 2 18:18:13 2013 -0700
Stop shared hash key TARGs from being shared
A CV has a list of pads containing a different pad for each recursion
level. pad_push, which adds a new pad to the list, copies some items
from pad no. 1 but not others. In particular op targets¹ are created
anew, but, under ithreads, constants² are not. Historically, these
were distinguished from each other by flags on the value. Any read-
only or shared hash key scalar was considered to be a constant, and
hence shared between pads. The target returned by ref() is a shared
hash key scalar, so it was being shared between recursion levels.
Recent commits have made it possible to distinguish between constants
and targets based on the name. Formerly, both were nameless pad
entries. Now constants have zero-length names (PadnamePV(name) &&
!PadnameLEN(name)). So we can use that to fix this bug.
¹ Many operators return the same scalar each time, for efficiency.
This is stored in the pad, and is known as the target, or TARG.
² Constanst are stored in the pad under ithreads, instead of being
attached directly to ops, as they are under non-threaded builds.
M pad.c
M t/op/sub.t
commit 9a73d45015b65a9cb8e4f302abc65613c9a9a4f3
Author: Father Chrysostomos <[email protected]>
Date: Sun Jun 30 20:26:34 2013 -0700
[perl #79908] Stop sub inlining from breaking closures
When a closure closes over a variable, it references the variable
itself, as opposed to taking a snapshot of its value.
This was broken by the constant optimisation added for
constant.pmâs sake:
{
my $x;
sub () { $x }; # takes a snapshot of the current value of $x
}
constant.pm no longer uses that mechanism, except on older perls, so
we can remove this hack, causing code like this this to start work-
ing again:
BEGIN{
my $x = 5;
*foo = sub(){$x};
$x = 6
}
print foo; # now prints 6, not 5
M embed.fnc
M embed.h
M op.c
M pad.c
M proto.h
M t/op/sub.t
commit f0c1538cbbe18a0a2fb47eb1f70f8934f8109d87
Author: Father Chrysostomos <[email protected]>
Date: Sun Jun 30 14:51:37 2013 -0700
Make sub(){42} return a mutable value
But only make it do so in lvalue context. This will be just as fast
in true rvalue context. In either case, it is still inlined.
This makes sub () { 42 } and sub () { return 42 } do the same thing.
It also means that sub () { '-'x75 } reverts back to returning a muta-
ble value, the way it did in 5.16. From now on, tweaks to constant
folding will no longer affect the mutability of the return value of a
nullary function.
âuse constantâ is unaffected. It still returns read-only values.
This was brought up in ticket #109744.
M op.c
M t/op/sub.t
commit 21f39391f4d8eee9b5ba4b260918e8c2e44d95cf
Author: Father Chrysostomos <[email protected]>
Date: Sun Jun 30 00:40:58 2013 -0700
Tweak optree_constants.t for inlined list consts
M ext/B/t/optree_constants.t
commit 10e9e186e960fab8793f4525ad0fa832a32df296
Author: Father Chrysostomos <[email protected]>
Date: Sun Jun 30 00:26:46 2013 -0700
Update constant.pm to reflect list inlinement
I also removed âsome symbols may be redefined without generating a
warningâ. I know not to what it refers. It has been there as long as
constant.pm has. If a constant is clobbered by another constant with
the same value, there is no warning, as it is harmless. That may be
to what it refers, but we donât need a caveat for that.
M dist/constant/lib/constant.pm
commit 9bb5f49feea8dd64af33c65b106a20f2fe6a0e2d
Author: Father Chrysostomos <[email protected]>
Date: Sun Jun 30 00:20:33 2013 -0700
Inline list constants
These are inlined the same way as 1..5. We have two ops:
rv2av
|
`-- const
The const op returns an AV, which is stored in the op tree, and then
rv2av flattens it.
M embed.fnc
M embed.h
M op.c
M proto.h
M toke.c
commit 07db20b809c1f2392493f7027f995f060844df06
Author: Father Chrysostomos <[email protected]>
Date: Sat Jun 29 23:59:48 2013 -0700
constant.pm: Make list constants read-only
Here we take advantage of the array-ref-stash-elem mechanism added in
the previous commit, which causes the actual elements of the stored
array to be pushed on to the stack.
M dist/constant/lib/constant.pm
M dist/constant/t/constant.t
commit ba8577db57fe72e341032397fa4ed17fd69a12d8
Author: Father Chrysostomos <[email protected]>
Date: Sat Jun 29 23:33:14 2013 -0700
Allow stash elems to be array refs
These turn into CVs that return the contents of the array. Future
commits will make constant.pm use these and also make them inlinable.
Even without inlining, these subs are faster, because they are XSUBs:
$ time ./perl -Ilib -e 'my @a=1..1000000; sub foo { @a } () = foo for 1..10'
real 0m3.725s
user 0m3.407s
sys 0m0.227s
$ time ./perl -Ilib -e 'my @a=1..1000000; BEGIN { $::{foo} = \@a } () = foo
for 1..10'
real 0m2.153s
user 0m1.949s
sys 0m0.121s
M gv.c
M op.c
M pod/perldiag.pod
M t/op/gv.t
M t/uni/gv.t
commit b0dd022779725503f89e0f6dce483c1b9bde8290
Author: Father Chrysostomos <[email protected]>
Date: Fri Jun 28 18:25:28 2013 -0700
Increase $constant::VERSION to 1.28
M dist/constant/lib/constant.pm
commit 7fbfec1bce4852043ef2872fff2f0d568dccf222
Author: Father Chrysostomos <[email protected]>
Date: Fri Jun 28 01:27:48 2013 -0700
Stop constant.pm from (ab)using subs for scalars
under versions that support $::{foo} = \1.
This changes no behaviour. Future commits will change the way
sub(){42} and sub(){$forty_two} work. (The former will return a muta-
ble value; the latter will not be inlined at all.)
M dist/constant/lib/constant.pm
commit 70656cc052fa18bb53aa4d5aaed509f5387e6d6f
Author: Father Chrysostomos <[email protected]>
Date: Thu Jun 27 18:11:48 2013 -0700
[perl #78194] Make re-evals copy PADTMPs
So that \$_ == \$_ will be true in "$foo$bar" =~ /(?{...})/.
Many ops return the same scalar each time, for efficiency; refgen (\)
knows about that and copies them, to hide the implementation detail,
but constructs, such as regular expression code blocks in this case,
need to do the same thing.
M regexec.c
M t/re/rxcode.t
commit 13dd5023f7017376a988cd049de7794efedcac79
Author: Father Chrysostomos <[email protected]>
Date: Thu Jun 27 14:37:14 2013 -0700
[perl #78194] Make sort copy PADTMPs
Copy PADTMPs (op return values) when there is a sort block/sub that is
not optimised away and we are not sorting in place, so that \$a == \$a
will return true.
Many ops return the same scalar each time, for efficiency; refgen (\)
knows about that and copies them, to hide the implementation detail,
but other ops (sort in this case) need to do the same thing.
M pp_sort.c
M t/op/sort.t
commit 88831f9bf1f2937c06b8a168f0ef5355b9399248
Author: Father Chrysostomos <[email protected]>
Date: Mon Jun 24 21:28:36 2013 -0700
[perl #78194] Make x copy PADTMPs in lv cx
So that \(("$a")x2) will give two references to the same scalar, the
way that \(($a)x2) does.
M pp.c
M t/op/repeat.t
commit e01dc5ebf15e267a549e809bc92fca3a17a837e0
Author: Father Chrysostomos <[email protected]>
Date: Mon Jun 24 20:16:30 2013 -0700
[perl #78194] Make list slices copy PADTMPs in lv cx
So that slices that reference the same value multiple times (such as
(...)[1,1]) will exhibit referential identity (\(...)[1,1] will return
two references to the same scalar).
M pp.c
M t/op/list.t
commit 470a94e911be0964eccd9a5160caae395e7cc1a9
Author: Father Chrysostomos <[email protected]>
Date: Sun Jun 23 07:08:24 2013 -0700
Stop folding of ops from changing mutability
If $a+$b produces a mutable value, then so should 1+2.
M ext/Devel-Peek/t/Peek.t
M op.c
M t/comp/fold.t
M t/op/readline.t
M t/op/vec.t
M t/uni/readline.t
commit 2a2b52b9928e647c6ed21da0cb483dcd7008c111
Author: Father Chrysostomos <[email protected]>
Date: Sun Jun 23 07:06:49 2013 -0700
[perl #78194] Make foreach copy pad tmps
before aliasing them to $_.
This caused one to-do test in sub.t to pass, but the bug it is testing
for has not been fixed, so I added another one. I didnât remove the
to-do test that started passing, because it is still a good test to
have (the only test we have for interactions betwen foreach, shared
hash keys, and recursion).
M pp_hot.c
M t/cmd/for.t
M t/op/sub.t
commit b3775c462efa6aa8447a0c86e75e18abf9ffa3b0
Author: Father Chrysostomos <[email protected]>
Date: Sat Jun 22 23:04:13 2013 -0700
Correct list.t descr in MANIFEST
We donât use the term âarrayâ for lists any more. Also, mention
slices, since list slices are tested here.
M MANIFEST
commit 4b9e12f1df0ac090ccb86b1ab2a0da2d1a67162c
Author: Father Chrysostomos <[email protected]>
Date: Sat Jun 22 18:46:00 2013 -0700
[perl #3105] Make 1..3 modification safe
This construct is optimised at compile time to an anonymous array with
an implicit @{} around it if both arguments are constant. Modifying
elements of that array produces wrong results the next time the same
code is executed.
If we mark each element of the array as PADTMP, then it will be
treated like an operatorâs return value (which it is) and get copied
as appropriate.
M op.c
M t/op/range.t
M t/op/svleak.t
commit 84bfc1fb645868c33c1f8abb3269573141d170fc
Author: Father Chrysostomos <[email protected]>
Date: Sat Jun 22 00:26:12 2013 -0700
constant.t: Correct version
I didnât have this done in time for 5.19.1.
M dist/constant/t/constant.t
commit 1b87ae8a9c777a1ad2fbe8251be9dc1bad9ebad4
Author: Father Chrysostomos <[email protected]>
Date: Thu Jun 20 14:32:15 2013 -0700
[perl #78194] Make sub calls copy pad tmps
before aliasing them to elements of @_.
M ext/Devel-Peek/t/Peek.t
M pp_hot.c
M t/op/sub.t
M t/op/tie.t
commit 74b60d89bbce5b986e09b72d6f1fd84a75e9bc44
Author: Father Chrysostomos <[email protected]>
Date: Thu Jun 20 14:13:20 2013 -0700
Increase $OptreeCheck::VERSION to 0.10
though I still donât understand why it has a version at all.
M ext/B/t/OptreeCheck.pm
commit 04003124f62fa5f629842db7802fb655600d416d
Author: Father Chrysostomos <[email protected]>
Date: Thu Jun 20 06:04:59 2013 -0700
[perl #78194] Make grep/map copy pad tmps
before aliasing them to $_.
And make sure the copies go back on the stack for grep, since modi-
fying $_ in the grep block or expression is supposed to modify the
item returned.
M pp_ctl.c
M pp_hot.c
M t/op/grep.t
commit ec5e64f0db04fcfddfd64edb198704552b4b4aa5
Author: Father Chrysostomos <[email protected]>
Date: Thu Jun 20 05:25:01 2013 -0700
pad.c: cast before comparing signed with unsigned
M pad.c
commit 4815f62622139f13e34a6756073702454a64774b
Author: Father Chrysostomos <[email protected]>
Date: Tue Jun 18 20:34:21 2013 -0700
op.c: Stop copying constants under ithreads
This fixes bugs #21979, #89188, #109746, #114838 and #115388 and
mostly fixes #109744 and #105906 (other issues still remain in those
two tickets).
Because the PADTMP flag was doing double duty, indicating that a
pad slot was in use in addition to indicating a target, constants
could not be shared between pad slots, as freeing one const op (and
turning of its PADTMP flag in pad_free) would mark some other pad
slot as free.
I believe this may have been fixed already by change 3b1c21fabed,
which made const ops use pad_swipe (which removes the SV from the
pad) instead of pad_free (which marks it as available for reuse). But
the copying still happens.
In any case, as of the previous commit, whether a pad slot for a con-
stant is in use is now stored in the pad name. Slots in use for const
ops now have &PL_sv_no names.
So there is no longer any reason to copy the constants.
The difference can be observed thus:
Before:
$ ./perl -lIlib -MDevel::Peek -e 'sub foo(){42} Dump foo; Dump foo'
SV = IV(0x7f94ea02ef10) at 0x7f94ea02ef20
REFCNT = 2
FLAGS = (PADTMP,IOK,READONLY,pIOK)
IV = 42
SV = IV(0x7f94ea02eeb0) at 0x7f94ea02eec0
REFCNT = 1
FLAGS = (PADTMP,IOK,READONLY,pIOK)
IV = 42
After:
$ ./perl -lIlib -MDevel::Peek -e 'sub foo(){42} Dump foo; Dump foo'
SV = IV(0x7f894882ef10) at 0x7f894882ef20
REFCNT = 3
FLAGS = (IOK,READONLY,pIOK)
IV = 42
SV = IV(0x7f894882ef10) at 0x7f894882ef20
REFCNT = 3
FLAGS = (IOK,READONLY,pIOK)
IV = 42
Notice the different addresses.
There are still special cases for copying &PL_sv_undef, which I need
to tackle.
Since most constants created by âuse constantâ have the PADMY flag on
(since they reside in lexical variables inside constant.pm) and PADMY
and PADTMP are exclusive, I have stop turning on PADTMP for constants.
It is no longer necessary now, since before its purpose was to mark
pad entries as being in use. That means many to-do tests pass.
M dist/constant/t/constant.t
M ext/B/t/OptreeCheck.pm
M ext/B/t/optree_constants.t
M op.c
M pad.c
M t/op/not.t
M t/op/ref.t
commit 47fb65f0f5c27003ba867ac4944a9d8ca66c94a3
Author: Father Chrysostomos <[email protected]>
Date: Tue Jun 18 16:51:57 2013 -0700
pad.c: Expand pad_push SVf_READONLY explanation
M pad.c
commit 4074839c490cccea01797c87314bb30442f2a6be
Author: Father Chrysostomos <[email protected]>
Date: Sun Jun 16 14:00:01 2013 -0700
pad.c: Use &PL_sv_no for const pad names
Currently &PL_sv_undef as a pad name can indicate either a free slot
available for use by pad_alloc or a slot used by an op target (or,
under ithreads, a constant or GV).
Currently pad_alloc distinguishes between free slots and unnamed slots
based on whether the value in the pad has PADMY or PADTMP set. If
neither is set, then the slot is free. If either is set, the slot
is in use.
This makes it rather difficult to distinguish between constants stored
in the pad (under ithreads) and targets. The latter need to be copied
when referenced, to give the impression that a new scalar is being
returned by an operator each time. (So \"$a" has to return a refer-
ence to a new scalar each time, but \1 should return the same one.)
Also, constants are shared between recursion levels. Currently, if
the value is marked READONLY or is a shared hash key scalar, it is
shared. But targets can also me shared hash keys, resulting in bugs.
It also makes it impossible for the same constant to be shared by mul-
tiple pad slots, as freeing one const op will turn off the PADTMP flag
while the other slot still uses it, making the latter appear to be
free. Hence a lot of copying occurs under ithreads. (Actually, that
may not be true any more since 3b1c21fabed, as freed const ops swipe
their constants from the pad. But right now, a lot of copying does
still happen.)
Also, XS modules may want to create const ops that return the same
mutable SV each time. That is currently not possible without
various workarounds including custom ops and references. (See
<https://rt.perl.org/rt3/Ticket/Display.html?id=105906#txn-1075354>.)
This commit changes pad_alloc and pad_free to use &PL_sv_no for con-
stants and updates other code to keep all tests passing. Subsequent
commits will actually use that information to fix bugs.
This will probably break PadWalker, but I think it is an acceptable
trade-off. The alternative would be to make PadnamePV forever more
complex than necessary, by giving it a special case for &PL_sv_no and
having it return NULL.
I gave PadnameLEN a special case for &PL_sv_undef, so it may appear
that I have simply shifted the complexity around. But if pad names
stop being SVs, then this exception will simply disappear, since the
global &PL_padname_undef will have 0 in its length field.
M ext/XS-APItest/APItest.xs
M op.c
M pad.c
M pad.h
M perl.c
M perly.c
commit 2ef69ef9841f205f20885a0e6668f6271fb47166
Author: Father Chrysostomos <[email protected]>
Date: Sun Jun 16 12:40:05 2013 -0700
Re(mov|writ)e two comments from pad.c:pad_alloc
The thing about "foreach" index vars was added in bbce6d697 (insepar-
able changes from patch from perl5.003_08 to perl5.003_09, presuma-
bly the âLexical scoping cleanupâ part). It is not valid, because
âforeachâ doesnât aliases a pad entry to a non-pad (not marked PADMY
or PADTMP) value until run time, and pad_alloc happens at compile
time. The real reason we need this loop is that entries that close
over unavailable variables are not marked PADMY. That may have been a
mistake, but it works because of this loop. The reason for the loop
also may have changed over time.
The comment about copying to sv is not valid, because it is used later
on in the same condition when compared to &PL_sv_undef. It was added
in commit dd2155a49b.
M pad.c
commit fc3cce251c7c56d1df41d16f4121b75a7d927595
Author: Father Chrysostomos <[email protected]>
Date: Sat Jun 15 23:17:59 2013 -0700
op.c:S_fold_constants: Add assertion
This code correctly handles a value returned by a folded constant that
is a target or a mortal.
If it is neither, then it takes ownership of a reference count (with-
out doing SvREFCNT_inc), so it ends up sharing a reference count with
whatever owned it before. That is only safe to do with immortals,
which is (afaict) the only other type of scalar that can get through
this code, so it is actually correct.
Changes elsewhere could easily break this, though, so add an
assertion.
M op.c
commit cb5ce88533e405a4c0c7324879bc130c3bd985ee
Author: Father Chrysostomos <[email protected]>
Date: Sat Jun 15 21:52:07 2013 -0700
Change the tests for #3105 into to-dos
instead of testing for the incorrect behaviour
M t/op/range.t
commit b039623ec43e24adb031374ada357a4dd6f4758a
Author: Father Chrysostomos <[email protected]>
Date: Sat Jun 15 20:41:49 2013 -0700
Test readonliness of overload constants
including one to-do test
M lib/overload.t
commit c7b286d4985f899455c5fd53470bb8736d411f63
Author: Father Chrysostomos <[email protected]>
Date: Sat Jun 15 20:29:16 2013 -0700
To-do test for #109746
M t/op/ref.t
commit 83a428073f5397211d6b1746fc3c998c0ccd0104
Author: Father Chrysostomos <[email protected]>
Date: Sat Jun 15 20:24:55 2013 -0700
Test (im)mutability of constants and constant-like subs
including many to-do tests
M dist/constant/t/constant.t
M t/op/sub.t
commit c732edd0c592af1caeaae7842cc916fb7923a9e3
Author: Father Chrysostomos <[email protected]>
Date: Sat Jun 15 19:42:35 2013 -0700
Test !0 and !1 immutability and singletonness
The latter (for bug #114838) is a to-do test under ithreads.
M t/op/not.t
commit 627e4f22adae0976e797602acc289d775032e5c1
Author: Father Chrysostomos <[email protected]>
Date: Sat Jun 15 19:29:27 2013 -0700
Test that literal numbers and strings are read-only
including ${\3}, which currently fails under ithreads (and is hence a
to-do test).
M t/op/ref.t
commit 0340b30e065c61654244ff3ad1522bb6ee38206f
Author: Father Chrysostomos <[email protected]>
Date: Sat Jun 15 19:14:14 2013 -0700
To-do tests for perl #78194
plus a regular (not to-do) test for an lvalue sub case that already
works properly.
M t/cmd/for.t
M t/op/grep.t
M t/op/list.t
M t/op/repeat.t
M t/op/sort.t
M t/op/sub.t
M t/op/sub_lval.t
M t/op/tie.t
M t/re/rxcode.t
commit 194e36c072ba0ab7b06c30fee0d6591670995d17
Author: Father Chrysostomos <[email protected]>
Date: Sat Jun 15 11:57:56 2013 -0700
ref.t: To-do test for retvals of folded ops
M t/comp/fold.t
commit d5bebcaa979b4ac541c6df6bbc4f92bb9db32b38
Author: Father Chrysostomos <[email protected]>
Date: Sat Jun 15 11:41:57 2013 -0700
sub.t: To-do test for recursive shared-hash-keys TARGs
This is only buggy under ithreads.
sub a {
for (${\""}.${\""}) {
$_ = $_[0] || __PACKAGE__;
print "$_\n";
a("road") unless $_[0];
print "$_\n";
}
}
a();
The outer call sets the scalar returned by ${\""}.${\""} to the cur-
rent package name.
The inner call sets it to "road".
Each call prints it twice, the outer call surrounding the inner call.
The output in 5.10-5.18 is:
main
road
road
road
because the inner call is clobbering the same scalar. If __PACKAGE__
is changed to "main", it works, and prints
main
road
road
main
(as the script above also prints in 5.8.8).
M t/op/sub.t
-----------------------------------------------------------------------
--
Perl5 Master Repository