Davem/xspod #23795
Open
Davem/xspod #23795
+4,590
−1,871
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tonycoz
reviewed
Oct 6, 2025
tonycoz
reviewed
Oct 6, 2025
tonycoz
reviewed
Oct 6, 2025
johannessen
suggested changes
Oct 7, 2025
Contributor
johannessen
left a comment
johannessen
left a comment
There was a problem hiding this comment.
I left some comments, mostly about minor things.
Overall, I found the document to be well structured and the prose to be easily readable, in spite of the fairly complex topic.
Leont
reviewed
Oct 8, 2025
Leont
reviewed
Oct 8, 2025
Contributor
Author
|
On Wed, Oct 08, 2025 at 04:53:18PM -0700, Leon Timmermans wrote:
Should we still use PREINIT? I see it's use-case in C89, but in C99 it
can be confusing (mainly because it runs before argument handling).
I didn't look too closely, but my quick experimentation seemed to
indicate that the flag(s) telling perl to use C99 weren't necessarily
being propagated to XS compilation. So I was playing safe. Also if people
want to write code that runs on older perls, or have any reason to
declare and possibly initialise a var before argument processing, then
PREINIT should be the official way.
|
Contributor
|
Yes, the -std flag is set by the cflags script and not in Some systems (I think non-x86 BSDs) can use old gccs that don't default to c99 or later. |
Contributor
Author
|
On Sat, Oct 11, 2025 at 03:00:31PM -0700, Leon Timmermans wrote:
@Leont commented on this pull request.
> + require XSLoader;
+ XSLoader::load(__PACKAGE__, $VERSION);
That automatically adds the module name but not the expected version. Personally I wouldn't use it..
I was just following the form of the Foo.pm which h2xs generates. I have
no knowledge or strong opinions about the usage of XSLoader.
|
wolfsage
reviewed
Oct 17, 2025
Contributor
Author
|
On Thu, Oct 16, 2025 at 05:44:45PM -0700, Matthew Horsfall (alh) wrote:
@wolfsage commented on this pull request.
Is this intended to be a link to other documentation? Because this is in
a code block, it won't be treated that way by pod parsers
Yes, it was originally intended to link. Then I realised that (as you point
out) it doesn't link, but I forgot to update it.
|
tonycoz
reviewed
Oct 20, 2025
tonycoz
reviewed
Oct 21, 2025
tonycoz
reviewed
Oct 21, 2025
Contributor
|
I'm done for now. |
Contributor
Author
|
On Mon, Oct 20, 2025 at 07:58:13PM -0700, Tony Cook wrote:
> +=head3 Perl OPs
...
99%* of XS code doesn't touch OPs, I don't think it needs to be the
first heading, or even mentioned at all here.
I mentioned OPs mainly as a way of introducing OP_ENTERSUB, which I needed
to do to explain how XSUBs get called.
Also, I wanted to give readers a bit of background knowledge abut how the
interpreter works, to make sense of things when debugging.
I'll think about this some more.
|
Contributor
Author
|
On Mon, Oct 20, 2025 at 08:35:05PM -0700, Tony Cook wrote:
I'm not sure it's good to go into so much detail on the internal
structure of SVs here.
I expect nearly all XS code is fine with newSViv()/sv_setiv()/SvIV() etc
rather than looking at any flags beyond SvOK() and SvUTF8().
I wanted to educate people into the difference between SvIVX() and SvIV(),
for example. I've emphasised that they should generally use the latter,
but at least now they're aware of the difference when they then try and
cargo-cult something from an existing distro that incorrectly uses
SvIVX().
|
wolfsage
reviewed
Oct 22, 2025
Contributor
wolfsage
left a comment
wolfsage
left a comment
There was a problem hiding this comment.
(more to come, don't want this to get lost)
wolfsage
reviewed
Oct 23, 2025
Contributor
wolfsage
left a comment
wolfsage
left a comment
There was a problem hiding this comment.
Alright that's enough for one night, more later
wolfsage
reviewed
Oct 23, 2025
Contributor
Author
|
On Sun, Oct 26, 2025 at 05:34:58PM -0700, Tony Cook wrote:
+ This SV has a lifetime which is the same as the sub which the OP is
+ a part of
No, it lives as long as the sub
I don't understand. Aren't we both saying the same thing?
|
Contributor
You and I are, wolfsage seemed to think otherwise. |
wolfsage
reviewed
Nov 1, 2025
Contributor
wolfsage
left a comment
wolfsage
left a comment
There was a problem hiding this comment.
Final review done! Few more small things found.
The XS parser supported an extremely obscure bit of functionality which
made use of the %v package variable to maintain state between different
bits of typemap processing. This was accidentally broken in 5.10.0:
refactoring removed the 'use vars "%v"' line, and no one seemed to
notice or care.
Also, the sole example of its use in the docs seemed to be obscure,
confusing and probably wrong.
There was a consensus in the discussion at
http://nntp.perl.org/group/perl.perl5.porters/267667
that we should stop documenting this feature rather than trying to fix
it.
The various XS code examples had odd and inconsistent indentation (often with 5 leading spaces) and inconsistent formatting, e.g. foo(a,b) vs foo( a, b ) vs foo(a, b). Fix that, and also remove any tab chars. Whitespace-only change.
This commit is a simple cut which deletes several '=head2' sections from
perlxs.pod. The next commit will tidy up and fix any broken links etc.
These sections are more tutorial-like, and aren't in line with the goal
of this branch that perlxs.pod becomes purely a reference manual for XS.
Any relevant information from these sections may be incorporated later
into new sections in perlxs.pod and/or be included in a future rewrite
of perlxstut.pod.
The sections deleted are:
=head2 Introduction
=head2 On The Road
=head2 The Anatomy of an XSUB
=head2 The Argument Stack
=head2 The RETVAL Variable
=head2 Returning SVs, AVs and HVs through RETVAL
=head2 Returning Undef And Empty Lists
=head2 Interface Strategy
=head2 Perl Objects And C Structures
The previous commit deleted several sections from perlxs.pod. This commit fixes things up; done as a separate commit so that the changes aren't drowned out in the diff listing.
This big commit does a series of plain cut+pastes to reorder all the
=head2 sections within the file.
This changes the order from semi-random into roughly the order the
various XS keywords would appear within an XS file, and then within an
XSUB declaration/definition.
No changes have been made to the text: simply that all lines from a
particular '^=head2' up until the next head2 have been cut+paste as
a single unit.
No attempt has been made yet to make the text consistent with the new
ordering; that will be done by the subsequent commits of this branch.
The previous ordering in this file was:
=head1 NAME
=head1 DESCRIPTION
=head2 The MODULE Keyword
=head2 The PACKAGE Keyword
=head2 The PREFIX Keyword
=head2 The OUTPUT: Keyword
=head2 The NO_OUTPUT Keyword
=head2 The CODE: Keyword
=head2 The INIT: Keyword
=head2 The NO_INIT Keyword
=head2 The TYPEMAP: Keyword
=head2 Initializing Function Parameters
=head2 Default Parameter Values
=head2 The PREINIT: Keyword
=head2 The SCOPE: Keyword
=head2 The INPUT: Keyword
=head2 The IN/OUTLIST/IN_OUTLIST/OUT/IN_OUT Keywords
=head2 The C<length(NAME)> Keyword
=head2 Variable-length Parameter Lists
=head2 The C_ARGS: Keyword
=head2 The PPCODE: Keyword
=head2 The REQUIRE: Keyword
=head2 The CLEANUP: Keyword
=head2 The POSTCALL: Keyword
=head2 The BOOT: Keyword
=head2 The VERSIONCHECK: Keyword
=head2 The PROTOTYPES: Keyword
=head2 The PROTOTYPE: Keyword
=head2 The ALIAS: Keyword
=head2 The OVERLOAD: Keyword
=head2 The FALLBACK: Keyword
=head2 The INTERFACE: Keyword
=head2 The INTERFACE_MACRO: Keyword
=head2 The INCLUDE: Keyword
=head2 The INCLUDE_COMMAND: Keyword
=head2 The CASE: Keyword
=head2 The EXPORT_XSUB_SYMBOLS: Keyword
=head2 The & Unary Operator
=head2 Inserting POD, Comments and C Preprocessor Directives
=head2 Using XS With C++
=head2 Safely Storing Static Data in XS
=head3 MY_CXT REFERENCE
=head1 EXAMPLES
=head1 CAVEATS
=head2 Use of standard C library functions
=head2 Event loops and control flow
=head1 XS VERSION
=head1 AUTHOR DIAGNOSTICS
=head1 AUTHOR
and is now:
=head1 NAME
=head1 DESCRIPTION
=head2 The MODULE Keyword
=head2 The PACKAGE Keyword
=head2 The PREFIX Keyword
=head2 Inserting POD, Comments and C Preprocessor Directives
=head2 The REQUIRE: Keyword
=head2 The VERSIONCHECK: Keyword
=head2 The PROTOTYPES: Keyword
=head2 The EXPORT_XSUB_SYMBOLS: Keyword
=head2 The INCLUDE: Keyword
=head2 The INCLUDE_COMMAND: Keyword
=head2 The TYPEMAP: Keyword
=head2 The BOOT: Keyword
=head2 The FALLBACK: Keyword
=head2 The NO_OUTPUT Keyword
=head2 The IN/OUTLIST/IN_OUTLIST/OUT/IN_OUT Keywords
=head2 Default Parameter Values
=head2 The C<length(NAME)> Keyword
=head2 Variable-length Parameter Lists
=head2 The PREINIT: Keyword
=head2 The INPUT: Keyword
=head2 The NO_INIT Keyword
=head2 Initializing Function Parameters
=head2 The & Unary Operator
=head2 The SCOPE: Keyword
=head2 The INIT: Keyword
=head2 The C_ARGS: Keyword
=head2 The CODE: Keyword
=head2 The PPCODE: Keyword
=head2 The POSTCALL: Keyword
=head2 The OUTPUT: Keyword
=head2 The CLEANUP: Keyword
=head2 The PROTOTYPE: Keyword
=head2 The OVERLOAD: Keyword
=head2 The ALIAS: Keyword
=head2 The INTERFACE: Keyword
=head2 The INTERFACE_MACRO: Keyword
=head2 The CASE: Keyword
=head2 Using XS With C++
=head2 Safely Storing Static Data in XS
=head3 MY_CXT REFERENCE
=head1 EXAMPLES
=head1 CAVEATS
=head2 Use of standard C library functions
=head2 Event loops and control flow
=head1 XS VERSION
=head1 AUTHOR DIAGNOSTICS
=head1 AUTHOR
Following the previous commit's reordering of the all the =head2 sections, demote most of the =head2 headers to =head3, and add some new =head2 headers which group together related headers. Also add some =head3's for a few missing keywords. Subsequent commits will flesh out the new sections.
Add text for the new '=head2 The XSUB Input Part' section, and rewrite the existing entry for the PREINIT keyword.
This commit completely rewrites this section and subsections:
=head3 The INPUT: Keyword
=head4 The NO_INIT Keyword
=head4 Initializing Function Parameters
=head4 The & Unary Operator
It de-emphasises the INPUT keyword and suggests using ANSI XS signatures
etc instead.
Add text for the new '=head2 The XSUB Init Part' section, and rewrite the existing entry for the INIT keyword.
Add text to the new
=head2 The XSUB Code Part
=head3 Auto-calling a C function
sections, and rewrite the existing
=head4 The C_ARGS: Keyword
section
Rewrite these sections:
=head3 The CODE: Keyword
=head3 The PPCODE: Keyword
This keyword formerly wasn't documented. The docs now say "this is what it is, but don't use it".
Add text to the new
=head2 The XSUB Output Part
section, and rewrite the text in these existing sections:
=head3 The POSTCALL: Keyword
=head3 The OUTPUT: Keyword
Add text to the new
=head2 The XSUB Cleanup Part
section, and rewrite the text in this existing section:
=head3 The CLEANUP: Keyword
Add text to the new
=head2 XSUB Generic Keywords
section, and rewrite the text in this existing section:
=head3 The PROTOTYPE: Keyword
Explain that a 'C' parameter type in an XSUB declaration can actually
be a Perl package name or similar, e.g.
Foo::Bar
f(Foo::Bar obj, char *s)
First, add a new subsection
=head3 T_PTROBJ and opaque handles
to the TYPEMAPs section explaining how this typemap can be used to
map between Perl objects and C library handles. It provides a
fully-worked example of wrapping a simple arithmetic library.
Then completely rewrite the
=head3 The OVERLOAD: Keyword
section. In particular, it now refers to the new T_PTROBJ example and
shows how it can be extended to use overloading.
This keyword was undocumented, even though it had been added 25 years ago.
Populate the introduction to this new section.
Rewrite this section:
=head3 The ALIAS: Keyword
Rewrite these sections: =head3 The INTERFACE: Keyword =head3 The INTERFACE_MACRO: Keyword also demote the second to be a head4 child of the first. Then expand the T_PTROBJ example to use INTERFACE as an alternative to ALIAS.
Rewrite this section:
=head3 The CASE: Keyword
Populate this new section (except for the T_PTROBJ subsection, which had already been added by an earlier commit within this branch). Note that the "Common typemaps" subsection could probably benefit from some further expansion by someone familiar with which built-in T_FOO entries are useful.
Rewrite this section: =head2 Using XS With C++ Disclaimer: I've never written a proper C++ program. I had to (literally) dust off my 34-year old copy of Stroustrup(*) and also do some Googling. Hopefully what I've written is sane. (*) This was bought back in the days when people used to to learn things by buying books, and when I thought that I ought to know something about this newfangled C++ thing. I never got round to reading all of it: I discovered Perl around the same time, which looked to be a lot more fun.
Revise the text in this section:
=head2 Safely Storing Static Data in XS
Rewrite this section:
=head1 EXAMPLES
Basically, delete the one big example in this section and instead
provide links to various other examples already present in this document
instead.
Tweak the final few sections of perlxs.pod.
After the big rewrite, various bits of text which describe when a particular feature was introduced or changed have ended up using a random mixture of Perl, ParseXS and xsubpp version numbers. This commit standardises on xsubpp version numbers. These are mostly the same as ParseXS, but this handles the cases before ParseXS was split off from xsubpp. Perl versions suffer from not exactly matching when an xsubpp version number was incremented, and not matching what is used by the "REQUIRE:" keyword. This commit also adds a short new section which tries to explain how the three sets of version numbers are related.
The many commits in this branch have completely rewritten perlxs.pod. This commit applies all the minor tweaks suggested by reviewers. (It was far too much like hard work to try and update each individual commit with the various changes.)
Contributor
Author
|
Thanks for all the feedback so far. The two commits I've just appended / pushed to this branch contain fixups for every comment from your reviews. As a general rule, if I didn't reply to a comment, it means I implicitly accepted the point. I made the standardisation of xsubpp version numbers into a separate commit because all the changes were related. The second commit is just a big pile of random minor fixes and rewordings. |
tonycoz
reviewed
Nov 9, 2025
Contributor
Please mark comments as resolved when you're done with them, that cleans up this thread immensely for anyone reading it. Now it isn't even rendering everything because there are so many of them. |
Contributor
Author
|
Does anyone still intend to do any further review? Or is this PR approaching being mergeable? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Rewrite perlxs.pod
This branch completely rewrites and modernises the XS reference manual,
perlxs.pod.
The new file is about twice the size of the old one.
This branch:
deletes some obsolete sections;
reorders the existing sections into a more logical order;
adds a large new introductory/overview part, which explains
all the background needed to understand what XSUBs do, including
SVs, the stack, reference counts, magic etc.
includes a BNF syntax section
modernises: e.g. it uses "ANSI" parameter syntax throughout
has a fully-worked example using T_PTROBJ
Note that although each commit in this branch may have a complex-looking
diff for the updating of a particular section, in reality most sections
haver been rewritten from scratch, and the diff output is showing
paragraph breaks as fixed unchanging points, so that it appears as lots of
individual paragraph changes rather than "delete all this text, add new
text". If reviewing, it may be easier to just read the final perlxs.pod
file instead of looking at the diffs.