Front page | perl.perl5.porters |
Postings from July 2007
Re: [PATCH] ExtUtils::ParseXS - Error reporting problem with INTERFACE and ALIAS keywords
Thread Previous
|
Thread Next
From:
Ken Williams
Date:
July 19, 2007 21:00
Subject:
Re: [PATCH] ExtUtils::ParseXS - Error reporting problem with INTERFACE and ALIAS keywords
Message ID:
49E28BE1-0BBC-4DFA-81B2-87B71C8CC627@mathforum.org
Hi Robert, hi p5p,
I throw myself at the mercy of p5p on this one. Despite being the
maintainer of EU::PXS I'm far from an expert on XS->C processing.
P5p, any opinion on which alternative to apply? I like the cosmetic
looks of #2 also.
-Ken
On Jul 13, 2007, at 6:47 PM, Robert May wrote:
> [ 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.<patch.1><patch.2>
Thread Previous
|
Thread Next