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. --- scope.c | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'scope.c') diff --git a/scope.c b/scope.c index dfaab806aa..7da26a48fe 100644 --- a/scope.c +++ b/scope.c @@ -330,6 +330,17 @@ Perl_save_gp(pTHX_ GV *gv, I32 empty) { PERL_ARGS_ASSERT_SAVE_GP; + /* XXX For now, we just upgrade any coderef in the stash to a full GV + during localisation. Maybe at some point we could make localis- + ation work without needing the upgrade. (In which case our + callers should probably call a different function, not save_gp.) + */ + if (!isGV(gv)) { + assert(isGV_or_RVCV(gv)); + (void)CvGV(SvRV((SV *)gv)); /* CvGV does the upgrade */ + assert(isGV(gv)); + } + save_pushptrptr(SvREFCNT_inc(gv), GvGP(gv), SAVEt_GP); if (empty) { -- cgit v1.2.1