tweak, and heavily add comments to the Extutils::ParseXS code #22445
Conversation
|
For anyone reviewing this PR: note that in the last commit ("add many code comments"), GH by default hides the huge diff against ParseXS.pm because it's too big. So it's easy to miss. |
| # Note that the pod for this module is separate in ParseXS.pod. | ||
| # | ||
| # This module provides the guts for the xsubpp XS-to-C translator utility. | ||
| # By having it as a module separate from xsubpp, it makes it easier to |
There was a problem hiding this comment.
This is an important advantage, but to the best of my knowledge that's not why most of the code was moduled out. The direct cause was making it more usable from Module::Build and friends (so they don't have to shell out).
|
|
||
| =over 4 | ||
| =item C<<$self->death(@messages)>> |
There was a problem hiding this comment.
This is not rendering correctly in POD formatters like perldoc or pod2text. You are trying to say $self-> but it's only coming out as $self-. I believe the tip of the arrow needs to be an escaped greater-than sign. The same problem occurs in these instances:
$ pod2text < dist/ExtUtils-ParseXS/lib/ExtUtils/ParseXS/Utilities.pm | ack '\$self-(?!>)'
conditional nesting. In particular, it updates "@{$self-"{XSStack}}>
"<$self-"Warn(@messages)>>
"<$self-"blurt(@messages)>>
"<$self-"death(@messages)>>
"<$self-"WarnHint(@messages, $hints)>>
Warn if the lines in "@{ $self-"{line} }> don't have balanced "#if",
There was a problem hiding this comment.
The issue is that the syntax to use the double-chevron POD escape is: C<< ... >> - there must be whitespace after the double chevron. Otherwise it is parsed as C<...>death(@messages)>>, where the part in the code escape is only <$self-.
| =item * Arguments | ||
| It is used for something like a syntax error, where parsing can't | ||
| continue. However, this is inconvenient for testing purposes, as the | ||
| error can't be trapped. So if <$self> is created with the C<die_on_error> |
There was a problem hiding this comment.
is <$self> meant to be C<$self> ?
|
|
||
| =item * Return Value | ||
| This is a more obscure twin to C<Warn>, which does the same as C<Warn>, | ||
| but afterwards, outputs any lines contained in the <$hints> string, with |
|
All comments so far have been addressed; rebased and pushed. |
It's easy to confuse these two:
XS::Typemap
ExtUtils::Typemaps
Update the description of the XS::Typemap files in MANIFEST to aid
in deconfusion.
Add a few lines to the top of this file explaining what its purpose is.
Crude tool to collect before and after snapshots of all .c files created by xsubpp, in order to see what (if anything) has changed in functionality while working on ParseXS files.
Consolidate the POD for the various warning- and error-raising methods into a single block of text, to give a better overview of which method should be used when (e.g. warning vs deferred errors vs die immediately) The diff looks confusing, but its basically deleting the individual chunk of pod directly above each method, then adding a complete new block of pod. This contains completely new text, rather than copying and pasting.
To aid the understanding of this module and its sub-modules: - add lots of code comments - add blank lines - reformat and/or line-wrap a few long code lines There should be no functional changes. In particular, the line count of ParseXS.pm is increased by about 60% with this commit. I've tried to consistently use the word 'emit' rather than 'print' or 'output' in comments about the code that gets generated and ends up in the .c file. At the moment most of this code code is indeed just immediately printed to STDOUT, but in the longer term I would like to separate out code generation and output stages.
A few trivial commits, followed by one huge commit which adds many code comments to ParseXS.pm and related files.