Skip to content

Commit

Permalink
Replace most calls of MUTABLE_xV(SvRV()) with xV_FROM_REF()
Browse files Browse the repository at this point in the history 
This results in shorter neater code, and additional debugging assertions
that the dereferenced SVs really are the requested type when built under
`-DDEBUGGING`.

When I added the xV_FROM_REF() macros, I searched for `(TYPE)SvRV` style
cast expressions, but forgot to additionally look for `MUTABLE_xV()`
calls.

There are additionally two spots in pp_hot.c that cannot be modified,
because despite casting the result to a CV pointer, the SV isn't
actually a CV. I've added a comment on these lines as to why they're not
altered.
  • Loading branch information
@leonerd
leonerd committed Jul 22, 2024
1 parent 381bfe3 commit 7fa858d
Show file tree
Hide file tree
Showing 12 changed files with 42 additions and 42 deletions.
2 changes: 1 addition & 1 deletion mg.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3697,7 +3697,7 @@ Perl_perly_sighandler(int sig, Siginfo_t *sip PERL_UNUSED_DECL,
}
}
/* sv_2cv is too complicated, try a simpler variant first: */
if (!SvROK(PL_psig_ptr[sig]) || !(cv = MUTABLE_CV(SvRV(PL_psig_ptr[sig])))
if (!SvROK(PL_psig_ptr[sig]) || !(cv = CV_FROM_REF(PL_psig_ptr[sig]))
|| SvTYPE(cv) != SVt_PVCV) {
HV *st;
cv = sv_2cv(PL_psig_ptr[sig], &st, &gv, GV_ADD);
Expand Down
6 changes: 3 additions & 3 deletions op.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3145,7 +3145,7 @@ Perl_op_lvalue_flags(pTHX_ OP *o, I32 type, U32 flags)
cv = isGV(gv)
? GvCV(gv)
: SvROK(gv) && SvTYPE(SvRV(gv)) == SVt_PVCV
? MUTABLE_CV(SvRV(gv))
? CV_FROM_REF((SV *)gv)
: NULL;
if (!cv)
break;
Expand Down Expand Up @@ -7934,7 +7934,7 @@ static U16 S_extract_shortver(pTHX_ SV *sv)
if(!SvRV(sv) || !SvOBJECT(rv = SvRV(sv)) || !sv_derived_from(sv, "version"))
return 0;

AV *av = MUTABLE_AV(SvRV(*hv_fetchs(MUTABLE_HV(rv), "version", 0)));
AV *av = AV_FROM_REF(*hv_fetchs(MUTABLE_HV(rv), "version", 0));

U16 shortver = 0;

Expand Down Expand Up @@ -14221,7 +14221,7 @@ Perl_rv2cv_op_cv(pTHX_ OP *cvop, U32 flags)
gv = cGVOPx_gv(rvop);
if (!isGV(gv)) {
if (SvROK(gv) && SvTYPE(SvRV(gv)) == SVt_PVCV) {
cv = MUTABLE_CV(SvRV(gv));
cv = CV_FROM_REF((SV *)gv);
gv = NULL;
break;
}
Expand Down
4 changes: 2 additions & 2 deletions perlio.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -757,7 +757,7 @@ static int
perlio_mg_set(pTHX_ SV *sv, MAGIC *mg)
{
if (SvROK(sv)) {
IO * const io = GvIOn(MUTABLE_GV(SvRV(sv)));
IO * const io = GvIOn(GV_FROM_REF(sv));
PerlIO * const ifp = IoIFP(io);
PerlIO * const ofp = IoOFP(io);
Perl_warn(aTHX_ "set %" SVf " %p %p %p",
Expand All @@ -770,7 +770,7 @@ static int
perlio_mg_get(pTHX_ SV *sv, MAGIC *mg)
{
if (SvROK(sv)) {
IO * const io = GvIOn(MUTABLE_GV(SvRV(sv)));
IO * const io = GvIOn(GV_FROM_REF(sv));
PerlIO * const ifp = IoIFP(io);
PerlIO * const ofp = IoOFP(io);
Perl_warn(aTHX_ "get %" SVf " %p %p %p",
Expand Down
2 changes: 1 addition & 1 deletion pp.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ PP(pp_rv2cv)
if (cv) NOOP;
else if ((flags == (GV_ADD|GV_NOEXPAND)) && gv && SvROK(gv)) {
cv = SvTYPE(SvRV(gv)) == SVt_PVCV
? MUTABLE_CV(SvRV(gv))
? CV_FROM_REF((SV *)gv)
: MUTABLE_CV(gv);
}
else
Expand Down
40 changes: 20 additions & 20 deletions pp_ctl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3182,7 +3182,7 @@ PP(pp_goto)
/* This egregious kludge implements goto &subroutine */
I32 cxix;
PERL_CONTEXT *cx;
CV *cv = MUTABLE_CV(SvRV(sv));
CV *cv = CV_FROM_REF(sv);
AV *arg = GvAV(PL_defgv);
CV *old_cv = NULL;

Expand Down Expand Up @@ -4399,7 +4399,7 @@ S_require_version(pTHX_ SV *sv)
SV * const pv = *hv_fetchs(MUTABLE_HV(req), "original", FALSE);

/* get the left hand term */
lav = MUTABLE_AV(SvRV(*hv_fetchs(MUTABLE_HV(req), "version", FALSE)));
lav = AV_FROM_REF(*hv_fetchs(MUTABLE_HV(req), "version", FALSE));

first = SvIV(*av_fetch(lav,0,0));
if ( first > (int)PERL_REVISION /* probably 'use 6.0' */
Expand Down Expand Up @@ -4720,7 +4720,7 @@ S_require_file(pTHX_ SV *sv)
if (SvTYPE(SvRV(loader)) == SVt_PVAV
&& !SvOBJECT(SvRV(loader)))
{
loader = *av_fetch(MUTABLE_AV(SvRV(loader)), 0, TRUE);
loader = *av_fetch(AV_FROM_REF(loader), 0, TRUE);
if (SvGMAGICAL(loader)) {
SvGETMAGIC(loader);
SV *l = sv_newmortal();
Expand Down Expand Up @@ -5936,12 +5936,12 @@ S_do_smartmatch(pTHX_ HV *seen_this, HV *seen_other, const bool copied)
else if (SvROK(d) && SvTYPE(SvRV(d)) == SVt_PVHV) {
/* Check that the key-sets are identical */
HE *he;
HV *other_hv = MUTABLE_HV(SvRV(d));
HV *other_hv = HV_FROM_REF(d);
bool tied;
bool other_tied;
U32 this_key_count = 0,
other_key_count = 0;
HV *hv = MUTABLE_HV(SvRV(e));
HV *hv = HV_FROM_REF(e);

DEBUG_M(Perl_deb(aTHX_ " applying rule Hash-Hash\n"));
/* Tied hashes don't know how many keys they have. */
Expand Down Expand Up @@ -5989,10 +5989,10 @@ S_do_smartmatch(pTHX_ HV *seen_this, HV *seen_other, const bool copied)
goto ret_yes;
}
else if (SvROK(d) && SvTYPE(SvRV(d)) == SVt_PVAV) {
AV * const other_av = MUTABLE_AV(SvRV(d));
AV * const other_av = AV_FROM_REF(d);
const Size_t other_len = av_count(other_av);
Size_t i;
HV *hv = MUTABLE_HV(SvRV(e));
HV *hv = HV_FROM_REF(e);

DEBUG_M(Perl_deb(aTHX_ " applying rule Array-Hash\n"));
for (i = 0; i < other_len; ++i) {
Expand All @@ -6011,7 +6011,7 @@ S_do_smartmatch(pTHX_ HV *seen_this, HV *seen_other, const bool copied)
{
PMOP * const matcher = make_matcher((REGEXP*) SvRV(d));
HE *he;
HV *hv = MUTABLE_HV(SvRV(e));
HV *hv = HV_FROM_REF(e);

(void) hv_iterinit(hv);
while ( (he = hv_iternext(hv)) ) {
Expand All @@ -6029,7 +6029,7 @@ S_do_smartmatch(pTHX_ HV *seen_this, HV *seen_other, const bool copied)
else {
sm_any_hash:
DEBUG_M(Perl_deb(aTHX_ " applying rule Any-Hash\n"));
if (hv_exists_ent(MUTABLE_HV(SvRV(e)), d, 0))
if (hv_exists_ent(HV_FROM_REF(e), d, 0))
goto ret_yes;
else
goto ret_no;
Expand All @@ -6041,7 +6041,7 @@ S_do_smartmatch(pTHX_ HV *seen_this, HV *seen_other, const bool copied)
goto sm_any_array; /* Treat objects like scalars */
}
else if (SvROK(d) && SvTYPE(SvRV(d)) == SVt_PVHV) {
AV * const other_av = MUTABLE_AV(SvRV(e));
AV * const other_av = AV_FROM_REF(e);
const Size_t other_len = av_count(other_av);
Size_t i;

Expand All @@ -6051,16 +6051,16 @@ S_do_smartmatch(pTHX_ HV *seen_this, HV *seen_other, const bool copied)

DEBUG_M(Perl_deb(aTHX_ " testing for key existence...\n"));
if (svp) { /* ??? When can this not happen? */
if (hv_exists_ent(MUTABLE_HV(SvRV(d)), *svp, 0))
if (hv_exists_ent(HV_FROM_REF(d), *svp, 0))
goto ret_yes;
}
}
goto ret_no;
}
if (SvROK(d) && SvTYPE(SvRV(d)) == SVt_PVAV) {
AV *other_av = MUTABLE_AV(SvRV(d));
AV *other_av = AV_FROM_REF(d);
DEBUG_M(Perl_deb(aTHX_ " applying rule Array-Array\n"));
if (av_count(MUTABLE_AV(SvRV(e))) != av_count(other_av))
if (av_count(AV_FROM_REF(e)) != av_count(other_av))
goto ret_no;
else {
Size_t i;
Expand All @@ -6073,7 +6073,7 @@ S_do_smartmatch(pTHX_ HV *seen_this, HV *seen_other, const bool copied)
seen_other = (HV*)newSV_type_mortal(SVt_PVHV);
}
for(i = 0; i < other_len; ++i) {
SV * const * const this_elem = av_fetch(MUTABLE_AV(SvRV(e)), i, FALSE);
SV * const * const this_elem = av_fetch(AV_FROM_REF(e), i, FALSE);
SV * const * const other_elem = av_fetch(other_av, i, FALSE);

if (!this_elem || !other_elem) {
Expand Down Expand Up @@ -6115,11 +6115,11 @@ S_do_smartmatch(pTHX_ HV *seen_this, HV *seen_other, const bool copied)
sm_regex_array:
{
PMOP * const matcher = make_matcher((REGEXP*) SvRV(d));
const Size_t this_len = av_count(MUTABLE_AV(SvRV(e)));
const Size_t this_len = av_count(AV_FROM_REF(e));
Size_t i;

for(i = 0; i < this_len; ++i) {
SV * const * const svp = av_fetch(MUTABLE_AV(SvRV(e)), i, FALSE);
SV * const * const svp = av_fetch(AV_FROM_REF(e), i, FALSE);
DEBUG_M(Perl_deb(aTHX_ " testing element against pattern...\n"));
if (svp && matcher_matches_sv(matcher, *svp)) {
destroy_matcher(matcher);
Expand All @@ -6132,12 +6132,12 @@ S_do_smartmatch(pTHX_ HV *seen_this, HV *seen_other, const bool copied)
}
else if (!SvOK(d)) {
/* undef ~~ array */
const Size_t this_len = av_count(MUTABLE_AV(SvRV(e)));
const Size_t this_len = av_count(AV_FROM_REF(e));
Size_t i;

DEBUG_M(Perl_deb(aTHX_ " applying rule Undef-Array\n"));
for (i = 0; i < this_len; ++i) {
SV * const * const svp = av_fetch(MUTABLE_AV(SvRV(e)), i, FALSE);
SV * const * const svp = av_fetch(AV_FROM_REF(e), i, FALSE);
DEBUG_M(Perl_deb(aTHX_ " testing for undef element...\n"));
if (!svp || !SvOK(*svp))
goto ret_yes;
Expand All @@ -6148,11 +6148,11 @@ S_do_smartmatch(pTHX_ HV *seen_this, HV *seen_other, const bool copied)
sm_any_array:
{
Size_t i;
const Size_t this_len = av_count(MUTABLE_AV(SvRV(e)));
const Size_t this_len = av_count(AV_FROM_REF(e));

DEBUG_M(Perl_deb(aTHX_ " applying rule Any-Array\n"));
for (i = 0; i < this_len; ++i) {
SV * const * const svp = av_fetch(MUTABLE_AV(SvRV(e)), i, FALSE);
SV * const * const svp = av_fetch(AV_FROM_REF(e), i, FALSE);
if (!svp)
continue;

Expand Down
6 changes: 3 additions & 3 deletions pp_hot.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1658,7 +1658,7 @@ PP(pp_readline)
/* is it *FOO, $fh, or 'FOO' ? */
if (!isGV_with_GP(PL_last_in_gv)) {
if (SvROK(PL_last_in_gv) && isGV_with_GP(SvRV(PL_last_in_gv)))
PL_last_in_gv = MUTABLE_GV(SvRV(PL_last_in_gv));
PL_last_in_gv = GV_FROM_REF((SV *)PL_last_in_gv);
else {
rpp_xpush_1(MUTABLE_SV(PL_last_in_gv));
Perl_pp_rv2gv(aTHX);
Expand Down Expand Up @@ -6217,7 +6217,7 @@ PP(pp_entersub)

/* a non-magic-RV -> CV ? */
if (LIKELY( (SvFLAGS(sv) & (SVf_ROK|SVs_GMG)) == SVf_ROK)) {
cv = MUTABLE_CV(SvRV(sv));
cv = MUTABLE_CV(SvRV(sv)); /* might not actually be a CV */
if (UNLIKELY(SvOBJECT(cv))) /* might be overloaded */
goto do_ref;
}
Expand Down Expand Up @@ -6265,7 +6265,7 @@ PP(pp_entersub)
cv = get_cvn_flags(sym, len, GV_ADD|SvUTF8(sv));
break;
}
cv = MUTABLE_CV(SvRV(sv));
cv = MUTABLE_CV(SvRV(sv)); /* might not actually be a CV */
if (LIKELY(SvTYPE(cv) == SVt_PVCV))
break;
/* FALLTHROUGH */
Expand Down
4 changes: 2 additions & 2 deletions regcomp.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12320,7 +12320,7 @@ Perl_set_ANYOF_arg(pTHX_ RExC_state_t* const pRExC_state,
}

SV * const rv = MUTABLE_SV(RExC_rxi->data->data[i]);
AV * const av = MUTABLE_AV(SvRV(rv));
AV * const av = AV_FROM_REF(rv);

/* If the already encountered class has data that won't be known
* until runtime (stored in the final element of the array), we
Expand Down Expand Up @@ -12461,7 +12461,7 @@ Perl_get_re_gclass_aux_data(pTHX_ const regexp *prog, const regnode* node, bool

if (data->what[n] == 's') {
SV * const rv = MUTABLE_SV(data->data[n]);
AV * const av = MUTABLE_AV(SvRV(rv));
AV * const av = AV_FROM_REF(rv);
SV **const ary = AvARRAY(av);

invlist = ary[INVLIST_INDEX];
Expand Down
2 changes: 1 addition & 1 deletion regcomp_study.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,7 @@ S_get_ANYOF_cp_list_for_ssc(pTHX_ const RExC_state_t *pRExC_state,
else if (ANYOF_HAS_AUX(node)) {
const U32 n = ARG1u(node);
SV * const rv = MUTABLE_SV(RExC_rxi->data->data[n]);
AV * const av = MUTABLE_AV(SvRV(rv));
AV * const av = AV_FROM_REF(rv);
SV **const ary = AvARRAY(av);

if (av_tindex_skip_len_mg(av) >= DEFERRED_USER_DEFINED_INDEX) {
Expand Down
2 changes: 1 addition & 1 deletion regexec.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12093,7 +12093,7 @@ Perl_reg_named_buff_scalar(pTHX_ REGEXP * const r, const U32 flags)
return newSViv(HvTOTALKEYS(RXp_PAREN_NAMES(rx)));
} else if (flags & RXapif_ONE) {
ret = CALLREG_NAMED_BUFF_ALL(r, (flags | RXapif_REGNAMES));
av = MUTABLE_AV(SvRV(ret));
av = AV_FROM_REF(ret);
length = av_count(av);
SvREFCNT_dec_NN(ret);
return newSViv(length);
Expand Down
2 changes: 1 addition & 1 deletion sv.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13049,7 +13049,7 @@ Perl_sv_vcatpvfn_flags(pTHX_ SV *const sv, const char *const pat, const STRLEN p
* vectorize happen normally
*/
if (sv_isobject(vecsv) && sv_derived_from(vecsv, "version")) {
if ( hv_existss(MUTABLE_HV(SvRV(vecsv)), "alpha") ) {
if ( hv_existss(HV_FROM_REF(vecsv), "alpha") ) {
Perl_ck_warner_d(aTHX_ packWARN(WARN_PRINTF),
"vector argument not supported with alpha versions");
vecsv = &PL_sv_no;
Expand Down
4 changes: 2 additions & 2 deletions universal.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -763,7 +763,7 @@ XS(XS_Internals_hv_clear_placehold)
if (items != 1 || !SvROK(ST(0)))
croak_xs_usage(cv, "hv");
else {
HV * const hv = MUTABLE_HV(SvRV(ST(0)));
HV * const hv = HV_FROM_REF(ST(0));
hv_clear_placeholders(hv);
XSRETURN(0);
}
Expand Down Expand Up @@ -1015,7 +1015,7 @@ XS(XS_re_regnames)
if (!ret)
XSRETURN_UNDEF;

av = MUTABLE_AV(SvRV(ret));
av = AV_FROM_REF(ret);
length = av_count(av);

EXTEND(SP, length); /* better extend stack just once */
Expand Down
10 changes: 5 additions & 5 deletions vutil.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,7 @@ Perl_new_version(pTHX_ SV *ver)
if(svp)
(void)hv_stores(MUTABLE_HV(hv), "original", newSVsv(*svp));
}
sav = MUTABLE_AV(SvRV(*hv_fetchs(MUTABLE_HV(ver), "version", FALSE)));
sav = AV_FROM_REF(*hv_fetchs(MUTABLE_HV(ver), "version", FALSE));
/* This will get reblessed later if a derived class*/
for ( key = 0; key <= av_len(sav); key++ )
{
Expand Down Expand Up @@ -994,7 +994,7 @@ Perl_vnumify(pTHX_ SV *vs)
}

/* attempt to retrieve the version array */
if ( !(av = MUTABLE_AV(SvRV(*hv_fetchs(MUTABLE_HV(vs), "version", FALSE))) ) ) {
if ( !(av = AV_FROM_REF(*hv_fetchs(MUTABLE_HV(vs), "version", FALSE)) ) ) {
return newSVpvs("0");
}

Expand Down Expand Up @@ -1056,7 +1056,7 @@ Perl_vnormal(pTHX_ SV *vs)
if ( ! vs )
Perl_croak(aTHX_ "Invalid version object");

av = MUTABLE_AV(SvRV(*hv_fetchs(MUTABLE_HV(vs), "version", FALSE)));
av = AV_FROM_REF(*hv_fetchs(MUTABLE_HV(vs), "version", FALSE));

len = av_len(av);
if ( len == -1 )
Expand Down Expand Up @@ -1161,10 +1161,10 @@ Perl_vcmp(pTHX_ SV *lhv, SV *rhv)
Perl_croak(aTHX_ "Invalid version object");

/* get the left hand term */
lav = MUTABLE_AV(SvRV(*hv_fetchs(MUTABLE_HV(lhv), "version", FALSE)));
lav = AV_FROM_REF(*hv_fetchs(MUTABLE_HV(lhv), "version", FALSE));

/* and the right hand term */
rav = MUTABLE_AV(SvRV(*hv_fetchs(MUTABLE_HV(rhv), "version", FALSE)));
rav = AV_FROM_REF(*hv_fetchs(MUTABLE_HV(rhv), "version", FALSE));

l = av_len(lav);
r = av_len(rav);
Expand Down

0 comments on commit 7fa858d

Please sign in to comment.