Skip to content

Commit 2fbf369

Browse files
committed
ParseXS: refactor: unify sig and INPUT params
The series of approx 45 refactoring commits in this branch have added E::P::Node::Param and E::P::Node:Sig objects, to allow an XSUB's signature to be stored as a list of Param objects in a Sig object. So in a straightforward XSUB like: int foo(a, b = 999) int a int b a pair of Param objects is created and stored in a Sig object to represent the two items in the signature, e.g. { var => 'a', arg_num => 1 }, { var => 'b', arg_num => 2, default => 999 }, But up until now, when the lines in the implicit INPUT section were subsequently parsed, a *second* pair of temporary Param objects were created using both data obtained from the Sig objects and from the INPUT line. The as_code() method was then called on each temp Param object, and the temp object was then thrown away. This commit changes it so that instead, any extra info obtained from an INPUT line is added to the original Param object in the Sig object, (e.g. the 'type' value) and the as_code() method is called on the *original* object. No temporary Param objects are created. This commit is effectively what the previous chain of commits have been preparing for. Before that, all info obtained from the signature was stored in a series of hash refs stored in the ExtUtils::ParseXS object, such as $self->{sub_map_argname_to_default}{b} = 999; The new way is cleaner, conceptually simpler, gathers related code and data into one location, and has fewer special cases that need handling. The main thing it doesn't do yet is defer code generation. It still emits a C var declaration directly after each INPUT line is read and parsed. Eventually I would like for the whole signature/INPUT sequence to be read in and parsed and *then* emit all the code one go (and in the long term, to read in the *whole* XS file before emitting any code). One intended side effect of this commit is that it now detects duplicate 'alien' parameters again - this was broken during the earlier refactoring. An alien param is one declared in INPUT but not in the signature: int foo(a) int a # normal parameter int b # alien parameter int b # duplicate alien parameter A test has been added for this. A test has also been added for looking up the prototype char associated with a type when the type can change, as with a synthetic parameter like THIS.
1 parent ac6d839 commit 2fbf369

File tree

3 files changed

+50
-39
lines changed

3 files changed

