Skip to content

Commit

Permalink
[MERGE] ParseXS: notype param and SV* placeholders
Browse files Browse the repository at this point in the history
These two commits:

- add tests for the existing behaviour of allowing an XSUB parameter
  which doesn't have a type to act as a placeholder;

- and as a special case, restores the recently changed behaviour of also
  allowing 'SV*' to denote a placeholder, and adds tests
  • Loading branch information
iabyn committed Nov 19, 2024
2 parents aec2cdc + 1b37187 commit 2ac0488
Show file tree
Hide file tree
Showing 3 changed files with 188 additions and 4 deletions.
2 changes: 1 addition & 1 deletion dist/ExtUtils-ParseXS/lib/ExtUtils/ParseXS.pm
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down
20 changes: 19 additions & 1 deletion dist/ExtUtils-ParseXS/lib/ExtUtils/ParseXS/Node.pm
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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}}) {
Expand All @@ -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;

Expand Down
170 changes: 168 additions & 2 deletions dist/ExtUtils-ParseXS/t/001-basic.t
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#!/usr/bin/perl

use strict;
use Test::More tests => 435;
use Test::More tests => 472;
use Config;
use DynaLoader;
use ExtUtils::CBuilder;
Expand Down Expand Up @@ -1631,14 +1631,24 @@ EOF
],

[
# shady but legal
# shady but legal - placeholder
"auto-generated proto with no type",
[
'void',
'foo(a, b, c = 0)',
],
[ 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);
Expand Down Expand Up @@ -2668,3 +2678,159 @@ EOF
test_many($preamble, 'XS_Foo_', \@test_fns);
}
{
# Test placeholders - various semi-official ways to to mark an
# argument as 'unused'.
my $preamble = Q(<<'EOF');
|MODULE = Foo PACKAGE = Foo
|
|PROTOTYPES: DISABLE
|
EOF
my @test_fns = (
[
"placeholder: typeless param with CODE",
[ Q(<<'EOF') ],
|int
|foo(int AAA, BBB, int CCC)
| CODE:
| XYZ;
EOF
[ 0, 0, qr/_usage\(cv,\s*"AAA, BBB, 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, 1, qr/\bBBB;/, "no BBB decl" ],
],
[
"placeholder: typeless param bodiless",
[ Q(<<'EOF') ],
|int
|foo(int AAA, BBB, int CCC)
EOF
[ 0, 0, qr/_usage\(cv,\s*"AAA, BBB, CCC"\)/, "usage" ],
# Note that autocall uses the BBB var even though it isn't
# declared. It would be up to the coder to use C_ARGS, or add
# such a var via PREINIT.
[ 0, 0, qr/\bRETVAL\s*=\s*\Qfoo(AAA, BBB, CCC);/, "autocall" ],
[ 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, 1, qr/\bBBB;/, "no BBB decl" ],
],
[
# this is the only IN/OUT etc one which works, since IN is the
# default.
"placeholder: typeless IN param with CODE",
[ Q(<<'EOF') ],
|int
|foo(int AAA, IN BBB, int CCC)
| CODE:
| XYZ;
EOF
[ 0, 0, qr/_usage\(cv,\s*"AAA, BBB, 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, 1, qr/\bBBB;/, "no BBB decl" ],
],
[
"placeholder: typeless OUT param with CODE",
[ Q(<<'EOF') ],
|int
|foo(int AAA, OUT BBB, int CCC)
| CODE:
| XYZ;
EOF
[ 1, 0, qr/Can't determine output type for 'BBB'/, "got type err" ],
],
[
"placeholder: typeless IN_OUT param with CODE",
[ Q(<<'EOF') ],
|int
|foo(int AAA, IN_OUT BBB, int CCC)
| CODE:
| XYZ;
EOF
[ 1, 0, qr/Can't determine output type for 'BBB'/, "got type err" ],
],
[
"placeholder: typeless OUTLIST param with CODE",
[ Q(<<'EOF') ],
|int
|foo(int AAA, OUTLIST BBB, int CCC)
| CODE:
| XYZ;
EOF
[ 1, 0, qr/Can't determine output type for 'BBB'/, "got type err" ],
],
[
# a placeholder with a default value may not seem to make much
# sense, but it allows an argument to still be passed (or
# not), even if it;s no longer used.
"placeholder: typeless default param with CODE",
[ Q(<<'EOF') ],
|int
|foo(int AAA, BBB = 888, int CCC = 999)
| CODE:
| XYZ;
EOF
[ 0, 0, qr/_usage\(cv,\s*"AAA, BBB = 888, CCC\s*= 999"\)/,"usage" ],
[ 0, 0, qr/\bint\s+AAA\s*=\s*.*\Q(ST(0))/, "AAA is ST(0)" ],
[ 0, 0, qr/\bCCC\s*=\s*.*\Q(ST(2))/, "CCC is ST(2)" ],
[ 0, 1, qr/\bBBB;/, "no BBB decl" ],
[ 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" ],
],
);
test_many($preamble, 'XS_Foo_', \@test_fns);
}

0 comments on commit 2ac0488

Please sign in to comment.