From 6881372e19f63014452bb62329f9954deb042b2e Mon Sep 17 00:00:00 2001 From: Father Chrysostomos Date: Thu, 21 Sep 2017 07:06:05 -0700 Subject: [perl #129916] Allow sub-in-stash outside of main MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sub-in-stash optimization introduced in 2eaf799e only applied to subs in the main stash, not in other stashes, due to a problem with the logic in newATTRSUB. This comment: Also, we may be called from load_module at run time, so PL_curstash (which sets CvSTASH) may not point to the stash the sub is stored in. explains why we need the PL_curstash != CopSTASH(PL_curcop) check. (Perl_load_module will fail without it.) But that logic does not work properly at compile time (when PL_curcop == &PL_compiling). The value of CopSTASH(&PL_compiling) is never actually used. It is always set to the main stash. So if we check that PL_curstash != CopSTASH(PL_curcop) and forego the optimization in that case, we will never optimize subs outside of the main stash. What we really need is to check IN_PERL_RUNTIME && PL_curstash != opSTASH(PL_curcop). I.e., forego the optimization at run time if the stashes differ. That is what this commit implements. One observable side effect of this change is that deleting a stash element no longer anonymizes the CV if the CV had no GV that it was depending on to provide its name. Since the main thing in such situa- tions is that we do not get a crash, I think this change (arguably an improvement) is acceptable.) ----------- A bit of explanation of various other changes: gv.c:require_tie_mod needed a bit of help, since it could not handle sub refs in stashes. To keep localisation of stash elements working the same way, local($Stash::{foo}) now upgrades a coderef to a full GV before the localisation. (Changes in two pp*.c files and in scope.c:save_gp.) t/op/stash.t contains a test that makes sure that perl does not crash when a GV with a CV pointing to it gets deleted. This commit tweaks the test so that it continues to test that. (There has to be a GV for the test to test what it is meant to test.) Similarly with t/uni/caller.t and t/uni/stash.t. op.c:rv2cv_op_cv with the _MAYBE_NAME_GV flag was returning the cal- ling GV in those cases where a GV-less sub is called via a GV. E.g., *main = \&Foo::foo; main(). This meant that errors like ‘Not enough arguments’ were giving the wrong sub name. newATTRSUB was not calling mro_method_changed_in when storing a sub as an RV. gv_init needs to arrange for the new GV to have the file and line num- ber corresponding to the sub in it. These are taken from CvSTART, which may be off by a few lines, but is the closest we have to the place the sub was declared. --- t/uni/caller.t | 4 ++++ t/uni/stash.t | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) (limited to 't/uni') diff --git a/t/uni/caller.t b/t/uni/caller.t index de314b0a31..c48018c1ee 100644 --- a/t/uni/caller.t +++ b/t/uni/caller.t @@ -26,6 +26,9 @@ sub { @c = caller(0) } -> (); # Bug 20020517.003 (#9367), used to dump core sub ｆｏｏ { @c = caller(0) } +# The subroutine only gets anonymised if it is relying on a real GV +# for its name. +() = *{"ｆｏｏ"}; # with quotes so that the op tree doesn’t reference the GV my $fooref = delete $ｍａｉｎ::{ｆｏｏ}; $fooref -> (); ::is( $c[3], "ｍａｉｎ::__ANON__", "deleted subroutine name" ); @@ -55,6 +58,7 @@ sub { ｆ() } -> (); ::ok( $c[4], "hasargs true with anon sub" ); sub ｆｏｏ2 { ｆ() } +() = *{"ｆｏｏ2"}; # see ｆｏｏ notes above my $fooref2 = delete $ｍａｉｎ::{ｆｏｏ2}; $fooref2 -> (); ::is( $c[3], "ｍａｉｎ::__ANON__", "deleted subroutine name" ); diff --git a/t/uni/stash.t b/t/uni/stash.t index 31d6c9d9b2..e329faab25 100644 --- a/t/uni/stash.t +++ b/t/uni/stash.t @@ -170,7 +170,7 @@ plan( tests => 49 ); package ＦŌŌ３; sub 남えㄉ {}; my $anon = sub {}; - my $남えㄉ = eval q[\&남えㄉ]; + my $남えㄉ = eval q[*남えㄉ{CODE}]; # not \&남えㄉ; need a real GV package main; delete $ＦŌŌ３::{남えㄉ}; # make named anonymous -- cgit v1.2.1