ParseXS: allow SV* as placeholder

ExtUtils::ParseXS, up until and including perl 5.40.0, accepted
unparseable XSUB parameter declarations: if the parameter couldn't be
parsed, it was just silently skipped. This had the side effect that
any old line noise could be used as a placeholder: no parameter would
be declared, but during arg processing, the arg would be skipped.

During my recent refactoring work, I instead made it an error if the
parameter couldn't be parsed, but this broke some modules which were
using 'SV*' as a placeholder, e.g.

    int
    foo(int a, SV*, int b)
        ...

To partially restore backwards compatibility, this commit recognises
the special case of 'SV*' being used as a placeholder. All other
unparseable parameter declarations will continue to be an error.

It will however now still emit an error if 'SV*' is used on a bodiless sub.
It used to cause the call to the C function to be emitted as (from the
example above),

    RETVAL = foo(a, SV*, b);

which isn't legal C code. So making it an error isn't breaking backwards
compatibility.

The coder can still use C_ARGS, e.g.

    int
    foo(int a, SV*, int b)
        C_ARGS: a, b

Author: iabyn

dist/ExtUtils-ParseXS/lib/ExtUtils/ParseXS.pm

@@ -1209,7 +1209,7 @@ EOF
 
           my $sig  = $self->{xsub_sig};
           my $args = $sig->{auto_function_sig_override}; # C_ARGS
-          $args = $sig->C_func_signature()
+          $args = $sig->C_func_signature($self)
             unless defined $args;
           print "$self->{xsub_func_name}($args);\n";
 

dist/ExtUtils-ParseXS/lib/ExtUtils/ParseXS/Node.pm

@@ -687,7 +687,18 @@ sub parse {
                 /x;
 
         unless (defined $name) {
-            $pxs->blurt("Unparseable XSUB parameter: '$_'");
+            if (/^ SV \s* \* $/x) {
+                # special-case SV* as a placeholder for backwards
+                # compatibility.
+                push @{$self->{params}},
+                    ExtUtils::ParseXS::Node::Param->new( {
+                        var     => 'SV *',
+                        arg_num => ++$nargs,
+                    });
+            }
+            else {
+                $pxs->blurt("Unparseable XSUB parameter: '$_'");
+            }
             next;
         }
 
@@ -839,6 +850,7 @@ sub usage_string {
 
 sub C_func_signature {
     my ExtUtils::ParseXS::Node::Sig $self = shift;
+    my ExtUtils::ParseXS            $pxs  = shift;
 
     my @args;
     for my $param (@{$self->{params}}) {
@@ -853,6 +865,12 @@ sub C_func_signature {
             next;
         }
 
+        if ($param->{var} eq 'SV *') {
+            #backcompat placeholder
+            $pxs->blurt("Error: parameter 'SV *' not valid as a C argument");
+            next;
+        }
+
         my $io = $param->{in_out};
         $io = '' unless defined $io;
 

dist/ExtUtils-ParseXS/t/001-basic.t

@@ -1,7 +1,7 @@
 #!/usr/bin/perl
 
 use strict;
-use Test::More tests => 460;
+use Test::More tests => 472;
 use Config;
 use DynaLoader;
 use ExtUtils::CBuilder;
@@ -1639,6 +1639,16 @@ EOF
             ],
             [ 0, 0, qr/"\$\$;\$"/, ""  ],
         ],
+
+        [
+            "auto-generated proto with backcompat SV* placeholder",
+            [
+                'void',
+                'foo(int a, SV*, char *c = "")',
+                'C_ARGS: a, c',
+            ],
+            [ 0, 0, qr/"\$\$;\$"/, ""  ],
+        ],
     );
 
     test_many($preamble, 'boot_Foo', \@test_fns);
@@ -2780,6 +2790,45 @@ EOF
             [ 0, 1, qr/\b888\s*;/,                            "no 888 usage" ],
         ],
 
+        [
+            "placeholder: allow SV *",
+            [ Q(<<'EOF') ],
+                |int
+                |foo(int AAA, SV *, int CCC)
+                |   CODE:
+                |      XYZ;
+EOF
+            [ 0, 0, qr/_usage\(cv,\s*\Q"AAA, SV *, CCC")/,    "usage" ],
+            [ 0, 0, qr/\bint\s+AAA\s*=\s*.*\Q(ST(0))/,        "AAA is ST(0)" ],
+            [ 0, 0, qr/\bint\s+CCC\s*=\s*.*\Q(ST(2))/,        "CCC is ST(2)" ],
+        ],
+
+        [
+            # Bodiless XSUBs can't use SV* as a placeholder ...
+            "placeholder: SV *, bodiless",
+            [ Q(<<'EOF') ],
+                |int
+                |foo(int AAA, SV    *, int CCC)
+EOF
+            [ 1, 0, qr/Error: parameter 'SV \*' not valid as a C argument/,
+                                                           "got arg err" ],
+        ],
+
+        [
+            # ... unless they use C_ARGS to define how the C fn should
+            # be called.
+            "placeholder: SV *, bodiless C_ARGS",
+            [ Q(<<'EOF') ],
+                |int
+                |foo(int AAA, SV    *, int CCC)
+                |    C_ARGS: AAA, CCC
+EOF
+            [ 0, 0, qr/_usage\(cv,\s*\Q"AAA, SV *, CCC")/,    "usage" ],
+            [ 0, 0, qr/\bint\s+AAA\s*=\s*.*\Q(ST(0))/,        "AAA is ST(0)" ],
+            [ 0, 0, qr/\bint\s+CCC\s*=\s*.*\Q(ST(2))/,        "CCC is ST(2)" ],
+            [ 0, 0, qr/\bRETVAL\s*=\s*\Qfoo(AAA, CCC);/,      "autocall" ],
+        ],
+
 
     );