Skip to content

add the new vstring API to Devel::PPPort #23160

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 5 commits into from
Apr 2, 2025
Merged

Conversation

tonycoz
Copy link
Contributor

@tonycoz tonycoz commented Mar 27, 2025

Adds the new vstring APIs to ppport.h

Also improves a Devel::PPPort test and extends Porting/test-dist-modules.pl to aid in development.


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

@Leont
Copy link
Contributor

Leont commented Mar 27, 2025

I think it also needs to be added to parts/, otherwise the ppport.h script will not detect that an XS module using SvVSTRING/sv_vstring_get will need to define NEED_sv_vstring_get.

I've never been 100% sure of how that part works, but I suspect it needs to be set to #T in parts/todo/5008000, and to #U in parts/base/5041010.

tonycoz added a commit to tonycoz/perl5 that referenced this pull request Mar 31, 2025
Based on @Leont's comment on Perl#23160.

This seems to produce reasonable results:

tony@venus:.../git/perl6$ cat foo.c
SvVSTRING
tony@venus:.../git/perl6$ ./perl -Ilib dist/Devel-PPPort/ppport.h --nofilter foo.c
Scanning foo.c ...
=== Analyzing foo.c ===
Uses SvVSTRING, which depends on sv_vstring_get, SvVSTRING_mg, mg_find, PERL_MAGIC_vstring, SvMAGICAL
File needs sv_vstring_get, adding static request
Needs to include 'ppport.h'
Analysis completed
Suggested changes:
--- foo.c       2025-04-01 10:51:39.040415623 +1100
+++ foo.c.patched       2025-04-01 10:55:11.347014468 +1100
@@ -1 +1,3 @@
+#define NEED_sv_vstring_get
+#include "ppport.h"
 SvVSTRING
Copy link
Contributor

@Leont Leont left a comment

Choose a reason for hiding this comment

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

It looks good to me (though I would squash the fixes)

tonycoz added 5 commits April 2, 2025 10:08
A change I made caused this to fail, tracking it down was rough.
This is especially handy when tracking down problems with
Devel::PPPort, since so much is generated.
vstrings were originally added in perl-5.8.0-82-g92f0c26562,
SvVSTRING_mg() was originally added in perl-5.8.0-8018-gb0a11fe104 so
technically in the same release.

I expect there were some uses of intermediate versions 19 years ago,
but I don't think we need to worry about it now.
Based on @Leont's comment on Perl#23160.

This seems to produce reasonable results:

tony@venus:.../git/perl6$ cat foo.c
SvVSTRING
tony@venus:.../git/perl6$ ./perl -Ilib dist/Devel-PPPort/ppport.h --nofilter foo.c
Scanning foo.c ...
=== Analyzing foo.c ===
Uses SvVSTRING, which depends on sv_vstring_get, SvVSTRING_mg, mg_find, PERL_MAGIC_vstring, SvMAGICAL
File needs sv_vstring_get, adding static request
Needs to include 'ppport.h'
Analysis completed
Suggested changes:
--- foo.c       2025-04-01 10:51:39.040415623 +1100
+++ foo.c.patched       2025-04-01 10:55:11.347014468 +1100
@@ -1 +1,3 @@
+#define NEED_sv_vstring_get
+#include "ppport.h"
 SvVSTRING
@tonycoz tonycoz force-pushed the 23075-vstring-api branch from 996e27b to 0a223ec Compare April 1, 2025 23:10
@tonycoz tonycoz merged commit 61a68ee into Perl:blead Apr 2, 2025
33 checks passed
@tonycoz tonycoz deleted the 23075-vstring-api branch April 2, 2025 02:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants