Front page | perl.perl5.porters |
Postings from May 2008
Re: [PATCH] ExtUtils::ParseXS - Error reporting problem with INTERFACE and ALIAS keywords
Thread Previous
|
Thread Next
From:
Nicholas Clark
Date:
May 21, 2008 06:46
Subject:
Re: [PATCH] ExtUtils::ParseXS - Error reporting problem with INTERFACE and ALIAS keywords
Message ID:
20080521134559.GG6780@plum.flirble.org
On Thu, Jul 19, 2007 at 11:00:17PM -0500, Ken Williams wrote:
> 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.
I'm not an expert either, but I think #2 is more correct. Comments follow.
> On Jul 13, 2007, at 6:47 PM, Robert May wrote:
> >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
Looking at the code in dump.c for dumping out shit stuff, I see
void
Perl_do_gvgv_dump(pTHX_ I32 level, PerlIO *file, const char *name, GV *sv)
{
PERL_ARGS_ASSERT_DO_GVGV_DUMP;
Perl_dump_indent(aTHX_ level, file, "%s = 0x%"UVxf, name, PTR2UV(sv));
if (sv && GvNAME(sv)) {
const char *hvname;
PerlIO_printf(file, "\t\"");
if (GvSTASH(sv) && (hvname = HvNAME_get(GvSTASH(sv))))
PerlIO_printf(file, "%s\" :: \"", hvname);
PerlIO_printf(file, "%s\"\n", GvNAME(sv));
}
else
PerlIO_putc(file, '\n');
}
so it looks like the core doesn't assume that these are always going to be
non-NULL. I've just added this function to blead:
void
Perl_croak_xs_usage(pTHX_ const CV *const cv, const char *const params)
{
const GV *const gv = CvGV(cv);
PERL_ARGS_ASSERT_CROAK_XS_USAGE;
if (gv) {
const char *const gvname = GvNAME(gv);
const HV *const stash = GvSTASH(gv);
const char *const hvname = stash ? HvNAME_get(stash) : NULL;
if (hvname)
Perl_croak(aTHX_ "Usage: %s::%s(%s)", hvname, gvname, params);
else
Perl_croak(aTHX_ "Usage: %s(%s)", gvname, params);
} else {
/* Pants. I don't think that it should be possible to get here. */
Perl_croak(aTHX_ "Usage: CODE(%"UVXf")(%s)", (UV)cv, params);
}
}
and made all the core XS code (in universal.c, xsutils.c and mro.c) use it,
which results in a small space saving.
Assuming we think it's sane, I'd assume that it will appear in 5.8.9 and
5.10.1, at which point ExtUtils::ParseXS could call it as is. For everything
else, I think it would be viable to patch ExtUtils::ParseXS to see if it
needs it, and if so emit it as a static function.
(I think all it needs is an array to stack the output of print in the loop
starting
PARAGRAPH:
while (fetch_para()) {
and a variable to track if any $cond is non-zero. If any are, spit out that
function above as a static function. Then, whatever, spit out all the array
of lines.)
Does this seem viable?
Nicholas Clark
Thread Previous
|
Thread Next