+50
-39
lines changed

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

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2363,9 +2363,11 @@ sub INPUT_handler {
23632363
$self->{xsub_seen_RETVAL_in_INPUT} |= $var_name eq "RETVAL";
23642364

23652365
my ($var_num, $is_alien);
2366-
my $sig_param = $self->{xsub_sig}{names}{$var_name};
23672366

2368-
if (defined $sig_param) {
2367+
my ExtUtils::ParseXS::Node::Param $param
2368+
= $self->{xsub_sig}{names}{$var_name};
2369+
2370+
if (defined $param) {
23692371
# The var appeared in the signature too.
23702372

23712373
# Check for duplicate definitions of a particular parameter name.
@@ -2374,21 +2376,28 @@ sub INPUT_handler {
23742376
# and thus shouldn't be defined again. The exception to this are
23752377
# synthetic params like THIS, which are assigned a provisional type
23762378
# which can be overridden.
2377-
if ( $sig_param->{in_input}
2378-
or (!$sig_param->{is_synthetic} and exists $sig_param->{type})
2379+
if ( $param->{in_input}
2380+
or (!$param->{is_synthetic} and exists $param->{type})
23792381
) {
23802382
$self->blurt(
23812383
"Error: duplicate definition of argument '$var_name' ignored");
23822384
next;
23832385
}
2384-
$sig_param->{in_input} = 1;
2385-
$var_num = $sig_param->{arg_num};
2386+
$param->{in_input} = 1;
2387+
$var_num = $param->{arg_num};
23862388
}
23872389
else {
23882390
# The var is in an INPUT line, but not in signature. Treat it as a
23892391
# general var declaration (which really should have been in a
23902392
# PREINIT section). Legal but nasty: flag is as 'alien'
23912393
$is_alien = 1;
2394+
$param = ExtUtils::ParseXS::Node::Param->new({
2395+
var => $var_name,
2396+
is_alien => 1,
2397+
});
2398+
2399+
push @{$self->{xsub_sig}{params}}, $param;
2400+
$self->{xsub_sig}{names}{$var_name} = $param;
23922401
}
23932402

23942403
# Parse the initialisation part of the INPUT line (if any)
@@ -2442,8 +2451,16 @@ sub INPUT_handler {
24422451
$no_init = 1;
24432452
}
24442453

2445-
my ExtUtils::ParseXS::Node::Param $param =
2446-
ExtUtils::ParseXS::Node::Param->new({
2454+
if ( defined $param->{in_out}
2455+
and $param->{in_out} =~ /^OUT/
2456+
and !defined $init_op
2457+
) {
2458+
# OUT* class: skip initialisation
2459+
$no_init = 1;
2460+
}
2461+
2462+
%$param = (
2463+
%$param,
24472464
type => $var_type,
24482465
arg_num => $var_num,
24492466
var => $var_name,
@@ -2452,8 +2469,7 @@ sub INPUT_handler {
24522469
init_op => $init_op,
24532470
no_init => $no_init,
24542471
is_addr => !!$var_addr,
2455-
is_alien=> $is_alien,
2456-
});
2472+
);
24572473

24582474
$param->check($self)
24592475
or next;
@@ -2464,7 +2480,7 @@ sub INPUT_handler {
24642480
# Synthetic params like THIS will be emitted later - they
24652481
# are treated like ANSI params, except the type can overridden
24662482
# within an INPUT statement
2467-
next if $sig_param->{is_synthetic};
2483+
next if $param->{is_synthetic};
24682484

24692485
$param->as_code($self);
24702486

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

Lines changed: 6 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -115,8 +115,7 @@ BEGIN {
115115

116116

117117
# check(): for a parsed INPUT line and/or typed parameter in a signature,
118-
# update some global state and do some checks (e.g. "duplicate argument"
119-
# error).
118+
# update some global state and do some checks
120119
#
121120
# Return true if checks pass.
122121

@@ -128,35 +127,16 @@ sub check {
128127

129128
# Get the overridden prototype character, if any, associated with the
130129
# typemap entry for this var's type.
130+
# Note that something with a provisional type such as THIS can get
131+
# the type changed later. It is important to update each time.
132+
# It also can't be looked up only at BOOT code emitting time, because
133+
# potentiall, the typmap may been bee updated last in the XS file
134+
# after the XSUB was parsed.
131135
if ($self->{arg_num}) {
132136
my $typemap = $pxs->{typemaps_object}->get_typemap(ctype => $type);
133137
my $p = $typemap && $typemap->proto;
134138
$self->{proto} = $p if defined $p && length $p;
135139
}
136-
137-
# XXX tmp workaround during code refactoring
138-
# Copy any relevant fields from the param object (if any) for the
139-
# param of the same name declared in the signature, to the INPUT param
140-
# object.
141-
my ExtUtils::ParseXS::Node::Param $sigp =
142-
$pxs->{xsub_sig}{names}{$self->{var}};
143-
144-
if ($sigp) {
145-
for (qw(default)) {
146-
$self->{$_} = $sigp->{$_} if exists $sigp->{$_};
147-
}
148-
if ( defined $sigp->{in_out}
149-
and $sigp->{in_out} =~ /^OUT/
150-
and !defined($self->{init_op})
151-
) {
152-
# OUT* class: skip initialisation
153-
$self->{no_init} = 1;
154-
}
155-
# XXX also tmp copy some stuff back to the sig param
156-
for(qw(is_addr in_out proto type)) {
157-
$sigp->{$_} = $self->{$_} if exists $self->{$_};
158-
}
159-
}
160140

161141
return 1;
162142
}

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

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
#!/usr/bin/perl
22

33
use strict;
4-
use Test::More tests => 179;
4+
use Test::More tests => 182;
55
use Config;
66
use DynaLoader;
77
use ExtUtils::CBuilder;
@@ -883,14 +883,16 @@ EOF
883883
| int b;
884884
| int b;
885885
| int c;
886+
| int alien;
887+
| int alien;
886888
EOF
887889

888890
tie *FH, 'Capture';
889891
my $stderr = PrimitiveCapture::capture_stderr(sub {
890892
$pxs->process_file( filename => \$text, output => \*FH);
891893
});
892894

893-
for my $var (qw(a b c)) {
895+
for my $var (qw(a b c alien)) {
894896
my $count = () =
895897
$stderr =~ /duplicate definition of argument '$var'/g;
896898
is($count, 1, "One dup error for \"$var\"");
@@ -1481,6 +1483,9 @@ EOF
14811483
|X::Y * T_OBJECT
14821484
|const X::Y * T_OBJECT \&
14831485
|
1486+
|P::Q * T_OBJECT @
1487+
|const P::Q * T_OBJECT %
1488+
|
14841489
|INPUT
14851490
|T_OBJECT
14861491
| $var = my_in($arg);
@@ -1555,6 +1560,16 @@ EOF
15551560
[ 0, 0, qr/"\$\$\$\$;\$\@"/, "" ],
15561561
],
15571562

1563+
[
1564+
"auto-generated proto with overridden THIS type",
1565+
[
1566+
'void',
1567+
'P::Q::foo()',
1568+
' const P::Q * THIS'
1569+
],
1570+
[ 0, 0, qr/"%"/, "" ],
1571+
],
1572+
15581573
[
15591574
"explicit prototype",
15601575
[

0 commit comments

Comments
 (0)