Skip to content

op.c auto-use attributes.pm; newSVpvs("attributes") -> newSVpvs_share() #23424

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

Open
wants to merge 1 commit into
base: blead
Choose a base branch
from

Conversation

bulk88
Copy link
Contributor

@bulk88 bulk88 commented Jul 11, 2025

  • faster, it is guaranteed "use attributes;" creates a hash key named "attributes" that exists for the rest of the process

  • This set of changes does not require a perldelta entry.

- faster, it is guaranteed "use attributes;" creates a hash key
  named "attributes" that exists for the rest of the process
@bulk88 bulk88 force-pushed the op_c_attributes_auto_use_auto_load_add_SV_HEK_COWs_for_pkg branch from 5d0e4c4 to 81ca02a Compare July 11, 2025 03:08
@@ -3888,7 +3888,7 @@ S_apply_attrs(pTHX_ HV *stash, SV *target, OP *attrs)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The commit message is not comprehensible to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have a suggestion something better than my wording of "op.c auto-use attributes.pm;"?

I have no preference on the wording of the commit title. Its for future ppl who are not me to read.

There are 3 different places in op.c that do C level equivalent of PP use attributes; or require attributes; every time they execute

void Perl_apply_attrs_string(pTHX_ const char *stashpv, CV *cv, const char *attrstr, STRLEN len)
void S_apply_attrs_my(pTHX_ HV *stash, OP *target, OP *attrs, OP **imopsp)
void S_apply_attrs(pTHX_ HV *stash, SV *target, OP *attrs)

What would you call the use attributes; from C feature in them? I can't think of anything better than "auto-use" and just writing "use attributes" is too confusing, PP level? C level? the keyword "use" or "use the attributes API"?

I know "auto-use Foo.pm;" means somewhere Perl_load_module() is going to be called. But I think nobody knows what Perl_load_module() is or does off the top of their head. Its super rare for me see Perl_load_module() anywhere in core or CPAN XS.

I believe its not a day 1 Perl API call and was added in the 5.6.x era or 5.9.x or 5.10.x or 5.12.x era, because predecessor void Perl_require_pv(pTHX_ const char *pv) was a very quick hack job that wakes up the yylexer API.

perl5/perl.c

Line 3517 in 94b9bf6

Perl_require_pv(pTHX_ const char *pv)

and I think XS devs are afraid of Perl_load_module() because it is the 1 and only function call in the entire Public API takes over ownership of the callers SV*, excluding very obvious and very basic hv_store()/av_store() calls.

Perl_load_module() is also "too cool for school" to a CPAN XS person, since its C code looks like the output of B::ByteLoader or the output of B::C or B::CC.

perl5/op.c

Line 8471 in 94b9bf6

modname = newSVOP(OP_CONST, 0, name);

I dont mind it being copy pasted "B:CC" codegen, its a smart and faster design choice than parsing an ASCII string . But Perl_load_module() scares off 90% of high skill set CPAN XS/C devs, IDK why, Perl_load_module() just does. So b/c is so rarely used by Perl XS ppl, I didn't want to put its name in a commit title.

Perl C ppl don't remember what it does off the top of their head, so I would be making a future reader who reads the title, searching for "what commit could be causing my Zoo bug in the interp's Foo subsystem?", waste time reading its diff code, or just reading the title only and then spend 30 seconds greping perlapi and perlguts to remember what it actually does. I know everyone knows what use attributes; does and can keep scrolling the git log with the mouse wheel. Commit title op.c: Perl_load_module(): newSVxxv() -> newSVyyv() is useless without opening it.

So I thought a "PP" worded title like "op.c auto-use attributes.pm; newSVpvs("attributes") -> newSVpvs_share()" is much better than writing a title like "op.c Perl_load_module(), newSVpvs("attributes") -> newSVpvs_share()" which nobody knows what on earth this commit is doing inside of op.c without opening it because op.c is as huge as beach but with code instead of sand and every commit is the size of dime.

I think "auto-use attributes.pm;" best describes the "location" or "area" of op.c being touched with GPS level accuracy, writing

op.c: Perl_apply_attrs_string S_apply_attrs_my S_apply_attrs: Perl_load_module: newSVpvs -> newSVpvs_share

is too long to be a commit title and unreadable except for the most hard core C devs at P5P, and I try to consider readability of a title for non-C ppl so `"op.c: auto-use attributes;" sounds friendly to read vs the way too list of C symbols above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants