Skip to content

Commit

Permalink
Define and use macro families for typed refcount increment
Browse files Browse the repository at this point in the history
Create specific macros for incrementing the refcount of an AV, CV, GV or
HV, returning a typed pointer to avoid the caller needing to cast the
result. This yields neater easier-to-read code.

While not implemented here, this gives an opportunity to add an
`assert()` check on the SvTYPE of the structure being adjusted, for
extra debug checking during debugging builds.
  • Loading branch information
leonerd committed Jul 22, 2024
1 parent 1c8b66f commit de762f9
Show file tree
Hide file tree
Showing 14 changed files with 101 additions and 37 deletions.
18 changes: 18 additions & 0 deletions av.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,24 @@ struct xpvav {
SV** xav_alloc; /* pointer to beginning of C array of SVs */
};

/*
=for apidoc_section $AV
=for apidoc Am|AV *|AvREFCNT_inc|AV *av
=for apidoc_item |AV *|AvREFCNT_inc_simple|AV *av
=for apidoc_item |AV *|AvREFCNT_inc_simple_NN|AV *av
These all increment the reference count of the given SV, which must be an AV.
They are useful when assigning the result into a typed pointer as they avoid
the need to cast the result to the appropriate type.
=cut
*/

#define AvREFCNT_inc(av) ((AV *)SvREFCNT_inc((SV *)av))
#define AvREFCNT_inc_simple(av) ((AV *)SvREFCNT_inc_simple((SV *)av))
#define AvREFCNT_inc_simple_NN(av) ((AV *)SvREFCNT_inc_simple_NN((SV *)av))

/* SV* xav_arylen; */

/* SVpav_REAL is set for all AVs whose xav_array contents are refcounted
Expand Down
2 changes: 1 addition & 1 deletion class.c
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ XS(injected_constructor)

SV *instance = newSVobject(aux->xhv_class_next_fieldix);
SvOBJECT_on(instance);
SvSTASH_set(instance, MUTABLE_HV(SvREFCNT_inc_simple(stash)));
SvSTASH_set(instance, HvREFCNT_inc_simple(stash));

SV *self = sv_2mortal(newRV_noinc(instance));

Expand Down
18 changes: 18 additions & 0 deletions cv.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,24 @@ struct xpvcv {
_XPVCV_COMMON;
};

/*
=for apidoc_section $CV
=for apidoc Am|CV *|CvREFCNT_inc|CV *cv
=for apidoc_item |CV *|CvREFCNT_inc_simple|CV *cv
=for apidoc_item |CV *|CvREFCNT_inc_simple_NN|CV *cv
These all increment the reference count of the given SV, which must be a CV.
They are useful when assigning the result into a typed pointer as they avoid
the need to cast the result to the appropriate type.
=cut
*/

#define CvREFCNT_inc(cv) ((CV *)SvREFCNT_inc((SV *)cv))
#define CvREFCNT_inc_simple(cv) ((CV *)SvREFCNT_inc_simple((SV *)cv))
#define CvREFCNT_inc_simple_NN(cv) ((CV *)SvREFCNT_inc_simple_NN((SV *)cv))

/*
=for apidoc Ayh||CV
Expand Down
7 changes: 3 additions & 4 deletions gv.c
Original file line number Diff line number Diff line change
Expand Up @@ -1888,8 +1888,7 @@ S_parse_gv_stash_name(pTHX_ HV **stash, GV **gv, const char **name,
if (SvTYPE(*gv) != SVt_PVGV) {
gv_init_pvn(*gv, PL_defstash, "main::", 6,
GV_ADDMULTI);
GvHV(*gv) =
MUTABLE_HV(SvREFCNT_inc_simple(PL_defstash));
GvHV(*gv) = HvREFCNT_inc_simple(PL_defstash);
}
}
goto ok;
Expand Down Expand Up @@ -3294,7 +3293,7 @@ Perl_Gv_AMupdate(pTHX_ HV *stash, bool destructing)
cv = MUTABLE_CV(gv);
filled = 1;
}
amt.table[i]=MUTABLE_CV(SvREFCNT_inc_simple(cv));
amt.table[i] = CvREFCNT_inc_simple(cv);

if (gv) {
switch (i) {
Expand Down Expand Up @@ -3878,7 +3877,7 @@ Perl_amagic_call(pTHX_ SV *left, SV *right, int method, int flags)
SvOBJECT_on(newref);
/* No need to do SvAMAGIC_on here, as SvAMAGIC macros
delegate to the stash. */
SvSTASH_set(newref, MUTABLE_HV(SvREFCNT_inc(SvSTASH(tmpRef))));
SvSTASH_set(newref, HvREFCNT_inc(SvSTASH(tmpRef)));
return newref;
}
}
Expand Down
18 changes: 18 additions & 0 deletions gv.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,24 @@ struct gp {
HEK * gp_file_hek; /* file first declared in (for -w) */
};

