From: Nicholas Clark Date: Sat, 21 Apr 2007 11:42:43 +0000 (+0000) Subject: Where possible, use SvIV instead of SvIVX, SvNV instead of SvNVX, X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=4ea561bc94841f378b6950ed75b669dc60767dfa;p=p5sagit%2Fp5-mst-13.2.git Where possible, use SvIV instead of SvIVX, SvNV instead of SvNVX, SvUV instead of SvUVX, and SvPV* variants instead of SvPVX*. Document that the non-x variants are preferable whenever the expression has no side effects. (Compilers perform common subexression elimination). Likewise SvREFCNT_inc simple variants are valid for all cases apart from expressions with side effects. p4raw-id: //depot/perl@31010 --- diff --git a/doio.c b/doio.c index e1acc67..2d901fd 100644 --- a/doio.c +++ b/doio.c @@ -1609,7 +1609,7 @@ Perl_apply(pTHX_ I32 type, register SV **mark, register SV **sp) case OP_CHMOD: APPLY_TAINT_PROPER(); if (++mark <= sp) { - val = SvIVx(*mark); + val = SvIV(*mark); APPLY_TAINT_PROPER(); tot = sp - mark; while (++mark <= sp) { @@ -1703,7 +1703,7 @@ nothing in the core. Perl_croak(aTHX_ "Unrecognized signal name \"%s\"",s); } else - val = SvIVx(*mark); + val = SvIV(*mark); APPLY_TAINT_PROPER(); tot = sp - mark; #ifdef VMS @@ -1716,7 +1716,7 @@ nothing in the core. * CRTL's emulation of Unix-style signals and kill() */ while (++mark <= sp) { - I32 proc = SvIVx(*mark); + I32 proc = SvIV(*mark); register unsigned long int __vmssts; APPLY_TAINT_PROPER(); if (!((__vmssts = sys$delprc(&proc,0)) & 1)) { @@ -1740,7 +1740,7 @@ nothing in the core. if (val < 0) { val = -val; while (++mark <= sp) { - const I32 proc = SvIVx(*mark); + const I32 proc = SvIV(*mark); APPLY_TAINT_PROPER(); #ifdef HAS_KILLPG if (PerlProc_killpg(proc,val)) /* BSD */ @@ -1752,7 +1752,7 @@ nothing in the core. } else { while (++mark <= sp) { - const I32 proc = SvIVx(*mark); + const I32 proc = SvIV(*mark); APPLY_TAINT_PROPER(); if (PerlProc_kill(proc, val)) tot--; @@ -1810,16 +1810,16 @@ nothing in the core. else { Zero(&utbuf, sizeof utbuf, char); #ifdef HAS_FUTIMES - utbuf[0].tv_sec = (long)SvIVx(accessed); /* time accessed */ + utbuf[0].tv_sec = (long)SvIV(accessed); /* time accessed */ utbuf[0].tv_usec = 0; - utbuf[1].tv_sec = (long)SvIVx(modified); /* time modified */ + utbuf[1].tv_sec = (long)SvIV(modified); /* time modified */ utbuf[1].tv_usec = 0; #elif defined(BIG_TIME) - utbuf.actime = (Time_t)SvNVx(accessed); /* time accessed */ - utbuf.modtime = (Time_t)SvNVx(modified); /* time modified */ + utbuf.actime = (Time_t)SvNV(accessed); /* time accessed */ + utbuf.modtime = (Time_t)SvNV(modified); /* time modified */ #else - utbuf.actime = (Time_t)SvIVx(accessed); /* time accessed */ - utbuf.modtime = (Time_t)SvIVx(modified); /* time modified */ + utbuf.actime = (Time_t)SvIV(accessed); /* time accessed */ + utbuf.modtime = (Time_t)SvIV(modified); /* time modified */ #endif } APPLY_TAINT_PROPER(); diff --git a/op.c b/op.c index 55f0571..5ebbe17 100644 --- a/op.c +++ b/op.c @@ -4812,7 +4812,7 @@ Perl_newLOOPEX(pTHX_ I32 type, OP *label) o = newOP(type, OPf_SPECIAL); else { o = newPVOP(type, 0, savesharedpv(label->op_type == OP_CONST - ? SvPVx_nolen_const(((SVOP*)label)->op_sv) + ? SvPV_nolen_const(((SVOP*)label)->op_sv) : "")); } #ifdef PERL_MAD @@ -5249,11 +5249,11 @@ Perl_newATTRSUB(pTHX_ I32 floor, OP *o, OP *proto, OP *attrs, OP *block) = (block || attrs || (CvFLAGS(PL_compcv) & CVf_BUILTIN_ATTRS) || PL_madskills) ? GV_ADDMULTI : GV_ADDMULTI | GV_NOINIT; - const char * const name = o ? SvPVx_nolen_const(cSVOPo->op_sv) : NULL; + const char * const name = o ? SvPV_nolen_const(cSVOPo->op_sv) : NULL; if (proto) { assert(proto->op_type == OP_CONST); - ps = SvPVx_const(((SVOP*)proto)->op_sv, ps_len); + ps = SvPV_const(((SVOP*)proto)->op_sv, ps_len); } else ps = NULL; diff --git a/pod/perlapi.pod b/pod/perlapi.pod index a36ab88..7fc7e84 100644 --- a/pod/perlapi.pod +++ b/pod/perlapi.pod @@ -3968,7 +3968,8 @@ Found in file sv.h X Coerces the given SV to an integer and returns it. Guarantees to evaluate -sv only once. Use the more efficient C otherwise. +C only once. Only use this if C is an expression with side effects, +otherwise use the more efficient C. IV SvIVx(SV* sv) @@ -4138,7 +4139,8 @@ Found in file sv.h X Coerces the given SV to a double and returns it. Guarantees to evaluate -sv only once. Use the more efficient C otherwise. +C only once. Only use this if C is an expression with side effects, +otherwise use the more efficient C. NV SvNVx(SV* sv) @@ -4378,7 +4380,9 @@ Found in file sv.h =item SvPVx X -A version of C which guarantees to evaluate sv only once. +A version of C which guarantees to evaluate C only once. +Only use this if C is an expression with side effects, otherwise use the +more efficient C. char* SvPVx(SV* sv, STRLEN len) @@ -4489,9 +4493,8 @@ Found in file sv.h =item SvREFCNT_inc_simple X -Same as SvREFCNT_inc, but can only be used with simple variables, not -expressions or pointer dereferences. Since we don't have to store a -temporary value, it's faster. +Same as SvREFCNT_inc, but can only be used with expressions without side +effects. Since we don't have to store a temporary value, it's faster. SV* SvREFCNT_inc_simple(SV* sv) @@ -4775,7 +4778,8 @@ Found in file sv.h X Coerces the given SV to an unsigned integer and returns it. Guarantees to -evaluate sv only once. Use the more efficient C otherwise. +C only once. Only use this if C is an expression with side effects, +otherwise use the more efficient C. UV SvUVx(SV* sv) diff --git a/pp.c b/pp.c index 208a1a2..4903264 100644 --- a/pp.c +++ b/pp.c @@ -1475,7 +1475,7 @@ PP(pp_repeat) count = (IV)nv; } else - count = SvIVx(sv); + count = SvIV(sv); if (GIMME == G_ARRAY && PL_op->op_private & OPpREPEAT_DOLIST) { dMARK; static const char oom_list_extend[] = "Out of memory during list extend"; @@ -3863,7 +3863,7 @@ PP(pp_aslice) register SV **svp; I32 max = -1; for (svp = MARK + 1; svp <= SP; svp++) { - const I32 elem = SvIVx(*svp); + const I32 elem = SvIV(*svp); if (elem > max) max = elem; } @@ -3872,7 +3872,7 @@ PP(pp_aslice) } while (++MARK <= SP) { register SV **svp; - I32 elem = SvIVx(*MARK); + I32 elem = SvIV(*MARK); if (elem > 0) elem -= arybase; @@ -4118,7 +4118,7 @@ PP(pp_lslice) register SV **lelem; if (GIMME != G_ARRAY) { - I32 ix = SvIVx(*lastlelem); + I32 ix = SvIV(*lastlelem); if (ix < 0) ix += max; else @@ -4137,7 +4137,7 @@ PP(pp_lslice) } for (lelem = firstlelem; lelem <= lastlelem; lelem++) { - I32 ix = SvIVx(*lelem); + I32 ix = SvIV(*lelem); if (ix < 0) ix += max; else @@ -4216,7 +4216,7 @@ PP(pp_splice) SP++; if (++MARK < SP) { - offset = i = SvIVx(*MARK); + offset = i = SvIV(*MARK); if (offset < 0) offset += AvFILLp(ary) + 1; else diff --git a/pp_ctl.c b/pp_ctl.c index 6f91d02..861e8a5 100644 --- a/pp_ctl.c +++ b/pp_ctl.c @@ -3276,7 +3276,7 @@ PP(pp_require) || (*name == ':' && name[1] != ':' && strchr(name+2, ':')) #endif ) { - const char *dir = SvPVx_nolen_const(dirsv); + const char *dir = SvPV_nolen_const(dirsv); #ifdef MACOS_TRADITIONAL char buf1[256]; char buf2[256]; diff --git a/pp_sys.c b/pp_sys.c index aa36bef..4fc8196 100644 --- a/pp_sys.c +++ b/pp_sys.c @@ -3416,7 +3416,7 @@ PP(pp_chdir) gv = (GV*)SvRV(sv); } else { - tmps = SvPVx_nolen_const(sv); + tmps = SvPV_nolen_const(sv); } } diff --git a/sv.c b/sv.c index 917f806..5a76a93 100644 --- a/sv.c +++ b/sv.c @@ -8578,7 +8578,7 @@ Perl_sv_vcatpvfn(pTHX_ SV *sv, const char *pat, STRLEN patlen, va_list *args, SV has_precis = TRUE; } argsv = (SV*)va_arg(*args, void*); - eptr = SvPVx_const(argsv, elen); + eptr = SvPV_const(argsv, elen); if (DO_UTF8(argsv)) is_utf8 = TRUE; goto string; @@ -8837,7 +8837,7 @@ Perl_sv_vcatpvfn(pTHX_ SV *sv, const char *pat, STRLEN patlen, va_list *args, SV case 'c': if (vectorize) goto unknown; - uv = (args) ? va_arg(*args, int) : SvIVx(argsv); + uv = (args) ? va_arg(*args, int) : SvIV(argsv); if ((uv > 255 || (!UNI_IS_INVARIANT(uv) && SvUTF8(sv))) && !IN_BYTES) { @@ -8871,7 +8871,7 @@ Perl_sv_vcatpvfn(pTHX_ SV *sv, const char *pat, STRLEN patlen, va_list *args, SV } } else { - eptr = SvPVx_const(argsv, elen); + eptr = SvPV_const(argsv, elen); if (DO_UTF8(argsv)) { I32 old_precis = precis; if (has_precis && precis < elen) { @@ -8943,7 +8943,7 @@ Perl_sv_vcatpvfn(pTHX_ SV *sv, const char *pat, STRLEN patlen, va_list *args, SV } } else { - IV tiv = SvIVx(argsv); /* work around GCC bug #13488 */ + IV tiv = SvIV(argsv); /* work around GCC bug #13488 */ switch (intsize) { case 'h': iv = (short)tiv; break; case 'l': iv = (long)tiv; break; @@ -9028,7 +9028,7 @@ Perl_sv_vcatpvfn(pTHX_ SV *sv, const char *pat, STRLEN patlen, va_list *args, SV } } else { - UV tuv = SvUVx(argsv); /* work around GCC bug #13488 */ + UV tuv = SvUV(argsv); /* work around GCC bug #13488 */ switch (intsize) { case 'h': uv = (unsigned short)tuv; break; case 'l': uv = (unsigned long)tuv; break; @@ -9150,7 +9150,7 @@ Perl_sv_vcatpvfn(pTHX_ SV *sv, const char *pat, STRLEN patlen, va_list *args, SV #else va_arg(*args, double) #endif - : SvNVx(argsv); + : SvNV(argsv); need = 0; if (c != 'e' && c != 'E') { diff --git a/sv.h b/sv.h index ba873ea..54f319d 100644 --- a/sv.h +++ b/sv.h @@ -177,9 +177,8 @@ to return a meaningful value, or check for NULLness, so it's smaller and faster. =for apidoc Am|SV*|SvREFCNT_inc_simple|SV* sv -Same as SvREFCNT_inc, but can only be used with simple variables, not -expressions or pointer dereferences. Since we don't have to store a -temporary value, it's faster. +Same as SvREFCNT_inc, but can only be used with expressions without side +effects. Since we don't have to store a temporary value, it's faster. =for apidoc Am|SV*|SvREFCNT_inc_simple_NN|SV* sv Same as SvREFCNT_inc_simple, but can only be used if you know I @@ -1550,7 +1549,9 @@ stringified version becoming C. Handles 'get' magic. See also C for a version which guarantees to evaluate sv only once. =for apidoc Am|char*|SvPVx|SV* sv|STRLEN len -A version of C which guarantees to evaluate sv only once. +A version of C which guarantees to evaluate C only once. +Only use this if C is an expression with side effects, otherwise use the +more efficient C. =for apidoc Am|char*|SvPV_nomg|SV* sv|STRLEN len Like C but doesn't process magic. @@ -1569,7 +1570,8 @@ Like C but doesn't process magic. =for apidoc Am|IV|SvIVx|SV* sv Coerces the given SV to an integer and returns it. Guarantees to evaluate -sv only once. Use the more efficient C otherwise. +C only once. Only use this if C is an expression with side effects, +otherwise use the more efficient C. =for apidoc Am|NV|SvNV|SV* sv Coerce the given SV to a double and return it. See C for a version @@ -1577,7 +1579,8 @@ which guarantees to evaluate sv only once. =for apidoc Am|NV|SvNVx|SV* sv Coerces the given SV to a double and returns it. Guarantees to evaluate -sv only once. Use the more efficient C otherwise. +C only once. Only use this if C is an expression with side effects, +otherwise use the more efficient C. =for apidoc Am|UV|SvUV|SV* sv Coerces the given SV to an unsigned integer and returns it. See C @@ -1588,7 +1591,8 @@ Like C but doesn't process magic. =for apidoc Am|UV|SvUVx|SV* sv Coerces the given SV to an unsigned integer and returns it. Guarantees to -evaluate sv only once. Use the more efficient C otherwise. +C only once. Only use this if C is an expression with side effects, +otherwise use the more efficient C. =for apidoc Am|bool|SvTRUE|SV* sv Returns a boolean indicating whether Perl would evaluate the SV as true or