develooper Front page | perl.perl5.porters | Postings from July 2007

[PATCH] ExtUtils::ParseXS - Error reporting problem with INTERFACE and ALIAS keywords

Thread Next
From:
Robert May
Date:
July 14, 2007 11:22
Subject:
[PATCH] ExtUtils::ParseXS - Error reporting problem with INTERFACE and ALIAS keywords
Message ID:
54bdc7510707131647t1582cdf7w37a4a345f400dc42@mail.gmail.com
[ CC: Ken Williams <kwilliams@cpan.org>, listed maintainer of
ExtUtils::ParseXS ]

I have a small issue with the following XSUB

C:\WINDOWS\Desktop\test>type MyTest.xs
#include "EXTERN.h"
#include "perl.h"
#include "XSUB.h"

int one()   { return 1; }
int two()   { return 2; }
int three() { return 3; }

MODULE = MyTest   PACKAGE = MyTest

PROTOTYPES: DISABLE

int
one()

int
two()
    ALIAS:
        two_x = 1
        FOO::two = 2

int
interface_v_i()
    INTERFACE:
        three

C:\WINDOWS\Desktop\test>type ntest.pl
#!perl -w
use strict;
use warnings;

use MyTest;

my @funcs = qw( MyTest::one
                MyTest::two MyTest::two_x FOO::two
                MyTest::three
              );

for my $func (@funcs) {
    my $code = \&$func;
    eval { $code->('somearg') };
    warn $@ if $@;
}

C:\WINDOWS\Desktop\test>perl -Mblib ntest.pl
Usage: MyTest::one() at ntest.pl line 14.
Usage: two() at ntest.pl line 14.
Usage: two_x() at ntest.pl line 14.
Usage: two() at ntest.pl line 14.
Usage: MyTest::interface_v_i() at ntest.pl line 14.

Particularly the last line, which is generated by calling MyTest::three()
with the wrong number of arguments.  I think it is very confusing for a
module user to be presented with an error message referring to a sub they
didn't call (and is unlikely to be documented).   This run also highlights
that the code generated by xsubpp is inconsistent in whether it uses fully
qualified sub names or not when reporting the error.

I propose 2 (alternative) patches to ExtUtils::ParseXS (and ideally to the
same area of xsubpp in maint).  The full attached patches are against the CPAN
release of ExtUtils::ParseXS (2.18), include tests, and update MANIFEST.

ALTERNATIVE 1:
--------------
This is the simplest change I could see, and fixes the error message
when using the INTERFACE keyword.

diff -urN ExtUtils-ParseXS-orig\lib\ExtUtils\ParseXS.pm
ExtUtils-ParseXS\lib\ExtUtils\ParseXS.pm
--- ExtUtils-ParseXS-orig\lib\ExtUtils\ParseXS.pm	Tue Jan 30 02:56:14 2007
+++ ExtUtils-ParseXS\lib\ExtUtils\ParseXS.pm	Fri Jul 13 19:43:24 2007
@@ -590,7 +590,7 @@
 #    *errbuf = '\0';
 EOF

-    if ($ALIAS)
+    if ($ALIAS || $INTERFACE)
       { print Q(<<"EOF") if $cond }
 #    if ($cond)
 #       Perl_croak(aTHX_ "Usage: %s(%s)", GvNAME(CvGV(cv)), "$report_args");

C:\WINDOWS\Desktop\test>perl -Mblib ntest.pl
Usage: MyTest::one() at ntest.pl line 14.
Usage: two() at ntest.pl line 14.
Usage: two_x() at ntest.pl line 14.
Usage: two() at ntest.pl line 14.
Usage: three() at ntest.pl line 14.

ALTERNATIVE 2:
--------------
This change makes the errors use fully qualified sub names when using both
the INTERFACE and ALIAS keywords.

This wants reviewing, as I am dabbling in areas of internals that I don't
know well (at all?), but it attempts to *always* report using the
fully qualified
sub name (as typically happens when not using the INTERFACE or ALIAS keyword).

diff -urN ExtUtils-ParseXS-orig\lib\ExtUtils\ParseXS.pm
ExtUtils-ParseXS\lib\ExtUtils\ParseXS.pm
--- ExtUtils-ParseXS-orig\lib\ExtUtils\ParseXS.pm	Tue Jan 30 02:56:14 2007
+++ ExtUtils-ParseXS\lib\ExtUtils\ParseXS.pm	Fri Jul 13 23:11:30 2007
@@ -590,20 +590,17 @@
 #    *errbuf = '\0';
 EOF

-    if ($ALIAS)
-      { print Q(<<"EOF") if $cond }
+    if($cond)
+      { print Q(<<"EOF"); }
 #    if ($cond)
-#       Perl_croak(aTHX_ "Usage: %s(%s)", GvNAME(CvGV(cv)), "$report_args");
+#       Perl_croak(aTHX_ "Usage: %s::%s(%s)", HvNAME(GvSTASH(CvGV(cv))),
+#                                             GvNAME(CvGV(cv)),
+#                                             "$report_args");
 EOF
-    else
-      { print Q(<<"EOF") if $cond }
-#    if ($cond)
-#       Perl_croak(aTHX_ "Usage: %s(%s)", "$pname", "$report_args");
-EOF
-
+    else {
      # cv doesn't seem to be used, in most cases unless we go in
      # the if of this else
-     print Q(<<"EOF");
+     print Q(<<"EOF"); }
 #    PERL_UNUSED_VAR(cv); /* -W */
 EOF

C:\WINDOWS\Desktop\test>perl -Mblib ntest.pl
Usage: MyTest::one() at ntest.pl line 14.
Usage: MyTest::two() at ntest.pl line 14.
Usage: MyTest::two_x() at ntest.pl line 14.
Usage: FOO::two() at ntest.pl line 14.
Usage: MyTest::three() at ntest.pl line 14.

I'd welcome comments on which approach is better (I prefer alternative 2),
and will happily try to wrap any suggestions into further patches.  This
is my first attempt at submitting a core patch, so if I've missed anything
please let me know and I'll try to rectify things.

Regards,
Rob.

Thread Next


nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About