Use a static inline function instead of macro body for SvCANEXISTDELETE

The macro body needed two variables to use as temporaries. This meant
lots of callsites with apparently-unused variables. Worse, an
unsuspecting author might use this macro and accidentally corrupt
existing variables called `mg` or `stash` in subtle hard-to-find bugs.

Since we can now use static inline functions this is much neater moved
into such a function, avoiding the risk of breaking those variables.

Author: leonerd

inline.h

@@ -1177,6 +1177,20 @@ Perl_rpp_invoke_xs(pTHX_ CV *cv)
 }
 
 
+/* for SvCANEXISTDELETE() macro in pp.h */
+PERL_STATIC_INLINE bool
+Perl_sv_can_existdelete(pTHX_ SV *sv)
+{
+    /* Anything without tie magic is fine */
+    MAGIC *mg;
+    if(!SvRMAGICAL(sv) || !(mg = mg_find(sv, PERL_MAGIC_tied)))
+        return true;
+
+    HV *stash = SvSTASH(SvRV(SvTIED_obj(sv, mg)));
+    return stash &&
+        gv_fetchmethod_autoload(stash, "EXISTS", TRUE) &&
+        gv_fetchmethod_autoload(stash, "DELETE", TRUE);
+}
 
 
 /* ----------------------------- regexp.h ----------------------------- */

pp.c

@@ -5349,9 +5349,6 @@ PP(pp_aslice)
         bool can_preserve = FALSE;
 
         if (localizing) {
-            MAGIC *mg;
-            HV *stash;
-
             can_preserve = SvCANEXISTDELETE(av);
         }
 
@@ -5571,8 +5568,6 @@ S_do_delete_local(pTHX)
 {
     dSP;
     const U8 gimme = GIMME_V;
-    const MAGIC *mg;
-    HV *stash;
     const bool sliced = cBOOL(PL_op->op_private & OPpSLICE);
     SV **unsliced_keysv = sliced ? NULL : sp--;
     SV * const osv = POPs;
@@ -5874,9 +5869,6 @@ PP(pp_hslice)
     bool can_preserve = FALSE;
 
     if (localizing) {
-        MAGIC *mg;
-        HV *stash;
-
         if (SvCANEXISTDELETE(hv))
             can_preserve = TRUE;
     }
@@ -6546,9 +6538,6 @@ PP_wrapped(pp_reverse, 0, 1)
             if (SvMAGICAL(av)) {
                 SSize_t i, j;
                 SV *tmp = sv_newmortal();
-                /* For SvCANEXISTDELETE */
-                HV *stash;
-                const MAGIC *mg;
                 bool can_preserve = SvCANEXISTDELETE(av);
 
                 for (i = 0, j = av_top_index(av); i < j; ++i, --j) {
@@ -7490,8 +7479,6 @@ PP(pp_refassign)
     case SVt_PVAV:
         assert(key);
         if (UNLIKELY(PL_op->op_private & OPpLVAL_INTRO)) {
-            MAGIC *mg;
-            HV *stash;
             S_localise_aelem_lval(aTHX_ (AV *)left, key,
                                         SvCANEXISTDELETE(left));
         }
@@ -7500,8 +7487,6 @@ PP(pp_refassign)
     case SVt_PVHV:
         if (UNLIKELY(PL_op->op_private & OPpLVAL_INTRO)) {
             assert(key);
-            MAGIC *mg;
-            HV *stash;
             S_localise_helem_lval(aTHX_ (HV *)left, key,
                                         SvCANEXISTDELETE(left));
         }
@@ -7538,8 +7523,6 @@ PP_wrapped(pp_lvref,
         mg->mg_flags |= MGf_PERSIST;
     if (UNLIKELY(PL_op->op_private & OPpLVAL_INTRO)) {
       if (elem) {
-        MAGIC *mg;
-        HV *stash;
         assert(arg);
         {
             const bool can_preserve = SvCANEXISTDELETE(arg);
@@ -7568,8 +7551,6 @@ PP_wrapped(pp_lvrefslice, 0, 1)
     bool can_preserve = FALSE;
 
     if (UNLIKELY(localizing)) {
-        MAGIC *mg;
-        HV *stash;
         SV **svp;
 
         can_preserve = SvCANEXISTDELETE(av);

pp.h

@@ -701,14 +701,7 @@ True if this op will be the return value of an lvalue subroutine
 =cut */
 #define LVRET ((PL_op->op_private & OPpMAYBE_LVSUB) && is_lvalue_sub())
 
-#define SvCANEXISTDELETE(sv) \
- (!SvRMAGICAL(sv)            \
-  || !(mg = mg_find((const SV *) sv, PERL_MAGIC_tied))           \
-  || (   (stash = SvSTASH(SvRV(SvTIED_obj(MUTABLE_SV(sv), mg)))) \
-      && gv_fetchmethod_autoload(stash, "EXISTS", TRUE)          \
-      && gv_fetchmethod_autoload(stash, "DELETE", TRUE)          \
-     )                       \
-  )
+#define SvCANEXISTDELETE(sv) Perl_sv_can_existdelete(aTHX_ MUTABLE_SV(sv))
 
 #ifdef PERL_CORE
 

pp_hot.c

@@ -4356,9 +4356,6 @@ PP(pp_helem)
     }
 
     if (localizing) {
-        MAGIC *mg;
-        HV *stash;
-
         /* Try to preserve the existence of a tied hash
          * element by using EXISTS and DELETE if possible.
          * Fall back to FETCH and STORE otherwise. */
@@ -4604,9 +4601,6 @@ PP(pp_multideref)
                     SV** svp;
 
                     if (UNLIKELY(localizing)) {
-                        MAGIC *mg;
-                        HV *stash;
-
                         /* Try to preserve the existence of a tied array
                          * element by using EXISTS and DELETE if possible.
                          * Fall back to FETCH and STORE otherwise. */
@@ -4797,9 +4791,6 @@ PP(pp_multideref)
                     HE* he;
 
                     if (UNLIKELY(localizing)) {
-                        MAGIC *mg;
-                        HV *stash;
-
                         /* Try to preserve the existence of a tied hash
                          * element by using EXISTS and DELETE if possible.
                          * Fall back to FETCH and STORE otherwise. */
@@ -6652,9 +6643,6 @@ PP(pp_aelem)
     }
 
     if (UNLIKELY(localizing)) {
-        MAGIC *mg;
-        HV *stash;
-
         /* Try to preserve the existence of a tied array
          * element by using EXISTS and DELETE if possible.
          * Fall back to FETCH and STORE otherwise. */