Thrift compiler code cleanup, comments, php inline generation, etc


git-svn-id: https://svn.apache.org/repos/asf/incubator/thrift/trunk@664822 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/compiler/cpp/src/generate/t_cpp_generator.cc b/compiler/cpp/src/generate/t_cpp_generator.cc
index bda43ea..1a2c8ae 100644
--- a/compiler/cpp/src/generate/t_cpp_generator.cc
+++ b/compiler/cpp/src/generate/t_cpp_generator.cc
@@ -40,10 +40,12 @@
     "#include <transport/TTransport.h>" << endl <<
     endl;
 
+  // Include the types file
   f_types_impl_ <<
     "#include \"" << program_name_ <<"_types.h\"" << endl <<
     endl;
 
+  // Open namespace
   ns_open_ = namespace_open(tprogram->get_namespace());
   ns_close_ = namespace_close(tprogram->get_namespace());
 
@@ -60,7 +62,7 @@
  * Closes the output files.
  */
 void t_cpp_generator::close_generator(t_program* tprogram) {
-  // Close ns
+  // Close namespace
   f_types_ <<
     ns_close_ << endl <<
     endl;
@@ -83,8 +85,7 @@
  */
 void t_cpp_generator::generate_typedef(t_typedef* ttypedef) {
   f_types_ <<
-    indent() << "typedef " << type_name(ttypedef->get_type()) << " " << 
-    ttypedef->get_symbolic() << ";" << endl <<
+    indent() << "typedef " << type_name(ttypedef->get_type()) << " " << ttypedef->get_symbolic() << ";" << endl <<
     endl;
 }
 
@@ -97,7 +98,6 @@
 void t_cpp_generator::generate_enum(t_enum* tenum) {
   f_types_ <<
     indent() << "enum " << tenum->get_name() << " {" << endl;
-
   indent_up();
 
   vector<t_constant*> constants = tenum->get_constants();
@@ -119,7 +119,6 @@
   }
 
   indent_down();
-
   f_types_ <<
     endl <<
     "};" << endl <<
@@ -127,9 +126,9 @@
 }
 
 /**
- * Generates a struct definition for a thrift data type. In C++, this is just
- * simple C struct with basic data members. There are no constructors,
- * initializers, etc.
+ * Generates a struct definition for a thrift data type. This is a class
+ * with data members and a read/write() function, plus a mirroring isset
+ * inner class.
  *
  * @param tstruct The struct definition
  */
@@ -140,7 +139,7 @@
 }
 
 /**
- * Writes the struct def.
+ * Writes the struct definition into the header file
  *
  * @param out Output stream
  * @param tstruct The struct
@@ -152,13 +151,12 @@
     indent() << "class " << tstruct->get_name() << " {" << endl <<
     indent() << " public:" << endl <<
     endl;
-  
   indent_up();
 
   // Get members
   vector<t_field*>::const_iterator m_iter; 
   const vector<t_field*>& members = tstruct->get_members();
-
+  
   // Default constructor
   bool init_ctor = false;
   for (m_iter = members.begin(); m_iter != members.end(); ++m_iter) {
@@ -180,14 +178,14 @@
   if (init_ctor) {
     out << " {} " << endl;
   }
-
+  
   // Declare all fields
   for (m_iter = members.begin(); m_iter != members.end(); ++m_iter) {
     indent(out) <<
       declare_field(*m_iter) << endl;
   }
-
-  // Isset vector
+  
+  // Isset struct has boolean fields
   if (members.size() > 0) {
     out <<
       endl <<
@@ -213,6 +211,7 @@
         indent(out) <<
           "bool " << (*m_iter)->get_name() << ";" << endl;
       }
+
     indent_down();
     indent(out) <<
       "} __isset;" << endl;  
@@ -220,12 +219,8 @@
 
   out <<
     endl <<
-    indent() << "uint32_t read(" <<
-    "boost::shared_ptr<const facebook::thrift::protocol::TProtocol> iprot, " <<
-    "boost::shared_ptr<facebook::thrift::transport::TTransport> itrans);" << endl <<
-    indent() << "uint32_t write(" <<
-    "boost::shared_ptr<const facebook::thrift::protocol::TProtocol> oprot, " <<
-    "boost::shared_ptr<facebook::thrift::transport::TTransport> otrans) const;" << endl <<
+    indent() << "uint32_t read(boost::shared_ptr<const facebook::thrift::protocol::TProtocol> iprot, boost::shared_ptr<facebook::thrift::transport::TTransport> itrans);" << endl <<
+    indent() << "uint32_t write(boost::shared_ptr<const facebook::thrift::protocol::TProtocol> oprot, boost::shared_ptr<facebook::thrift::transport::TTransport> otrans) const;" << endl <<
     endl;
 
   indent_down(); 
@@ -243,9 +238,7 @@
 void t_cpp_generator::generate_struct_reader(ofstream& out,
                                              t_struct* tstruct) {
   indent(out) <<
-    "uint32_t " << tstruct->get_name() << "::read(" <<
-    "boost::shared_ptr<const facebook::thrift::protocol::TProtocol> iprot, " <<
-    "boost::shared_ptr<facebook::thrift::transport::TTransport> itrans) {" << endl;
+    "uint32_t " << tstruct->get_name() << "::read(boost::shared_ptr<const facebook::thrift::protocol::TProtocol> iprot, boost::shared_ptr<facebook::thrift::transport::TTransport> itrans) {" << endl;
   indent_up();
 
   const vector<t_field*>& fields = tstruct->get_members();
@@ -320,7 +313,7 @@
 }
 
 /**
- * Makes a helper function to gen a struct writer.
+ * Generates the write function.
  *
  * @param out Stream to write to
  * @param tstruct The struct
@@ -332,9 +325,7 @@
   vector<t_field*>::const_iterator f_iter;
 
   indent(out) <<
-    "uint32_t " << tstruct->get_name() << "::write(" <<
-    "boost::shared_ptr<const facebook::thrift::protocol::TProtocol> oprot, " <<
-    "boost::shared_ptr<facebook::thrift::transport::TTransport> otrans) const {" << endl;
+    "uint32_t " << tstruct->get_name() << "::write(boost::shared_ptr<const facebook::thrift::protocol::TProtocol> oprot, boost::shared_ptr<facebook::thrift::transport::TTransport> otrans) const {" << endl;
   indent_up();
 
   out <<
@@ -382,9 +373,7 @@
   vector<t_field*>::const_iterator f_iter;
 
   indent(out) <<
-    "uint32_t " << tstruct->get_name() << "::write(" <<
-    "boost::shared_ptr<const facebook::thrift::protocol::TProtocol> oprot, " <<
-    "boost::shared_ptr<facebook::thrift::transport::TTransport> otrans) const {" << endl;
+    "uint32_t " << tstruct->get_name() << "::write(boost::shared_ptr<const facebook::thrift::protocol::TProtocol> oprot, boost::shared_ptr<facebook::thrift::transport::TTransport> otrans) const {" << endl;
   indent_up();
 
   out <<
@@ -478,13 +467,14 @@
     ns_open_ << endl <<
     endl;
 
+  // Generate all the components
   generate_service_interface(tservice);
   generate_service_helpers(tservice);
   generate_service_client(tservice);
   generate_service_server(tservice);
   generate_service_multiface(tservice);
 
-
+  // Close the namespace
   f_service_ <<
     ns_close_ << endl <<
     endl;
@@ -494,12 +484,14 @@
   f_header_ <<
     "#endif" << endl;
 
+  // Close the files
   f_service_.close();
   f_header_.close();
 }
 
 /**
- * Generates helper functions for a service.
+ * Generates helper functions for a service. Basically, this generates types
+ * for all the arguments and results to functions.
  *
  * @param tservice The service to generate a header definition for
  */
