Front page | perl.perl5.porters |
Postings from February 2007
Re: refactor dump.c
Thread Previous
|
Thread Next
From:
Steve Peters
Date:
February 22, 2007 06:03
Subject:
Re: refactor dump.c
Message ID:
20070222140254.GA11949@kirk.peters.homeunix.org
On Wed, Feb 21, 2007 at 10:35:53PM -0700, Jim Cromie wrote:
>
> dump.c has a large swath of code in op_dump which has been cut-pasted,
> 756..956 and 2549.. with the former differing by the addition
> and subsequent use of an intermediate var;
>
> const OPCODE optype = o->op_type;
>
> this 3 patch set refactors that code, then calls it
>
> - adds t/run/debugging.t - with 1 test of op-dump
> - refactor Perl_do_op_dump - somewhat ideosyncratic to lower diffsize
> - reuse refactoring in Perl_do_op_xmldump
I've got a few comments about the attached code. First, I'll give the
test a run in a bit, and if it seems OK, I may add it regardless of the
other changes.
> +/* forward decls - minimize diff */
> +STATIC SV* S_dump_op_flags(pTHX_ const OP* o);
> +STATIC SV* S_dump_op_flags_private(pTHX_ const OP* o);
> +static void S_dump_op_mad(pTHX_ I32 level, PerlIO *file, const OP *o);
> +static void S_dump_op_rest(pTHX_ I32 level, PerlIO *file, const OP *o);
> +
I'd rather see these done in embed.fnc than left in here.
> +#ifndef PERL_MAD
> +static void S_dump_op_mad (pTHX_ I32 level, PerlIO *file, const OP *o)
> +{
> + PERL_UNUSED_ARG(level);
> + PERL_UNUSED_ARG(file);
> + PERL_UNUSED_ARG(o);
> +}
> +#else
> +static void S_dump_op_mad (pTHX_ I32 level, PerlIO *file, const OP *o)
> +{
> if (PL_madskills && o->op_madprop) {
> SV * const tmpsv = newSVpvn("", 0);
> MADPROP* mp = o->op_madprop;
For this, I'd really prefer to see one function declaration rather than
two. I think instead that the code should be #ifndef'd inside the functions
rather than #ifndef'ing the whole thing. I have two reasons for it. First,
its less code. Second, its less error prone. Someone will change one but
not the other.
Otherwise, I really like this idea and I hope with these tweaks it will
help dump quite a bit of code out of dump.c.
Steve Peters
steve@fisharerojo.org
Thread Previous
|
Thread Next