ParseXS: refactor: turn param parsing into a sub

Currently the parsing of an XSUB's signature, and the parsing of
the individual comma-separated items within that signature, are done in
the same function, Params->parse(). This commit is the first of three
which will extract out the latter into a separate Param->parse() method.

For now, the per-param code is kept in-place (to make the diff easier to
understand), but is wrapped within an immediately-called anon sub, in
preparation to be moved.

So before, the code was (very simplified):

    for (split /,/, $params_text) {
        ... parse type, name, init etc ...
        next if can't parse;
        my $param = Param->new(var = $var, type => $type, ...);
        push @{$params->{kids}}, $param;
    }

After this commit, it looks more like:

    for (split /,/, $params_text) {
        my $param = Param->new();
        sub {
            my $param = shift;
            ...
            ... parse type, name, init etc ...
            return if can't parse;
            $param->{var} = $var; ...
            return 1;
        }->{$param, ...)
            or next;

        push @{$params->{kids}}, $param;
    }

Note that the inner sub leaves pushing the new param, updating the names
hash and setting the arg_num to the caller.

In theory there are no functional changes, except that when a synthetic
RETVAL is being kept (but its position within kids moved), we now keep
the Param hash and update its contents, rather than replace it with a new
hash. This shouldn't make any difference.

Author: iabyn

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

@@ -1989,7 +1989,7 @@ sub parse {
         $self->{names}{RETVAL} = $param;
     }
 
-    for (@param_texts) {
+    for my $param_text (@param_texts) {
         # Process each parameter. A parameter is of the general form:
         #
         #    OUT char* foo = expression
@@ -2007,19 +2007,29 @@ sub parse {
         #    = default      default value - only allowed under
         #                      $pxs->{config_allow_argtypes}
 
-        s/^\s+//;
-        s/\s+$//;
+        $param_text =~ s/^\s+//;
+        $param_text =~ s/\s+$//;
 
         # Process ellipsis (...)
 
         $pxs->blurt("further XSUB parameter seen after ellipsis (...)")
             if $self->{seen_ellipsis};
 
-        if ($_ eq '...') {
+        if ($param_text eq '...') {
             $self->{seen_ellipsis} = 1;
             next;
         }
 
+        my $param = ExtUtils::ParseXS::Node::Param->new();
+
+    sub {
+        my ExtUtils::ParseXS::Node::Param  $param  = shift;
+        my ExtUtils::ParseXS               $pxs    = shift;
+        my                                 $params = shift; # parent Params
+        my $param_text                             = shift;
+
+        $_ = $param_text;
+
         # Decompose parameter into its components.
         # Note that $name can be either 'foo' or 'length(foo)'
 
@@ -2046,55 +2056,47 @@ sub parse {
             if (/^ SV \s* \* $/x) {
                 # special-case SV* as a placeholder for backwards
                 # compatibility.
-                push @{$self->{kids}},
-                    ExtUtils::ParseXS::Node::Param->new( {
-                        var     => 'SV *',
-                        arg_num => ++$nargs,
-                    });
-            }
-            else {
-                $pxs->blurt("Unparseable XSUB parameter: '$_'");
+                $param->{var} = 'SV *';
+                return 1;
             }
-            next;
+            $pxs->blurt("Unparseable XSUB parameter: '$_'");
+            return;
         }
 
         undef $type unless length($type) && $type =~ /\S/;
-
-        my ExtUtils::ParseXS::Node::Param $param
-                = ExtUtils::ParseXS::Node::Param->new( {
-                        var => $name,
-                    });
+        $param->{var} = $name;
 
         # Check for duplicates
 
-        my $old_param = $self->{names}{$name};
+        my $old_param = $params->{names}{$name};
         if ($old_param) {
+            # Normally a dup parameter is an error, but we allow RETVAL as
+            # a real parameter, which overrides the synthetic one which
+            # was added earlier if the return value isn't void.
             if (    $name eq 'RETVAL'
                     and $old_param->{is_synthetic}
                     and !defined $old_param->{arg_num})
             {
                 # RETVAL is currently fully synthetic. Now that it has
                 # been declared as a parameter too, override any implicit
                 # RETVAL declaration. Delete the original param from the
-                # param list.
-                @{$self->{kids}} = grep $_ != $old_param, @{$self->{kids}};
+                # param list and later re-add it as a parameter in it's
+                # correct position.
+                @{$params->{kids}} = grep $_ != $old_param, @{$params->{kids}};
                 # If the param declaration includes a type, it becomes a
                 # real parameter. Otherwise the param is kept as
                 # 'semi-real' (synthetic, but with an arg_num) until such
                 # time as it gets a type set in INPUT, which would remove
                 # the synthetic/no_init.
-                $param = $old_param if !defined $type;
+                %$param = %$old_param unless defined $type;
             }
             else {
                 $pxs->blurt(
                         "Error: duplicate definition of parameter '$name' ignored");
-                next;
+                return;
             }
         }
 
-        push @{$self->{kids}}, $param;
-        $self->{names}{$name} = $param;
-
         # Process optional IN/OUT etc modifier
 
         if (defined $out_type) {
@@ -2155,7 +2157,6 @@ sub parse {
         # to be used in "usage: ..." error messages.
         my $report_def = '';
         if (defined $default) {
-            $opt_args++;
             # The default expression for reporting usage. For backcompat,
             # sometimes preserve the spaces either side of the '='
             $report_def =    ((defined $type or $is_length) ? '' : $sp1)
@@ -2164,12 +2165,18 @@ sub parse {
             $param->{default} = $default;
         }
 
-        if ($out_type eq "OUTLIST" or $is_length) {
-            $param->{arg_num} = undef;
-        }
-        else {
-            $param->{arg_num} = ++$nargs;
-        }
+        1;
+    }->($param, $pxs, $self, $param_text)
+            or next;
+
+        push @{$self->{kids}}, $param;
+        $self->{names}{$param->{var}} = $param unless $param->{var} eq 'SV *';
+        $opt_args++ if defined $param->{default};
+        # Give the param a number if it will consume one of the passed args
+        $param->{arg_num} = ++$nargs
+            unless (  defined $param->{in_out} && $param->{in_out} eq "OUTLIST"
+                    or $param->{is_length})
+
     } # for (@param_texts)
 
     $self->{nargs}    = $nargs;