Patch for ops2pm.pl

All --

Its been quite some time since I did any committing, so I figured I post 
this for comment
rather than just commit it. Without objection, I'll commit.

This patch makes the code and documentation for the program match, removes 
dead
code, and slightly improves (IMHO) the way the program goes about its 
business. Also,
it adds an "XXX" comment about how usage of this program and its 
supporting libraries
has drifted from the initial intent, and calls attention to two ways of 
bringing things back
into line.


Regards,

-- Gregor

----------------------------------------------------

Index: ops2pm.pl
===================================================================
RCS file: /cvs/public/parrot/ops2pm.pl,v
retrieving revision 1.7
diff -u -r1.7 ops2pm.pl
--- ops2pm.pl   30 Jan 2002 04:20:37 -0000      1.7
+++ ops2pm.pl   26 Oct 2002 19:30:30 -0000
@@ -2,7 +2,32 @@
 #
 # ops2pm.pl
 #
-# Generate a Perl module from the operation definitions in an .ops file.
+# Generate a Perl module from the operation definitions in .ops files.
+#
+# The first .ops file on the command line is read in and its name is used 
to
+# determine the name of the Perl module that will contain the op info.
+#
+# Any remaining .ops files on the command line are read in, and their op
+# info objects are appended, in order, to the op info array obtained from 
the
+# first .ops file.
+#
+# WARNING: Generating a combined Perl module for a set of .ops files that
+# you do not later turn into a combined opcode table with the same 
content
+# and order is a recipe for disaster.
+# 
+# XXX: The original design of the .ops processing code was intended to be 
a
+# read-only representation of what was in a particular .ops file. It was
+# not originally intended that it was a mechanism for building a bigger
+# virtual .ops file from multiple physical .ops files. This code does 
half of
+# that job (the other half is getting them to compile together instead of
+# separately in a *_ops.c file). You can see evidence of this by the way 
this
+# code reaches in to the internal OPS hash key to do its concatenation, 
and
+# the way it twiddles each op's CODE hash key after that. If the op and 
oplib
+# Perl modules are going to be used for modifying information read from 
..ops
+# files in addition to reading it, they should be changed to make the 
above
+# operations explicitly supported. Otherwise, the Parrot build and 
interpreter
+# start-up logic should be modified so that it doesn't need to 
concatenate 
+# separate .ops files.
 #
 
 use strict;
@@ -14,6 +39,8 @@
 #$Data::Dumper::Terse  = 1;
 #$Data::Dumper::Indent = 0;
 
+my $moddir  = "lib/Parrot/OpLib";
+
 sub Usage {
     print STDERR <<_EOF_;
 usage: $0 input.ops [input2.ops ...]
@@ -21,34 +48,31 @@
     exit;
 }
 
+Usage() unless @ARGV;
+
 
 #
-# Process command-line argument:
+# Use the first .ops file to determine the output file name, and read in 
its
+# ops:
 #
 
-Usage() unless @ARGV;
-
-my $file = 'core.ops';
-
-my $base = $file;
-$base =~ s/\.ops$//;
+my $file = shift @ARGV;
 
+my ($base)  = ($file =~ m{^(.*)\.ops$});
 my $package = "${base}";
-my $moddir  = "lib/Parrot/OpLib";
 my $module  = "lib/Parrot/OpLib/${package}.pm";
 
+die "$0: Could not find ops file '$file'!\n" unless -e $file;
+my $ops = new Parrot::OpsFile $file;
+die "$0: Could not read ops file '$file'!\n" unless defined $ops;
+
 
 #
-# Read the first input file:
+# Copy the ops from the remaining .ops files to the object just created.
 #
 
-use Data::Dumper;
-
-$file = shift @ARGV;
-die "$0: Could not read ops file '$file'!\n" unless -e $file;
-my $ops = new Parrot::OpsFile $file;
-
 my %seen;
+
 for $file (@ARGV) {
     if ($seen{$file}) {
       print STDERR "$0: Ops file '$file' mentioned more than once!\n";
@@ -56,20 +80,24 @@
     }
     $seen{$file} = 1;
 
-    die "$0: Could not read ops file '$file'!\n" unless -e $file;
+    die "$0: Could not find ops file '$file'!\n" unless -e $file;
     my $temp_ops = new Parrot::OpsFile $file;
-    for(@{$temp_ops->{OPS}}) {
-       push @{$ops->{OPS}},$_;
-    }
+    die "$0: Could not read ops file '$file'!\n" unless defined 
$temp_ops;
+
+    push @{$ops->{OPS}}, @{$temp_ops->{OPS}};
 }
+
+
+#
+# Renumber the ops based on their new absolute positions (they started 
out
+# being numbered according to their relative position within their 
respective
+# .ops files):
+#
+
 my $cur_code = 0;
 for(@{$ops->{OPS}}) {
     $_->{CODE}=$cur_code++;
 }
-
-
-my $num_ops     = scalar $ops->ops;
-my $num_entries = $num_ops + 1; # For trailing NULL
 
 
 #

• Patch for ops2pm.pl by gregor