In perl.git, the branch smoke-me/padconst has been created
<http://perl5.git.perl.org/perl.git/commitdiff/39e8e695a253bd1a4dc5ea023266a1f5a1e26bad?hp=0000000000000000000000000000000000000000>
at 39e8e695a253bd1a4dc5ea023266a1f5a1e26bad (commit)
- Log -----------------------------------------------------------------
commit 39e8e695a253bd1a4dc5ea023266a1f5a1e26bad
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 33f7e999aa522b874114d86ad6c75533acf2dae1
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 419eb821cccbd6ba9bef7278fcd3aa83991f40e4
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 e06f7c00959d919830d744698644c6802ffc52dc
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 b44c3d9d727ac910151bd1d6d317733d0ba72519
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 b39404c3013eb5f199f1ee38361b8e114eca7251
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 7080baf31af07e8cdc7fd689b0e9f67f3d6f1268
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 d3ed2e4ae704cacc008e2ab05ef889b19e66df8c
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 d0fe699c7567132c94539cc0ea6dd1e596ff0111
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 f9508d174d790e2b8f6c09f0413fead93d8d9213
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 76b01c45f37ced2f24a7c33fe42c72ee6e6ef24b
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 d033ec06e2e1bb01bbc8832b221e38c104284cf1
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 5ba574acd1aa046b52e6e954e4238395319bacec
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 1396f3ff07bc51fb5ec46d37487df2a5eb92659c
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 003edd6ca31b6ad15b4c19867870fd4b917cc593
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 a853f4d24d872d672f400bbf3330a7bfece08bc7
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 8d09dd75b8b7593fc0e2f6f5a03ed0be8d1cb1dc
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 f24a5d1f46c04680c671d0f204d46b8819d5b702
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 other 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