@@ -546,7 +538,7 @@
  * @param tservice The service to generate a multiserver for.
  */
 void t_cpp_generator::generate_service_multiface(t_service* tservice) {
-    // Generate the dispatch methods
+  // Generate the dispatch methods
   vector<t_function*> functions = tservice->get_functions();
   vector<t_function*>::iterator f_iter; 
 
@@ -639,16 +631,18 @@
 
   indent_up();
   f_header_ <<
-    indent() << service_name_ << "Client" <<
-    "(boost::shared_ptr<facebook::thrift::transport::TTransport> trans, boost::shared_ptr<const facebook::thrift::protocol::TProtocol> prot) : " <<
-    "_itrans(trans), _otrans(trans), " <<
-    "_iprot(prot), _oprot(prot) {}" << endl;
+    indent() << service_name_ << "Client(boost::shared_ptr<facebook::thrift::transport::TTransport> trans, boost::shared_ptr<const facebook::thrift::protocol::TProtocol> prot) : " << endl <<
+    indent() << "  _itrans(trans)," << endl <<
+    indent() << "  _otrans(trans)," << endl <<
+    indent() << "  _iprot(prot)," << endl <<
+    indent() << "  _oprot(prot) {}" << endl;
+
   f_header_ <<
-    indent() << service_name_ << "Client" <<
-    "(boost::shared_ptr<facebook::thrift::transport::TTransport> itrans, boost::shared_ptr<facebook::thrift::transport::TTransport> otrans," <<
-    " boost::shared_ptr<const facebook::thrift::protocol::TProtocol> iprot, boost::shared_ptr<const facebook::thrift::protocol::TProtocol> oprot) : " <<
-    "_itrans(itrans), _otrans(otrans), " <<
-    "_iprot(iprot), _oprot(oprot) {}" << endl;
+    indent() << service_name_ << "Client(boost::shared_ptr<facebook::thrift::transport::TTransport> itrans, boost::shared_ptr<facebook::thrift::transport::TTransport> otrans, boost::shared_ptr<const facebook::thrift::protocol::TProtocol> iprot, boost::shared_ptr<const facebook::thrift::protocol::TProtocol> oprot) : " << endl <<
+    indent() << "  _itrans(itrans)," << endl <<
+    indent() << "  _otrans(otrans)," << endl <<
+    indent() << "  _iprot(iprot)," << endl <<
+    indent() << "  _oprot(oprot) {}" << endl;
 
   vector<t_function*> functions = tservice->get_functions();
   vector<t_function*>::const_iterator f_iter; 
@@ -723,6 +717,7 @@
     scope_down(f_service_);
     f_service_ << endl;
 
+    // Function for sending
     t_function send_function(g_program->get_void_type(),
                              string("send_") + (*f_iter)->get_name(),
                              (*f_iter)->get_arglist());
@@ -732,6 +727,7 @@
       function_signature(&send_function, scope) << endl;
     scope_up(f_service_);
 
+    // Function arguments and results
     string argsname = (*f_iter)->get_name() + "_args";
     string resultname = (*f_iter)->get_name() + "_result";
 
@@ -787,8 +783,7 @@
         indent() << "_iprot->readMessageEnd(_itrans);" << endl <<
         endl;
 
-
-      // Careful, only return _result if not a void function
+      // Careful, only look for _result if not a void function
       if (!(*f_iter)->get_returntype()->is_void()) {
         f_service_ <<
           indent() << "if (__result.__isset.success) {" << endl <<
@@ -806,7 +801,7 @@
           indent() << "}" << endl;
       }
 
-      // Careful, only return _result if not a void function
+      // We only get here if we are a void function
       if ((*f_iter)->get_returntype()->is_void()) {
         indent(f_service_) <<
           "return;" << endl;
@@ -854,8 +849,7 @@
   indent_up();
   for (f_iter = functions.begin(); f_iter != functions.end(); ++f_iter) {
     indent(f_header_) <<
-      "void process_" << (*f_iter)->get_name() <<
-      "(int32_t seqid, boost::shared_ptr<facebook::thrift::transport::TTransport> _itrans, boost::shared_ptr<facebook::thrift::transport::TTransport> _otrans);" << endl;
+      "void process_" << (*f_iter)->get_name() << "(int32_t seqid, boost::shared_ptr<facebook::thrift::transport::TTransport> _itrans, boost::shared_ptr<facebook::thrift::transport::TTransport> _otrans);" << endl;
   }
   indent_down();
 
@@ -876,12 +870,16 @@
 
   f_header_ << 
     " public: " << endl <<
-    indent() << service_name_ << "Processor(boost::shared_ptr<" << service_name_ << "If> iface, boost::shared_ptr<const facebook::thrift::protocol::TProtocol> prot) : " <<
-    "_iface(iface), _iprot(prot), _oprot(prot) {" << endl <<
+    indent() << service_name_ << "Processor(boost::shared_ptr<" << service_name_ << "If> iface, boost::shared_ptr<const facebook::thrift::protocol::TProtocol> prot) :" << endl <<
+    indent() << "  _iface(iface)," << endl <<
+    indent() << "  _iprot(prot)," << endl <<
+    indent() << "  _oprot(prot) {" << endl <<
     declare_map <<
     indent() << "}" << endl <<
-    indent() << service_name_ << "Processor(boost::shared_ptr<" << service_name_ << "If> iface, boost::shared_ptr<const facebook::thrift::protocol::TProtocol> iprot, boost::shared_ptr<const facebook::thrift::protocol::TProtocol> oprot) : " <<
-    "_iface(iface), _iprot(iprot), _oprot(oprot) {" << endl <<
+    indent() << service_name_ << "Processor(boost::shared_ptr<" << service_name_ << "If> iface, boost::shared_ptr<const facebook::thrift::protocol::TProtocol> iprot, boost::shared_ptr<const facebook::thrift::protocol::TProtocol> oprot) :" << endl <<
+    indent() << "  _iface(iface)," << endl <<
+    indent() << "  _iprot(iprot)," << endl <<
+    indent() << "  _oprot(oprot) {" << endl <<
     declare_map <<
     indent() << "}" << endl <<
     endl <<
@@ -891,11 +889,9 @@
   f_header_ <<
     "};" << endl << endl;
 
-
   // Generate the server implementation
   f_service_ <<
-    "bool " << service_name_ << "Processor::" <<
-    "process(boost::shared_ptr<facebook::thrift::transport::TTransport> itrans, boost::shared_ptr<facebook::thrift::transport::TTransport> otrans) {" << endl;
+    "bool " << service_name_ << "Processor::process(boost::shared_ptr<facebook::thrift::transport::TTransport> itrans, boost::shared_ptr<facebook::thrift::transport::TTransport> otrans) {" << endl;
   indent_up();
 
   f_service_ <<
@@ -921,35 +917,6 @@
     indent() << "  (this->*(pfn->second))(seqid, itrans, otrans);" << endl <<
     indent() << "}" << endl;  
 
-  // DEPRECATED: if else if bullshit
-  /*
-  bool first = true;
-  for (f_iter = functions.begin(); f_iter != functions.end(); ++f_iter) {
-    if (!first) {
-      f_service_ << " else ";
-    } else {
-      f_service_ << indent();
-      first = false;
-    }
-    f_service_ <<
-      "if (fname.compare(\"" << (*f_iter)->get_name() <<"\") == 0) {" << endl;
-    indent_up();
-    indent(f_service_) <<
-      "process_" << (*f_iter)->get_name() <<
-      "(seqid, itrans, otrans);" << endl;
-    indent_down();
-    indent(f_service_) << "}";
-  }
-  indent(f_service_) <<
-    " else {" << endl;
-  indent_up();
-  indent(f_service_) <<
-    "throw facebook::thrift::Exception(\"Unknown function name: '\"+fname+\"'\");" << endl;
-  indent_down();
-  indent(f_service_) <<
-    "}" << endl;
-  */
-
   // Read end of args field, the T_STOP, and the struct close
   f_service_ <<
     indent() << "return true;" << endl;
@@ -963,7 +930,6 @@
   for (f_iter = functions.begin(); f_iter != functions.end(); ++f_iter) {
     generate_process_function(tservice, *f_iter);
   }
-
 }
 
 /**
@@ -1129,7 +1095,6 @@
   } else if (type->is_container()) {
     generate_deserialize_container(out, tfield->get_type(), name);
   } else if (type->is_base_type() || type->is_enum()) {
-
     indent(out) <<
       "xfer += iprot->";
     
@@ -1223,11 +1188,9 @@
 
   // For loop iterates over elements
   string i = tmp("_i");
-  indent(out) <<
-    "uint32_t " << i << ";" << endl;
-  indent(out) <<
-    "for (" <<
-    i << " = 0; " << i << " < " << size << "; ++" << i << ")" << endl;
+  out <<
+    indent() << "uint32_t " << i << ";" << endl <<
+    indent() << "for (" << i << " = 0; " << i << " < " << size << "; ++" << i << ")" << endl;
   
     scope_up(out);
     
@@ -1265,10 +1228,9 @@
   t_field fkey(tmap->get_key_type(), key);
   t_field fval(tmap->get_val_type(), val);
 
-  indent(out) <<
-    declare_field(&fkey) << endl;
-  indent(out) <<
-    declare_field(&fval) << endl;
+  out <<
+    indent() << declare_field(&fkey) << endl <<
+    indent() << declare_field(&fval) << endl;
 
   generate_deserialize_field(out, &fkey);
   generate_deserialize_field(out, &fval);
@@ -1422,15 +1384,10 @@
   }
 
   string iter = tmp("_iter");
-  indent(out) <<
-    type_name(ttype) << "::const_iterator " << iter << ";" << endl;
-  indent(out) <<
-    "for (" << iter << " = " << prefix  << ".begin(); " <<
-    iter << " != " << prefix << ".end(); " <<
-    "++" << iter << ")" << endl;
-
-    scope_up(out);
-
+  out <<
+    indent() << type_name(ttype) << "::const_iterator " << iter << ";" << endl <<
+    indent() << "for (" << iter << " = " << prefix  << ".begin(); " << iter << " != " << prefix << ".end(); ++" << iter << ")" << endl;
+  scope_up(out);
     if (ttype->is_map()) {
       generate_serialize_map_element(out, (t_map*)ttype, iter);
     } else if (ttype->is_set()) {
@@ -1438,19 +1395,18 @@
     } else if (ttype->is_list()) {
       generate_serialize_list_element(out, (t_list*)ttype, iter);
     }
-
-    scope_down(out);
+  scope_down(out);
     
-    if (ttype->is_map()) {
-      indent(out) <<
-        "xfer += oprot->writeMapEnd(otrans);" << endl;
-    } else if (ttype->is_set()) {
-      indent(out) <<
-        "xfer += oprot->writeSetEnd(otrans);" << endl;
-    } else if (ttype->is_list()) {
-      indent(out) <<
-        "xfer += oprot->writeListEnd(otrans);" << endl;
-    }    
+  if (ttype->is_map()) {
+    indent(out) <<
+      "xfer += oprot->writeMapEnd(otrans);" << endl;
+  } else if (ttype->is_set()) {
+    indent(out) <<
+      "xfer += oprot->writeSetEnd(otrans);" << endl;
+  } else if (ttype->is_list()) {
+    indent(out) <<
+      "xfer += oprot->writeListEnd(otrans);" << endl;
+  }
 
   scope_down(out);  
 }
@@ -1533,11 +1489,11 @@
   return result;
 }
 
-
 /**
  * Returns a C++ type name
  *
  * @param ttype The type
+ * @return String of the type name, i.e. std::set<type>
  */
 string t_cpp_generator::type_name(t_type* ttype) {
   if (ttype->is_base_type()) {
@@ -1562,6 +1518,7 @@
  * Returns the C++ type that corresponds to the thrift type.
  *
  * @param tbase The base type
+ * @return Explicit C++ type, i.e. "int32_t"
  */
 string t_cpp_generator::base_type_name(t_base_type::t_base tbase) {
   switch (tbase) {
@@ -1590,6 +1547,7 @@
  * Declares a field, which may include initialization as necessary.
  *
  * @param ttype The type
+ * @return Field declaration, i.e. int x = 0;
  */
 string t_cpp_generator::declare_field(t_field* tfield, bool init) {
   // TODO(mcslee): do we ever need to initialize the field?
@@ -1646,6 +1604,9 @@
 
 /**
  * Renders a field list
+ *
+ * @param tstruct The struct definition
+ * @return Comma sepearated list of all field names in that struct
  */
 string t_cpp_generator::argument_list(t_struct* tstruct) {
   string result = "";
@@ -1666,6 +1627,9 @@
 
 /**
  * Converts the parse type to a C++ enum string for the given type.
+ *
+ * @param type Thrift Type
+ * @return String of C++ code to definition of that type constant
  */
 string t_cpp_generator::type_to_enum(t_type* type) {
   while (type->is_typedef()) {
diff --git a/compiler/cpp/src/generate/t_cpp_generator.h b/compiler/cpp/src/generate/t_cpp_generator.h
index 55c7c2f..37c36a1 100644
--- a/compiler/cpp/src/generate/t_cpp_generator.h
+++ b/compiler/cpp/src/generate/t_cpp_generator.h
@@ -8,7 +8,7 @@
 
 #include "t_oop_generator.h"
 
-// TODO(mcslee: Paramaterize the output dir
+// TODO(mcslee): Paramaterize the output dir
 #define T_CPP_DIR "gen-cpp"
 
 /**
@@ -19,27 +19,31 @@
 class t_cpp_generator : public t_oop_generator {
  public:
   t_cpp_generator() {}
-  ~t_cpp_generator() {}
 
-  /** Init and close methods */
+  /**
+   * Init and close methods
+   */
 
   void init_generator(t_program *tprogram);
   void close_generator(t_program *tprogram);
 
-  /** Program-level generation functions */
+  /**
+   * Program-level generation functions
+   */
 
   void generate_typedef  (t_typedef*  ttypedef);
   void generate_enum     (t_enum*     tenum);
   void generate_struct   (t_struct*   tstruct);
   void generate_service  (t_service*  tservice);
 
-  void generate_struct_definition (std::ofstream& out, t_struct* tstruct);
-
-  void generate_struct_reader     (std::ofstream& out, t_struct* tstruct);
-  void generate_struct_writer     (std::ofstream& out, t_struct* tstruct);
+  void generate_struct_definition    (std::ofstream& out, t_struct* tstruct);
+  void generate_struct_reader        (std::ofstream& out, t_struct* tstruct);
+  void generate_struct_writer        (std::ofstream& out, t_struct* tstruct);
   void generate_struct_result_writer (std::ofstream& out, t_struct* tstruct);
 
-  /** Service-level generation functions */
+  /**
+   * Service-level generation functions
+   */
 
   void generate_service_interface (t_service* tservice);
   void generate_service_multiface (t_service* tservice);
@@ -47,10 +51,11 @@
   void generate_service_client    (t_service* tservice);
   void generate_service_server    (t_service* tservice);
   void generate_process_function  (t_service* tservice, t_function* tfunction);
-
   void generate_function_helpers  (t_function* tfunction);
 
-  /** Serialization constructs */
+  /**
+   * Serialization constructs
+   */
 
   void generate_deserialize_field        (std::ofstream& out,
                                           t_field*    tfield, 
@@ -100,7 +105,9 @@
                                           t_list*     tlist,
                                           std::string iter);
 
-  /** Helper rendering functions */
+  /**
+   * Helper rendering functions
+   */
 
   std::string namespace_open(std::string ns);
   std::string namespace_close(std::string ns);
@@ -113,14 +120,20 @@
 
  private:
 
-  /** File streams */
+  /**
+   * Strings for namespace, computed once up front then used directly
+   */
 
   std::string ns_open_;
   std::string ns_close_;
 
+  /**
+   * File streams, stored here to avoid passing them as parameters to every
+   * function.
+   */
+
   std::ofstream f_types_;
   std::ofstream f_types_impl_;
-
   std::ofstream f_header_;
   std::ofstream f_service_;
 };
diff --git a/compiler/cpp/src/generate/t_generator.cc b/compiler/cpp/src/generate/t_generator.cc
index 99baf33..dc09ba1 100644
--- a/compiler/cpp/src/generate/t_generator.cc
+++ b/compiler/cpp/src/generate/t_generator.cc
@@ -7,6 +7,7 @@
  * program to perform the correct actions.
  *
  * @param program The thrift program to compile into C++ source
+ * @author Mark Slee <mcslee@facebook.com>
  */
 void t_generator::generate_program(t_program *tprogram) {
   // Set program name
diff --git a/compiler/cpp/src/generate/t_generator.h b/compiler/cpp/src/generate/t_generator.h
index 16d8084..07eaf38 100644
--- a/compiler/cpp/src/generate/t_generator.h
+++ b/compiler/cpp/src/generate/t_generator.h
@@ -15,7 +15,10 @@
  */
 class t_generator {
  public:
-  t_generator() { tmp_ = 0; }
+  t_generator() {
+    tmp_ = 0;
+  }
+
   virtual ~t_generator() {}
 
   /**
@@ -26,45 +29,67 @@
   void generate_program  (t_program*  tprogram);
 
  protected:
-  /** Optional methods that may be imlemented by subclasses. */
+
+  /**
+   * Optional methods that may be imlemented by subclasses to take necessary
+   * steps at the beginning or end of code generation.
+   */
 
   virtual void init_generator    (t_program*  tprogram) {}
   virtual void close_generator   (t_program*  tprogram) {}
 
-  /** Pure virtual methods implemented by the generator subclasses. */
+  /**
+   * Pure virtual methods implemented by the generator subclasses.
+   */
 
   virtual void generate_typedef  (t_typedef*  ttypedef)  = 0;
   virtual void generate_enum     (t_enum*     tenum)     = 0;
   virtual void generate_struct   (t_struct*   tstruct)   = 0;
+  virtual void generate_service  (t_service*  tservice)  = 0;
   virtual void generate_xception (t_struct*   txception) {
+    // By default exceptions are the same as structs
     generate_struct(txception);
   }
-  virtual void generate_service  (t_service*  tservice)  = 0;
 
-  /** Method to get the program name, may be overridden */
-
+  /**
+   * Method to get the program name, may be overridden
+   */
   virtual std::string get_program_name(t_program* tprogram) {
     return tprogram->get_name();
   }
 
-  /** Method to get the service name, may be overridden */
+  /**
+   * Method to get the service name, may be overridden
+   */
   virtual std::string get_service_name(t_service* tservice) {
     return tservice->get_name();
   }
 
-  /** Creates a unique temporary variable name. */
+  /**
+   * Creates a unique temporary variable name, which is just "name" with a
+   * number appended to it (i.e. name35)
+   */
   std::string tmp(std::string name) {
     std::ostringstream out;
     out << name << tmp_++;
     return out.str();
   }
 
-  /** Indentation level modifiers */
+  /**
+   * Indentation level modifiers
+   */
 
-  void indent_up()   { ++indent_; }
-  void indent_down() { --indent_; }
+  void indent_up(){
+    ++indent_;
+  }
 
-  /** Indentation print function */
+  void indent_down() {
+    --indent_;
+  }
+
+  /**
+   * Indentation print function
+   */
   std::string indent() {
     std::string ind = "";
     int i;
@@ -74,23 +99,35 @@
     return ind;
   }
 
-  /** Indentation utility wrapper */
+  /**
+   * Indentation utility wrapper
+   */
   std::ostream& indent(std::ostream &os) {
     return os << indent();
   }
 
  protected:
-  /** Quick accessor for formatted program name */
+  /**
+   * Quick accessor for formatted program name that is currently being
+   * generated.
+   */
   std::string program_name_;
 
-  /** Quick accessor for formatted service name */
+  /**
+   * Quick accessor for formatted service name that is currently being
+   * generated.
+   */
   std::string service_name_;
 
  private:
-  /** Indentation level */
+  /**
+   * Current code indentation level
+   */
   int indent_;
 
-  /** Temporary variable counter */
+  /**
+   * Temporary variable counter, for making unique variable names
+   */
   int tmp_;
 };
 
diff --git a/compiler/cpp/src/generate/t_java_generator.cc b/compiler/cpp/src/generate/t_java_generator.cc
index 987a755..b99c748 100644
--- a/compiler/cpp/src/generate/t_java_generator.cc
+++ b/compiler/cpp/src/generate/t_java_generator.cc
@@ -18,14 +18,17 @@
 
 /**
  * Packages the generated file
+ *
+ * @return String of the package, i.e. "package com.facebook.thriftdemo;"
  */
 string t_java_generator::java_package() {
-  // TODO(mcslee): Allow destination package to be specified in .thrift file
   return string("package ") + package_name_ + ";\n\n";
 }
 
 /**
  * Prints standard java imports
+ *
+ * @return List of imports for Java types that are used in here
  */
 string t_java_generator::java_type_imports() {
   return
@@ -38,6 +41,8 @@
 
 /**
  * Prints standard java imports
+ *
+ * @return List of imports necessary for thrift
  */
 string t_java_generator::java_thrift_imports() {
   return
@@ -47,20 +52,21 @@
 }
 
 /**
- * Does nothing in Java
+ * Nothing in Java
  */
 void t_java_generator::close_generator(t_program *tprogram) {}
 
 /**
- * Generates a typedef. This is not done in Java.
+ * Generates a typedef. This is not done in Java, since it does
+ * not support arbitrary name replacements, and it'd be a wacky waste
+ * of overhead to make wrapper classes.
  *
  * @param ttypedef The type definition
  */
 void t_java_generator::generate_typedef(t_typedef* ttypedef) {}
 
 /**
- * Generates code for an enumerated type. In C++, this is essentially the same
- * as the thrift definition itself, using the enum keyword in C++.
+ * Enums are a class with a set of static constants. 
  *
  * @param tenum The enumeration
  */
@@ -70,6 +76,7 @@
   ofstream f_enum;
   f_enum.open(f_enum_name.c_str());
 
+  // Comment and package it
   f_enum <<
     autogen_comment() <<
     java_package() << endl;
@@ -94,12 +101,12 @@
   }
 
   scope_down(f_enum);
+  f_enum.close();
 }
 
 /**
- * Generates a struct definition for a thrift data type. In C++, this is just
- * simple C struct with basic data members. There are no constructors,
- * initializers, etc.
+ * Generates a struct definition for a thrift data type. This is a class
+ * with data members, read(), write(), and an inner Isset class.
  *
  * @param tstruct The struct definition
  */
@@ -107,10 +114,21 @@
   generate_java_struct(tstruct, false);
 }
 
+/**
+ * Exceptions are structs, but they inherit from Exception
+ *
+ * @param tstruct The struct definition
+ */
 void t_java_generator::generate_xception(t_struct* txception) {
   generate_java_struct(txception, true);
 }
 
+
+/**
+ * Java struct definition.
+ *
+ * @param tstruct The struct definition
+ */
 void t_java_generator::generate_java_struct(t_struct* tstruct,
                                             bool is_exception) {
   // Make output file
@@ -130,6 +148,16 @@
   f_struct.close();
 }
 
+/**
+ * Java struct definition. This has various parameters, as it could be
+ * generated standalone or inside another class as a helper. If it
+ * is a helper than it is a static class.
+ *
+ * @param tstruct      The struct definition
+ * @param is_exception Is this an exception?
+ * @param in_class     If inside a class, needs to be static class
+ * @param is_result    If this is a result it needs a different writer
+ */
 void t_java_generator::generate_java_struct_definition(ofstream &out,
                                                        t_struct* tstruct,
                                                        bool is_exception,
@@ -144,6 +172,7 @@
   
   scope_up(out);
 
+  // Members are public
   const vector<t_field*>& members = tstruct->get_members();
   vector<t_field*>::const_iterator m_iter; 
   for (m_iter = members.begin(); m_iter != members.end(); ++m_iter) {
@@ -151,6 +180,7 @@
       "public " << declare_field(*m_iter, true) << endl;
   }
 
+  // Inner Isset class
   if (members.size() > 0) {
     out <<
       endl <<
@@ -177,6 +207,11 @@
   out << endl;
 }
 
+/**
+ * Generates a function to read all the fields of the struct.
+ *
+ * @param tstruct The struct definition
+ */
 void t_java_generator::generate_java_struct_reader(ofstream& out,
                                                    t_struct* tstruct) {
   out <<
@@ -194,7 +229,6 @@
   // Loop over reading in fields
   indent(out) <<
     "while (true)" << endl;
-
     scope_up(out);
     
     // Read beginning field marker
@@ -253,6 +287,11 @@
     endl;
 }
 
+/**
+ * Generates a function to write all the fields of the struct
+ *
+ * @param tstruct The struct definition
+ */
 void t_java_generator::generate_java_struct_writer(ofstream& out,
                                                    t_struct* tstruct) {
   out <<
@@ -292,6 +331,14 @@
     indent() << "}" << endl;
 }
 
+/**
+ * Generates a function to write all the fields of the struct,
+ * which is a function result. These fields are only written
+ * if they are set in the Isset array, and only one of them
+ * can be set at a time.
+ *
+ * @param tstruct The struct definition
+ */
 void t_java_generator::generate_java_struct_result_writer(ofstream& out,
                                                           t_struct* tstruct) {
   out <<
@@ -409,6 +456,11 @@
     endl;
 }
 
+/**
+ * Generates structs for all the service args and return types
+ *
+ * @param tservice The service
+ */
 void t_java_generator::generate_service_helpers(t_service* tservice) {
   vector<t_function*> functions = tservice->get_functions();
   vector<t_function*>::iterator f_iter; 
@@ -541,14 +593,14 @@
         "public " << function_signature(&recv_function) << endl;
       scope_up(f_service_);
            
-      // TODO(mcslee): Message validation here
-
       f_service_ <<
         indent() << "TMessage _msg = _iprot.readMessageBegin(_itrans);" << endl <<
         indent() << resultname << " __result = new " << resultname << "();" << endl <<
         indent() << "__result.read(_iprot, _itrans);" << endl <<
         indent() << "_iprot.readMessageEnd(_itrans);" << endl;
 
+      // TODO(mcslee): Message validation here, was the seqid etc ok?
+
       // Careful, only return _result if not a void function
       if (!(*f_iter)->get_returntype()->is_void()) {
         f_service_ <<
@@ -567,7 +619,7 @@
           indent() << "}" << endl;
       }
 
-      // Careful, only return _result if not a void function
+      // If you get here it's an exception, unless a void function
       if ((*f_iter)->get_returntype()->is_void()) {
         indent(f_service_) <<
           "return;" << endl;
@@ -627,14 +679,13 @@
   
   // Generate the server implementation
   indent(f_service_) <<
-    "public boolean process(TTransport _itrans, TTransport _otrans) " <<
-    "throws TException" << endl;
+    "public boolean process(TTransport _itrans, TTransport _otrans) throws TException" << endl;
   scope_up(f_service_);
 
   f_service_ <<
     indent() << "TMessage _msg = _iprot.readMessageBegin(_itrans);" << endl;
 
-  // TODO(mcslee): validate message
+  // TODO(mcslee): validate message, was the seqid etc. legit?
 
   bool first = true;
   for (f_iter = functions.begin(); f_iter != functions.end(); ++f_iter) {
@@ -648,8 +699,7 @@
       "if (_msg.name.equals(\"" << (*f_iter)->get_name() <<"\")) {" << endl;
     indent_up();
     indent(f_service_) <<
-      "process_" << (*f_iter)->get_name() <<
-      "(_msg.seqid, _itrans, _otrans);" << endl;
+      "process_" << (*f_iter)->get_name() << "(_msg.seqid, _itrans, _otrans);" << endl;
     indent_down();
     indent(f_service_) << "}";
   }
@@ -657,8 +707,7 @@
     " else {" << endl;
   indent_up();
   indent(f_service_) <<
-    "System.err.println" <<
-    "(\"Unknown function: '\" + _msg.name + \"'\");" << endl;
+    "System.err.println(\"Unknown function: '\" + _msg.name + \"'\");" << endl;
   indent_down();
   indent(f_service_) <<
     "}" << endl;
@@ -716,8 +765,7 @@
                                                  t_function* tfunction) {
   // Open function
   indent(f_service_) <<
-    "private void process_" << tfunction->get_name() <<
-    "(int seqid, TTransport _itrans, TTransport _otrans) throws TException" << endl;
+    "private void process_" << tfunction->get_name() << "(int seqid, TTransport _itrans, TTransport _otrans) throws TException" << endl;
   scope_up(f_service_);
 
   string argsname = tfunction->get_name() + "_args";
@@ -815,6 +863,9 @@
 
 /**
  * Deserializes a field of any type.
+ *
+ * @param tfield The field
+ * @param prefix The variable name or container for this field
  */
 void t_java_generator::generate_deserialize_field(ofstream& out,
                                                   t_field* tfield,
@@ -885,10 +936,7 @@
 }
 
 /**
- * Generates an unserializer for a variable. This makes two key assumptions,
- * first that there is a const char* variable named data that points to the
- * buffer for deserialization, and that there is a variable protocol which
- * is a reference to a TProtocol serialization object.
+ * Generates an unserializer for a struct, invokes read()
  */
 void t_java_generator::generate_deserialize_struct(ofstream& out,
                                                    t_struct* tstruct,
@@ -898,6 +946,9 @@
     indent() << prefix << ".read(_iprot, _itrans);" << endl;
 }
 
+/**
+ * Deserializes a container by reading its size and then iterating
+ */
 void t_java_generator::generate_deserialize_container(ofstream& out,
                                                       t_type* ttype,
                                                       string prefix) {
@@ -981,6 +1032,9 @@
     prefix << ".put(" << key << ", " << val << ");" << endl;
 }
 
+/**
+ * Deserializes a set element
+ */
 void t_java_generator::generate_deserialize_set_element(ofstream& out,
                                                         t_set* tset,
                                                         string prefix) {
@@ -996,6 +1050,9 @@
     prefix << ".add(" << elem << ");" << endl;
 }
 
+/**
+ * Deserializes a list element
+ */
 void t_java_generator::generate_deserialize_list_element(ofstream& out,
                                                          t_list* tlist,
                                                          string prefix) {
@@ -1102,6 +1159,12 @@
     indent() << prefix << ".write(_oprot, _otrans);" << endl;
 }
 
+/**
+ * Serializes a container by writing its size then the elements.
+ *
+ * @param ttype  The type of container
+ * @param prefix String prefix for fields
+ */
 void t_java_generator::generate_serialize_container(ofstream& out,
                                                     t_type* ttype,
                                                     string prefix) {
@@ -1174,7 +1237,6 @@
 
 /**
  * Serializes the members of a map.
- *
  */ 
 void t_java_generator::generate_serialize_map_element(ofstream& out,
                                                       t_map* tmap,
@@ -1182,7 +1244,6 @@
                                                       string map) {
   t_field kfield(tmap->get_key_type(), iter);
   generate_serialize_field(out, &kfield, "");
-
   t_field vfield(tmap->get_val_type(), map + ".get(" + iter + ")");
   generate_serialize_field(out, &vfield, "");
 }
@@ -1212,6 +1273,7 @@
  *
  * @param ttype The type
  * @param container Is the type going inside a container?
+ * @return Java type name, i.e. HashMap<Key,Value>
  */
 string t_java_generator::type_name(t_type* ttype, bool in_container) {
   // In Java typedefs are just resolved to their real type
@@ -1326,8 +1388,7 @@
                                             string prefix) {
   t_type* ttype = tfunction->get_returntype();
   std::string result =
-    type_name(ttype) + " " + prefix + tfunction->get_name() +
-    "(" + argument_list(tfunction->get_arglist()) + ") throws ";
+    type_name(ttype) + " " + prefix + tfunction->get_name() + "(" + argument_list(tfunction->get_arglist()) + ") throws ";
   t_struct* xs = tfunction->get_xceptions();
   const std::vector<t_field*>& xceptions = xs->get_members();
   vector<t_field*>::const_iterator x_iter;
@@ -1339,7 +1400,7 @@
 }
 
 /**
- * Renders a field list
+ * Renders a comma separated field list, with type names
  */
 string t_java_generator::argument_list(t_struct* tstruct) {
   string result = "";
diff --git a/compiler/cpp/src/generate/t_java_generator.h b/compiler/cpp/src/generate/t_java_generator.h
index 2c8e247..e528c34 100644
--- a/compiler/cpp/src/generate/t_java_generator.h
+++ b/compiler/cpp/src/generate/t_java_generator.h
@@ -19,14 +19,17 @@
 class t_java_generator : public t_oop_generator {
  public:
   t_java_generator() {}
-  ~t_java_generator() {}
 
-  /** Init and close methods */
+  /**
+   * Init and close methods
+   */
 
   void init_generator(t_program *tprogram);
   void close_generator(t_program *tprogram);
 
-  /** Program-level generation functions */
+  /**
+   * Program-level generation functions
+   */
 
   void generate_typedef (t_typedef*  ttypedef);
   void generate_enum    (t_enum*     tenum);
@@ -34,7 +37,9 @@
   void generate_xception(t_struct*   txception);
   void generate_service (t_service*  tservice);
 
-  /** Service-level generation functions */
+  /**
+   * Service-level generation functions
+   */
 
   void generate_java_struct(t_struct* tstruct, bool is_exception);
 
@@ -51,7 +56,9 @@
   void generate_service_server    (t_service* tservice);
   void generate_process_function  (t_service* tservice, t_function* tfunction);
 
-  /** Serialization constructs */
+  /**
+   * Serialization constructs
+   */
 
   void generate_deserialize_field        (std::ofstream& out,
                                           t_field*    tfield, 
@@ -102,7 +109,9 @@
                                           t_list*     tlist,
                                           std::string iter);
 
-  /** Helper rendering functions */
+  /**
+   * Helper rendering functions
+   */
 
   std::string java_package();
   std::string java_type_imports();
@@ -116,9 +125,11 @@
 
  private:
 
-  /** File streams */
-  std::string package_name_;
+  /**
+   * File streams
+   */
 
+  std::string package_name_;
   std::ofstream f_service_;
 };
 
diff --git a/compiler/cpp/src/generate/t_oop_generator.h b/compiler/cpp/src/generate/t_oop_generator.h
index 7b7ada1..fc47bc6 100644
--- a/compiler/cpp/src/generate/t_oop_generator.h
+++ b/compiler/cpp/src/generate/t_oop_generator.h
@@ -6,13 +6,16 @@
 
 /**
  * Class with utility methods shared across common object oriented languages.
+ * Specifically, most of this stuff is for C++/Java.
  *
  * @author Mark Slee <mcslee@facebook.com>
  */
 class t_oop_generator : public t_generator {
  public:
 
-  /** Scoping */
+  /**
+   * Scoping, using curly braces!
+   */
 
   void scope_up(std::ostream& out) {
     indent(out) << "{" << std::endl;
@@ -25,7 +28,8 @@
   }
 
   /**
-   * Generates a comment about this code being autogenerated.
+   * Generates a comment about this code being autogenerated, using C++ style
+   * comments, which are also fair game in Java / PHP, yay!
    *
    * @return C-style comment mentioning that this file is autogenerated.
    */
diff --git a/compiler/cpp/src/generate/t_php_generator.cc b/compiler/cpp/src/generate/t_php_generator.cc
index 3507a70..61df589 100644
--- a/compiler/cpp/src/generate/t_php_generator.cc
+++ b/compiler/cpp/src/generate/t_php_generator.cc
@@ -34,7 +34,7 @@
 }
 
 /**
- * Does nothing in PHP
+ * Close up (or down) some filez.
  */
 void t_php_generator::close_generator(t_program *tprogram) {
   // Close types file
@@ -101,6 +101,9 @@
   f_types_ << "}" << endl << endl;
 }
 
+/**
+ * Make a struct
+ */
 void t_php_generator::generate_struct(t_struct* tstruct) {
   generate_php_struct(tstruct, false);
 }
@@ -115,6 +118,9 @@
   generate_php_struct(txception, true);  
 }
 
+/**
+ * Structs can be normal or exceptions.
+ */
 void t_php_generator::generate_php_struct(t_struct* tstruct,
                                           bool is_exception) {
   generate_php_struct_definition(f_types_, tstruct, is_exception);
@@ -143,10 +149,6 @@
   indent_up();
 
   for (m_iter = members.begin(); m_iter != members.end(); ++m_iter) {
-    // This fills in default values, as opposed to nulls
-    //indent(out) <<
-    //"public " << declare_field(*m_iter, true) << endl;
-
     indent(out) <<
       "public $" << (*m_iter)->get_name() << " = null;" << endl;
   }
@@ -162,13 +164,21 @@
     endl;
 }
 
+/**
+ * Generates the read() method for a struct
+ */
 void t_php_generator::generate_php_struct_reader(ofstream& out,
                                                  t_struct* tstruct) {
   const vector<t_field*>& fields = tstruct->get_members();
   vector<t_field*>::const_iterator f_iter;
 
-  indent(out) <<
-    "public function read($iprot, $itrans) " << endl;
+  if (binary_inline_) {
+    indent(out) <<
+      "public function read($itrans) " << endl;
+  } else {
+    indent(out) <<
+      "public function read($iprot, $itrans) " << endl;
+  }
   scope_up(out);
 
   out <<
@@ -231,10 +241,13 @@
       }
       
       // In the default case we skip the field
-      out <<
-        indent() <<  "default:" << endl <<
-        indent() <<  "  $xfer += $iprot->skip($itrans, $ftype);" << endl <<
-        indent() <<  "  break;" << endl;
+      indent(out) <<  "default:" << endl;
+      if (binary_inline_) {
+        indent(out) <<  "  $xfer += TProtocol::skipBinary($itrans, $ftype);" << endl;
+      } else {
+        indent(out) <<  "  $xfer += $iprot->skip($itrans, $ftype);" << endl;
+      }
+      indent(out) <<  "  break;" << endl;
       
       scope_down(out);
       
@@ -260,6 +273,9 @@
     endl;
 }
 
+/**
+ * Generates the write() method for a struct
+ */
 void t_php_generator::generate_php_struct_writer(ofstream& out,
                                                  t_struct* tstruct) {
   string name = tstruct->get_name();
@@ -283,7 +299,11 @@
       "$xfer += $oprot->writeStructBegin($otrans, '" << name << "');" << endl;
   }
 
-  for (f_iter = fields.begin(); f_iter != fields.end(); ++f_iter) {
+  for (f_iter = fields.begin(); f_iter != fields.end(); ++f_iter) {   
+    out <<
+      indent() << "if ($this->" << (*f_iter)->get_name() << " !== null) {" << endl;
+    indent_up();
+
     // Write field header
     if (binary_inline_) {
       out <<
@@ -305,13 +325,16 @@
       indent(out) <<
         "$xfer += $oprot->writeFieldEnd($otrans);" << endl;
     }
+
+    indent_down();
+    indent(out) <<
+      "}" << endl;
   }
 
   if (binary_inline_) {
     out <<
       indent() << "$_output .= pack('c', TType::STOP);" << endl;
   } else {
-    // Write the struct map
     out <<
       indent() << "$xfer += $oprot->writeFieldStop($otrans);" << endl <<
       indent() << "$xfer += $oprot->writeStructEnd($otrans);" << endl;
@@ -347,7 +370,7 @@
   generate_service_interface(tservice);
   generate_service_client(tservice);
   generate_service_helpers(tservice);
-  // generate_service_server(tservice);
+  generate_service_processor(tservice);
   
   // Close service file
   f_service_ << "?>" << endl;
@@ -355,6 +378,203 @@
 }
 
 /**
+ * Generates a service server definition.
+ *
+ * @param tservice The service to generate a server for.
+ */
+void t_php_generator::generate_service_processor(t_service* tservice) {
+  // Generate the dispatch methods
+  vector<t_function*> functions = tservice->get_functions();
+  vector<t_function*>::iterator f_iter; 
+
+  // Generate the header portion
+  f_service_ <<
+    "class " << service_name_ << "Processor {" << endl;
+  indent_up();
+
+  f_service_ <<
+    indent() << "private $_handler = null;" << endl;
+  if (!binary_inline_) {
+    f_service_ << 
+      indent() << "private $_iprot = null;" << endl <<
+      indent() << "private $_oprot = null;" << endl <<
+      endl;
+  }
+
+  if (binary_inline_) {
+    f_service_ <<
+      indent() << "public function __construct($handler) {" << endl << 
+      indent() << "  $this->_handler = $handler;" << endl <<
+      indent() << "}" << endl <<
+      endl;
+  } else {
+    f_service_ <<
+      indent() << "public function __construct($handler, $iprot, $oprot=null) {" << endl << 
+      indent() << "  $this->_handler = $handler;" << endl <<
+      indent() << "  $this->_iprot = $iprot;" << endl <<
+      indent() << "  $this->_oprot = $oprot ? $oprot : $iprot;" << endl <<
+      indent() << "}" << endl <<
+      endl;
+  }
+
+  // Generate the server implementation
+  indent(f_service_) <<
+    "public function process($itrans, $otrans) {" << endl;
+  indent_up();
+
+  f_service_ <<
+    indent() << "$rseqid = 0;" << endl <<
+    indent() << "$fname = null;" << endl <<
+    indent() << "$mtype = 0;" << endl <<
+    endl;
+
+  if (binary_inline_) {
+    t_field ffname(g_program->get_string_type(), "fname");
+    t_field fmtype(g_program->get_byte_type(), "mtype");
+    t_field fseqid(g_program->get_i32_type(), "rseqid");
+    generate_deserialize_field(f_service_, &ffname, "", true);
+    generate_deserialize_field(f_service_, &fmtype, "", true);
+    generate_deserialize_field(f_service_, &fseqid, "", true);
+  } else {
+    f_service_ <<
+      indent() << "$this->_iprot->readMessageBegin($itrans, $fname, $mtype, $rseqid);" << endl;
+  }
+
+  // TODO(mcslee): validate message 
+
+  // HOT: check for method implementation
+  f_service_ <<
+    indent() << "$methodname = 'process_'.$fname;" << endl << 
+    indent() << "if (!method_exists($this, $methodname)) {" << endl <<
+    indent() << "  throw new Exception('Function '.$fname.' not implemented.');" << endl <<
+    indent() << "}" << endl <<
+    indent() << "$this->$methodname($rseqid, $itrans, $otrans);" << endl;
+  indent_down();
+  f_service_ <<
+    indent() << "}" << endl <<
+    endl;
+
+  // Generate the process subfunctions
+  for (f_iter = functions.begin(); f_iter != functions.end(); ++f_iter) {
+    generate_process_function(tservice, *f_iter);
+  }
+
+  indent_down();
+  f_service_ << "}" << endl;
+}
+
+/**
+ * Generates a process function definition.
+ *
+ * @param tfunction The function to write a dispatcher for
+ */
+void t_php_generator::generate_process_function(t_service* tservice,
+                                                t_function* tfunction) {
+  // Open function
+  indent(f_service_) <<
+    "private function process_" << tfunction->get_name() <<
+    "($seqid, $itrans, $otrans) {" << endl;
+  indent_up();
+
+  string argsname = service_name_ + "_" + tfunction->get_name() + "_args";
+  string resultname = service_name_ + "_" + tfunction->get_name() + "_result";
+
+  f_service_ <<
+    indent() << "$__args = new " << argsname << "();" << endl;
+  if (binary_inline_) {
+    f_service_ <<
+      indent() << "$__args->read($itrans);" << endl;
+  } else {
+    f_service_ <<
+      indent() << "$__args->read($this->_iprot, $itrans);" << endl <<
+      indent() << "$this->_iprot->readMessageEnd($itrans);" << endl;
+  }
+
+  t_struct* xs = tfunction->get_xceptions();
+  const std::vector<t_field*>& xceptions = xs->get_members();
+  vector<t_field*>::const_iterator x_iter;
+
+  // Declare result for non async function
+  if (!tfunction->is_async()) {
+    f_service_ <<
+      indent() << "$__result = new " << resultname << "();" << endl;
+  }
+
+  // Try block for a function with exceptions
+  if (xceptions.size() > 0) {
+    f_service_ <<
+      indent() << "try {" << endl;
+    indent_up();
+  }
+ 
+  // Generate the function call
+  t_struct* arg_struct = tfunction->get_arglist();
+  const std::vector<t_field*>& fields = arg_struct->get_members();
+  vector<t_field*>::const_iterator f_iter;
+
+  f_service_ << indent();
+  if (!tfunction->is_async() && !tfunction->get_returntype()->is_void()) {
+    f_service_ << "$__result->success = ";
+  }
+  f_service_ <<
+    "$this->_handler->" << tfunction->get_name() << "(";
+  bool first = true;
+  for (f_iter = fields.begin(); f_iter != fields.end(); ++f_iter) {
+    if (first) {
+      first = false;
+    } else {
+      f_service_ << ", ";
+    }
+    f_service_ << "$__args->" << (*f_iter)->get_name();
+  }
+  f_service_ << ");" << endl;
+
+  if (!tfunction->is_async() && xceptions.size() > 0) {
+    indent_down();
+    for (x_iter = xceptions.begin(); x_iter != xceptions.end(); ++x_iter) {
+      f_service_ <<
+        indent() << "} catch (" << (*x_iter)->get_type()->get_name() << " $" << (*x_iter)->get_name() << ") {" << endl;
+      if (!tfunction->is_async()) {
+        indent_up();
+        f_service_ <<
+          indent() << "$__result->" << (*x_iter)->get_name() << " = $" << (*x_iter)->get_name() << ";" << endl;
+        indent_down();
+        f_service_ << indent();
+      }
+    }
+    f_service_ << "}" << endl;
+  }
+
+  // Shortcut out here for async functions
+  if (tfunction->is_async()) {
+    f_service_ <<
+      indent() << "return;" << endl;
+    indent_down();
+    f_service_ << endl;
+    return;
+  }
+
+  // Serialize the request header
+  if (binary_inline_) {
+    f_service_ <<
+      indent() << "$_output = '';" << endl <<
+      indent() << "$_output .= pack('N', strlen('" << tfunction->get_name() << "'));" << endl <<
+      indent() << "$_output .= '" << tfunction->get_name() << "';" << endl <<
+      indent() << "$_output .= pack('cN', TMessageType::REPLY, $seqid);" << endl;
+  } else {
+    f_service_ <<
+      indent() << "$this->_oprot->writeMessageBegin($otrans, '" << tfunction->get_name() << "', TMessageType::REPLY, $seqid);" << endl <<
+      indent() << "$__result->write($this->_oprot, $otrans);" << endl <<
+      indent() << "$otrans->flush();" << endl;
+  }
+
+  // Close function
+  indent_down();
+  f_service_ <<
+    indent() << "}" << endl;
+}
+
+/**
  * Generates helper functions for a service.
  *
  * @param tservice The service to generate a header definition for
@@ -592,8 +812,14 @@
       // TODO(mcslee): Validate message reply here
 
       f_service_ <<
-        indent() << "$__result = new " << resultname << "();" << endl <<
-        indent() << "$__result->read($this->_iprot, $this->_otrans);" << endl;
+        indent() << "$__result = new " << resultname << "();" << endl;
+      if (binary_inline_) {
+        f_service_ <<
+          indent() << "$__result->read($this->_otrans);" << endl;
+      } else {
+        f_service_ <<
+          indent() << "$__result->read($this->_iprot, $this->_otrans);" << endl;
+      }
 
       if (!binary_inline_) {
         f_service_ <<
@@ -786,7 +1012,7 @@
 
   } else {
     printf("DO NOT KNOW HOW TO DESERIALIZE FIELD '%s' TYPE '%s'\n",
-           tfield->get_name().c_str(), type_name(type).c_str());
+           tfield->get_name().c_str(), type->get_name().c_str());
   }  
 }
 
@@ -800,8 +1026,14 @@
                                                   t_struct* tstruct,
                                                   string prefix) {
   out <<
-    indent() << "$" << prefix << " = new " << tstruct->get_name() << "();" << endl <<
-    indent() << "$xfer += $" << prefix << "->read($iprot, $itrans);" << endl;
+    indent() << "$" << prefix << " = new " << tstruct->get_name() << "();" << endl;
+  if (binary_inline_) {
+    out <<
+      indent() << "$xfer += $" << prefix << "->read($itrans);" << endl;
+  } else {
+    out <<
+      indent() << "$xfer += $" << prefix << "->read($iprot, $itrans);" << endl;
+  }
 }
 
 void t_php_generator::generate_deserialize_container(ofstream &out,
@@ -1067,7 +1299,7 @@
     printf("DO NOT KNOW HOW TO SERIALIZE FIELD '%s%s' TYPE '%s'\n",
            prefix.c_str(),
            tfield->get_name().c_str(),
-           type_name(type).c_str());
+           type->get_name().c_str());
   }
 }
 
@@ -1089,6 +1321,9 @@
   }
 }
 
+/**
+ * Writes out a container
+ */
 void t_php_generator::generate_serialize_container(ofstream &out,
                                                    t_type* ttype,
                                                    string prefix) {
@@ -1214,65 +1449,6 @@
 }
 
 /**
- * Returns a Java type name
- *
- * @param ttype The type
- */
-string t_php_generator::type_name(t_type* ttype) {
-  // In Java typedefs are just resolved to their real type
-  while (ttype->is_typedef()) {
-    ttype = ((t_typedef*)ttype)->get_type();
-  }
-
-  if (ttype->is_base_type()) {
-    return base_type_name(((t_base_type*)ttype)->get_base());
-  } else if (ttype->is_enum()) {
-    return "Int32";
-  } else if (ttype->is_map()) {
-    t_map* tmap = (t_map*) ttype;
-    return "HashMap<" +
-      type_name(tmap->get_key_type()) + "," +
-      type_name(tmap->get_val_type()) + ">";
-  } else if (ttype->is_set()) {
-    t_set* tset = (t_set*) ttype;
-    return "HashSet<" + type_name(tset->get_elem_type()) + ">";
-  } else if (ttype->is_list()) {
-    t_list* tlist = (t_list*) ttype;
-    return "ArrayList<" + type_name(tlist->get_elem_type()) + ">";
-  } else {
-    return ttype->get_name();
-  }
-}
-
-/**
- * Returns the C++ type that corresponds to the thrift type.
- *
- * @param tbase The base type
- */
-string t_php_generator::base_type_name(t_base_type::t_base tbase) {
-  switch (tbase) {
-  case t_base_type::TYPE_VOID:
-    return "void";
-  case t_base_type::TYPE_STRING:
-    return "TString";
-  case t_base_type::TYPE_BOOL:
-    return "bool";
-  case t_base_type::TYPE_BYTE:
-    return "UInt8";
-  case t_base_type::TYPE_I16:
-    return "Int16";
-  case t_base_type::TYPE_I32:
-    return "Int32";
-  case t_base_type::TYPE_I64:
-    return "Int64";
-  case t_base_type::TYPE_DOUBLE:
-    return "Double";
-  default:
-    throw "compiler error: no PHP name for base type " + tbase;
-  }
-}
-
-/**
  * Declares a field, which may include initialization as necessary.
  *
  * @param ttype The type
diff --git a/compiler/cpp/src/generate/t_php_generator.h b/compiler/cpp/src/generate/t_php_generator.h
index 0cc837f..0e6869b 100644
--- a/compiler/cpp/src/generate/t_php_generator.h
+++ b/compiler/cpp/src/generate/t_php_generator.h
@@ -9,7 +9,7 @@
 #include "t_oop_generator.h"
 
 /**
- * Java code generator.
+ * PHP code generator.
  *
  * @author Mark Slee <mcslee@facebook.com>
  */
@@ -24,14 +24,16 @@
     }
   }
   
-  ~t_php_generator() {}
-
-  /** Init and close methods */
+  /**
+   * Init and close methods
+   */
 
   void init_generator(t_program *tprogram);
   void close_generator(t_program *tprogram);
 
-  /** Program-level generation functions */
+  /**
+   * Program-level generation functions
+   */
 
   void generate_typedef  (t_typedef*  ttypedef);
   void generate_enum     (t_enum*     tenum);
@@ -39,21 +41,29 @@
   void generate_xception (t_struct*   txception);
   void generate_service  (t_service*  tservice);
 
+  /**
+   * Structs!
+   */
 
   void generate_php_struct(t_struct* tstruct, bool is_exception);
   void generate_php_struct_definition(std::ofstream& out, t_struct* tstruct, bool is_xception=false);
   void generate_php_struct_reader(std::ofstream& out, t_struct* tstruct);
   void generate_php_struct_writer(std::ofstream& out, t_struct* tstruct);
+  void generate_php_function_helpers(t_function* tfunction);
 
-  void generate_php_function_helpers  (t_function* tfunction);
+  /**
+   * Service-level generation functions
+   */
 
-  /** Service-level generation functions */
-
-  void generate_service_helpers(t_service*  tservice);
+  void generate_service_helpers   (t_service* tservice);
   void generate_service_interface (t_service* tservice);
   void generate_service_client    (t_service* tservice);
+  void generate_service_processor (t_service* tservice);
+  void generate_process_function  (t_service* tservice, t_function* tfunction);
 
-  /** Serialization constructs */
+  /**
+   * Serialization constructs
+   */
 
   void generate_deserialize_field        (std::ofstream &out,
                                           t_field*    tfield, 
@@ -105,11 +115,11 @@
                                           t_list*     tlist,
                                           std::string iter);
 
-  /** Helper rendering functions */
+  /**
+   * Helper rendering functions
+   */
 
   std::string php_includes();
-  std::string type_name(t_type* ttype);
-  std::string base_type_name(t_base_type::t_base tbase);
   std::string declare_field(t_field* tfield, bool init=false, bool obj=false);
   std::string function_signature(t_function* tfunction, std::string prefix="");
   std::string argument_list(t_struct* tstruct);
@@ -117,15 +127,21 @@
 
  private:
 
-  /** File streams */
+  /**
+   * Output directory
+   */
   char* T_PHP_DIR;
 
+  /**
+   * File streams
+   */
   std::ofstream f_types_;
   std::ofstream f_helpers_;
   std::ofstream f_service_;
 
-  /** Generate protocol-independent template? Or Binary inline code? */
-
+  /**
+   * Generate protocol-independent template? Or Binary inline code?
+   */
   bool binary_inline_;
 
 };
diff --git a/compiler/cpp/src/generate/t_py_generator.cc b/compiler/cpp/src/generate/t_py_generator.cc
index b666e51..9838df6 100644
--- a/compiler/cpp/src/generate/t_py_generator.cc
+++ b/compiler/cpp/src/generate/t_py_generator.cc
@@ -40,7 +40,7 @@
 }
 
 /**
- * Prints standard java imports
+ * Prints standard thrift imports
  */
 string t_py_generator::py_imports() {
   return
@@ -48,7 +48,7 @@
 }
 
 /**
- * Does nothing in Python
+ * Closes the type files
  */
 void t_py_generator::close_generator(t_program *tprogram) {
   // Close types file
@@ -63,8 +63,8 @@
 void t_py_generator::generate_typedef(t_typedef* ttypedef) {}
 
 /**
- * Generates code for an enumerated type. Since define is expensive to lookup
- * in PHP, we use a global array for this.
+ * Generates code for an enumerated type. Done using a class to scope
+ * the values.
  *
  * @param tenum The enumeration
  */
@@ -91,6 +91,9 @@
   f_types_ << endl;
 }
 
+/**
+ * Generates a python struct
+ */
 void t_py_generator::generate_struct(t_struct* tstruct) {
   generate_py_struct(tstruct, false);
 }
@@ -105,6 +108,9 @@
   generate_py_struct(txception, true);  
 }
 
+/**
+ * Generates a python struct
+ */
 void t_py_generator::generate_py_struct(t_struct* tstruct,
                                         bool is_exception) {
   generate_py_struct_definition(f_types_, tstruct, is_exception);
@@ -160,12 +166,10 @@
   out << endl;
 
   generate_py_struct_reader(out, tstruct);
-  if (is_result) {
-    generate_py_struct_result_writer(out, tstruct);
-  } else {
-    generate_py_struct_writer(out, tstruct);
-  }
+  generate_py_struct_writer(out, tstruct);
 
+  // Printing utilities so that on the command line thrift
+  // structs look pretty like dictionaries
   out <<
     indent() << "def __str__(self): " << endl <<
     indent() << "  return str(self.__dict__)" << endl <<
@@ -177,6 +181,9 @@
   indent_down();
 }
 
+/**
+ * Generates the read method for a struct
+ */
 void t_py_generator::generate_py_struct_reader(ofstream& out,
                                                 t_struct* tstruct) {
   const vector<t_field*>& fields = tstruct->get_members();
@@ -259,45 +266,6 @@
   for (f_iter = fields.begin(); f_iter != fields.end(); ++f_iter) {
     // Write field header
     indent(out) <<
-      "oprot.writeFieldBegin(otrans, " <<
-      "'" << (*f_iter)->get_name() << "', " <<
-      type_to_enum((*f_iter)->get_type()) << ", " <<
-      (*f_iter)->get_key() << ")" << endl;
-
-    // Write field contents
-    generate_serialize_field(out, *f_iter, "self.");
-
-    // Write field closer
-    indent(out) <<
-      "oprot.writeFieldEnd(otrans)" << endl;
-  }
-
-  // Write the struct map
-  out <<
-    indent() << "oprot.writeFieldStop(otrans)" << endl <<
-    indent() << "oprot.writeStructEnd(otrans)" << endl;
-
-  indent_down();
-  out <<
-    endl;
-}
-
-void t_py_generator::generate_py_struct_result_writer(ofstream& out,
-                                                      t_struct* tstruct) {
-  string name = tstruct->get_name();
-  const vector<t_field*>& fields = tstruct->get_members();
-  vector<t_field*>::const_iterator f_iter;
-
-  indent(out) <<
-    "def write(self, oprot, otrans):" << endl;
-  indent_up();
-  
-  indent(out) <<
-    "oprot.writeStructBegin(otrans, '" << name << "')" << endl;
-
-  for (f_iter = fields.begin(); f_iter != fields.end(); ++f_iter) {
-    // Write field header
-    indent(out) <<
       "if self." << (*f_iter)->get_name() << " != None:" << endl;
     indent_up();
     indent(out) <<
@@ -393,7 +361,6 @@
   for (f_iter = fields.begin(); f_iter != fields.end(); ++f_iter) {
     result.append(*f_iter);
   }
-
   generate_py_struct_definition(f_service_, &result, false, true);
 }
 
@@ -525,7 +492,7 @@
       f_service_ <<
         indent() << "(fname, mtype, rseqid) = self.__iprot.readMessageBegin(self.__itrans)" << endl;
 
-      // TODO(mcslee): Validate message reply here
+      // TODO(mcslee): Validate message reply here, seq ids etc.
 
       f_service_ <<
         indent() << "__result = " << resultname << "()" << endl <<
@@ -560,8 +527,7 @@
 
     // Close function
     indent_down();
-    f_service_ << endl;
-    
+    f_service_ << endl;   
   }
 
   indent_down();
@@ -695,7 +661,7 @@
   // Close service file
   f_remote.close();
   
-  // Make file executable
+  // Make file executable, love that bitwise OR action
   chmod(f_remote_name.c_str(),
         S_IRUSR |
         S_IWUSR |
@@ -752,34 +718,12 @@
 
   // TODO(mcslee): validate message
 
-  // HOT: dictionary lookup
+  // HOT: dictionary function lookup
   f_service_ <<
     indent() << "if name not in self.__processMap:" << endl <<
     indent() << "  print 'Unknown function %s' % (name)" << endl <<
     indent() << "else:" << endl <<
     indent() << "  self.__processMap[name](self, seqid, itrans, otrans)" << endl;
-  // DEPRECATED: if else if bullshit
-  /*
-  bool first = true;
-  for (f_iter = functions.begin(); f_iter != functions.end(); ++f_iter) {
-    if (!first) {
-      f_service_ << indent() << "el";
-    } else {
-      f_service_ << indent();
-      first = false;
-    }
-    f_service_ <<
-      "if name == \"" << (*f_iter)->get_name() << "\":" << endl;
-    indent_up();
-    indent(f_service_) <<
-      "self.process_" << (*f_iter)->get_name() << "(seqid, itrans, otrans)" << endl;
-    indent_down();
-  }
-  f_service_ <<
-    indent() << "else:" << endl <<
-    indent() << "  print 'Unknown function %s' % (name)" << endl;
-  f_service_ << endl;
-  */  
 
   // Read end of args field, the T_STOP, and the struct close
   f_service_ <<
@@ -961,15 +905,12 @@
 
   } else {
     printf("DO NOT KNOW HOW TO DESERIALIZE FIELD '%s' TYPE '%s'\n",
-           tfield->get_name().c_str(), type_name(type).c_str());
+           tfield->get_name().c_str(), type->get_name().c_str());
   }  
 }
 
 /**
- * Generates an unserializer for a variable. This makes two key assumptions,
- * first that there is a const char* variable named data that points to the
- * buffer for deserialization, and that there is a variable protocol which
- * is a reference to a TProtocol serialization object.
+ * Generates an unserializer for a struct, calling read()
  */
 void t_py_generator::generate_deserialize_struct(ofstream &out,
                                                   t_struct* tstruct,
@@ -979,6 +920,10 @@
     indent() << prefix << ".read(iprot, itrans)" << endl;
 }
 
+/**
+ * Serialize a container by writing out the header followed by
+ * data and then a footer.
+ */
 void t_py_generator::generate_deserialize_container(ofstream &out,
                                                     t_type* ttype,
                                                     string prefix) {
@@ -1060,34 +1005,30 @@
     prefix << "[" << key << "] = " << val << endl;
 }
 
+/**
+ * Write a set element
+ */
 void t_py_generator::generate_deserialize_set_element(ofstream &out,
                                                        t_set* tset,
                                                        string prefix) {
   string elem = tmp("_elem");
   t_field felem(tset->get_elem_type(), elem);
 
-  /*
-  indent(out) <<
-    "$" << elem << " = null;" << endl;
-  */
-
   generate_deserialize_field(out, &felem);
 
   indent(out) <<
     prefix << ".append(" << elem << ")" << endl;
 }
 
+/**
+ * Write a list element
+ */
 void t_py_generator::generate_deserialize_list_element(ofstream &out,
                                                         t_list* tlist,
                                                         string prefix) {
   string elem = tmp("_elem");
   t_field felem(tlist->get_elem_type(), elem);
 
-  /*
-  indent(out) <<
-    "$" << elem << " = null;" << endl;
-  */
-
   generate_deserialize_field(out, &felem);
 
   indent(out) <<
@@ -1169,7 +1110,7 @@
     printf("DO NOT KNOW HOW TO SERIALIZE FIELD '%s%s' TYPE '%s'\n",
            prefix.c_str(),
            tfield->get_name().c_str(),
-           type_name(type).c_str());
+           type->get_name().c_str());
   }
 }
 
@@ -1180,15 +1121,15 @@
  * @param prefix  String prefix to attach to all fields
  */
 void t_py_generator::generate_serialize_struct(ofstream &out,
-                                                t_struct* tstruct,
-                                                string prefix) {
+                                               t_struct* tstruct,
+                                               string prefix) {
   indent(out) <<
     prefix << ".write(oprot, otrans)" << endl;
 }
 
 void t_py_generator::generate_serialize_container(ofstream &out,
-                                                   t_type* ttype,
-                                                   string prefix) {
+                                                  t_type* ttype,
+                                                  string prefix) {
   if (ttype->is_map()) {
     indent(out) <<
       "oprot.writeMapBegin(otrans, " <<
@@ -1207,31 +1148,30 @@
       "len(" << prefix << "))" << endl;
   }
 
-    if (ttype->is_map()) {
-      string kiter = tmp("_kiter");
-      string viter = tmp("_viter");
-      indent(out) << 
-        "for " << kiter << "," << viter << " in " << prefix << ":" << endl;
-      indent_up();
-      generate_serialize_map_element(out, (t_map*)ttype, kiter, viter);
-      indent_down();
-    } else if (ttype->is_set()) {
-      string iter = tmp("_iter");
-      indent(out) << 
-        "for " << iter << " in " << prefix << ":" << endl;
-      indent_up();
-      generate_serialize_set_element(out, (t_set*)ttype, iter);
-      indent_down();
-    } else if (ttype->is_list()) {
-      string iter = tmp("_iter");
-      indent(out) << 
-        "for " << iter << " in " << prefix << ":" << endl;
-      indent_up();
-      generate_serialize_list_element(out, (t_list*)ttype, iter);
-      indent_down();
-    }
+  if (ttype->is_map()) {
+    string kiter = tmp("_kiter");
+    string viter = tmp("_viter");
+    indent(out) << 
+      "for " << kiter << "," << viter << " in " << prefix << ":" << endl;
+    indent_up();
+    generate_serialize_map_element(out, (t_map*)ttype, kiter, viter);
+    indent_down();
+  } else if (ttype->is_set()) {
+    string iter = tmp("_iter");
+    indent(out) << 
+      "for " << iter << " in " << prefix << ":" << endl;
+    indent_up();
+    generate_serialize_set_element(out, (t_set*)ttype, iter);
+    indent_down();
+  } else if (ttype->is_list()) {
+    string iter = tmp("_iter");
+    indent(out) << 
+      "for " << iter << " in " << prefix << ":" << endl;
+    indent_up();
+    generate_serialize_list_element(out, (t_list*)ttype, iter);
+    indent_down();
+  }
     
-
   if (ttype->is_map()) {
     indent(out) <<
       "oprot.writeMapEnd(otrans)" << endl;
@@ -1280,65 +1220,6 @@
 }
 
 /**
- * Returns a Java type name
- *
- * @param ttype The type
- */
-string t_py_generator::type_name(t_type* ttype) {
-  // In Java typedefs are just resolved to their real type
-  while (ttype->is_typedef()) {
-    ttype = ((t_typedef*)ttype)->get_type();
-  }
-
-  if (ttype->is_base_type()) {
-    return base_type_name(((t_base_type*)ttype)->get_base());
-  } else if (ttype->is_enum()) {
-    return "int";
-  } else if (ttype->is_map()) {
-    t_map* tmap = (t_map*) ttype;
-    return "map<" +
-      type_name(tmap->get_key_type()) + "," +
-      type_name(tmap->get_val_type()) + ">";
-  } else if (ttype->is_set()) {
-    t_set* tset = (t_set*) ttype;
-    return "set<" + type_name(tset->get_elem_type()) + ">";
-  } else if (ttype->is_list()) {
-    t_list* tlist = (t_list*) ttype;
-    return "list<" + type_name(tlist->get_elem_type()) + ">";
-  } else {
-    return ttype->get_name();
-  }
-}
-
-/**
- * Returns the C++ type that corresponds to the thrift type.
- *
- * @param tbase The base type
- */
-string t_py_generator::base_type_name(t_base_type::t_base tbase) {
-  switch (tbase) {
-  case t_base_type::TYPE_VOID:
-    return "void";
-  case t_base_type::TYPE_STRING:
-    return "TString";
-  case t_base_type::TYPE_BOOL:
-    return "bool";
-  case t_base_type::TYPE_BYTE:
-    return "UInt8";
-  case t_base_type::TYPE_I16:
-    return "Int16";
-  case t_base_type::TYPE_I32:
-    return "Int32";
-  case t_base_type::TYPE_I64:
-    return "Int64";
-  case t_base_type::TYPE_DOUBLE:
-    return "Double";
-  default:
-    throw "compiler error: no PHP name for base type " + tbase;
-  }
-}
-
-/**
  * Declares a field, which may include initialization as necessary.
  *
  * @param ttype The type
@@ -1400,6 +1281,7 @@
  */
 string t_py_generator::function_signature(t_function* tfunction,
                                            string prefix) {
+  // TODO(mcslee): Nitpicky, no ',' if argument_list is empty
   return
     prefix + tfunction->get_name() +
     "(self, " + argument_list(tfunction->get_arglist()) + ")";
@@ -1426,7 +1308,7 @@
 }
 
 /**
- * Converts the parse type to a C++ enum string for the given type.
+ * Converts the parse type to a Python tyoe
  */
 string t_py_generator::type_to_enum(t_type* type) {
   while (type->is_typedef()) {
diff --git a/compiler/cpp/src/generate/t_py_generator.h b/compiler/cpp/src/generate/t_py_generator.h
index 2f6c6ce..238d10e 100644
--- a/compiler/cpp/src/generate/t_py_generator.h
+++ b/compiler/cpp/src/generate/t_py_generator.h
@@ -18,14 +18,17 @@
 class t_py_generator : public t_oop_generator {
  public:
   t_py_generator() {}
-  ~t_py_generator() {}
 
-  /** Init and close methods */
+  /**
+   * Init and close methods
+   */
 
   void init_generator(t_program *tprogram);
   void close_generator(t_program *tprogram);
 
-  /** Program-level generation functions */
+  /**
+   * Program-level generation functions
+   */
 
   void generate_typedef  (t_typedef*  ttypedef);
   void generate_enum     (t_enum*     tenum);
@@ -33,27 +36,30 @@
   void generate_xception (t_struct*   txception);
   void generate_service  (t_service*  tservice);
 
+  /**
+   * Struct generation code
+   */
 
   void generate_py_struct(t_struct* tstruct, bool is_exception);
   void generate_py_struct_definition(std::ofstream& out, t_struct* tstruct, bool is_xception=false, bool is_result=false);
   void generate_py_struct_reader(std::ofstream& out, t_struct* tstruct);
-  void generate_py_struct_result_writer(std::ofstream& out, t_struct* tstruct);
   void generate_py_struct_writer(std::ofstream& out, t_struct* tstruct);
+  void generate_py_function_helpers(t_function* tfunction);
 
-  void generate_py_function_helpers  (t_function* tfunction);
+  /**
+   * Service-level generation functions
+   */
 
-  /** Service-level generation functions */
-
-  void generate_service_helpers(t_service*  tservice);
-  void generate_service_interface(t_service* tservice);
-  void generate_service_client(t_service* tservice);
-  void generate_service_remote(t_service* tservice);
-
-
+  void generate_service_helpers   (t_service*  tservice);
+  void generate_service_interface (t_service* tservice);
+  void generate_service_client    (t_service* tservice);
+  void generate_service_remote    (t_service* tservice);
   void generate_service_server    (t_service* tservice);
   void generate_process_function  (t_service* tservice, t_function* tfunction);
 
-  /** Serialization constructs */
+  /**
+   * Serialization constructs
+   */
 
   void generate_deserialize_field        (std::ofstream &out,
                                           t_field*    tfield, 
@@ -105,12 +111,12 @@
                                           t_list*     tlist,
                                           std::string iter);
 
-  /** Helper rendering functions */
+  /**
+   * Helper rendering functions
+   */
 
   std::string py_autogen_comment();
   std::string py_imports();
-  std::string type_name(t_type* ttype);
-  std::string base_type_name(t_base_type::t_base tbase);
   std::string declare_field(t_field* tfield, bool init=false, bool obj=false);
   std::string function_signature(t_function* tfunction, std::string prefix="");
   std::string argument_list(t_struct* tstruct);
@@ -118,7 +124,10 @@
 
  private:
 
-  /** File streams */
+  /**
+   * File streams
+   */
+
   std::ofstream f_types_;
   std::ofstream f_service_;
 
diff --git a/compiler/cpp/src/globals.h b/compiler/cpp/src/globals.h
index db7e392..7385565 100644
--- a/compiler/cpp/src/globals.h
+++ b/compiler/cpp/src/globals.h
@@ -3,13 +3,20 @@
 
 class t_program;
 
-/** Global variable: the master program parse tree */
+/**
+ * The master program parse tree. This is accessed from within the parser code
+ * to build up the program elements.
+ */
 extern t_program* g_program;
 
-/** Global debug state */
+/**
+ * Global debug state
+ */
 extern int g_debug;
 
-/** Global time string */
+/**
+ * Global time string, used in formatting error messages etc.
+ */
 extern char* g_time_str;
 
 #endif
diff --git a/compiler/cpp/src/main.cc b/compiler/cpp/src/main.cc
index 5cbf53e..9a8e29e 100644
--- a/compiler/cpp/src/main.cc
+++ b/compiler/cpp/src/main.cc
@@ -3,7 +3,8 @@
  *
  * This file contains the main compiler engine for Thrift, which invokes the
  * scanner/parser to build the thrift object tree. The interface generation
- * code for each language lives in a file by the language name.
+ * code for each language lives in a file by the language name under the
+ * generate/ folder, and all parse structures live in parse/
  *
  * @author Mark Slee <mcslee@facebook.com>
  */
@@ -13,7 +14,7 @@
 #include <stdarg.h>
 #include <string>
 
-// Careful: must include globals first
+// Careful: must include globals first here for extern/global definitions
 #include "globals.h"
 
 #include "main.h"
@@ -25,16 +26,21 @@
 
 using namespace std;
 
-/** Global program tree */
+/**
+ * Global program tree
+ */
 t_program* g_program;
 
-/** Global debug state */
+/**
+ * Global debug state
+ */
 int g_debug = 0;
 
-/** Global time string */
+/**
+ * Global time string
+ */
 char* g_time_str;
 
-
 /**
  * Report an error to the user. This is called yyerror for historical
  * reasons (lex and yacc expect the error reporting routine to be called
@@ -106,15 +112,17 @@
 }
 
 /**
- * Parse it up.. then spit it back out, in pretty much every language
+ * Parse it up.. then spit it back out, in pretty much every language. Alright
+ * not that many languages, but the cool ones that we care about.
  */
 int main(int argc, char** argv) {
   int i;
+
   bool gen_cpp = false;
   bool gen_java = false;
   bool gen_py = false;
   bool gen_php = false;
-  bool php_inline = false;
+  bool gen_phpi = false;
 
   // Setup time string
   time_t now = time(NULL);
@@ -125,6 +133,7 @@
     usage();
   }
 
+  // Hacky parameter handling... I didn't feel like using a library sorry!
   for (i = 1; i < argc-1; i++) {
     char* arg;
     arg = strtok(argv[i], " ");
@@ -137,10 +146,8 @@
         gen_java = true;
       } else if (strcmp(arg, "--php") == 0) {
         gen_php = true;
-        php_inline = false;
       } else if (strcmp(arg, "--phpi") == 0) {
-        gen_php = true;
-        php_inline = true;
+        gen_phpi = true;
       } else if (strcmp(arg, "--py") == 0) {
         gen_py = true;
       } else {
@@ -153,7 +160,7 @@
     }
   }
   
-  if (!gen_cpp && !gen_java && !gen_php && !gen_py) {
+  if (!gen_cpp && !gen_java && !gen_php && !gen_phpi && !gen_py) {
     fprintf(stderr, "!!! No output language(s) specified\n\n");
     usage();
   }
@@ -176,9 +183,10 @@
     name = name.substr(0, dot);
   }
   
-  // Parse it
+  // Instance of the global parse tree
   g_program = new t_program(name);
 
+  // Parse it!
   if (yyparse() != 0) {
     failure("Parser error.");
   }
@@ -198,16 +206,23 @@
     }
 
     if (gen_php) {
-      t_php_generator* php = new t_php_generator(php_inline);
+      t_php_generator* php = new t_php_generator(false);
       php->generate_program(g_program);
       delete php;
     }
 
+    if (gen_phpi) {
+      t_php_generator* phpi = new t_php_generator(true);
+      phpi->generate_program(g_program);
+      delete phpi;
+    }
+
     if (gen_py) {
       t_py_generator* py = new t_py_generator();
       py->generate_program(g_program);
       delete py;
     }
+
   } catch (string s) {
     printf("Error: %s\n", s.c_str());
   } catch (const char* exc) {
diff --git a/compiler/cpp/src/main.h b/compiler/cpp/src/main.h
index b26142c..1281cc9 100644
--- a/compiler/cpp/src/main.h
+++ b/compiler/cpp/src/main.h
@@ -1,17 +1,27 @@
 #ifndef T_MAIN_H
 #define T_MAIN_H
 
-/** Defined in the flex library */
-extern int   yylex(void);
-extern int   yyparse(void);
+/**
+ * Defined in the flex library
+ */
 
-/** Expected to be defined by Flex/Bison */
-extern void  yyerror(char* fmt, ...);
+extern int yylex(void);
+extern int yyparse(void);
 
-/** Parse debug output */
-extern void  pdebug(char* fmt, ...);
+/**
+ * Expected to be defined by Flex/Bison
+ */
+extern void yyerror(char* fmt, ...);
 
-/** Flex utilities */
+/**
+ * Parse debugging output, used to print warnings etc.
+ */
+extern void pdebug(char* fmt, ...);
+
+/**
+ * Flex utilities
+ */
+
 extern int   yylineno;
 extern char  yytext[];
 extern FILE* yyin;
diff --git a/compiler/cpp/src/parse/t_base_type.h b/compiler/cpp/src/parse/t_base_type.h
index 0e97207..837caa5 100644
--- a/compiler/cpp/src/parse/t_base_type.h
+++ b/compiler/cpp/src/parse/t_base_type.h
@@ -4,7 +4,8 @@
 #include "t_type.h"
 
 /**
- * A thrift base type, which must be one of the defined enumerated types.
+ * A thrift base type, which must be one of the defined enumerated types inside
+ * this definition.
  *
  * @author Mark Slee <mcslee@facebook.com>
  */
@@ -25,11 +26,20 @@
   };
 
   t_base_type(std::string name, t_base base) :
-    t_type(name), base_(base) {}
+    t_type(name),
+    base_(base) {}
     
-  t_base get_base() const { return base_; }
-  bool is_void() const { return base_ == TYPE_VOID; }
-  bool is_base_type() const { return true; }
+  t_base get_base() const {
+    return base_;
+  }
+
+  bool is_void() const {
+    return base_ == TYPE_VOID;
+  }
+
+  bool is_base_type() const {
+    return true;
+  }
     
  private:
   t_base base_;
diff --git a/compiler/cpp/src/parse/t_constant.h b/compiler/cpp/src/parse/t_constant.h
index 1f3c868..f502f75 100644
--- a/compiler/cpp/src/parse/t_constant.h
+++ b/compiler/cpp/src/parse/t_constant.h
@@ -3,19 +3,38 @@
 
 #include <string>
 
+/**
+ * A constant. These are used inside of enum definitions. Constants are just
+ * symbol identifiers that may or may not have an explicit value associated
+ * with them.
+ *
+ * @author Mark Slee <mcslee@facebook.com>
+ */
 class t_constant {
  public:
   t_constant(std::string name) :
-    name_(name), has_value_(false), value_(0) {}
+    name_(name),
+    has_value_(false),
+    value_(0) {}
 
   t_constant(std::string name, int value) :
-    name_(name), has_value_(true), value_(value) {}
+    name_(name),
+    has_value_(true),
+    value_(value) {}
 
   ~t_constant() {}
 
-  const std::string& get_name() { return name_; }
-  bool has_value() { return has_value_; }
-  int get_value() { return value_; }
+  const std::string& get_name() {
+    return name_;
+  }
+  
+  bool has_value() {
+    return has_value_;
+  }
+  
+  int get_value() {
+    return value_;
+  }
   
  private:
   std::string name_;
diff --git a/compiler/cpp/src/parse/t_enum.h b/compiler/cpp/src/parse/t_enum.h
index f089150..4b6fe36 100644
--- a/compiler/cpp/src/parse/t_enum.h
+++ b/compiler/cpp/src/parse/t_enum.h
@@ -4,16 +4,30 @@
 #include "t_constant.h"
 #include <vector>
 
+/**
+ * An enumerated type. A list of t_constant objects with a name for the type.
+ *
+ * @author Mark Slee <mcslee@facebook.com>
+ */
 class t_enum : public t_type {
  public:
   t_enum() {}
-  ~t_enum() {}
 
-  void set_name(std::string name) { name_ = name; }
-  void append(t_constant* constant) { constants_.push_back(constant); }
+  void set_name(std::string name) {
+    name_ = name;
+  }
+  
+  void append(t_constant* constant) {
+    constants_.push_back(constant);
+  }
 
-  const std::vector<t_constant*>& get_constants() { return constants_; }
-  bool is_enum() const { return true; }
+  const std::vector<t_constant*>& get_constants() {
+    return constants_;
+  }
+
+  bool is_enum() const {
+    return true;
+  }
 
  private:
   std::vector<t_constant*> constants_;
diff --git a/compiler/cpp/src/parse/t_field.h b/compiler/cpp/src/parse/t_field.h
index 7f4d0e9..93e14d8 100644
--- a/compiler/cpp/src/parse/t_field.h
+++ b/compiler/cpp/src/parse/t_field.h
@@ -4,24 +4,36 @@
 #include <string>
 
 /**
- * Class to represent a field in a thrift structure or in a set of arguments
- * to a thrift function.
+ * Class to represent a field in a thrift structure. A field has a data type,
+ * a symbolic name, and a numeric identifier.
  *
  * @author Mark Slee <mcslee@facebook.com>
  */
 class t_field {
  public:
   t_field(t_type* type, std::string name) :
-    type_(type), name_(name), key_(0) {}
+    type_(type),
+    name_(name),
+    key_(0) {}
 
   t_field(t_type* type, std::string name, int32_t key) :
-    type_(type), name_(name), key_(key) {}
+    type_(type),
+    name_(name),
+    key_(key) {}
 
   ~t_field() {}
 
-  t_type* get_type() const { return type_; }
-  const std::string& get_name() const { return name_; }
-  int32_t get_key() const { return key_; }
+  t_type* get_type() const {
+    return type_;
+  }
+
+  const std::string& get_name() const {
+    return name_;
+  }
+
+  int32_t get_key() const {
+    return key_;
+  }
 
  private:
   t_type* type_;
diff --git a/compiler/cpp/src/parse/t_function.h b/compiler/cpp/src/parse/t_function.h
index 58667d8..3d07171 100644
--- a/compiler/cpp/src/parse/t_function.h
+++ b/compiler/cpp/src/parse/t_function.h
@@ -6,8 +6,9 @@
 #include "t_struct.h"
 
 /**
- * Representation of a function. Key parst are return type, function name,
- * optional modifiers, and an argument list.
+ * Representation of a function. Key parts are return type, function name,
+ * optional modifiers, and an argument list, which is implemented as a thrift
+ * struct.
  *
  * @author Mark Slee <mcslee@facebook.com>
  */
@@ -24,7 +25,6 @@
     xceptions_ = new t_struct;
   }
 
-
   t_function(t_type* returntype,
              std::string name,
              t_struct* arglist,
@@ -38,11 +38,25 @@
 
   ~t_function() {}
 
-  t_type*      get_returntype() const { return returntype_; }
-  const std::string& get_name() const { return name_; }
-  t_struct*    get_arglist()    const { return arglist_; }
-  t_struct*    get_xceptions()  const { return xceptions_; }
-  bool         is_async()       const { return async_; }
+  t_type* get_returntype() const {
+    return returntype_;
+  }
+
+  const std::string& get_name() const {
+    return name_;
+  }
+
+  t_struct* get_arglist() const {
+    return arglist_;
+  }
+
+  t_struct* get_xceptions() const {
+    return xceptions_;
+  }
+
+  bool is_async() const {
+    return async_;
+  }
 
  private:
   t_type* returntype_;
diff --git a/compiler/cpp/src/parse/t_list.h b/compiler/cpp/src/parse/t_list.h
index 65874cc..5f9b5aa 100644
--- a/compiler/cpp/src/parse/t_list.h
+++ b/compiler/cpp/src/parse/t_list.h
@@ -3,13 +3,23 @@
 
 #include "t_type.h"
 
+/**
+ * A list is a lightweight container type that just wraps another data type.
+ *
+ * @author Mark Slee <mcslee@facebook.com>
+ */
 class t_list : public t_type {
  public:
-  t_list(t_type* elem_type) : elem_type_(elem_type) {}
-  ~t_list() {}
+  t_list(t_type* elem_type) :
+    elem_type_(elem_type) {}
 
-  t_type* get_elem_type() const { return elem_type_; }
-  bool is_list() const { return true; }
+  t_type* get_elem_type() const {
+    return elem_type_;
+  }
+  
+  bool is_list() const {
+    return true;
+  }
 
  private:
   t_type* elem_type_;
diff --git a/compiler/cpp/src/parse/t_map.h b/compiler/cpp/src/parse/t_map.h
index 1f61843..ff1c569 100644
--- a/compiler/cpp/src/parse/t_map.h
+++ b/compiler/cpp/src/parse/t_map.h
@@ -1,15 +1,29 @@
 #ifndef T_MAP_H
 #define T_MAP_H
 
+/**
+ * A map is a lightweight container type that just wraps another two data
+ * types.
+ *
+ * @author Mark Slee <mcslee@facebook.com>
+ */
 class t_map : public t_type {
  public:
   t_map(t_type* key_type, t_type* val_type) :
-    key_type_(key_type), val_type_(val_type) {}
-  ~t_map() {}
+    key_type_(key_type),
+    val_type_(val_type) {}
 
-  t_type* get_key_type() const { return key_type_; }
-  t_type* get_val_type() const { return val_type_; }
-  bool is_map() const { return true; }
+  t_type* get_key_type() const {
+    return key_type_;
+  }
+
+  t_type* get_val_type() const {
+    return val_type_;
+  }
+
+  bool is_map() const {
+    return true;
+  }
 
  private:
   t_type* key_type_;
diff --git a/compiler/cpp/src/parse/t_program.h b/compiler/cpp/src/parse/t_program.h
index 74b9360..5e58654 100644
--- a/compiler/cpp/src/parse/t_program.h
+++ b/compiler/cpp/src/parse/t_program.h
@@ -24,12 +24,15 @@
  *   Exceptions
  *   Services
  *
+ * The program module also contains the definitions of the base types.
+ *
  * @author Mark Slee <mcslee@facebook.com>
  */
 class t_program {
  public:
   t_program(std::string name) :
-    name_(name), namespace_() {
+    name_(name),
+    namespace_() {
     type_void   = new t_base_type("void",   t_base_type::TYPE_VOID);
     type_string = new t_base_type("string", t_base_type::TYPE_STRING);
     type_bool   = new t_base_type("bool",   t_base_type::TYPE_BOOL);
@@ -51,10 +54,14 @@
   }
 
   // Name accessor
-  const std::string& get_name() const { return name_; }
+  const std::string& get_name() const {
+    return name_;
+  }
 
   // Namespace
-  const std::string& get_namespace() const { return namespace_; }
+  const std::string& get_namespace() const {
+    return namespace_;
+  }
 
   // Accessors for program elements
   const std::vector<t_typedef*>& get_typedefs()  const { return typedefs_;  }
@@ -79,6 +86,7 @@
   }
 
   // New program element addition
+
   void set_namespace(std::string name) {
     namespace_ = name;
   }
@@ -87,23 +95,28 @@
     typedefs_.push_back(td);
     add_custom_type(td->get_symbolic(), td);
   }
+
   void add_enum(t_enum* te) {
     enums_.push_back(te);
     add_custom_type(te->get_name(), te);
   }
+
   void add_struct(t_struct* ts) {
     structs_.push_back(ts);
     add_custom_type(ts->get_name(), ts);
   }
+
   void add_xception(t_struct* tx) {
     xceptions_.push_back(tx);
     add_custom_type(tx->get_name(), tx);
   }
+
   void add_service(t_service* ts) {
     services_.push_back(ts);
   }
 
  private:
+
   // Add custom type for lookup
   void add_custom_type(std::string name, t_type* type) {
     custom_types_[name] = type;
diff --git a/compiler/cpp/src/parse/t_service.h b/compiler/cpp/src/parse/t_service.h
index 92e59b2..6bb5e5d 100644
--- a/compiler/cpp/src/parse/t_service.h
+++ b/compiler/cpp/src/parse/t_service.h
@@ -4,16 +4,30 @@
 #include "t_function.h"
 #include <vector>
 
+/**
+ * A service consists of a set of functions.
+ *
+ * @author Mark Slee <mcslee@facebook.com>
+ */
 class t_service {
  public:
   t_service() {}
-  ~t_service() {}
 
-  void set_name(std::string name) { name_ = name; }
-  void add_function(t_function* func) { functions_.push_back(func); }
+  void set_name(std::string name) {
+    name_ = name;
+  }
 
-  const std::string& get_name() const { return name_; }
-  const std::vector<t_function*>& get_functions() const { return functions_; }
+  void add_function(t_function* func) {
+    functions_.push_back(func);
+  }
+
+  const std::string& get_name() const {
+    return name_;
+  }
+
+  const std::vector<t_function*>& get_functions() const {
+    return functions_;
+  }
 
  private:
   std::string name_;
diff --git a/compiler/cpp/src/parse/t_set.h b/compiler/cpp/src/parse/t_set.h
index 3d34ace..e39e420 100644
--- a/compiler/cpp/src/parse/t_set.h
+++ b/compiler/cpp/src/parse/t_set.h
@@ -3,13 +3,23 @@
 
 #include "t_type.h"
 
+/**
+ * A set is a lightweight container type that just wraps another data type.
+ *
+ * @author Mark Slee <mcslee@facebook.com>
+ */
 class t_set : public t_type {
  public:
-  t_set(t_type* elem_type) : elem_type_(elem_type) {}
-  ~t_set() {}
+  t_set(t_type* elem_type) :
+    elem_type_(elem_type) {}
 
-  t_type* get_elem_type() const { return elem_type_; }
-  bool is_set() const { return true; }
+  t_type* get_elem_type() const {
+    return elem_type_;
+  }
+
+  bool is_set() const {
+    return true;
+  }
 
  private:
   t_type* elem_type_;
diff --git a/compiler/cpp/src/parse/t_struct.h b/compiler/cpp/src/parse/t_struct.h
index a365cd4..8768c5f 100644
--- a/compiler/cpp/src/parse/t_struct.h
+++ b/compiler/cpp/src/parse/t_struct.h
@@ -7,24 +7,29 @@
 #include "t_type.h"
 #include "t_field.h"
 
+/**
+ * A struct is a container for a set of member fields that has a name. Structs
+ * are also used to implement exception types.
+ *
+ * @author Mark Slee <mcslee@facebook.com>
+ */
 class t_struct : public t_type {
  public:
-  t_struct() : is_xception_(false) {}
-  t_struct(const std::string& name) : t_type(name), is_xception_(false) {}
+  t_struct() :
+    is_xception_(false) {}
 
-  ~t_struct() {}
+  t_struct(const std::string& name) :
+    t_type(name),
+    is_xception_(false) {}
 
-  /** Set the struct name */
   void set_name(const std::string& name) {
     name_ = name;
   }
 
-  /** Mark as an exception */
   void set_xception(bool is_xception) {
     is_xception_ = is_xception;
   }
 
-  /** Add a new field to the list */
   void append(t_field* elem) {
     members_.push_back(elem);
   }
diff --git a/compiler/cpp/src/parse/t_type.h b/compiler/cpp/src/parse/t_type.h
index 6328961..27b85a5 100644
--- a/compiler/cpp/src/parse/t_type.h
+++ b/compiler/cpp/src/parse/t_type.h
@@ -4,7 +4,11 @@
 #include <string>
 
 /**
- * Generic representation of a thrift type.
+ * Generic representation of a thrift type. These objects are used by the
+ * parser module to build up a tree of object that are all explicitly typed.
+ * The generic t_type class exports a variety of useful methods that are
+ * used by the code generator to branch based upon different handling for the
+ * various types.
  *
  * @author Mark Slee <mcslee@facebook.com>
  */
@@ -24,7 +28,9 @@
   virtual bool is_set()       const { return false; }
   virtual bool is_map()       const { return false; }
 
-  bool is_container() const { return is_map() || is_set() || is_list(); }
+  bool is_container() const {
+    return is_map() || is_set() || is_list();
+  }
 
  protected:
   t_type() {}
diff --git a/compiler/cpp/src/parse/t_typedef.h b/compiler/cpp/src/parse/t_typedef.h
index 97284be..8973201 100644
--- a/compiler/cpp/src/parse/t_typedef.h
+++ b/compiler/cpp/src/parse/t_typedef.h
@@ -5,26 +5,36 @@
 #include "t_type.h"
 
 /**
- * A typedef is a mapping from a symbolic name to another type.
+ * A typedef is a mapping from a symbolic name to another type. In dymanically
+ * typed languages (i.e. php/python) the code generator can actually usually
+ * ignore typedefs and just use the underlying type directly, though in C++
+ * the symbolic naming can be quite useful for code clarity.
  *
  * @author Mark Slee <mcslee@facebook.com>
  */
 class t_typedef : public t_type {
  public:
   t_typedef(t_type* type, std::string symbolic) :
-    t_type(symbolic), type_(type), symbolic_(symbolic) {}
+    t_type(symbolic),
+    type_(type),
+    symbolic_(symbolic) {}
 
   ~t_typedef() {}
 
-  t_type* get_type() const { return type_; }
-  const std::string& get_symbolic() const { return symbolic_; }
-  bool is_typedef() const { return true; }
+  t_type* get_type() const {
+    return type_;
+  }
+
+  const std::string& get_symbolic() const {
+    return symbolic_;
+  }
+
+  bool is_typedef() const {
+    return true;
+  }
 
  private:
-  /** Type that this definition inherits from */
   t_type* type_;
-
-  /** Symbolic name for this type */
   std::string symbolic_;
 };
 
diff --git a/compiler/cpp/src/thrift.l b/compiler/cpp/src/thrift.l
index 484e43b..64a2bf8 100644
--- a/compiler/cpp/src/thrift.l
+++ b/compiler/cpp/src/thrift.l
@@ -4,20 +4,29 @@
  * Tokenizes a thrift definition file.
  * @author Mark Slee <mcslee@facebook.com>
  */
+
 %{
 
 #include "main.h"
 #include "parse/t_program.h"
 
-/** Must be included AFTER parse/t_program.h */
+/**
+ * Must be included AFTER parse/t_program.h, but I can't remember why anymore
+ * because I wrote this a while ago.
+ */
 #include "thrift.tab.hh"
 
 %}
 
-/** Provides yylineno global */
+/**
+ * Provides the yylineno global, useful for debugging output
+ */
 %option lex-compat 	
 
-/** Helper definitions */
+/** 
+ * Helper definitions, comments, constants, and whatnot
+ */
+
 intconstant  ([0-9]+)
 identifier   ([a-zA-Z_][\.a-zA-Z_0-9]*)
 whitespace   ([ \t\r\n]*)
@@ -36,31 +45,27 @@
 {symbol}      { return yytext[0]; }
 
 "namespace"   { return tok_namespace; }
-
-"void"        { return tok_void;     }
-"bool"        { return tok_bool;     }
-"byte"        { return tok_byte;     }
-"i16"         { return tok_i16;      }
-"i32"         { return tok_i32;      }
-"i64"         { return tok_i64;      }
-"double"      { return tok_double;   }
-"string"      { return tok_string;   }
-
-"map"         { return tok_map;      }
-"list"        { return tok_list;     }
-"set"         { return tok_set;      }
-
-"async"       { return tok_async;    }
-
-"typedef"     { return tok_typedef;  }
-"struct"      { return tok_struct;   }
-"exception"   { return tok_xception; }
-"throws"      { return tok_throws;   }
-"service"     { return tok_service;  }
-"enum"        { return tok_enum;     }
-
+"void"        { return tok_void;      }
+"bool"        { return tok_bool;      }
+"byte"        { return tok_byte;      }
+"i16"         { return tok_i16;       }
+"i32"         { return tok_i32;       }
+"i64"         { return tok_i64;       }
+"double"      { return tok_double;    }
+"string"      { return tok_string;    }
+"map"         { return tok_map;       }
+"list"        { return tok_list;      }
+"set"         { return tok_set;       }
+"async"       { return tok_async;     }
+"typedef"     { return tok_typedef;   }
+"struct"      { return tok_struct;    }
+"exception"   { return tok_xception;  }
+"throws"      { return tok_throws;    }
+"service"     { return tok_service;   }
+"enum"        { return tok_enum;      }
 
 {intconstant} { yylval.iconst = atoi(yytext); return tok_int_constant; }
+
 {identifier}  { yylval.id = strdup(yytext); return tok_identifier; }
 
 %%
diff --git a/compiler/cpp/src/thrift.y b/compiler/cpp/src/thrift.y
index 4b2dde7..404c80b 100644
--- a/compiler/cpp/src/thrift.y
+++ b/compiler/cpp/src/thrift.y
@@ -13,10 +13,19 @@
 #include "globals.h"
 #include "parse/t_program.h"
 
+/**
+ * This global variable is used for automatic numbering of field indices etc.
+ * when parsing the members of a struct. Field values are automatically
+ * assigned starting from -1 and working their way down.
+ */
 int y_field_val = -1;
 
 %}
 
+/**
+ * This structure is used by the parser to hold the data types associated with
+ * various parse nodes.
+ */
 %union {
   char*       id;
   int         iconst;
@@ -31,14 +40,25 @@
   t_constant* tconstant;
 }
 
-/** Strings and constants */
+/**
+ * Strings identifier
+ */
 %token<id>     tok_identifier
+
+/**
+ * Integer constant value
+ */
 %token<iconst> tok_int_constant
 
-/** Namespace */
+/**
+ * Namespace keyword
+ */
 %token tok_namespace
 
-/** Base datatypes */
+/**
+ * Base datatype keywords
+ */
+%token tok_void
 %token tok_bool
 %token tok_byte
 %token tok_string
@@ -47,18 +67,21 @@
 %token tok_i64
 %token tok_double
 
-/** Complex Types */
+/**
+ * Complex type keywords
+ */
 %token tok_map
 %token tok_list
 %token tok_set
 
-/** Function types */
-%token tok_void
-
-/** Modifiers */ 
+/**
+ * Function modifiers
+ */ 
 %token tok_async
 
-/** Thrift actions */
+/**
+ * Thrift language keywords
+ */
 %token tok_typedef
 %token tok_struct
 %token tok_xception
@@ -66,22 +89,24 @@
 %token tok_service
 %token tok_enum
 
-/** Types */
+/**
+ * Grammar nodes
+ */
+
+%type<id>        Namespace
+
 %type<ttype>     BaseType
 %type<ttype>     ContainerType
 %type<ttype>     MapType
 %type<ttype>     SetType
 %type<ttype>     ListType
 
-%type<id>        Namespace
-
 %type<ttypedef>  Typedef
 %type<ttype>     DefinitionType
 
 %type<tfield>    Field
 %type<ttype>     FieldType
 %type<tstruct>   FieldList
-%type<tstruct>   ThrowsOptional
 
 %type<tenum>     Enum
 %type<tenum>     EnumDefList
@@ -89,18 +114,24 @@
 
 %type<tstruct>   Struct
 %type<tstruct>   Xception
-
 %type<tservice>  Service
 
 %type<tfunction> Function
 %type<ttype>     FunctionType
 %type<tservice>  FunctionList
 
+%type<tstruct>   ThrowsOptional
 %type<tbool>     AsyncOptional
 
 %%
 
-/** Thrift Grammar */
+/**
+ * Thrift Grammar Implementation.
+ *
+ * For the most part this source file works its way top down from what you
+ * might expect to find in a typical .thrift file, i.e. type definitions and
+ * namespaces up top followed by service definitions using those types.
+ */
 
 Program:
   DefinitionList