From 72876cce4ecc7d8756e00d284e32df0b943d0da9 Mon Sep 17 00:00:00 2001 From: David Mitchell Date: Mon, 4 Feb 2019 13:48:13 +0000 Subject: [PATCH] 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. --- gv.c | 10 +++++++--- op.h | 5 +++++ pod/perlhacktips.pod | 2 +- pod/perlinterp.pod | 21 +++++++++++---------- pp.h | 5 +++-- 5 files changed, 27 insertions(+), 16 deletions(-) diff --git a/gv.c b/gv.c index 798c3ae..f7ffbfa 100644 --- 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 --- a/op.h +++ b/op.h @@ -99,7 +99,12 @@ Deprecated. Use C 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 --- 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 --- a/pod/perlinterp.pod +++ b/pod/perlinterp.pod @@ -781,26 +781,27 @@ See L 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 - which pops from the top of the argument stack two NVs (hence C) and puts them into the variables C and C, hence the C. These are the two operands to the addition operator. diff --git a/pp.h b/pp.h index 55efa0b..61e26c5 100644 --- a/pp.h +++ b/pp.h @@ -553,7 +553,7 @@ Does not use C. See also C>, C> and C>. #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. See also C>, C> and C>. 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. See also C>, C> and C>. } STMT_END +/* 2019: no longer used in core */ #define opASSIGN (PL_op->op_flags & OPf_STACKED) /* -- 1.8.3.1