Add MAYBE modifier to ExtUtils::ParseXS by Leont · Pull Request #24557 · Perl/perl5

Leont · 2026-07-13T11:38:14Z

As I suggested here before, I've added a MAYBE modifier to arguments and return values. This will map undef to/from a sentinel value such as NULL, but otherwise use the typemap as usual.

This set of changes does not require a perldelta entry, because ExtUtils::ParseXS has its own Changes file

leonerd

Overall this feels good, but a few small comments about trying to be clearer in the docs / code.

tonycoz · 2026-07-15T01:19:22Z

+        $if_null .= "\n\t\tSTRLEN_length_of_\$var = 0;" if $self->{length_param};
+
+        $init_template = <<END;
+	if (SvOK(\$arg)) {


Nothing has called SvGETMAGIC() at this point, so SvOK() may not be valid (fixing that doesn't seem pretty, unless we accept extra magic calls.)

That's a good one. I think I've handled it everywhere I can now. There's no SvRV_nomg so I guess it doesn't matter enough in that corner.

The existing XS should[1] SvGETMAGIC() before SvROK(), SvRV() anyway, but you still need the SvGETMAGIC() before the SvOK().

If I fix T_PTRREF to properly SvGETMAGIC():

[ "test MAYBE PTRREF", Q(<<'EOF'), |TYPEMAP: <<EOF |myclass T_MYPTRREF |INPUT |T_MYPTRREF | SvGETMAGIC($arg); | if (SvROK($arg)) { | IV tmp = SvIV((SV*)SvRV($arg)); | $var = INT2PTR($type,tmp); | } | else | Perl_croak_nocontext("%s: %s is not a reference", | ${$ALIAS?\q[GvNAME(CvGV(cv))]:\qq["$pname"]}, | "$var") |EOF | |void |foo(MAYBE myclass input) EOF [ 0, qr/fail me/, 'I want to see the code' ], ],

We get:

# XS_EUPXS(XS_Foo_foo) # { # dVAR; dXSARGS; # if (items != 1) # croak_xs_usage(cv, "input"); # { # myclass input; # # SvGETMAGIC(ST(0)); # if (SvOK(ST(0))) { # SvGETMAGIC(ST(0)); # if (SvROK(ST(0))) { # IV tmp = SvIV_nomg((SV*)SvRV(ST(0))); # input = INT2PTR(myclass,tmp); # } # else # Perl_croak_nocontext("%s: %s is not a reference", # "Foo::foo", # "input") # ; # } else { # input = NULL;; # } # ; # # foo(input); # } # XSRETURN_EMPTY; # }'

Which has two problems:

an extraneous SvGETMAGIC(ST(0));

removed the magic from an SvIV() that wasn't ST(0)

I think it is safer to ~~be extra magical~~ just have extra magic calls.

[1] though lib/ExtUtils/typemap is painfully inconsistent about it, and thinking about this lead to tonycoz/imager#566

I hadn't realized SvRV never does SvGETMAGIC, that does change things. Now it's always added.

If I modify a match for the "test MAYBE with length" test to fail I see:

# XS_EUPXS(XS_Foo_foo) # { # dVAR; dXSARGS; # if (items != 1) # croak_xs_usage(cv, "input"); # { # STRLEN STRLEN_length_of_input; # size_t XSauto_length_of_input; # char* input; # # SvGETMAGIC(ST(0)); # if (SvOK(ST(0))) { # input = (char *)SvPV(ST(0), STRLEN_length_of_input) # ; # } else { # input = NULL; # STRLEN_length_of_input = 0;; # } # ; # # XSauto_length_of_input = STRLEN_length_of_input; # # foo(input, XSauto_length_of_input); # } # XSRETURN_EMPTY; # }'

The code looks like you're trying to support this case, but matching against the closing ) means this still uses the magical SvPV() instead of the non-magical variant.

though lib/ExtUtils/typemap is painfully inconsistent about it

Now #24589

tonycoz · 2026-07-20T22:55:47Z


    $self->{no_output} = 1 if $type =~ s/^NO_OUTPUT\s+//;

+    $self->{retval_sentinel} = defined($1) ? $1 : 'NULL' if $type =~ s/^\s* MAYBE (?: \( (-? \w+) \) )? \s*//x;


One thing that occurred to me this morning, was whether this should accept C++ namespace/class qualified names, ie. allow colons.

My ideal would be to accept an expression like MAYBE(Perl_nan()) or MAYBE(std::numeric_limits<NV>::quiet_NaN()) but I can see why you'd want to avoid that.

One thing that occurred to me this morning, was whether this should accept C++ namespace/class qualified names, ie. allow colons.

I kind of doubt it does in practice, but we can always make it more permissive in the future.

tonycoz · 2026-07-22T00:06:12Z


+    if ($init_template && defined $sentinel) {
+        $init_template =~ s/^(?!\A)/\t/g;
+        $init_template =~ s/(Sv(?:IV|UV|PV(?:|byte|utf8)?))(_nolen|)(\(\$arg\))/$1_nomg$2$3/;


SvNV is missing and does have a _nomg variant.

Something that occurred to me while I was out led to:

[ "test MAYBE byte", Q(<<'EOF'), |TYPEMAP: <<TM |BYTE * T_BYTEV |INPUT |T_BYTEV | $var = ($type)SvPVbyte_nolen($arg); |TM |void |foobyte(MAYBE BYTE *input) EOF [ 0, qr/if $SvOK\(ST\(0$\)\)/, 'check for SvOK' ], [ 0, qr/input = $BYTE \*$SvPVbyte_nomg_nolen$ST\(0$\)/ ], [ 0, qr/xinput = NULL;/, 'falls back to setting to null'], ],

xinput to force a fail:

# XS_EUPXS(XS_Foo_foobyte) # { # dVAR; dXSARGS; # if (items != 1) # croak_xs_usage(cv, "input"); # { # BYTE * input; # # SvGETMAGIC(ST(0)); # if (SvOK(ST(0))) { # input = (BYTE *)SvPVbyte_nomg_nolen(ST(0)) # ; # } else { # input = NULL;; # } # ; # # foobyte(input); # } # XSRETURN_EMPTY; # }' # doesn't match '(?^:xinput = NULL;)'

but:

tony@venus:.../git/perl6$ git grep SvPVbyte_nomg_nolen dist/ExtUtils-ParseXS/t/005-parse-parameters.t: [ 0, qr/input = $BYTE \*$SvPVbyte_nomg_nolen$ST\(0$\)/ ],

there is no such thing as SvPVbyte_nomg_nolen.

My original thought was that not all the various combinations of SvPV(byte|utf8)?(_nomg)?(_nolen)? have existed over the history of perls that EU::PXS needs to support.

SvNV is missing and does have a _nomg variant.

D'oh!

there is no such thing as SvPVbyte_nomg_nolen.

That is most annoying. I guess that needs to be SvPVbyte_nomg($arg, PL_na) then?

SvTRUE() is also missing (encountered while looking for the versions below)

I guess that needs to be SvPVbyte_nomg($arg, PL_na) then?

That looks right, but...

My original thought was that not all the various combinations of SvPV(byte|utf8)?(_nomg)?(_nolen)? have existed over the history of perls that EU::PXS needs to support.

I think this is still an issue: the replacements need to be perl version dependent:

SvPV(byte|utf8)_nomg were added in 5.31.3 (757fc32).

SvNV_nomg() was added in 5.13.1 (6f1401d)

SvPV_nomg_nolen() was added in v5.13.6-122-g71eb6d8cfb

SvTRUE_nomg() was added in v5.13.5-120-g06c841cf64 (if you add SvTRUE())

Sv(IV|UV)_nomg() were added in perl-5.8.0-3191-g891f9566d7 (which probably isn't an issue)

SvPV_nomg was added in perl-5.6.0-3776-g8d6d96c1bf

SvPV_nomg() was added in perl-5.005_02-822-g1fa8b10d8c

(git grep -S foo is my friend)

Alternatively MAYBE could reject old perls.

Alternatively MAYBE could reject old perls.

Or just require ppport.h on older perls?

Or just require ppport.h on older perls?

That's an option, though SvPVbyte_nomg() isn't defined by ppport.h, I haven't checked any others.

Oh that's annoying. We may have to fix that. And in the mean time I guess it doesn't work on older perls.

The code is currently put after short argument typemaps, but before long ones. This was fine when STRLEN_length_of_* was only set by short ones, but in the follow up commit it will be set by a longer one, so it needs to be moved after the long ones.

This adds the MAYBE modifier to arguments. This will map undef to the given sentinel (or NULL if none is given), and otherwise use the usual typemapping machinery.

This adds the MAYBE modifier to return values and output arguments. This will map undef to the given sentinel (or NULL if none is given), and otherwise use the usual typemapping machinery.

tonycoz · 2026-07-23T00:55:00Z

+                | MAYBE char* foo()
+EOF
+            [  0, qr/if \(RETVAL == NULL\)/, '' ],
+            # [  0, qr/asdf/, ''],


left over from testing?

Yeah, I'll remove that

still there

tonycoz · 2026-08-02T23:34:24Z

+typemap logic for any defined values. If an arguments it given to the C<MAYBE>
+(e.g. C<MAYBE(-1)>) then that value is used as the sentinel, otherwise C<NULL>
+is used.
+


This only describes input parameters ("mapping undef to NULL" etc). It should at least briefly mention output parameters. The prose added further up seems to just be about return values.

iabyn

Apart from the PADTMP performance issue, all my comments are small niggles, and I'm otherwise generally happy with this PR.

iabyn · 2026-08-04T11:29:09Z

-        # to change SvPV_nolen() to SvPV() or similar.
-        #
-        # The final assign should be deferred to come after all
-        # declarations.


I'd prefer this chunk of code comments to be left where it is. Just above, it's explained what set of code lines should be produced for a length(s), and now these comments explain where some of those lines are generated. Just change the 'The final assign' bit to say that that line is added near the end of this function.

iabyn · 2026-08-04T11:40:17Z

    'is_synthetic',  # Bool: var like 'THIS': we pretend it was in the sig

    # values derived from both the XSUB's signature and/or INPUT line
+    'sentinel',      # Str:  The C constant expression to map undef to


The 'sentinel' field line should be added 2 lines earlier, before the 'values derived from both the XSUB's signature and/or INPUT line' comment, since you can't have a MAYBE in an INPUT line.

iabyn · 2026-08-04T11:59:09Z

+        ],
+
+        [
+            "test MAYBE with length",


Presumably "MAYBE length(s)" an error. If so, there should be a test for it.

iabyn · 2026-08-04T12:01:58Z

    }

-    return ($init_template, $eval_vars, 1);
+    return ($init_template, $eval_vars, $sentinel, 1);


Since lookup_input_typemap() now returns an extra value, the code comments at the top of the function should be updated to reflect this. I.e. the 'Returns ($expr, $eval_vars, $is_template)' bit plus an extra line a few lines further down describing the new value.

iabyn · 2026-08-04T12:16:03Z

+        SvPVutf8_nolen => 'SvPVutf8_nomg(%s, PL_na',
+    );
+    if ($init_template && defined $sentinel) {
+        $init_template =~ s/^(?!\A)/\t/g;


I have no idea what the s/^(?!\A)/\t/g; is intended to do. Perhaps a comment at the end of it explaining it?

iabyn · 2026-08-04T15:31:06Z

        #   SV * targ = (PL_op->op_private & OPpENTERSUB_HASTARG)
        #               ? PAD_SV(PL_op->op_targ) : sv_newmortal()

-        if (   $pxs->{config_optimize}


This new section seems very sub-optimal. In the presence of a MAYBE return modifier, TARG is never used, and a so new mortal is created for each return (even if the sentinel value is never encountered).

iabyn · 2026-08-04T15:35:15Z

+            [ 0, qr/sv_setsv\(ST\(0\), &PL_sv_undef\)/, ''],
+        ],
+
+        [


Really aught to have tests for OUTLIST and default parameters too.

iabyn · 2026-08-04T15:45:04Z

 which allow you to return Perl's true and false values, or to return
 an empty list.

+Since ExtUtils::ParseXS 3.64, you can also use the C<MAYBE> modifier to


This new section doesn't actually say what the sentinel is for, or what values it can have. Perhaps something like:
If RETVAL is equal to the specified sentinel value (-1 in this example), then an undef-valued SV is returned instead. The sentinel is any simple C value. The value "NULL" is used if not specified.

iabyn · 2026-08-04T15:49:19Z

 of C<int> is only meaningful for declaring C<RETVAL> and for doing the
 autocall.

+It can also be prefixed with C<MAYBE> or C<MAYBE(sentinel)>. In such an XSUB,


This section's title should be updated to include the 'MAYBE' word, so it's more likely to be found when looking at a contents page. e.g.

=head3 An XSUB's return type and the NO_OUTPUT and MAYBE keywords

The previous '=head2 An XSUB Declaration' list of examples also aught have the MAYBE keyword somewhere.

iabyn · 2026-08-04T15:58:38Z

    foo(int a, b, char *c)
        C_ARGS: a, c

+=head3 XSUB parameters with sentinels


Again, this really needs 'MAYBE' in the section header, e.g.
=head3 MAYBE: XSUB parameters with sentinels.
This section also really needs a small code example.

iabyn · 2026-08-04T16:08:48Z

A few further comments: presumably a return type of 'MAYBE void' should give an error (and be tested for)?

The commit message for "ExtUtils::ParseXS: Add MAYBE support to return values and output" says "This will map undef to the given sentinel" which is the wrong way round for return/output values.

Should the docs mention that in the presence of length(), the length also gets set to 0 if the sentinel is triggered?

Leont requested review from iabyn and leonerd July 13, 2026 11:38

Leont added the defer-next-dev This PR should not be merged yet, but await the next development cycle label Jul 13, 2026

leonerd reviewed Jul 13, 2026

View reviewed changes

tonycoz reviewed Jul 15, 2026

View reviewed changes

Comment thread dist/ExtUtils-ParseXS/lib/ExtUtils/ParseXS/Node.pm Outdated

tonycoz reviewed Jul 15, 2026

View reviewed changes

Comment thread dist/ExtUtils-ParseXS/t/005-parse-parameters.t

jkeenan removed the defer-next-dev This PR should not be merged yet, but await the next development cycle label Jul 15, 2026

github-actions Bot added the hasConflicts label Jul 16, 2026

Leont force-pushed the parsexs-maybe branch from 32a7513 to a9e6fe0 Compare July 20, 2026 12:58

github-actions Bot removed the hasConflicts label Jul 20, 2026

Leont force-pushed the parsexs-maybe branch 3 times, most recently from 2bbac35 to 67abbe1 Compare July 20, 2026 19:23

tonycoz reviewed Jul 20, 2026

View reviewed changes

Leont force-pushed the parsexs-maybe branch 4 times, most recently from c5dfeff to 43c8fe7 Compare July 21, 2026 08:48

tonycoz reviewed Jul 21, 2026

View reviewed changes

Comment thread dist/ExtUtils-ParseXS/lib/perlxs.pod Outdated

tonycoz reviewed Jul 22, 2026

View reviewed changes

Comment thread dist/ExtUtils-ParseXS/t/005-parse-parameters.t Outdated

Leont force-pushed the parsexs-maybe branch 2 times, most recently from 0ad82b0 to 0a2a133 Compare July 22, 2026 13:24

Leont added 5 commits July 22, 2026 16:14

Bump ExtUtils::ParseXS version to 3.65

9131833

ExtUtils::ParseXS: Add MAYBE support to arguments

266c8df

This adds the MAYBE modifier to arguments. This will map undef to the given sentinel (or NULL if none is given), and otherwise use the usual typemapping machinery.

ExtUtils::ParseXS: Add MAYBE support to return values and output

c60ccd5

This adds the MAYBE modifier to return values and output arguments. This will map undef to the given sentinel (or NULL if none is given), and otherwise use the usual typemapping machinery.

ExtUtils::ParseXS: Document MAYBE functionality

0ebb3fc

Leont force-pushed the parsexs-maybe branch from 0a2a133 to 0ebb3fc Compare July 22, 2026 14:18

tonycoz reviewed Jul 23, 2026

View reviewed changes

tonycoz reviewed Aug 2, 2026

View reviewed changes

github-actions Bot added the hasConflicts label Aug 4, 2026

iabyn reviewed Aug 4, 2026

View reviewed changes


		$self->{no_output} = 1 if $type =~ s/^NO_OUTPUT\s+//;

		$self->{retval_sentinel} = defined($1) ? $1 : 'NULL' if $type =~ s/^\s* MAYBE (?: \( (-? \w+) \) )? \s*//x;

Uh oh!

Conversation

Leont commented Jul 13, 2026

Uh oh!

leonerd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Leont Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Leont Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iabyn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Leont Jul 21, 2026 •

edited

Loading

Leont Jul 22, 2026 •

edited

Loading