New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[PATCH 0/4] RFC op-type patchset #14203
Comments
From @jimcThis patchset cleans up op_type and op_ppaddr initialization, using Patch 1 uses CHANGE_TYPE in 49 callsites, ie "normal" init. TODO: CHANGE_TYPE would properly apply in a few more spots; perl.c could use it once, if it were hoisted into op.h. op.c could use it 2x, but for a macro/sequence-point issue with ++(o->op_type). pp.c could use it after patching OP_I_MODULO, but its a kinda special |
From @jimcThis patchset cleans up op_type and op_ppaddr initialization, using Patch 1 uses CHANGE_TYPE in 49 callsites. TODO: CHANGE_TYPE would properly apply in a few more spots; |
From @jimcCHANGE_TYPE has been around for 4 years, since 23a8159b759. Use it to op.c | 147 +++++++++++++++++++++++-------------------------------------------- Inline Patchdiff --git a/op.c b/op.c
index 397e3f1..dbb2886 100644
--- a/op.c
+++ b/op.c
@@ -1011,8 +1011,7 @@ Perl_op_null(pTHX_ OP *o)
return;
op_clear(o);
o->op_targ = o->op_type;
- o->op_type = OP_NULL;
- o->op_ppaddr = PL_ppaddr[OP_NULL];
+ CHANGE_TYPE(o, OP_NULL);
}
void
@@ -1771,23 +1770,19 @@ Perl_scalarvoid(pTHX_ OP *o)
break;
case OP_POSTINC:
- o->op_type = OP_PREINC; /* pre-increment is faster */
- o->op_ppaddr = PL_ppaddr[OP_PREINC];
+ CHANGE_TYPE(o, OP_PREINC); /* pre-increment is faster */
break;
case OP_POSTDEC:
- o->op_type = OP_PREDEC; /* pre-decrement is faster */
- o->op_ppaddr = PL_ppaddr[OP_PREDEC];
+ CHANGE_TYPE(o, OP_PREDEC); /* pre-decrement is faster */
break;
case OP_I_POSTINC:
- o->op_type = OP_I_PREINC; /* pre-increment is faster */
- o->op_ppaddr = PL_ppaddr[OP_I_PREINC];
+ CHANGE_TYPE(o, OP_I_PREINC); /* pre-increment is faster */
break;
case OP_I_POSTDEC:
- o->op_type = OP_I_PREDEC; /* pre-decrement is faster */
- o->op_ppaddr = PL_ppaddr[OP_I_PREDEC];
+ CHANGE_TYPE(o, OP_I_PREDEC); /* pre-decrement is faster */
break;
case OP_SASSIGN: {
@@ -1844,11 +1839,9 @@ Perl_scalarvoid(pTHX_ OP *o)
if (kid->op_type == OP_NOT
&& (kid->op_flags & OPf_KIDS)) {
if (o->op_type == OP_AND) {
- o->op_type = OP_OR;
- o->op_ppaddr = PL_ppaddr[OP_OR];
+ CHANGE_TYPE(o, OP_OR);
} else {
- o->op_type = OP_AND;
- o->op_ppaddr = PL_ppaddr[OP_AND];
+ CHANGE_TYPE(o, OP_AND);
}
op_null(kid);
}
@@ -2401,8 +2394,7 @@ S_lvref(pTHX_ OP *o, I32 type)
return;
}
slurpy:
- o->op_type = OP_LVAVREF;
- o->op_ppaddr = PL_ppaddr[OP_LVAVREF];
+ CHANGE_TYPE(o, OP_LVAVREF);
o->op_private &= OPpLVAL_INTRO|OPpPAD_STATE;
o->op_flags |= OPf_MOD|OPf_REF;
return;
@@ -2459,8 +2451,7 @@ S_lvref(pTHX_ OP *o, I32 type)
break;
case OP_ASLICE:
case OP_HSLICE:
- o->op_type = OP_LVREFSLICE;
- o->op_ppaddr = PL_ppaddr[OP_LVREFSLICE];
+ CHANGE_TYPE(o, OP_LVREFSLICE);
o->op_private &= OPpLVAL_INTRO|OPpLVREF_ELEM;
return;
case OP_NULL:
@@ -2493,8 +2484,7 @@ S_lvref(pTHX_ OP *o, I32 type)
PL_op_desc[type]));
return;
}
- o->op_type = OP_LVREF;
- o->op_ppaddr = PL_ppaddr[OP_LVREF];
+ CHANGE_TYPE(o, OP_LVREF);
o->op_private &=
OPpLVAL_INTRO|OPpLVREF_ELEM|OPpLVREF_TYPE|OPpPAD_STATE;
if (type == OP_ENTERLOOP)
@@ -2533,8 +2523,7 @@ Perl_op_lvalue_flags(pTHX_ OP *o, I32 type, U32 flags)
case OP_ENTERSUB:
if ((type == OP_UNDEF || type == OP_REFGEN || type == OP_LOCK) &&
!(o->op_flags & OPf_STACKED)) {
- o->op_type = OP_RV2CV; /* entersub => rv2cv */
- o->op_ppaddr = PL_ppaddr[OP_RV2CV];
+ CHANGE_TYPE(o, OP_RV2CV); /* entersub => rv2cv */
assert(cUNOPo->op_first->op_type == OP_NULL);
op_null(((LISTOP*)cUNOPo->op_first)->op_first);/* disable pushmark */
break;
@@ -2996,9 +2985,8 @@ Perl_doref(pTHX_ OP *o, I32 type, bool set_op_ref)
case OP_ENTERSUB:
if ((type == OP_EXISTS || type == OP_DEFINED) &&
!(o->op_flags & OPf_STACKED)) {
- o->op_type = OP_RV2CV; /* entersub => rv2cv */
- o->op_ppaddr = PL_ppaddr[OP_RV2CV];
- assert(cUNOPo->op_first->op_type == OP_NULL);
+ CHANGE_TYPE(o, OP_RV2CV); /* entersub => rv2cv */
+ assert(cUNOPo->op_first->op_type == OP_NULL);
op_null(((LISTOP*)cUNOPo->op_first)->op_first); /* disable pushmark */
o->op_flags |= OPf_SPECIAL;
}
@@ -3575,13 +3563,11 @@ Perl_op_scope(pTHX_ OP *o)
if (o) {
if (o->op_flags & OPf_PARENS || PERLDB_NOOPT || TAINTING_get) {
o = op_prepend_elem(OP_LINESEQ, newOP(OP_ENTER, 0), o);
- o->op_type = OP_LEAVE;
- o->op_ppaddr = PL_ppaddr[OP_LEAVE];
+ CHANGE_TYPE(o, OP_LEAVE);
}
else if (o->op_type == OP_LINESEQ) {
OP *kid;
- o->op_type = OP_SCOPE;
- o->op_ppaddr = PL_ppaddr[OP_SCOPE];
+ CHANGE_TYPE(o, OP_SCOPE);
kid = ((LISTOP*)o)->op_first;
if (kid->op_type == OP_NEXTSTATE || kid->op_type == OP_DBSTATE) {
op_null(kid);
@@ -4156,8 +4142,7 @@ S_gen_constant_list(pTHX_ OP *o)
Perl_pp_anonlist(aTHX);
PL_tmps_floor = oldtmps_floor;
- o->op_type = OP_RV2AV;
- o->op_ppaddr = PL_ppaddr[OP_RV2AV];
+ CHANGE_TYPE(o, OP_RV2AV);
o->op_flags &= ~OPf_REF; /* treat \(1..2) like an ordinary list */
o->op_flags |= OPf_PARENS; /* and flatten \(1..2,3) */
o->op_opt = 0; /* needs to be revisited in rpeep() */
@@ -4332,8 +4317,7 @@ Perl_op_convert_list(pTHX_ I32 type, I32 flags, OP *o)
}
}
- o->op_type = (OPCODE)type;
- o->op_ppaddr = PL_ppaddr[type];
+ CHANGE_TYPE(o, type);
o->op_flags |= flags;
o = CHECKOP(type, o);
@@ -4418,8 +4402,7 @@ Perl_newLISTOP(pTHX_ I32 type, I32 flags, OP *first, OP *last)
NewOp(1101, listop, 1, LISTOP);
- listop->op_type = (OPCODE)type;
- listop->op_ppaddr = PL_ppaddr[type];
+ CHANGE_TYPE(listop, type);
if (first || last)
flags |= OPf_KIDS;
listop->op_flags = (U8)flags;
@@ -4481,8 +4464,7 @@ Perl_newOP(pTHX_ I32 type, I32 flags)
|| (PL_opargs[type] & OA_CLASS_MASK) == OA_LOOPEXOP);
NewOp(1101, o, 1, OP);
- o->op_type = (OPCODE)type;
- o->op_ppaddr = PL_ppaddr[type];
+ CHANGE_TYPE(o, type);
o->op_flags = (U8)flags;
o->op_next = o;
@@ -4533,8 +4515,7 @@ Perl_newUNOP(pTHX_ I32 type, I32 flags, OP *first)
first = force_list(first, 1);
NewOp(1101, unop, 1, UNOP);
- unop->op_type = (OPCODE)type;
- unop->op_ppaddr = PL_ppaddr[type];
+ CHANGE_TYPE(unop, type);
unop->op_first = first;
unop->op_flags = (U8)(flags | OPf_KIDS);
unop->op_private = (U8)(1 | (flags >> 8));
@@ -4588,8 +4569,7 @@ S_newMETHOP_internal(pTHX_ I32 type, I32 flags, OP* dynamic_meth, SV* const_meth
methop->op_next = (OP*)methop;
}
- methop->op_type = (OPCODE)type;
- methop->op_ppaddr = PL_ppaddr[type];
+ CHANGE_TYPE(methop, type);
methop = (METHOP*) CHECKOP(type, methop);
if (methop->op_next) return (OP*)methop;
@@ -4650,8 +4630,7 @@ Perl_newBINOP(pTHX_ I32 type, I32 flags, OP *first, OP *last)
if (!first)
first = newOP(OP_NULL, 0);
- binop->op_type = (OPCODE)type;
- binop->op_ppaddr = PL_ppaddr[type];
+ CHANGE_TYPE(binop, type);
binop->op_first = first;
binop->op_flags = (U8)(flags | OPf_KIDS);
if (!last) {
@@ -5045,8 +5024,7 @@ Perl_newPMOP(pTHX_ I32 type, I32 flags)
assert((PL_opargs[type] & OA_CLASS_MASK) == OA_PMOP);
NewOp(1101, pmop, 1, PMOP);
- pmop->op_type = (OPCODE)type;
- pmop->op_ppaddr = PL_ppaddr[type];
+ CHANGE_TYPE(pmop, type);
pmop->op_flags = (U8)flags;
pmop->op_private = (U8)(0 | (flags >> 8));
@@ -5493,8 +5471,7 @@ Perl_newSVOP(pTHX_ I32 type, I32 flags, SV *sv)
|| (PL_opargs[type] & OA_CLASS_MASK) == OA_FILESTATOP);
NewOp(1101, svop, 1, SVOP);
- svop->op_type = (OPCODE)type;
- svop->op_ppaddr = PL_ppaddr[type];
+ CHANGE_TYPE(svop, type);
svop->op_sv = sv;
svop->op_next = (OP*)svop;
svop->op_flags = (U8)flags;
@@ -5559,8 +5536,7 @@ Perl_newPADOP(pTHX_ I32 type, I32 flags, SV *sv)
|| (PL_opargs[type] & OA_CLASS_MASK) == OA_FILESTATOP);
NewOp(1101, padop, 1, PADOP);
- padop->op_type = (OPCODE)type;
- padop->op_ppaddr = PL_ppaddr[type];
+ CHANGE_TYPE(padop, type);
padop->op_padix =
pad_alloc(type, isGV(sv) ? SVf_READONLY : SVs_PADTMP);
SvREFCNT_dec(PAD_SVl(padop->op_padix));
@@ -5627,8 +5603,7 @@ Perl_newPVOP(pTHX_ I32 type, I32 flags, char *pv)
|| (PL_opargs[type] & OA_CLASS_MASK) == OA_LOOPEXOP);
NewOp(1101, pvop, 1, PVOP);
- pvop->op_type = (OPCODE)type;
- pvop->op_ppaddr = PL_ppaddr[type];
+ CHANGE_TYPE(pvop, type);
pvop->op_pv = pv;
pvop->op_next = (OP*)pvop;
pvop->op_flags = (U8)flags;
@@ -6388,12 +6363,10 @@ Perl_newSTATEOP(pTHX_ I32 flags, char *label, OP *o)
NewOp(1101, cop, 1, COP);
if (PERLDB_LINE && CopLINE(PL_curcop) && PL_curstash != PL_debstash) {
- cop->op_type = OP_DBSTATE;
- cop->op_ppaddr = PL_ppaddr[ OP_DBSTATE ];
+ CHANGE_TYPE(cop, OP_DBSTATE);
}
else {
- cop->op_type = OP_NEXTSTATE;
- cop->op_ppaddr = PL_ppaddr[ OP_NEXTSTATE ];
+ CHANGE_TYPE(cop, OP_NEXTSTATE);
}
cop->op_flags = (U8)flags;
CopHINTS_set(cop, PL_hints);
@@ -7060,8 +7033,7 @@ Perl_newWHILEOP(pTHX_ I32 flags, I32 debuggable, LOOP *loop,
if (!loop) {
NewOp(1101,loop,1,LOOP);
- loop->op_type = OP_ENTERLOOP;
- loop->op_ppaddr = PL_ppaddr[OP_ENTERLOOP];
+ CHANGE_TYPE(loop, OP_ENTERLOOP);
loop->op_private = 0;
loop->op_next = (OP*)loop;
}
@@ -7120,8 +7092,7 @@ Perl_newFOROP(pTHX_ I32 flags, OP *sv, OP *expr, OP *block, OP *cont)
if (sv) {
if (sv->op_type == OP_RV2SV) { /* symbol table variable */
iterpflags = sv->op_private & OPpOUR_INTRO; /* for our $x () */
- sv->op_type = OP_RV2GV;
- sv->op_ppaddr = PL_ppaddr[OP_RV2GV];
+ CHANGE_TYPE(sv, OP_RV2GV);
/* The op_type check is needed to prevent a possible segfault
* if the loop variable is undeclared and 'strict vars' is in
@@ -8957,14 +8928,12 @@ Perl_oopsAV(pTHX_ OP *o)
switch (o->op_type) {
case OP_PADSV:
case OP_PADHV:
- o->op_type = OP_PADAV;
- o->op_ppaddr = PL_ppaddr[OP_PADAV];
+ CHANGE_TYPE(o, OP_PADAV);
return ref(o, OP_RV2AV);
case OP_RV2SV:
case OP_RV2HV:
- o->op_type = OP_RV2AV;
- o->op_ppaddr = PL_ppaddr[OP_RV2AV];
+ CHANGE_TYPE(o, OP_RV2AV);
ref(o, OP_RV2AV);
break;
@@ -8985,14 +8954,12 @@ Perl_oopsHV(pTHX_ OP *o)
switch (o->op_type) {
case OP_PADSV:
case OP_PADAV:
- o->op_type = OP_PADHV;
- o->op_ppaddr = PL_ppaddr[OP_PADHV];
+ CHANGE_TYPE(o, OP_PADHV);
return ref(o, OP_RV2HV);
case OP_RV2SV:
case OP_RV2AV:
- o->op_type = OP_RV2HV;
- o->op_ppaddr = PL_ppaddr[OP_RV2HV];
+ CHANGE_TYPE(o, OP_RV2HV);
ref(o, OP_RV2HV);
break;
@@ -9011,8 +8978,7 @@ Perl_newAVREF(pTHX_ OP *o)
PERL_ARGS_ASSERT_NEWAVREF;
if (o->op_type == OP_PADANY) {
- o->op_type = OP_PADAV;
- o->op_ppaddr = PL_ppaddr[OP_PADAV];
+ CHANGE_TYPE(o, OP_PADAV);
return o;
}
else if ((o->op_type == OP_RV2AV || o->op_type == OP_PADAV)) {
@@ -9037,8 +9003,7 @@ Perl_newHVREF(pTHX_ OP *o)
PERL_ARGS_ASSERT_NEWHVREF;
if (o->op_type == OP_PADANY) {
- o->op_type = OP_PADHV;
- o->op_ppaddr = PL_ppaddr[OP_PADHV];
+ CHANGE_TYPE(o, OP_PADHV);
return o;
}
else if ((o->op_type == OP_RV2HV || o->op_type == OP_PADHV)) {
@@ -9052,8 +9017,7 @@ Perl_newCVREF(pTHX_ I32 flags, OP *o)
{
if (o->op_type == OP_PADANY) {
dVAR;
- o->op_type = OP_PADCV;
- o->op_ppaddr = PL_ppaddr[OP_PADCV];
+ CHANGE_TYPE(o, OP_PADCV);
}
return newUNOP(OP_RV2CV, flags, scalar(o));
}
@@ -9066,8 +9030,7 @@ Perl_newSVREF(pTHX_ OP *o)
PERL_ARGS_ASSERT_NEWSVREF;
if (o->op_type == OP_PADANY) {
- o->op_type = OP_PADSV;
- o->op_ppaddr = PL_ppaddr[OP_PADSV];
+ CHANGE_TYPE(o, OP_PADSV);
return o;
}
return newUNOP(OP_RV2SV, 0, scalar(o));
@@ -9352,8 +9315,7 @@ Perl_ck_eval(pTHX_ OP *o)
enter->op_next = (OP*)enter;
o = op_prepend_elem(OP_LINESEQ, (OP*)enter, (OP*)kid);
- o->op_type = OP_LEAVETRY;
- o->op_ppaddr = PL_ppaddr[OP_LEAVETRY];
+ CHANGE_TYPE(o, OP_LEAVETRY);
enter->op_other = o;
return o;
}
@@ -10079,12 +10041,10 @@ Perl_ck_smartmatch(pTHX_ OP *o)
/* Implicitly take a reference to a regular expression */
if (first->op_type == OP_MATCH) {
- first->op_type = OP_QR;
- first->op_ppaddr = PL_ppaddr[OP_QR];
+ CHANGE_TYPE(first, OP_QR);
}
if (second->op_type == OP_MATCH) {
- second->op_type = OP_QR;
- second->op_ppaddr = PL_ppaddr[OP_QR];
+ CHANGE_TYPE(second, OP_QR);
}
}
@@ -10145,8 +10105,7 @@ Perl_ck_sassign(pTHX_ OP *o)
OP *const nullop = newCONDOP(0, first, o, other);
OP *const condop = first->op_next;
- condop->op_type = OP_ONCE;
- condop->op_ppaddr = PL_ppaddr[OP_ONCE];
+ CHANGE_TYPE(condop, OP_ONCE);
other->op_targ = target;
/* Store the initializedness of state vars in a separate
@@ -10460,8 +10419,7 @@ Perl_ck_select(pTHX_ OP *o)
if (o->op_flags & OPf_KIDS) {
kid = OP_SIBLING(cLISTOPo->op_first); /* get past pushmark */
if (kid && OP_HAS_SIBLING(kid)) {
- o->op_type = OP_SSELECT;
- o->op_ppaddr = PL_ppaddr[OP_SSELECT];
+ CHANGE_TYPE(o, OP_SSELECT);
o = ck_fun(o);
return fold_constants(op_integerize(op_std_init(o)));
}
@@ -10715,9 +10673,7 @@ Perl_ck_split(pTHX_ OP *o)
kid = pmruntime( newPMOP(OP_MATCH, OPf_SPECIAL), kid, 0, 0);
op_sibling_splice(o, NULL, 0, kid);
}
-
- kid->op_type = OP_PUSHRE;
- kid->op_ppaddr = PL_ppaddr[OP_PUSHRE];
+ CHANGE_TYPE(kid, OP_PUSHRE);
scalar(kid);
if (((PMOP *)kid)->op_pmflags & PMf_GLOBAL) {
Perl_ck_warner(aTHX_ packWARN(WARN_REGEXP),
@@ -12055,8 +12011,7 @@ Perl_rpeep(pTHX_ OP *o)
o->op_flags &=~ OPf_KIDS;
/* stub is a baseop; repeat is a binop */
assert(sizeof(OP) <= sizeof(BINOP));
- o->op_type = OP_STUB;
- o->op_ppaddr = PL_ppaddr[OP_STUB];
+ CHANGE_TYPE(o, OP_STUB);
o->op_private = 0;
break;
}
@@ -12284,8 +12239,7 @@ Perl_rpeep(pTHX_ OP *o)
* *always* formerly a pushmark */
assert(o->op_type == OP_PUSHMARK);
o->op_next = followop;
- o->op_type = OP_PADRANGE;
- o->op_ppaddr = PL_ppaddr[OP_PADRANGE];
+ CHANGE_TYPE(o, OP_PADRANGE);
o->op_targ = base;
/* bit 7: INTRO; bit 6..0: count */
o->op_private = (intro | count);
@@ -12370,8 +12324,7 @@ Perl_rpeep(pTHX_ OP *o)
o->op_private |= o->op_next->op_private & (OPpLVAL_INTRO
| OPpOUR_INTRO);
o->op_next = o->op_next->op_next;
- o->op_type = OP_GVSV;
- o->op_ppaddr = PL_ppaddr[OP_GVSV];
+ CHANGE_TYPE(o, OP_GVSV);
}
}
else if (o->op_next->op_type == OP_READLINE
@@ -12379,9 +12332,8 @@ Perl_rpeep(pTHX_ OP *o)
&& (o->op_next->op_next->op_flags & OPf_STACKED))
{
/* Turn "$a .= <FH>" into an OP_RCATLINE. AMS 20010917 */
- o->op_type = OP_RCATLINE;
+ CHANGE_TYPE(o, OP_RCATLINE);
o->op_flags |= OPf_STACKED;
- o->op_ppaddr = PL_ppaddr[OP_RCATLINE];
op_null(o->op_next->op_next);
op_null(o->op_next);
}
@@ -12682,8 +12634,7 @@ Perl_rpeep(pTHX_ OP *o)
sv_rvweaken(sv);
SvREADONLY_on(sv);
}
- o->op_type = OP_CONST;
- o->op_ppaddr = PL_ppaddr[OP_CONST];
+ CHANGE_TYPE(o, OP_CONST);
o->op_flags |= OPf_SPECIAL;
cSVOPo->op_sv = sv;
}
--
1.8.3.1 |
From @jimcCurrent codebase wires Perl_pp_mapstart to Perl_unimplemented_op (by This is all too clever by half, so this patch undoes the trickery, and I cant glean a reason for this historical arrangement: 65bca31 added Perl_unimplemented_op() used by 3 'unreachable' ops, c78ff97 moved a minimal/DIEing pp_mapstart implementation to f54cb97 did minor tweaks to a full pp_mapstart implementation. I looking at ck-grep(), the code doing [2] is from 22c35a8 in 1998. So anyway, I tried the following, it worked, it seems the historical [1] change regen/opcode.pl generated mapping this sets PL_ppaddr[OP_MAPSTART] = PL_ppaddr[OP_GREPSTART] during [2] Drop re-type-ing of MAPSTARTs as GREPSTARTs by Perl_ck_grep(OP* o) requires make regen op.c | 1 - Inline Patchdiff --git a/op.c b/op.c
index dbb2886..f259e68 100644
--- a/op.c
+++ b/op.c
@@ -9850,7 +9850,6 @@ Perl_ck_grep(pTHX_ OP *o)
PERL_ARGS_ASSERT_CK_GREP;
- o->op_ppaddr = PL_ppaddr[OP_GREPSTART];
/* don't allocate gwop here, as we may leak it if PL_parser->error_count > 0 */
if (o->op_flags & OPf_STACKED) {
diff --git a/opcode.h b/opcode.h
index 40cdc81..c367c09 100644
--- a/opcode.h
+++ b/opcode.h
@@ -44,7 +44,7 @@
#define Perl_pp_keys Perl_do_kv
#define Perl_pp_rv2hv Perl_pp_rv2av
#define Perl_pp_pop Perl_pp_shift
-#define Perl_pp_mapstart Perl_unimplemented_op
+#define Perl_pp_mapstart Perl_pp_grepstart
#define Perl_pp_dor Perl_pp_defined
#define Perl_pp_andassign Perl_pp_and
#define Perl_pp_orassign Perl_pp_or
@@ -1106,7 +1106,7 @@ EXT Perl_ppaddr_t PL_ppaddr[] /* or perlvars.h */
Perl_pp_reverse,
Perl_pp_grepstart,
Perl_pp_grepwhile,
- Perl_pp_mapstart, /* implemented by Perl_unimplemented_op */
+ Perl_pp_mapstart, /* implemented by Perl_pp_grepstart */
Perl_pp_mapwhile,
Perl_pp_range,
Perl_pp_flip,
diff --git a/regen/opcode.pl b/regen/opcode.pl
index b9d7042..df0fbf4 100755
--- a/regen/opcode.pl
+++ b/regen/opcode.pl
@@ -78,7 +78,7 @@ my %alias;
# Format is "this function" => "does these op names"
my @raw_alias = (
Perl_do_kv => [qw( keys values )],
- Perl_unimplemented_op => [qw(padany mapstart custom)],
+ Perl_unimplemented_op => [qw(padany custom)],
# All the ops with a body of { return NORMAL; }
Perl_pp_null => [qw(scalar regcmaybe lineseq scope)],
@@ -135,6 +135,7 @@ my @raw_alias = (
spwent epwent sgrent egrent)],
Perl_pp_shostent => [qw(snetent sprotoent sservent)],
Perl_pp_aelemfast => ['aelemfast_lex'],
+ Perl_pp_grepstart => ['mapstart'],
);
while (my ($func, $names) = splice @raw_alias, 0, 2) {
--
1.8.3.1 |
From @jimcA previous patch promoted CHANGE_TYPE as the "normal" way to It turns out that these are pretty normal too; since regen/opcode.pl op.c | 5 ++--- Inline Patchdiff --git a/op.c b/op.c
index f259e68..de5f93d 100644
--- a/op.c
+++ b/op.c
@@ -12282,15 +12282,14 @@ Perl_rpeep(pTHX_ OP *o)
op_null(pop);
o->op_flags |= pop->op_next->op_flags & OPf_MOD;
o->op_next = pop->op_next->op_next;
- o->op_ppaddr = PL_ppaddr[OP_AELEMFAST];
o->op_private = (U8)i;
if (o->op_type == OP_GV) {
gv = cGVOPo_gv;
GvAVn(gv);
- o->op_type = OP_AELEMFAST;
+ CHANGE_TYPE(o, OP_AELEMFAST);
}
else
- o->op_type = OP_AELEMFAST_LEX;
+ CHANGE_TYPE(o, OP_AELEMFAST_LEX);
}
if (o->op_type != OP_GV)
break;
--
1.8.3.1 |
From @jimcin Perl_rpeep, remove special handling of OP_AELEMFAST_LEX vs OP_AELEMFAST. Because PL_ppaddr[OP_AELEMFAST_LEX] == PL_ppaddr[OP_AELEMFAST] due to op.c | 5 ++--- Inline Patchdiff --git a/op.c b/op.c
index f259e68..de5f93d 100644
--- a/op.c
+++ b/op.c
@@ -12282,15 +12282,14 @@ Perl_rpeep(pTHX_ OP *o)
op_null(pop);
o->op_flags |= pop->op_next->op_flags & OPf_MOD;
o->op_next = pop->op_next->op_next;
- o->op_ppaddr = PL_ppaddr[OP_AELEMFAST];
o->op_private = (U8)i;
if (o->op_type == OP_GV) {
gv = cGVOPo_gv;
GvAVn(gv);
- o->op_type = OP_AELEMFAST;
+ CHANGE_TYPE(o, OP_AELEMFAST);
}
else
- o->op_type = OP_AELEMFAST_LEX;
+ CHANGE_TYPE(o, OP_AELEMFAST_LEX);
}
if (o->op_type != OP_GV)
break;
--
1.8.3.1 |
From @jimcS_alloc_LOGOP inits the op_type field, but not op_ppaddr. So CHANGE_TYPE()s postcondition pertains, lets say so. op.c | 10 +--------- Inline Patchdiff --git a/op.c b/op.c
index de5f93d..79307df 100644
--- a/op.c
+++ b/op.c
@@ -1226,7 +1226,7 @@ S_alloc_LOGOP(pTHX_ I32 type, OP *first, OP* other)
LOGOP *logop;
OP *kid = first;
NewOp(1101, logop, 1, LOGOP);
- logop->op_type = (OPCODE)type;
+ CHANGE_TYPE(logop, type);
logop->op_first = first;
logop->op_other = other;
logop->op_flags = OPf_KIDS;
@@ -5366,7 +5366,6 @@ Perl_pmruntime(pTHX_ OP *o, OP *expr, bool isreg, I32 floor)
}
rcop = S_alloc_LOGOP(aTHX_ OP_REGCOMP, scalar(expr), o);
- rcop->op_ppaddr = PL_ppaddr[OP_REGCOMP];
rcop->op_flags |= ((PL_hints & HINT_RE_EVAL) ? OPf_SPECIAL : 0)
| (reglist ? OPf_STACKED : 0);
rcop->op_targ = cv_targ;
@@ -5430,7 +5429,6 @@ Perl_pmruntime(pTHX_ OP *o, OP *expr, bool isreg, I32 floor)
}
else {
rcop = S_alloc_LOGOP(aTHX_ OP_SUBSTCONT, scalar(repl), o);
- rcop->op_ppaddr = PL_ppaddr[OP_SUBSTCONT];
rcop->op_private = 1;
/* establish postfix order */
@@ -6668,7 +6666,6 @@ S_new_logop(pTHX_ I32 type, I32 flags, OP** firstp, OP** otherp)
other->op_private |= OPpASSIGN_BACKWARDS; /* other is an OP_SASSIGN */
logop = S_alloc_LOGOP(aTHX_ type, first, LINKLIST(other));
- logop->op_ppaddr = PL_ppaddr[type];
logop->op_flags |= (U8)flags;
logop->op_private = (U8)(1 | (flags >> 8));
@@ -6738,7 +6735,6 @@ Perl_newCONDOP(pTHX_ I32 flags, OP *first, OP *trueop, OP *falseop)
return live;
}
logop = S_alloc_LOGOP(aTHX_ OP_COND_EXPR, first, LINKLIST(trueop));
- logop->op_ppaddr = PL_ppaddr[OP_COND_EXPR];
logop->op_flags |= (U8)flags;
logop->op_private = (U8)(1 | (flags >> 8));
logop->op_next = LINKLIST(falseop);
@@ -6789,7 +6785,6 @@ Perl_newRANGE(pTHX_ I32 flags, OP *left, OP *right)
PERL_ARGS_ASSERT_NEWRANGE;
range = S_alloc_LOGOP(aTHX_ OP_RANGE, left, LINKLIST(right));
- range->op_ppaddr = PL_ppaddr[OP_RANGE];
range->op_flags = OPf_KIDS;
leftstart = LINKLIST(left);
range->op_private = (U8)(1 | (flags >> 8));
@@ -7309,7 +7304,6 @@ S_newGIVWHENOP(pTHX_ OP *cond, OP *block,
PERL_ARGS_ASSERT_NEWGIVWHENOP;
enterop = S_alloc_LOGOP(aTHX_ enter_opcode, block, NULL);
- enterop->op_ppaddr = PL_ppaddr[enter_opcode];
enterop->op_targ = ((entertarg == NOT_IN_PAD) ? 0 : entertarg);
enterop->op_private = 0;
@@ -9309,7 +9303,6 @@ Perl_ck_eval(pTHX_ OP *o)
op_free(o);
enter = S_alloc_LOGOP(aTHX_ OP_ENTERTRY, NULL, NULL);
- enter->op_ppaddr = PL_ppaddr[OP_ENTERTRY];
/* establish postfix order */
enter->op_next = (OP*)enter;
@@ -9872,7 +9865,6 @@ Perl_ck_grep(pTHX_ OP *o)
kid = kUNOP->op_first;
gwop = S_alloc_LOGOP(aTHX_ type, o, LINKLIST(kid));
- gwop->op_ppaddr = PL_ppaddr[type];
kid->op_next = (OP*)gwop;
offset = pad_findmy_pvs("$_", 0);
if (offset == NOT_IN_PAD || PAD_COMPNAME_FLAGS_isOUR(offset)) {
--
1.8.3.1 |
From @jimcHere, op_type and op_ppaddr inits were separated by many lines. op.c | 4 ++-- Inline Patchdiff --git a/op.c b/op.c
index 79307df..2bb5637 100644
--- a/op.c
+++ b/op.c
@@ -9459,7 +9459,8 @@ Perl_ck_rvconst(pTHX_ OP *o)
&& SvTYPE(SvRV(gv)) != SVt_PVCV)
gv_fetchsv(kidsv, GV_ADDMULTI, SVt_PVCV);
}
- kid->op_type = OP_GV;
+ CHANGE_TYPE(kid, OP_GV);
+ kid->op_ppaddr = PL_ppaddr[OP_GV];
SvREFCNT_dec(kid->op_sv);
#ifdef USE_ITHREADS
/* XXX hack: dependence on sizeof(PADOP) <= sizeof(SVOP) */
@@ -9471,7 +9472,6 @@ Perl_ck_rvconst(pTHX_ OP *o)
kid->op_sv = SvREFCNT_inc_simple_NN(gv);
#endif
kid->op_private = 0;
- kid->op_ppaddr = PL_ppaddr[OP_GV];
/* FAKE globs in the symbol table cause weird bugs (#77810) */
SvFAKE_off(gv);
}
--
1.8.3.1 |
From @cpansproutOn Sun Nov 02 13:01:58 2014, yoduh wrote:
Please don’t use git send-email. While I *could* apply your patches, it would involve a lot of copying and pasting to put the headers back in the patch. If you could run: git format-patch --stdout -M blead.. > topic-branch-changes.patch and attach the file, it would save committers a lot of work. Thank you. -- Father Chrysostomos |
The RT System itself - Status changed from 'new' to 'open' |
From @tonycozOn Sun Nov 02 13:57:20 2014, sprout wrote:
I thought the new RT was meant to preserve the headers well enough to allow patches to be applied, but I guess I was mistaken, git am couldn't recognize the patch format. I've merged all eight tickets created into 123105. I'm not sure this was created directly with git send-email, there's 2 header emails, and the patch numbering is strange too, with some numbered /3 and the others /5, and both header emails as /4. Tony |
From @cpansproutOn Sun Nov 02 15:08:14 2014, tonyc wrote:
I didn’t realise that.
On the contrary, I just tried applying one after following the ‘with headers’ link, and it worked for me. But, still, it is easier to apply patch sequences that come in one file. -- Father Chrysostomos |
From @cpansproutOn Sun Nov 02 13:01:58 2014, yoduh wrote:
Thank you. I have applied three of your patches (1,2,4) as 10884df, 21b66d1 and ddacf10, respectively. Patch 3 increases the size of op.o. Patch 5 leaves a stray ppaddr assignment. -- Father Chrysostomos |
From @jimcOn Sun, Nov 2, 2014 at 7:07 PM, Father Chrysostomos via RT <
thank you. Ive fixed patch 5, and juggled the order (so it applies 1st). new spin of patchset created and attached, as per your request. Re old patch 3, the increase, while only 16 bytes, is mildly surprising. ==> op.report.before <== ==> op.report.after <== I had expected default -O2 settings on linux to detect and hoist the output suggests that the code is identical, Heres what Im seeing in the diff to conclude as above. And if you'll indulge me a bit here, break; --
|
From @jimcoptype-cleanups.patchFrom 432528294c62250ed17a490dc41946ce83bf0a2e Mon Sep 17 00:00:00 2001
From: Jim Cromie <jim.cromie@gmail.com>
Date: Sun, 2 Nov 2014 13:33:48 -0700
Subject: [PATCH 1/3] op.c: CHANGE_TYPE in Perl_ck_rvconst
Here, op_type and op_ppaddr inits were separated by many lines.
Move them together, then replace with CHANGE_TYPE call.
--
v2- actually remove the explicit op_ppaddr assignment.
---
op.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/op.c b/op.c
index 7a795e3..28811c7 100644
--- a/op.c
+++ b/op.c
@@ -9459,7 +9459,7 @@ Perl_ck_rvconst(pTHX_ OP *o)
&& SvTYPE(SvRV(gv)) != SVt_PVCV)
gv_fetchsv(kidsv, GV_ADDMULTI, SVt_PVCV);
}
- kid->op_type = OP_GV;
+ CHANGE_TYPE(kid, OP_GV);
SvREFCNT_dec(kid->op_sv);
#ifdef USE_ITHREADS
/* XXX hack: dependence on sizeof(PADOP) <= sizeof(SVOP) */
@@ -9471,7 +9471,6 @@ Perl_ck_rvconst(pTHX_ OP *o)
kid->op_sv = SvREFCNT_inc_simple_NN(gv);
#endif
kid->op_private = 0;
- kid->op_ppaddr = PL_ppaddr[OP_GV];
/* FAKE globs in the symbol table cause weird bugs (#77810) */
SvFAKE_off(gv);
}
--
1.8.3.1
From e39deb66d953f66a2e3e5ce17c96bb9e89a7f83a Mon Sep 17 00:00:00 2001
From: Jim Cromie <jim.cromie@gmail.com>
Date: Sat, 1 Nov 2014 17:24:50 -0600
Subject: [PATCH 2/3] op.c: regularize OP_AELEMFAST_LEX vs OP_AELEMFAST setup.
commit 10884df96723d4 promoted CHANGE_TYPE as the "normal" way to
initialize op_type and op_ppaddr fields. This suggests that
Perl_rpeep()s tweaking of OP_AELEMFAST and OP_AELEMFAST_LEX is
something of a special case.
It turns out that these are pretty normal too; since regen/opcode.pl
insures that PL_ppaddr[OP_AELEMFAST_LEX] == PL_ppaddr[OP_AELEMFAST],
the invariant: o->op_ppaddr == PL_ppaddr[o->op_type] holds after both
the OP_AELEMFAST and OP_AELEMFAST_LEX branches, and the stronger
semantics of CHANGE_TYPE() can be used to communicate this normality.
On a gcc x86-64 -DEBUGGING build, this enlarges 'size op.o' by 16
bytes, but before and after 'objdump -DS op.o' output suggests that
the code itself is identical.
---
op.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/op.c b/op.c
index 28811c7..c702813 100644
--- a/op.c
+++ b/op.c
@@ -12273,15 +12273,14 @@ Perl_rpeep(pTHX_ OP *o)
op_null(pop);
o->op_flags |= pop->op_next->op_flags & OPf_MOD;
o->op_next = pop->op_next->op_next;
- o->op_ppaddr = PL_ppaddr[OP_AELEMFAST];
o->op_private = (U8)i;
if (o->op_type == OP_GV) {
gv = cGVOPo_gv;
GvAVn(gv);
- o->op_type = OP_AELEMFAST;
+ CHANGE_TYPE(o, OP_AELEMFAST);
}
else
- o->op_type = OP_AELEMFAST_LEX;
+ CHANGE_TYPE(o, OP_AELEMFAST_LEX);
}
if (o->op_type != OP_GV)
break;
--
1.8.3.1
From 14952b14b341c1c227be80d9909b220e8e00861d Mon Sep 17 00:00:00 2001
From: Jim Cromie <jim.cromie@gmail.com>
Date: Sun, 2 Nov 2014 21:22:03 -0700
Subject: [PATCH 3/3] op.c: CHANGE_TYPE for op_integerize and ck_spair
These 2 callsites alter an OP's type and ppaddr to a closely related
op, by incrementing the op_type, then setting ppaddr, exactly what
CHANGE_TYPE does.
Since CHANGE_TYPE(o, ++type) expands type 2x, it has a problem
with the ++, avoid this using auto vars.
---
op.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/op.c b/op.c
index c702813..f2e5eef 100644
--- a/op.c
+++ b/op.c
@@ -3927,7 +3927,8 @@ S_op_integerize(pTHX_ OP *o)
if ((PL_opargs[type] & OA_OTHERINT) && (PL_hints & HINT_INTEGER))
{
dVAR;
- o->op_ppaddr = PL_ppaddr[++(o->op_type)];
+ type++; /* avoid ++ in macro arg */
+ CHANGE_TYPE(o, type);
}
if (type == OP_NEGATE)
@@ -9190,14 +9191,15 @@ OP *
Perl_ck_spair(pTHX_ OP *o)
{
dVAR;
+ OPCODE type;
PERL_ARGS_ASSERT_CK_SPAIR;
+ type = o->op_type;
if (o->op_flags & OPf_KIDS) {
OP* newop;
OP* kid;
OP* kidkid;
- const OPCODE type = o->op_type;
o = modkids(ck_fun(o), type);
kid = cUNOPo->op_first;
kidkid = kUNOP->op_first;
@@ -9219,8 +9221,9 @@ Perl_ck_spair(pTHX_ OP *o)
op_free(kidkid);
}
/* transforms OP_REFGEN into OP_SREFGEN, OP_CHOP into OP_SCHOP,
- * and OP_CHOMP into OP_SCHOMP */
- o->op_ppaddr = PL_ppaddr[++o->op_type];
+ * and OP_CHOMP into OP_SCHOMP, avoiding ++ in macro arg */
+ type++;
+ CHANGE_TYPE(o, type);
return ck_fun(o);
}
--
1.8.3.1
|
From @jimcOn Sun, Nov 2, 2014 at 4:08 PM, Tony Cook via RT <perlbug-followup@perl.org>
it was git send-email. set started as 3 patches, then added more, then I if you were curious...
|
From @craigberryOn Mon, Nov 3, 2014 at 1:59 AM, Jim Cromie <jim.cromie@gmail.com> wrote:
perlbug -p is available and was specifically intended for sending |
From @cpansproutOn Mon Nov 03 09:38:21 2014, craig.a.berry@gmail.com wrote:
Yes, of course. I’d forgotten about that. -- Father Chrysostomos |
From @cpansproutOn Sun Nov 02 23:54:23 2014, yoduh wrote:
Thank you. Applied as 72e8a95.
In this case, I’m not sure it makes the code clearer. To me it just seems more complex. But that is just my opinion. What do others think?
I don’t know the answer to that. However, I am seeing a 20-byte increase with g++ and debugging disabled. Do you see the same thing with debugging off? -- Father Chrysostomos |
From @iabynOn Sun, Nov 02, 2014 at 01:02:02PM -0800, Jim Cromie wrote:
Would anyone object if I renamed CHANGE_TYPE() to Op_SET_TYPE()? Because: * this macro is used to initialise the type/ppaddr of an op, in addition to -- |
From @tonycozOn Wed, Mar 11, 2015 at 05:19:08PM +0000, Dave Mitchell wrote:
+1 Tony |
From perl5-porters@perl.orgDave Mitchell asked:
OpTYPE_set. |
From @iabynOn Thu, Mar 12, 2015 at 04:21:32AM -0000, Father Chrysostomos wrote:
works for me. -- |
From @iabynOn Thu, Mar 12, 2015 at 03:37:21PM +0000, Dave Mitchell wrote:
Now done as b9a0709. -- |
From @jimcIm attaching a revised patchset for this ticket 0001-op.c-fix-latent-bug-in-OP_AELEMFAST_LEX-OP_AELEMFAST.patch p1 - its a bit of a stretch to call this a BUG, Father C noted this patch caused op.o to grow by ~20 bytes, p2 - these callsites use the (++o->op_type) idiom to modify an OP to its p3 - callsite added since 10884df was applied. p4 - hoist OpTYPE_set macro to op.h, use in perl.c p5 - obsoleted by patch in [perl #128112] |
From @jimc0001-op.c-fix-latent-bug-in-OP_AELEMFAST_LEX-OP_AELEMFAST.patchFrom f6257db99b53be401b3924614ecc67f8aa2d739b Mon Sep 17 00:00:00 2001
From: Jim Cromie <jim.cromie@gmail.com>
Date: Sat, 1 Nov 2014 17:24:50 -0600
Subject: [PATCH 1/5] op.c: fix latent bug in OP_AELEMFAST_LEX, OP_AELEMFAST
setup.
OpTYPE_set() is the "normal" way to initialize an OP.
Commit 10884df967 left a number of unusual use cases, we pick up one here.
In Perl_rpeep(): case OP_GV: code open-codes OpTYPE_set(), but
micro-optimizes by hoisting the op->pp_addr = PL_ppaddr[OP_AELEMFAST]
assignment out of the if-else.
This works, cuz:
PL_ppaddr[OP_AELEMFAST_LEX] == PL_ppaddr[OP_AELEMFAST] [*]
While this is guaranteed currently by regen/opcode.pl, it seems
rather cryptic and magical, and is certainly action at a distance.
On a gcc x86-64 -DEBUGGING build, this patch enlarges 'size op.o' by
24 bytes (last I checked). gcc evidently doesnt know that [*] holds.
This isnt a runtime hot-path, so doesnt warrant micro-optimiation, and
the OP setup is otherwise normal. OpTYPE_set()s postcondition,
o->op_ppaddr == PL_ppaddr[o->op_type], also holds, which is worth
saying clearly.
---
op.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/op.c b/op.c
index 93205fe..d4c5783 100644
--- a/op.c
+++ b/op.c
@@ -13878,15 +13878,14 @@ Perl_rpeep(pTHX_ OP *o)
op_null(pop);
o->op_flags |= pop->op_next->op_flags & OPf_MOD;
o->op_next = pop->op_next->op_next;
- o->op_ppaddr = PL_ppaddr[OP_AELEMFAST];
o->op_private = (U8)i;
if (o->op_type == OP_GV) {
gv = cGVOPo_gv;
GvAVn(gv);
- o->op_type = OP_AELEMFAST;
+ OpTYPE_set(o, OP_AELEMFAST);
}
else
- o->op_type = OP_AELEMFAST_LEX;
+ OpTYPE_set(o, OP_AELEMFAST_LEX);
}
if (o->op_type != OP_GV)
break;
--
2.5.5
|
From @jimc0002-op.c-use-OpTYPE_set-for-op_integerize-and-ck_spair.patchFrom e62d857d9fb68ee406daecd7309f200ac6b82d46 Mon Sep 17 00:00:00 2001
From: Jim Cromie <jim.cromie@gmail.com>
Date: Sun, 2 Nov 2014 21:22:03 -0700
Subject: [PATCH 2/5] op.c: use OpTYPE_set() for op_integerize and ck_spair
These 2 users employ the (++o->op_type) trick to modify an OP to its
optimized doppelganger. This is tricky, but wrt OP setup its
otherwize normal, in particular OpTYPE_set()s postcondition holds
true, ie (o->op_ppaddr == PL_ppaddr[o->op_type]), so using OpTYPE_set
conveys this "normality".
Also fix OpTYPE_set(o, ++type) expansion problem with an autovar
in the macro body.
---
op.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/op.c b/op.c
index d4c5783..86a8bf5 100644
--- a/op.c
+++ b/op.c
@@ -514,8 +514,9 @@ Perl_op_refcnt_dec(pTHX_ OP *o)
#define OpTYPE_set(o,type) \
STMT_START { \
- o->op_type = (OPCODE)type; \
- o->op_ppaddr = PL_ppaddr[type]; \
+ OPCODE tval = type; \
+ o->op_type = tval; \
+ o->op_ppaddr = PL_ppaddr[tval]; \
} STMT_END
STATIC OP *
@@ -4252,7 +4253,7 @@ S_op_integerize(pTHX_ OP *o)
if ((PL_opargs[type] & OA_OTHERINT) && (PL_hints & HINT_INTEGER))
{
dVAR;
- o->op_ppaddr = PL_ppaddr[++(o->op_type)];
+ OpTYPE_set(o, ++(o->op_type));
}
if (type == OP_NEGATE)
@@ -9430,14 +9431,15 @@ OP *
Perl_ck_spair(pTHX_ OP *o)
{
dVAR;
+ OPCODE type;
PERL_ARGS_ASSERT_CK_SPAIR;
+ type = o->op_type;
if (o->op_flags & OPf_KIDS) {
OP* newop;
OP* kid;
OP* kidkid;
- const OPCODE type = o->op_type;
o = modkids(ck_fun(o), type);
kid = cUNOPo->op_first;
kidkid = kUNOP->op_first;
@@ -9461,7 +9463,7 @@ Perl_ck_spair(pTHX_ OP *o)
}
/* transforms OP_REFGEN into OP_SREFGEN, OP_CHOP into OP_SCHOP,
* and OP_CHOMP into OP_SCHOMP */
- o->op_ppaddr = PL_ppaddr[++o->op_type];
+ OpTYPE_set(o, ++type);
return ck_fun(o);
}
--
2.5.5
|
From @jimc0003-use-OpTYPE_set-in-Perl_newUNOP_AUX.patchFrom 01f8b2262154cf4325eaa6a61becd19b32a38813 Mon Sep 17 00:00:00 2001
From: Jim Cromie <jim.cromie@gmail.com>
Date: Tue, 25 Aug 2015 18:06:59 -0600
Subject: [PATCH 3/5] use OpTYPE_set in Perl_newUNOP_AUX
Replace open-code with macro-call, dropping the (OPCODE) cast, as was
done numerous times in 10884df.
---
op.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/op.c b/op.c
index 86a8bf5..c3b9bd4 100644
--- a/op.c
+++ b/op.c
@@ -4876,8 +4876,7 @@ Perl_newUNOP_AUX(pTHX_ I32 type, I32 flags, OP *first, UNOP_AUX_item *aux)
|| type == OP_CUSTOM);
NewOp(1101, unop, 1, UNOP_AUX);
- unop->op_type = (OPCODE)type;
- unop->op_ppaddr = PL_ppaddr[type];
+ OpTYPE_set(unop, type);
unop->op_first = first;
unop->op_flags = (U8)(flags | (first ? OPf_KIDS : 0));
unop->op_private = (U8)((first ? 1 : 0) | (flags >> 8));
--
2.5.5
|
From @jimc0004-hoist-OpTYPE_set-to-op.h-from-op.c-use-in-perl.c.patchFrom c48f5fca2fd590f7d5a2805cfa4bd3eb5585ecdc Mon Sep 17 00:00:00 2001
From: Jim Cromie <jim.cromie@gmail.com>
Date: Sat, 15 Nov 2014 15:06:01 -0700
Subject: [PATCH 4/5] hoist OpTYPE_set() to op.h from op.c, use in perl.c
hoist OpTYPE_set() into op.h so its available to perl.c and others,
and use it in perl.c. Add parens around (o) so its robust with args
like &myop.
---
op.c | 7 -------
op.h | 6 ++++++
perl.c | 9 +++------
3 files changed, 9 insertions(+), 13 deletions(-)
diff --git a/op.c b/op.c
index c3b9bd4..2a823a3 100644
--- a/op.c
+++ b/op.c
@@ -512,13 +512,6 @@ Perl_op_refcnt_dec(pTHX_ OP *o)
#define RETURN_UNLIMITED_NUMBER (PERL_INT_MAX / 2)
-#define OpTYPE_set(o,type) \
- STMT_START { \
- OPCODE tval = type; \
- o->op_type = tval; \
- o->op_ppaddr = PL_ppaddr[tval]; \
- } STMT_END
-
STATIC OP *
S_no_fh_allowed(pTHX_ OP *o)
{
diff --git a/op.h b/op.h
index 3ded4bb..364b44f 100644
--- a/op.h
+++ b/op.h
@@ -993,6 +993,12 @@ C<sib> is non-null. For a higher-level interface, see C<L</op_sibling_splice>>.
#define OP_TYPE_ISNT_AND_WASNT(o, type) \
( (o) && OP_TYPE_ISNT_AND_WASNT_NN(o, type) )
+#define OpTYPE_set(o,type) \
+ STMT_START { \
+ OPCODE tval = type; \
+ (o)->op_type = tval; \
+ (o)->op_ppaddr = PL_ppaddr[tval]; \
+ } STMT_END
#ifdef PERL_OP_PARENT
# define OpHAS_SIBLING(o) (cBOOL((o)->op_moresib))
diff --git a/perl.c b/perl.c
index 1d8876b..76ef0e6 100644
--- a/perl.c
+++ b/perl.c
@@ -2783,15 +2783,12 @@ Perl_call_sv(pTHX_ SV *sv, VOL I32 flags)
method_op.op_next = (OP*)&myop;
PL_op = (OP*)&method_op;
if ( flags & G_METHOD_NAMED ) {
- method_op.op_ppaddr = PL_ppaddr[OP_METHOD_NAMED];
- method_op.op_type = OP_METHOD_NAMED;
+ OpTYPE_set(&method_op, OP_METHOD_NAMED);
method_op.op_u.op_meth_sv = sv;
} else {
- method_op.op_ppaddr = PL_ppaddr[OP_METHOD];
- method_op.op_type = OP_METHOD;
+ OpTYPE_set(&method_op, OP_METHOD);
}
- myop.op_ppaddr = PL_ppaddr[OP_ENTERSUB];
- myop.op_type = OP_ENTERSUB;
+ OpTYPE_set(&myop, OP_ENTERSUB);
}
if (!(flags & G_EVAL)) {
--
2.5.5
|
From @jimc0005-pp.c-use-OpTYPE_set-in-PP-pp_i_modulo-plus.patchFrom 9ea525e1c7b0566d4ede91bb99dd83aacf3f5aa1 Mon Sep 17 00:00:00 2001
From: Jim Cromie <jim.cromie@gmail.com>
Date: Mon, 17 Nov 2014 03:19:27 -0700
Subject: [PATCH 5/5] pp.c: use OpTYPE_set in PP(pp_i_modulo) plus..
PP(pp_i_modulo) is unusual code on several levels, and warrants some
explication:
For glibc version < 2.8, the PP-code detects a glibc _mod3i bug at
runtime (pp-code execution time), and updates both op->ppaddr and
PL_ppaddr[OP_I_MODULO] to work around it. This seems tricky:
- many ops may have been built prior to bug-detection, without correction.
- detector converts each different I_MODULO op at 1st execution.
- PL_ppaddr[OP_I_MODULO] updated repeatedly.
While the bug detection seems better done once in perl_construct(),
before code is parsed and optrees are built, but thats not the focus now.
This patch replaces the op_ppaddr assigment with OpTYPE_set,
tolerating the redundant assignment of the optype, effectively undoing
a micro-optimization which depends upon the optype not changing.
OpTYPE_set(op, type) has stronger postconditions:
(op->pp_addr == PL_ppaddr[op->optype])
Since the postcondition holds in this case even without the patch, its
best to communicate this by using macro.
Caveat: the OP-tweaking code is #idefd, and since commit bf3d06a
largely dormant on porters' boxes, so maybe leave it alone. Does
eliminating an ad-hoc o->op_ppaddr assignment, and reducing arbitrary
usage differences (code-entropy) outweigh the churn ?
---
pp.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/pp.c b/pp.c
index 4a2cde0..ae48c8c 100644
--- a/pp.c
+++ b/pp.c
@@ -2842,9 +2842,9 @@ PP(pp_i_modulo)
if (!right)
DIE(aTHX_ "Illegal modulus zero");
/* The assumption is to use hereafter the old vanilla version... */
- PL_op->op_ppaddr =
- PL_ppaddr[OP_I_MODULO] =
- Perl_pp_i_modulo_0;
+ PL_ppaddr[OP_I_MODULO] = &Perl_pp_i_modulo_0;
+ OpTYPE_set(PL_op, OP_I_MODULO);
+
/* .. but if we have glibc, we might have a buggy _moddi3
* (at least glibc 2.2.5 is known to have this bug), in other
* words our integer modulus with negative quad as the second
@@ -2860,9 +2860,9 @@ PP(pp_i_modulo)
if (l % r == -3) {
/* Yikes, we have the bug.
* Patch in the workaround version. */
- PL_op->op_ppaddr =
- PL_ppaddr[OP_I_MODULO] =
- &Perl_pp_i_modulo_1;
+ PL_ppaddr[OP_I_MODULO] = &Perl_pp_i_modulo_1;
+ OpTYPE_set(PL_op, OP_I_MODULO);
+
/* Make certain we work right this time, too. */
right = PERL_ABS(right);
}
--
2.5.5
|
Migrated from rt.perl.org#123105 (status was 'open')
Searchable as RT123105$
The text was updated successfully, but these errors were encountered: