# New Ticket Created by James Keenan
# Please include the string: [perl #41186]
# in the subject line of all future correspondence about this issue.
# <URL: http://rt.perl.org/rt3/Ticket/Display.html?id=41186 >
Particle has asked me to consider testing and refactoring
('phalanxing', for short) tools/build/ops2pm.pl and tools/build/
ops2c.pl in the same way that I did tools/build/pmc2c.pl.
In my first look at ops2pm.pl, I noticed a possible bug. $file
starts off as a package-scoped lexical variable. It is assigned the
first element remaining on @ARGV at the point of assignment.
115 my $file = shift @ARGV;
The script dies if $file does not exist or turns out not to be a
suitable argument for Parrot::OpsFile::new().
$file next turns up as the iterator variable in a 'for' loop over the
remaining elements in @ARGV:
126 for $file (@ARGV) {
127 if ( $seen{$file} ) {
...
150 }
I would normally have expected to see the iterator variable
differently named and lexically scoped to the 'for' loop:
for my $f (@ARGV) {
... in order to preserve $file for use in the script after the end of
the 'for' loop. At this point there's no big problem, because the
intention of the script is to process a series of files named on the
command line, the first of which is processed a bit differently from
all subsequent ones. But we have to realize that our package-scoped
$file is being assigned to -- albeit less than explicitly -- on each
iteration of the 'for' loop.
The problem arises later on when $file is called inside two HERE docs
being printed to newly created files.
225 my $preamble = <<END_C;
226 #! perl -w
227 #
228 # !!!!!!! DO NOT EDIT THIS FILE !!!!!!!
229 #
230 # This file is generated automatically from '$file'.
...
244 END_C
and
268 print $OUT <<END_C;
269 /* ex: set ro:
270 * !!!!!!! DO NOT EDIT THIS FILE !!!!!!!
271 *
272 * This file is generated automatically from '$file'
273 * by $0.
...
282 END_C
As I read this, the value of $file which will be used in these two
HERE docs is the *last* value assigned to $file when it was used as
the iterator of the 'for' loop -- which should be the *last* command-
line argument.
Is this what we're actually getting when make call ops2pm.pl?
Is this what is intended? If not, shouldn't we either (a)
distinguish between the package-scoped $file and the 'for' loop
iterator $file; and/or (b) make a provision for explicit assignment
to $file just before the HERE docs?
kid51
