pmc2c.pl: Proposed Strategy for Revisions and Testing

[James E Keenan]

Friends:

Last week I had my first encounter with the Parrot project when I
attended the Chicago Perl Hackathon. There Jerry Gay suggested that I
work on improving the tests applied to Parrot build/install tools
written in Perl 5 -- specifically, tools/build/pmc2c.pl. I devoted the
balance of my time at the Hackathon to that task and have continued
working on it during the past week as time has permitted.

I'd like to outline what I think should be done to test pmc2c.pl and
some of the questions I/we will be facing in doing so. I'm at a point
where I could use some feedback so that I don't invest a lot of time
writing patches that are unlikely to be accepted.

The latest version of pmc2c.pl was checked into version 15044 2006-10-29
by particle. It contains extensive documentation and 19 subroutines.
The script is written in a C-ish style wherein one of those subroutines,
main(), processes command-line options and calls some of the other 18
subroutines based on those options. AFAICT, pmc2c.pl is called only
three times in the codebase, all of them inside the Makefile.

PMC2CD = $(PERL) $(BUILD_TOOLS_DIR)/pmc2c.pl --dump
PMC2CC = $(PERL) $(BUILD_TOOLS_DIR)/pmc2c.pl --c
PMC2CV = $(PERL) $(BUILD_TOOLS_DIR)/pmc2c.pl --vtable

Based on my experience in taking over maintenance of CPAN distribution
ExtUtils::ModuleMaker and its associated command-line utility
'modulemaker', as well as some recent experiences at my day job, I've
come to believe that the best way to test Perl scripts that are part of
larger distributions is to extract as much as possible of their
functionality into subroutines which are then placed in a package of
their own. Those subroutines are then imported into both the executable
script and t/*.t test scripts that mimic the structure and flow of the
executable as much as possible.

Given that belief, I propose to:

1. Extract pmc2c.pl's 18 subroutines other than main() into a package
called Parrot::Pmc2c::Utils.

2. Rewrite pmc2c.pl to preserve its current functionality and import
subroutines from Parrot::Pmc2c::Utils as needed.

3. Write a test suite, t/tools/pmc2cutils/*.t, where tests are composed
according to these priorities:

a. Tests which mimic the functionality of pmc2c.pl as called with
the 3 options currently used in the Makefile.

b. Tests which test the working of pmc2c.pl with those options whose
use is described in pmc2c.pl's documentation but which are *not* called
in the Makefile (or, AFAICT, anywhere else).

4. Revise the subroutines in Parrot::Pmc2c::Utils as needed,
particularly as suggested by coverage analysis with Devel::Cover.

5. Revise the documentation in pmc2c.pl and Parrot::Pmc2c::Utils as
needed to reflect steps (1) through (4) above.

Findings

6. So far I've accomplished (1) and (2) and most of (3)(a). I've found
that of pmc2c.pl's 18 subroutines (other than main()), only 6 are
currently touched by the 3 calls to pmc2c.pl inside the Makefile.

find_file
dump_default
open_file
print_tree
read_dump
gen_c

That leaves 12 subroutines currently not used in Parrot's
build-and-install process:

extract_balanced
parse_flags
parse_method_attrs
inherit_attrs
parse_pmc
gen_parent_list
dump_1_pmc
gen_super_meths
add_defaulted
dump_is_newer
dump_pmc
gen_def

7. pmc2c.pl's documentation describes several options which are either
not called within pmc2c.pl itself or called anywhere else:

--library
--debug
--no-lines
--no-body

... and it contains one option which I can't find documented or used
anywhere:

--deb

Unlike code in a CPAN module, we can very specifically describe the way
in which pmc2c.pl is currently used. Consequently, we can very
specifically identify which code *inside* pmc2c.pl is needed to achieve
the results to which it is currently put (the 3 calls inside the Makefile).

When I act in the capacity of a maintenance programmer, my belief is
that any code in the code base I inherit that does not contribute to
accomplishing what the code is designed to do ought to be deprecated.
Taken to its logical conclusion, this would imply deprecating the 12
subroutines listed above which are currently not touched during Parrot's
build-and-install process. If those subroutines were deprecated, they
wouldn't appear in Parrot::Pmc2c::Utils and I wouldn't need to write
tests for them.

That would result in highly maintainable code, but it runs the risk of
eliminating code from the head of the repository which may have been
used in Parrot before I joined the project and which someone may be
planning to use in the future.

A more modest approach would be to maintain and test those subroutines
which, though not currently touched, are described in the documentation
(case (3)(b) above). I would then deprecate only those subroutines
which are neither touched via calls in the Makefile nor described in
pmc2c.pl's current documenation.

How do you think I should proceed?

Thank you very much.

Jim Keenan
On ircperlorg: kid51

[James E Keenan]

James E Keenan wrote:
[snip]

> Findings
>
> 6. So far I've accomplished (1) and (2) and most of (3)(a). I've found
> that of pmc2c.pl's 18 subroutines (other than main()), only 6 are
> currently touched by the 3 calls to pmc2c.pl inside the Makefile.
>
> find_file
> dump_default
> open_file
> print_tree
> read_dump
> gen_c
>
> That leaves 12 subroutines currently not used in Parrot's
> build-and-install process:
>

My analysis was incorrect. 11 of the 12 subroutines are, in fact,
touched when the current version of pmc2c.pl is called in the Makefile
with the '--dump' option.

> extract_balanced
> parse_flags
> parse_method_attrs
> inherit_attrs
> parse_pmc
> gen_parent_list
> dump_1_pmc
> gen_super_meths
> add_defaulted
> dump_is_newer
> dump_pmc

Only gen_def() is not touched by the current pmc2c.pl calls.

The other issues I raised in my first posting are still open and I
welcome your comments.

jimk

[Jonathan Worthington]

James E Keenan wrote:
> Only gen_def() is not touched by the current pmc2c.pl calls.

And I fear that's my mess. That dates back to when we generated .def
files for a PMC to list the symbols to export, but since PARROT_API
sprung into existence it's now redundant, I think.

Jonathan

[James E Keenan]

If no one else speaks up for it, I'll deprecate it. However, I have
quite a few subroutines still to test before I get to gen_def(). Thanks
for your response.

jimk

[Allison Randal]

James E Keenan wrote:
>
> That would result in highly maintainable code, but it runs the risk of
> eliminating code from the head of the repository which may have been
> used in Parrot before I joined the project and which someone may be
> planning to use in the future.
>
> A more modest approach would be to maintain and test those subroutines
> which, though not currently touched, are described in the documentation
> (case (3)(b) above). I would then deprecate only those subroutines
> which are neither touched via calls in the Makefile nor described in
> pmc2c.pl's current documenation.
>
> How do you think I should proceed?

Go with the more extensive and thoroughly tested refactor. Particle will
work with you (mentoring/code review) and make sure your changes get
checked in.

Thanks!
Allison

[James E Keenan]

Allison Randal wrote:

>
> Go with the more extensive and thoroughly tested refactor. Particle will
> work with you (mentoring/code review) and make sure your changes get
> checked in.
>

As my 2nd post noted, I made an error in counting the number of
subroutines that could be deprecated. Only gen_def() looks like a
likely candidate for being tossed. (See Jonathan Worthington's post.)

So my 2 options will pretty much collapse to 1. Thanks.

jimk