Skip to content

Commit 7bcccf4

Browse files
committed
Avoid redefining SvREADONLY_on in gv.c as that causes confusion
When reading the previous code, it was confusing to see calls to `SvREADONLY_on`, as without being aware the macro was redefined just above this function, the reader is unlikely to be aware that here it doesn't set the SVf_PROTECT flag. We should instead define a custom-purpose macro here and use that, to make its operation much clearer.
1 parent d98b252 commit 7bcccf4

File tree

1 file changed

+8
-14
lines changed

1 file changed

+8
-14
lines changed

gv.c

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2063,10 +2063,8 @@ S_find_default_stash(pTHX_ HV **stash, const char *name, STRLEN len,
20632063
}
20642064

20652065
/* gv_magicalize only turns on the SVf_READONLY flag, not SVf_PROTECT. So
2066-
redefine SvREADONLY_on for that purpose. We don’t use it later on in
2067-
this file. */
2068-
#undef SvREADONLY_on
2069-
#define SvREADONLY_on(sv) (SvFLAGS(sv) |= SVf_READONLY)
2066+
make a special-purpose macro just for that. */
2067+
#define SvREADONLY_NOPROTECT_on(sv) (SvFLAGS(sv) |= SVf_READONLY)
20702068

20712069
/* gv_magicalize() is called by gv_fetchpvn_flags when creating
20722070
* a new GV.
@@ -2187,7 +2185,7 @@ S_gv_magicalize(pTHX_ GV *gv, HV *stash, const char *name, STRLEN len,
21872185
const Size_t n = *name;
21882186

21892187
sv_magic(MUTABLE_SV(av), (SV*)n, PERL_MAGIC_regdata, NULL, 0);
2190-
SvREADONLY_on(av);
2188+
SvREADONLY_NOPROTECT_on(av);
21912189

21922190
require_tie_mod_s(gv, '+', "Tie::Hash::NamedCapture",0);
21932191

@@ -2359,7 +2357,7 @@ S_gv_magicalize(pTHX_ GV *gv, HV *stash, const char *name, STRLEN len,
23592357
{ /* $- $+ */
23602358
sv_magic(GvSVn(gv), MUTABLE_SV(gv), PERL_MAGIC_sv, name, len);
23612359
if (*name == '+')
2362-
SvREADONLY_on(GvSVn(gv));
2360+
SvREADONLY_NOPROTECT_on(GvSVn(gv));
23632361
}
23642362
{ /* %- %+ */
23652363
if (sv_type == SVt_PVHV || sv_type == SVt_PVGV)
@@ -2370,7 +2368,7 @@ S_gv_magicalize(pTHX_ GV *gv, HV *stash, const char *name, STRLEN len,
23702368
const Size_t n = *name;
23712369

23722370
sv_magic(MUTABLE_SV(av), (SV*)n, PERL_MAGIC_regdata, NULL, 0);
2373-
SvREADONLY_on(av);
2371+
SvREADONLY_NOPROTECT_on(av);
23742372
}
23752373
break;
23762374
case '*': /* $* */
@@ -2387,7 +2385,7 @@ S_gv_magicalize(pTHX_ GV *gv, HV *stash, const char *name, STRLEN len,
23872385
goto magicalize;
23882386
case '\023': /* $^S */
23892387
ro_magicalize:
2390-
SvREADONLY_on(GvSVn(gv));
2388+
SvREADONLY_NOPROTECT_on(GvSVn(gv));
23912389
/* FALLTHROUGH */
23922390
case '0': /* $0 */
23932391
case '^': /* $^ */
@@ -2431,7 +2429,7 @@ S_gv_magicalize(pTHX_ GV *gv, HV *stash, const char *name, STRLEN len,
24312429
if (!sv_derived_from(PL_patchlevel, "version"))
24322430
upg_version(PL_patchlevel, TRUE);
24332431
GvSV(gv) = vnumify(PL_patchlevel);
2434-
SvREADONLY_on(GvSV(gv));
2432+
SvREADONLY_NOPROTECT_on(GvSV(gv));
24352433
SvREFCNT_dec(sv);
24362434
break;
24372435
}
@@ -2440,7 +2438,7 @@ S_gv_magicalize(pTHX_ GV *gv, HV *stash, const char *name, STRLEN len,
24402438
{
24412439
SV * const sv = GvSV(gv);
24422440
GvSV(gv) = new_version(PL_patchlevel);
2443-
SvREADONLY_on(GvSV(gv));
2441+
SvREADONLY_NOPROTECT_on(GvSV(gv));
24442442
SvREFCNT_dec(sv);
24452443
break;
24462444
}
@@ -2458,10 +2456,6 @@ S_gv_magicalize(pTHX_ GV *gv, HV *stash, const char *name, STRLEN len,
24582456
|| (GvSV(gv) && (SvOK(GvSV(gv)) || SvMAGICAL(GvSV(gv))));
24592457
}
24602458

2461-
/* If we do ever start using this later on in the file, we need to make
2462-
sure we don’t accidentally use the wrong definition. */
2463-
#undef SvREADONLY_on
2464-
24652459
/* This function is called when the stash already holds the GV of the magic
24662460
* variable we're looking for, but we need to check that it has the correct
24672461
* kind of magic. For example, if someone first uses $! and then %!, the

0 commit comments

Comments
 (0)