/*
=for apidoc_section $GV
=for apidoc Am|GV *|GvREFCNT_inc|GV *gv
=for apidoc_item |GV *|GvREFCNT_inc_simple|GV *gv
=for apidoc_item |GV *|GvREFCNT_inc_simple_NN|GV *gv
These all increment the reference count of the given SV, which must be a GV.
They are useful when assigning the result into a typed pointer as they avoid
the need to cast the result to the appropriate type.
=cut
*/

#define GvREFCNT_inc(gv) ((GV *)SvREFCNT_inc((SV *)gv))
#define GvREFCNT_inc_simple(gv) ((GV *)SvREFCNT_inc_simple((SV *)gv))
#define GvREFCNT_inc_simple_NN(gv) ((GV *)SvREFCNT_inc_simple_NN((SV *)gv))

#define GvXPVGV(gv) ((XPVGV*)SvANY(gv))


Expand Down
18 changes: 18 additions & 0 deletions hv.h
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,24 @@ struct xpvhv_with_aux {
struct xpvhv_aux xhv_aux;
};

/*
=for apidoc_section $HV
=for apidoc Am|HV *|HvREFCNT_inc|HV *hv
=for apidoc_item |HV *|HvREFCNT_inc_simple|HV *hv
=for apidoc_item |HV *|HvREFCNT_inc_simple_NN|HV *hv
These all increment the reference count of the given SV, which must be a HV.
They are useful when assigning the result into a typed pointer as they avoid
the need to cast the result to the appropriate type.
=cut
*/

#define HvREFCNT_inc(hv) ((HV *)SvREFCNT_inc((SV *)hv))
#define HvREFCNT_inc_simple(hv) ((HV *)SvREFCNT_inc_simple((SV *)hv))
#define HvREFCNT_inc_simple_NN(hv) ((HV *)SvREFCNT_inc_simple_NN((SV *)hv))

/*
=for apidoc AmnU||HEf_SVKEY
This flag, used in the length slot of hash entries and magic structures,
Expand Down
5 changes: 2 additions & 3 deletions pad.c
Original file line number Diff line number Diff line change
Expand Up @@ -558,8 +558,7 @@ S_pad_alloc_name(pTHX_ PADNAME *name, U32 flags, HV *typestash,

if (typestash) {
PadnameFLAGS(name) |= PADNAMEf_TYPED;
PadnameTYPE(name) =
MUTABLE_HV(SvREFCNT_inc_simple_NN(MUTABLE_SV(typestash)));
PadnameTYPE(name) = HvREFCNT_inc_simple_NN(typestash);
}
if (ourstash) {
PadnameFLAGS(name) |= PADNAMEf_OUR;
Expand Down Expand Up @@ -1973,7 +1972,7 @@ S_cv_clone_pad(pTHX_ CV *proto, CV *cv, CV *outside, HV *cloned,
PL_compcv = cv;
if (newcv) SAVEFREESV(cv); /* in case of fatal warnings */

CvOUTSIDE(cv) = MUTABLE_CV(SvREFCNT_inc_simple(outside));
CvOUTSIDE(cv) = CvREFCNT_inc_simple(outside);

SAVESPTR(PL_comppad_name);
PL_comppad_name = protopad_name;
Expand Down
14 changes: 4 additions & 10 deletions perl.c
Original file line number Diff line number Diff line change
Expand Up @@ -4130,7 +4130,7 @@ S_init_main_stash(pTHX)
of the SvREFCNT_dec, only to add it again with hv_name_set */
SvREFCNT_dec(GvHV(gv));
hv_name_sets(PL_defstash, "main", 0);
GvHV(gv) = MUTABLE_HV(SvREFCNT_inc_simple(PL_defstash));
GvHV(gv) = HvREFCNT_inc_simple(PL_defstash);
SvREADONLY_on(gv);
PL_incgv = gv_HVadd(gv_AVadd(gv_fetchpvs("INC", GV_ADD|GV_NOTQUAL,
SVt_PVAV)));
Expand Down Expand Up @@ -4475,15 +4475,9 @@ Perl_init_debugger(pTHX)
PL_curstash = (HV *)SvREFCNT_inc_simple(PL_debstash);

Perl_init_dbargs(aTHX);
PL_DBgv = MUTABLE_GV(
SvREFCNT_inc(gv_fetchpvs("DB::DB", GV_ADDMULTI, SVt_PVGV))
);
PL_DBline = MUTABLE_GV(
SvREFCNT_inc(gv_fetchpvs("DB::dbline", GV_ADDMULTI, SVt_PVAV))
);
PL_DBsub = MUTABLE_GV(SvREFCNT_inc(
gv_HVadd(gv_fetchpvs("DB::sub", GV_ADDMULTI, SVt_PVHV))
));
PL_DBgv = GvREFCNT_inc(gv_fetchpvs("DB::DB", GV_ADDMULTI, SVt_PVGV));
PL_DBline = GvREFCNT_inc(gv_fetchpvs("DB::dbline", GV_ADDMULTI, SVt_PVAV));
PL_DBsub = GvREFCNT_inc(gv_HVadd(gv_fetchpvs("DB::sub", GV_ADDMULTI, SVt_PVHV)));
PL_DBsingle = GvSV((gv_fetchpvs("DB::single", GV_ADDMULTI, SVt_PV)));
if (!SvIOK(PL_DBsingle))
sv_setiv(PL_DBsingle, 0);
Expand Down
4 changes: 2 additions & 2 deletions pp_ctl.c
Original file line number Diff line number Diff line change
Expand Up @@ -3427,7 +3427,7 @@ PP(pp_goto)
exit, so point it at arg again. */
if (arg != GvAV(PL_defgv)) {
AV * const av = GvAV(PL_defgv);
GvAV(PL_defgv) = (AV *)SvREFCNT_inc_simple(arg);
GvAV(PL_defgv) = AvREFCNT_inc_simple(arg);
SvREFCNT_dec(av);
}
}
Expand Down Expand Up @@ -4008,7 +4008,7 @@ S_doeval_compile(pTHX_ U8 gimme, CV* outside, U32 seq, HV *hh)
CX_CUR()->blk_gimme = gimme;

CvOUTSIDE_SEQ(evalcv) = seq;
CvOUTSIDE(evalcv) = MUTABLE_CV(SvREFCNT_inc_simple(outside));
CvOUTSIDE(evalcv) = CvREFCNT_inc_simple(outside);

/* set up a scratch pad */

Expand Down
2 changes: 1 addition & 1 deletion pp_hot.c
Original file line number Diff line number Diff line change
Expand Up @@ -6395,7 +6395,7 @@ PP(pp_entersub)

defavp = &GvAV(PL_defgv);
cx->blk_sub.savearray = *defavp;
*defavp = MUTABLE_AV(SvREFCNT_inc_simple_NN(av));
*defavp = AvREFCNT_inc_simple_NN(av);

/* it's the responsibility of whoever leaves a sub to ensure
* that a clean, empty AV is left in pad[0]. This is normally
Expand Down
10 changes: 5 additions & 5 deletions pp_sort.c
Original file line number Diff line number Diff line change
Expand Up @@ -923,12 +923,12 @@ PP(pp_sort)
/* standard perl sub with values passed as $a and $b */
SAVEGENERICSV(PL_firstgv);
SAVEGENERICSV(PL_secondgv);
PL_firstgv = MUTABLE_GV(SvREFCNT_inc(
PL_firstgv = GvREFCNT_inc(
gv_fetchpvs("a", GV_ADD|GV_NOTQUAL, SVt_PV)
));
PL_secondgv = MUTABLE_GV(SvREFCNT_inc(
);
PL_secondgv = GvREFCNT_inc(
gv_fetchpvs("b", GV_ADD|GV_NOTQUAL, SVt_PV)
));
);
/* make sure the GP isn't removed out from under us for
* the SAVESPTR() */
save_gp(PL_firstgv, 0);
Expand Down Expand Up @@ -959,7 +959,7 @@ PP(pp_sort)
AV * const av0 = MUTABLE_AV(PAD_SVl(0));

