From 5743f2a37eb9fda061f42b2df6b4c8119b14eaf1 Mon Sep 17 00:00:00 2001 From: Father Chrysostomos Date: Sat, 31 Dec 2011 18:18:57 -0800 Subject: Update method caches for non-void stash elem deletions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit 71be2cb (inseparable changes from patch from perl5.003_13 to perl5.003_14) added code to hv_free_ent to reset method caches if necessary. Later, a bug fix in the deletion code (f50383f5) made it necessary for the value in the HE to be set to &PL_sv_placeholder, so it wouldn’t free the sv just yet. So the method cache code (which by then had changed from using PL_sub_generation to using mro_method_changed_in) got repeated in S_hv_delete_common. The only problem with all this is that hv_free_ent was the wrong place to put it to begin with. If delete is called in non-void context, the sv in it is not freed just yet, but returned; so hv_free_ent was already being called with a HE pointing to &PL_sv_placeholder. Commit f50383f5 only added the mro code for the void case, to avoid changing existing behaviour when rearranging the code. It turns out it needs to be done in S_hv_delete_common uncon- ditionally. Changing a test in t/op/method.t to use ()=delete instead of delete makes it fail, because method caches end up going stale. See the test in the diff. --- hv.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) (limited to 'hv.c') diff --git a/hv.c b/hv.c index 9a088a923a..e1f80400b5 100644 --- a/hv.c +++ b/hv.c @@ -1052,19 +1052,21 @@ S_hv_delete_common(pTHX_ HV *hv, SV *keysv, const char *key, STRLEN klen, if (d_flags & G_DISCARD) { sv = HeVAL(entry); HeVAL(entry) = &PL_sv_placeholder; - if (sv) { - /* deletion of method from stash */ - if (isGV(sv) && isGV_with_GP(sv) && GvCVu(sv) - && HvENAME_get(hv)) - mro_method_changed_in(hv); - SvREFCNT_dec(sv); - sv = NULL; - } } else { sv = sv_2mortal(HeVAL(entry)); HeVAL(entry) = &PL_sv_placeholder; } + if (sv) { + /* deletion of method from stash */ + if (isGV(sv) && isGV_with_GP(sv) && GvCVu(sv) + && HvENAME_get(hv)) + mro_method_changed_in(hv); + if (d_flags & G_DISCARD) { + SvREFCNT_dec(sv); + sv = NULL; + } + } /* * If a restricted hash, rather than really deleting the entry, put -- cgit v1.2.1