Skip to content

tick in symbols: rename new prototype scan_word and provide a forwarder #21165

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

Merged
merged 1 commit into from
Jun 23, 2023

Conversation

tonycoz
Copy link
Contributor

@tonycoz tonycoz commented Jun 22, 2023

The change in prototype of the non-API scan_word() function caused B::Hooks::Parser to fail to build.

Instead, rename scan_word() with the new prototype to scan_word2() and provide a scan_word() with the old signature.

The scan_word2() entry may be removed once tick in symbol becomes an error.

Fixes #21162

@leonerd
Copy link
Contributor

leonerd commented Jun 22, 2023

Tiniest comment: It's common in e.g. Linux kernel, when adding new functions with more arguments, to use the number of arguments to name the function. So here it'd be scan_word6. Do we want to consider doing similar in this case?

@ilmari
Copy link
Member

ilmari commented Jun 22, 2023

Tiniest comment: It's common in e.g. Linux kernel, when adding new functions with more arguments, to use the number of arguments to name the function. So here it'd be scan_word6. Do we want to consider doing similar in this case?

There's precedent for that in perl too:

~/src/perl (blead $)$ rg '^Perl_\w+\d+\(' proto.h |rg -vi 'utf8|latin1|[iu](8|16|32|64)'
Perl_csighandler1(int sig);
Perl_csighandler3(int sig, Siginfo_t *info, void *uap);
Perl_do_aexec5(pTHX_ SV *really, SV **mark, SV **sp, int fd, int do_report)
Perl_gv_efullname4(pTHX_ SV *sv, const GV *gv, const char *prefix, bool keepmain);
Perl_gv_fullname4(pTHX_ SV *sv, const GV *gv, const char *prefix, bool keepmain);
Perl_my_atof2(pTHX_ const char *orig, NV *value);
Perl_my_atof3(pTHX_ const char *orig, NV *value, const STRLEN len);
Perl_pregfree2(pTHX_ REGEXP *rx);
Perl_sighandler1(int sig)
Perl_sighandler3(int sig, Siginfo_t *info, void *uap)
Perl_sys_init3(int *argc, char ***argv, char ***env);
Perl_gv_efullname3(pTHX_ SV *sv, const GV *gv, const char *prefix);
Perl_gv_fullname3(pTHX_ SV *sv, const GV *gv, const char *prefix);
Perl_do_exec3(pTHX_ const char *incmd, int fd, int do_report)

The change in prototype of the non-API scan_word() function caused
B::Hooks::Parser to fail to build.

Instead, rename scan_word() with the new prototype to scan_word6() and
provide a scan_word() with the old signature that forwards calls to the
new function.

scan_word6() may be removed once tick in symbol becomes an error

Fixes Perl#21162
@tonycoz tonycoz force-pushed the 21162-scan_word-api-or-not branch from f20372e to c291c6e Compare June 23, 2023 00:09
@tonycoz
Copy link
Contributor Author

tonycoz commented Jun 23, 2023

Tiniest comment: It's common in e.g. Linux kernel, when adding new functions with more arguments, to use the number of arguments to name the function. So here it'd be scan_word6. Do we want to consider doing similar in this case?

Good idea, done.

Copy link
Contributor

@leonerd leonerd left a comment

Choose a reason for hiding this comment

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

LGTM

As it's a BBC issue, do we want to consider putting this in before 5.38.0-RC2?

@ilmari
Copy link
Member

ilmari commented Jun 23, 2023

As it's a BBC issue, do we want to consider putting this in before 5.38.0-RC2?

I thinks so, but I guess it's up to @rjbs to decide.

@rjbs rjbs merged commit 7e0355c into Perl:blead Jun 23, 2023
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.

BBC 5.37.x breaks B::Hooks::Parser
4 participants