cx->blk_sub.savearray = GvAV(PL_defgv);
GvAV(PL_defgv) = MUTABLE_AV(SvREFCNT_inc_simple(av0));
GvAV(PL_defgv) = AvREFCNT_inc_simple(av0);
}

}
Expand Down
2 changes: 1 addition & 1 deletion regcomp.c
Original file line number Diff line number Diff line change
Expand Up @@ -2325,7 +2325,7 @@ Perl_re_op_compile(pTHX_ SV ** const patternp, int pat_count,
RExC_rx->intflags |= PREGf_USE_RE_EVAL;

if (RExC_paren_names)
RXp_PAREN_NAMES(RExC_rx) = MUTABLE_HV(SvREFCNT_inc(RExC_paren_names));
RXp_PAREN_NAMES(RExC_rx) = HvREFCNT_inc(RExC_paren_names);
else
RXp_PAREN_NAMES(RExC_rx) = NULL;

Expand Down
10 changes: 5 additions & 5 deletions sv.c
Original file line number Diff line number Diff line change
Expand Up @@ -1248,7 +1248,7 @@ Perl_sv_upgrade(pTHX_ SV *const sv, svtype new_type)
DEBUG_o(Perl_deb(aTHX_ "sv_upgrade clearing PL_stashcache\n"));
hv_clear(PL_stashcache);

SvSTASH_set(io, MUTABLE_HV(SvREFCNT_inc(GvHV(iogv))));
SvSTASH_set(io, HvREFCNT_inc(GvHV(iogv)));
IoPAGE_LEN(sv) = 60;
}
if (old_type < SVt_PV) {
Expand Down Expand Up @@ -6687,7 +6687,7 @@ S_anonymise_cv_maybe(pTHX_ GV *gv, CV* cv)

CvANON_on(cv);
CvCVGV_RC_on(cv);
SvANY(cv)->xcv_gv_u.xcv_gv = MUTABLE_GV(SvREFCNT_inc(anongv));
SvANY(cv)->xcv_gv_u.xcv_gv = GvREFCNT_inc_simple(anongv);
}


Expand Down Expand Up @@ -10923,7 +10923,7 @@ Perl_sv_bless(pTHX_ SV *const sv, HV *const stash)
}
SvOBJECT_on(tmpRef);
SvUPGRADE(tmpRef, SVt_PVMG);
SvSTASH_set(tmpRef, MUTABLE_HV(SvREFCNT_inc_simple(stash)));
SvSTASH_set(tmpRef, HvREFCNT_inc_simple(stash));
SvREFCNT_dec(oldstash);

if(SvSMAGICAL(tmpRef))
Expand Down Expand Up @@ -14623,9 +14623,9 @@ S_sv_dup_hvaux(pTHX_ const SV *const ssv, SV *dsv, CLONE_PARAMS *const param)
? NULL
: saux->xhv_backreferences
? (SvTYPE(saux->xhv_backreferences) == SVt_PVAV)
? MUTABLE_AV(SvREFCNT_inc(
? AvREFCNT_inc(
sv_dup_inc((const SV *)
saux->xhv_backreferences, param)))
saux->xhv_backreferences, param))
: MUTABLE_AV(sv_dup((const SV *)
saux->xhv_backreferences, param))
: 0;
Expand Down
10 changes: 5 additions & 5 deletions toke.c
Original file line number Diff line number Diff line change
Expand Up @@ -928,11 +928,11 @@ Perl_lex_start(pTHX_ SV *line, PerlIO *rsfp, U32 flags)
parser->rsfp_filters =
!(flags & LEX_START_SAME_FILTER) || !oparser
? NULL
: MUTABLE_AV(SvREFCNT_inc(
: AvREFCNT_inc(
oparser->rsfp_filters
? oparser->rsfp_filters
: (oparser->rsfp_filters = newAV())
));
);

Newx(parser->lex_brackstack, 120, char);
Newx(parser->lex_casestack, 12, char);
Expand Down Expand Up @@ -2004,8 +2004,8 @@ S_incline(pTHX_ const char *s, const char *end)
alias the saved lines that are in the array.
Otherwise alias the whole array. */
if (CopLINE(PL_curcop) == line_num) {
GvHV(gv2) = MUTABLE_HV(SvREFCNT_inc(GvHV(cfgv)));
GvAV(gv2) = MUTABLE_AV(SvREFCNT_inc(GvAV(cfgv)));
GvHV(gv2) = HvREFCNT_inc(GvHV(cfgv));
GvAV(gv2) = AvREFCNT_inc(GvAV(cfgv));
}
else if (GvAV(cfgv)) {
AV * const av = GvAV(cfgv);
Expand Down Expand Up @@ -12860,7 +12860,7 @@ Perl_start_subparse(pTHX_ I32 is_format, U32 flags)

PL_subline = CopLINE(PL_curcop);
CvPADLIST(PL_compcv) = pad_new(padnew_SAVE|padnew_SAVESUB);
CvOUTSIDE(PL_compcv) = MUTABLE_CV(SvREFCNT_inc_simple(outsidecv));
CvOUTSIDE(PL_compcv) = CvREFCNT_inc_simple(outsidecv);
CvOUTSIDE_SEQ(PL_compcv) = PL_cop_seqmax;
if (outsidecv && CvPADLIST(outsidecv))
CvPADLIST(PL_compcv)->xpadl_outid = CvPADLIST(outsidecv)->xpadl_id;
Expand Down

0 comments on commit de762f9

Please sign in to comment.