Eliminate opASSIGN macro usage from core

This macro is defined as

    (PL_op->op_flags & OPf_STACKED)

and indicates, for ops which support it, that the mutator-variant of the
op is present (e.g. $x += 1).

This macro was mainly used as an arg for the old-style overloading
macros (tryAMAGICbin()) which were eliminated several years ago.

This commit removes its vestigial usage, and instead tests OPf_STACKED
directly at each location, along with adding a comment about the
significance of the flag.

This removes one item of obfuscation from the overloading code.

There is one potentially functional change in this commit:
Perl_try_amagic_bin() was sometimes testing for OPf_STACKED without
first checking that it had been called with the AMGf_assign flag (which
indicates that this op supports a mutator variant). With this commit, it
now checks first, so this is theoretically a bug fix. In practice that
section of code was never reached without AMGf_assign always being set
anyway.

diff --git a/gv.c b/gv.c
index 798c3ae..f7ffbfa 100644 (file)
--- a/gv.c
+++ b/gv.c
@@ -2998,8 +2998,12 @@ Perl_try_amagic_bin(pTHX_ int method, int flags) {
        SvGETMAGIC(right);
 
     if (SvAMAGIC(left) || SvAMAGIC(right)) {
-       SV * const tmpsv = amagic_call(left, right, method,
-                   ((flags & AMGf_assign) && opASSIGN ? AMGf_assign: 0)
+       SV * tmpsv;
+        /* STACKED implies mutator variant, e.g. $x += 1 */
+        bool mutator = (flags & AMGf_assign) && (PL_op->op_flags & OPf_STACKED);
+
+       tmpsv = amagic_call(left, right, method,
+                   (mutator ? AMGf_assign: 0)
                  | (flags & AMGf_numarg));
        if (tmpsv) {
            if (flags & AMGf_set) {
@@ -3009,7 +3013,7 @@ Perl_try_amagic_bin(pTHX_ int method, int flags) {
            else {
                dATARGET;
                (void)POPs;
-               if (opASSIGN || SvPADMY(TARG)) {
+               if (mutator || SvPADMY(TARG)) {
                    sv_setsv(TARG, tmpsv);
                    SETTARG;
                }

diff --git a/op.h b/op.h
index 6d9dae8..c9f05b2 100644 (file)
--- a/op.h
+++ b/op.h
@@ -99,7 +99,12 @@ Deprecated.  Use C<GIMME_V> instead.
 #define OPf_REF                16      /* Certified reference. */
                                /*  (Return container, not containee). */
 #define OPf_MOD                32      /* Will modify (lvalue). */
+
 #define OPf_STACKED    64      /* Some arg is arriving on the stack. */
+                                /*   Indicates mutator-variant of op for those
+                                 *     ops which support them, e.g. $x += 1
+                                 */
+
 #define OPf_SPECIAL    128     /* Do something weird for this op: */
                                /*  On local LVAL, don't init local value. */
                                /*  On OP_SORT, subroutine is inlined. */

diff --git a/pod/perlhacktips.pod b/pod/perlhacktips.pod
index 7ec2b40..d5c34dd 100644 (file)
--- a/pod/perlhacktips.pod
+++ b/pod/perlhacktips.pod
@@ -915,7 +915,7 @@ Lots of junk will go past as gdb reads in the relevant source files and
 libraries, and then:
 
     Breakpoint 1, Perl_pp_add () at pp_hot.c:309
-    309         dSP; dATARGET; tryAMAGICbin(add,opASSIGN);
+    1396    dSP; dATARGET; bool useleft; SV *svl, *svr;
     (gdb) step
     311           dPOPTOPnnrl_ul;
     (gdb)

diff --git a/pod/perlinterp.pod b/pod/perlinterp.pod
index 2d70737..b516bad 100644 (file)
--- a/pod/perlinterp.pod
+++ b/pod/perlinterp.pod
@@ -781,26 +781,27 @@ See L<perlguts/"Localizing changes"> for how to use the save stack.
 One thing you'll notice about the Perl source is that it's full of
 macros. Some have called the pervasive use of macros the hardest thing
 to understand, others find it adds to clarity. Let's take an example,
-the code which implements the addition operator:
+a stripped-down version the code which implements the addition operator:
 
    1  PP(pp_add)
    2  {
-   3      dSP; dATARGET; tryAMAGICbin(add,opASSIGN);
-   4      {
-   5        dPOPTOPnnrl_ul;
-   6        SETn( left + right );
-   7        RETURN;
-   8      }
-   9  }
+   3      dSP; dATARGET;
+   4      tryAMAGICbin_MG(add_amg, AMGf_assign|AMGf_numeric);
+   5      {
+   6        dPOPTOPnnrl_ul;
+   7        SETn( left + right );
+   8        RETURN;
+   9      }
+  10  }
 
 Every line here (apart from the braces, of course) contains a macro.
 The first line sets up the function declaration as Perl expects for PP
 code; line 3 sets up variable declarations for the argument stack and
-the target, the return value of the operation. Finally, it tries to see
+the target, the return value of the operation. Line 4 tries to see
 if the addition operation is overloaded; if so, the appropriate
 subroutine is called.
 
-Line 5 is another variable declaration - all variable declarations
+Line 6 is another variable declaration - all variable declarations
 start with C<d> - which pops from the top of the argument stack two NVs
 (hence C<nn>) and puts them into the variables C<right> and C<left>,
 hence the C<rl>. These are the two operands to the addition operator.

diff --git a/pp.h b/pp.h
index 55efa0b..61e26c5 100644 (file)
--- a/pp.h
+++ b/pp.h
@@ -553,7 +553,7 @@ Does not use C<TARG>.  See also C<L</XPUSHu>>, C<L</mPUSHu>> and C<L</PUSHu>>.
 
 #define AMGf_noright   1
 #define AMGf_noleft    2
-#define AMGf_assign    4
+#define AMGf_assign    4       /* op supports mutator variant, e.g. $x += 1 */
 #define AMGf_unary     8
 #define AMGf_numeric   0x10    /* for Perl_try_amagic_bin */
 #define AMGf_set       0x20    /* for Perl_try_amagic_bin */
@@ -608,7 +608,7 @@ Does not use C<TARG>.  See also C<L</XPUSHu>>, C<L</mPUSHu>> and C<L</PUSHu>>.
             else { /* AMGf_want_scalar */                       \
                 dATARGET; /* just use the arg's location */     \
                 sv_setsv(TARG, tmpsv);                          \
-                if (opASSIGN)                                   \
+                if (PL_op->op_flags & OPf_STACKED)              \
                     sp--;                                       \
                 SETTARG;                                        \
             }                                                   \
@@ -634,6 +634,7 @@ Does not use C<TARG>.  See also C<L</XPUSHu>>, C<L</mPUSHu>> and C<L</PUSHu>>.
     } STMT_END
 
 
+/* 2019: no longer used in core */
 #define opASSIGN (PL_op->op_flags & OPf_STACKED)
 
 /*