Add packfile commentary

I prefixed all my commentary with BCG: so they can be easily
searched for and killed when the time comes.

Author: Benabik

src/packfile/Constant.winxed

@@ -1,8 +1,14 @@
+// BCG: These might deserve to be more central types.
+
 class PACT.Packfile.Constant
 {
     var value;
+    // BCG: Probably want to specify a common API to all of them here.
 }
 
+// BCG: We may also want a Key class.  Keys are not terribly friendly to
+//      edit.
+
 class PACT.Packfile.Constant.Integer : PACT.Packfile.Constant
 {
     function Integer(int value)
@@ -15,6 +21,11 @@ class PACT.Packfile.Constant.Integer : PACT.Packfile.Constant
         return int(self.value);
     }
 
+    // BCG: We probably want to have a type enum/constants
+    //      String comparison is not speedy.
+    //      Alternative is single character types so we can switch on their ord
+    //      I'd like to get typing right quickly so that all levels can share
+    //      the type information.
     function get_type()
     {
         return "int";

src/packfile/Namespace.winxed

@@ -1,3 +1,5 @@
+// BCG: Probably also want subs stored in their namespace.
+//      That's not how the packfile is organized, but it's probably more sane.
 class PACT.Packfile.Namespace
 {
     var name;

src/packfile/Op.winxed

@@ -15,6 +15,8 @@ class PACT.Packfile.Op
         self.labels = [];
     }
 
+    // BCG: Not sure if results and operands can be separated sanely in
+    // all instances.
     function add_results(var results [slurpy])
     {
         for (var r in results)
@@ -38,6 +40,9 @@ class PACT.Packfile.Op
         push(self.labels, label);
     }
 
+    // BCG: We probably want an inverse of this method as well
+    // BCG: I would probably also do this as a stage rather than
+    //      put the guts in this class.  See docs/stages.mkd
     function get_opcode_pmc()
     {
         // TODO: Create and return a representation of this op expressed in

src/packfile/Packfile.winxed

@@ -22,20 +22,26 @@ class PACT.Packfile
         return self.root_ns.get_child_ns(name);
     }
 
+    // BCG: ns optional, perhaps?
     function new_sub(string name, var ns)
     {
         if (ns == null)
             ns = self.root_ns;
         var s = new PACT.Packfile.Subroutine(self, name, ns);
     }
 
+    // BCG: I'm not sure I'd worry about a constant table at this level.
+    //      I think the PackfileConstantTable has convenient methods to
+    //      deal with addition and de-dup when we output.
+    //      It's simpler if we don't have to worry about it until then.
     function add_constant(var constant)
     {
         // TODO: We need to de-duplicate. Do we do that on input or do that
         // on output?
         push(self.constants, constant);
     }
 
+    // BCG: Again with wanting an inverse and doing it as a stage.
     function get_packfile()
     {
         // TODO: This is the biggie. Create the entire Packfile PMC structure

src/packfile/Subroutine.winxed

@@ -1,12 +1,15 @@
 class PACT.Packfile.Subroutine
 {
-    var program;
+    var program; // BCG: What is this?  The packfile?
     var ops;
     var name;
     var flags;
     var parent_namespace;
     var annotations;
     var identifier;
+
+	// BCG: This feels like scope-like information that really
+	//      belongs at levels higher than Packfile.
     var variables;
     var temp_variables;
 
@@ -70,6 +73,8 @@ class PACT.Packfile.Subroutine
         }
     }
 
+    // BCG: These feel like they're in the wrong place.
+    //      The subroutine doesn't have to care about constants.
     function get_constant(int i)
     {
         var c = new PACT.Packfile.Constant.Integer(i);

src/packfile/Subroutine/Flag.winxed

@@ -1,6 +1,9 @@
+// BCG: Do the flags need to be separate classes?
+//      Could they just as sanely be stored in a hash?
+//      Or as variables in subroutine with an array for tags, perhaps?
 class PACT.Packfile.Subroutine.Flag
 {
-
+	// BCG: Subroutine uses name(), which isn't defined here
 }
 
 class PACT.Packfile.Subroutine.Flag.Main : PACT.Packfile.Subroutine.Flag

src/packfile/Variable.winxed

@@ -1,9 +1,19 @@
+// BCG: I would expect the Packfile classes to be too deep for things like
+//      Variables.  I would expect raw Registers instead.
+//      My vision for the Packfile classes was that it would be a PACT-oriented
+//      and simpler method of dealing with packfiles more-or-less as they
+//      are.  Once that's in place we can deal with adding things like
+//      variables and register allocation and the like.
+
 class PACT.Packfile.Variable
 {
     var id;     // A human-readable name for the variable
     var index;  // the actual register index (integer)
     // TODO: It might be nice to store first/last use lifetime information for
     // the register allocator to use.
+    // BCG: I would probably perform a separate lifetime analysis and store the
+    //      data there than relying on the Variable to keep it up to date.
+    //      Also, again with defining common API in the base class.
 }
 
 class PACT.Packfile.Variable.Integer : PACT.Packfile.Variable