From 4064a09238466b3576c3c93ff45b428dee69d37e Mon Sep 17 00:00:00 2001
From: Ryan Hunt <rhunt@eqrion.net>
Date: Tue, 20 Jun 2017 01:25:42 -0400
Subject: [PATCH] Do a bunch of clean up and add some comments

---
 README.md                   |  12 +-
 src/bindgen/annotation.rs   |  10 ++
 src/bindgen/cargo_expand.rs |   7 +-
 src/bindgen/cargo_toml.rs   |   1 +
 src/bindgen/cdecl.rs        |   9 +-
 src/bindgen/config.rs       | 280 ++++++++++++++++++------------------
 src/bindgen/items.rs        |  97 ++++++++-----
 src/bindgen/library.rs      |  65 ++++-----
 src/bindgen/mod.rs          |   3 +
 src/bindgen/utilities.rs    |  66 +++++----
 src/bindgen/writer.rs       |   7 +
 src/main.rs                 |   8 ++
 12 files changed, 316 insertions(+), 249 deletions(-)

diff --git a/README.md b/README.md
index 5b750fc..bb852f5 100644
--- a/README.md
+++ b/README.md
@@ -38,7 +38,9 @@ fn main() {
     let root = env::var("CARGO_MANIFEST_DIR").unwrap();
     let config = Config::from_root_or_default(&root);
 
-    Library::load(&root, &config)
+    Library::load_crate(Path::new(root),
+                        "CRATE_NAME",
+                        &config)
         .generate().unwrap()
         .write_to_file("bindings.h");
 }
@@ -59,8 +61,6 @@ See `compile-tests/` for some examples of rust source that can be handled.
 
 ## Future work
 
-1. Add a validation step to catch common issues
-2. Better support for types with fully specified names
-3. Better support for finding dependencies managed by Cargo
-4. Support for generating a FFI interface for a Struct+Impl
-5. ...
+1. Better support for types with fully specified names
+2. Support for generating a FFI interface for a Struct+Impl
+3. ...
diff --git a/src/bindgen/annotation.rs b/src/bindgen/annotation.rs
index e3e8218..b730231 100644
--- a/src/bindgen/annotation.rs
+++ b/src/bindgen/annotation.rs
@@ -38,11 +38,17 @@ impl AnnotationSet {
     pub fn parse(text: String) -> Result<AnnotationSet, String> {
         let mut annotations = HashMap::new();
 
+        // Look at each line for an annotation
         for line in text.lines().map(|x| x.trim_left_matches("///").trim()) {
+            // Skip lines that don't start with cbindgen
             if !line.starts_with("cbindgen:") {
                 continue;
             }
+
+            // Remove the "cbingen:" prefix
             let annotation = &line[9..];
+
+            // Split the annotation in two
             let parts: Vec<&str> = annotation.split("=")
                                             .map(|x| x.trim())
                                             .collect();
@@ -51,13 +57,16 @@ impl AnnotationSet {
                 return Err(format!("couldn't parse {}", line));
             }
 
+            // Grab the name that this annotation is modifying
             let name = parts[0];
 
+            // If the annotation only has a name, assume it's setting a bool flag
             if parts.len() == 1 {
                 annotations.insert(name.to_string(), AnnotationValue::Bool(true));
                 continue;
             }
 
+            // Parse the value we're setting the name to
             let value = parts[1];
 
             if let Some(x) = parse_list(value) {
@@ -111,6 +120,7 @@ impl AnnotationSet {
     }
 }
 
+/// Parse lists like "[x, y, z]". This is not implemented efficiently or well.
 fn parse_list(list: &str) -> Option<Vec<String>> {
     if list.len() < 2 {
         return None;
diff --git a/src/bindgen/cargo_expand.rs b/src/bindgen/cargo_expand.rs
index 96fc455..551c8bc 100644
--- a/src/bindgen/cargo_expand.rs
+++ b/src/bindgen/cargo_expand.rs
@@ -3,10 +3,11 @@ use std::path::Path;
 use std::process::Command;
 use std::str::from_utf8;
 
-type ExpandResult = Result<String, String>;
-
-pub fn expand(manifest_path: &Path, crate_name: &str) -> ExpandResult {
+/// Use rustc to expand and pretty print the crate into a single file,
+/// removing any macros in the process.
+pub fn expand(manifest_path: &Path, crate_name: &str) -> Result<String, String> {
     let cargo = env::var("CARGO").unwrap_or_else(|_| String::from("cargo"));
+
     let mut cmd = Command::new(cargo);
     cmd.arg("rustc");
     cmd.arg("--manifest-path");
diff --git a/src/bindgen/cargo_toml.rs b/src/bindgen/cargo_toml.rs
index 46408ea..c91c43e 100644
--- a/src/bindgen/cargo_toml.rs
+++ b/src/bindgen/cargo_toml.rs
@@ -35,6 +35,7 @@ pub struct Package {
     pub name: String,
 }
 
+/// Parse the Cargo.toml for a given path
 pub fn manifest(manifest_path: &Path) -> Result<Manifest, Error> {
     let mut s = String::new();
     let mut f = File::open(manifest_path)?;
diff --git a/src/bindgen/cdecl.rs b/src/bindgen/cdecl.rs
index c9595f5..f612d2f 100644
--- a/src/bindgen/cdecl.rs
+++ b/src/bindgen/cdecl.rs
@@ -89,8 +89,7 @@ impl CDecl {
         }
     }
 
-    fn to_string(&self, ident: Option<&str>) -> String
-    {
+    fn to_string(&self, ident: Option<&str>) -> String {
         // Build the left side (the type-specifier and type-qualifier),
         // and then build the right side (the declarators), and then
         // merge the result.
@@ -169,11 +168,9 @@ impl CDecl {
     }
 }
 
-pub fn write_func<F: Write>(out: &mut SourceWriter<F>, f: &Function)
-{
+pub fn write_func<F: Write>(out: &mut SourceWriter<F>, f: &Function) {
     out.write(&CDecl::from_func(f).to_string(Some(&f.name)));
 }
-pub fn write_type<F: Write>(out: &mut SourceWriter<F>, t: &Type, ident: &str)
-{
+pub fn write_type<F: Write>(out: &mut SourceWriter<F>, t: &Type, ident: &str) {
     out.write(&CDecl::from_type(t).to_string(Some(ident)));
 }
diff --git a/src/bindgen/config.rs b/src/bindgen/config.rs
index e33af99..c02a428 100644
--- a/src/bindgen/config.rs
+++ b/src/bindgen/config.rs
@@ -19,21 +19,6 @@ pub enum Language {
     C,
 }
 
-/// A style of braces to use for generating code.
-#[derive(Debug, Clone, PartialEq)]
-pub enum Braces {
-    SameLine,
-    NextLine,
-}
-
-/// A type of layout to use when generating long lines of code.
-#[derive(Debug, Clone, PartialEq)]
-pub enum Layout {
-    Horizontal,
-    Vertical,
-    Auto,
-}
-
 impl FromStr for Language {
     type Err = String;
 
@@ -53,6 +38,16 @@ impl FromStr for Language {
         }
     }
 }
+
+deserialize_enum_str!(Language);
+
+/// A style of braces to use for generating code.
+#[derive(Debug, Clone, PartialEq)]
+pub enum Braces {
+    SameLine,
+    NextLine,
+}
+
 impl FromStr for Braces {
     type Err = String;
 
@@ -66,6 +61,17 @@ impl FromStr for Braces {
         }
     }
 }
+
+deserialize_enum_str!(Braces);
+
+/// A type of layout to use when generating long lines of code.
+#[derive(Debug, Clone, PartialEq)]
+pub enum Layout {
+    Horizontal,
+    Vertical,
+    Auto,
+}
+
 impl FromStr for Layout {
     type Err = String;
 
@@ -82,67 +88,8 @@ impl FromStr for Layout {
     }
 }
 
-deserialize_enum_str!(Language);
-deserialize_enum_str!(Braces);
 deserialize_enum_str!(Layout);
 
-/// A collection of settings to customize the generated bindings.
-#[derive(Debug, Clone, Deserialize)]
-#[serde(rename_all = "snake_case")]
-#[serde(deny_unknown_fields)]
-#[serde(default)]
-pub struct Config {
-    /// Optional text to output at the beginning of the file
-    pub header: Option<String>,
-    /// Optional text to output at the end of the file
-    pub trailer: Option<String>,
-    /// Option name to use for an include guard
-    pub include_guard: Option<String>,
-    /// Optional text to output at major sections to deter manual editing
-    pub autogen_warning: Option<String>,
-    /// Include a comment with the version of cbindgen used to generate the file
-    pub include_version: bool,
-    /// The style to use for braces
-    pub braces: Braces,
-    /// The preferred length of a line, used for auto breaking function arguments
-    pub line_length: usize,
-    /// The amount of spaces in a tab
-    pub tab_width: usize,
-    /// The language to output bindings for
-    pub language: Language,
-    /// The names of crates to parse with `rustc --pretty=expanded`
-    pub expand: Vec<String>,
-    /// The configuration options for functions
-    #[serde(rename = "fn")]
-    pub function: FunctionConfig,
-    /// The configuration options for structs
-    #[serde(rename = "struct")]
-    pub structure: StructConfig,
-    /// The configuration options for enums
-    #[serde(rename = "enum")]
-    pub enumeration: EnumConfig,
-}
-
-impl Default for Config {
-    fn default() -> Config {
-        Config {
-            header: None,
-            trailer: None,
-            include_guard: None,
-            autogen_warning: None,
-            include_version: true,
-            braces: Braces::SameLine,
-            line_length: 100,
-            tab_width: 2,
-            language: Language::Cxx,
-            expand: Vec::new(),
-            function: FunctionConfig::default(),
-            structure: StructConfig::default(),
-            enumeration: EnumConfig::default(),
-        }
-    }
-}
-
 /// Settings to apply to generated functions.
 #[derive(Debug, Clone, Deserialize)]
 #[serde(rename_all = "snake_case")]
@@ -170,6 +117,22 @@ impl Default for FunctionConfig {
     }
 }
 
+impl FunctionConfig {
+    pub fn prefix(&self, annotations: &AnnotationSet) -> Option<String> {
+        if let Some(x) = annotations.atom("prefix") {
+            return x;
+        }
+        self.prefix.clone()
+    }
+
+    pub fn postfix(&self, annotations: &AnnotationSet) -> Option<String> {
+        if let Some(x) = annotations.atom("postfix") {
+            return x;
+        }
+        self.postfix.clone()
+    }
+}
+
 /// Settings to apply to generated structs.
 #[derive(Debug, Clone, Deserialize)]
 #[serde(rename_all = "snake_case")]
@@ -206,6 +169,45 @@ impl Default for StructConfig {
     }
 }
 
+impl StructConfig {
+    pub fn derive_eq(&self, annotations: &AnnotationSet) -> bool {
+        if let Some(x) = annotations.bool("derive-eq") {
+            return x;
+        }
+        self.derive_eq
+    }
+    pub fn derive_neq(&self, annotations: &AnnotationSet) -> bool {
+        if let Some(x) = annotations.bool("derive-neq") {
+            return x;
+        }
+        self.derive_neq
+    }
+    pub fn derive_lt(&self, annotations: &AnnotationSet) -> bool {
+        if let Some(x) = annotations.bool("derive-lt") {
+            return x;
+        }
+        self.derive_lt
+    }
+    pub fn derive_lte(&self, annotations: &AnnotationSet) -> bool {
+        if let Some(x) = annotations.bool("derive-lte") {
+            return x;
+        }
+        self.derive_lte
+    }
+    pub fn derive_gt(&self, annotations: &AnnotationSet) -> bool {
+        if let Some(x) = annotations.bool("derive-gt") {
+            return x;
+        }
+        self.derive_gt
+    }
+    pub fn derive_gte(&self, annotations: &AnnotationSet) -> bool {
+        if let Some(x) = annotations.bool("derive-gte") {
+            return x;
+        }
+        self.derive_gte
+    }
+}
+
 /// Settings to apply to generated enums.
 #[derive( Debug, Clone, Deserialize)]
 #[serde(rename_all = "snake_case")]
@@ -228,6 +230,72 @@ impl Default for EnumConfig {
     }
 }
 
+impl EnumConfig {
+    pub fn add_sentinel(&self, annotations: &AnnotationSet) -> bool {
+        if let Some(x) = annotations.bool("add-sentinel") {
+            return x;
+        }
+        self.add_sentinel
+    }
+}
+
+/// A collection of settings to customize the generated bindings.
+#[derive(Debug, Clone, Deserialize)]
+#[serde(rename_all = "snake_case")]
+#[serde(deny_unknown_fields)]
+#[serde(default)]
+pub struct Config {
+    /// Optional text to output at the beginning of the file
+    pub header: Option<String>,
+    /// Optional text to output at the end of the file
+    pub trailer: Option<String>,
+    /// Option name to use for an include guard
+    pub include_guard: Option<String>,
+    /// Optional text to output at major sections to deter manual editing
+    pub autogen_warning: Option<String>,
+    /// Include a comment with the version of cbindgen used to generate the file
+    pub include_version: bool,
+    /// The style to use for braces
+    pub braces: Braces,
+    /// The preferred length of a line, used for auto breaking function arguments
+    pub line_length: usize,
+    /// The amount of spaces in a tab
+    pub tab_width: usize,
+    /// The language to output bindings for
+    pub language: Language,
+    /// The names of crates to parse with `rustc --pretty=expanded`
+    pub expand: Vec<String>,
+    /// The configuration options for functions
+    #[serde(rename = "fn")]
+    pub function: FunctionConfig,
+    /// The configuration options for structs
+    #[serde(rename = "struct")]
+    pub structure: StructConfig,
+    /// The configuration options for enums
+    #[serde(rename = "enum")]
+    pub enumeration: EnumConfig,
+}
+
+impl Default for Config {
+    fn default() -> Config {
+        Config {
+            header: None,
+            trailer: None,
+            include_guard: None,
+            autogen_warning: None,
+            include_version: true,
+            braces: Braces::SameLine,
+            line_length: 100,
+            tab_width: 2,
+            language: Language::Cxx,
+            expand: Vec::new(),
+            function: FunctionConfig::default(),
+            structure: StructConfig::default(),
+            enumeration: EnumConfig::default(),
+        }
+    }
+}
+
 impl Config {
     pub fn from_file(file_name: &str) -> Result<Config, String> {
         fn read(file_name: &str) -> io::Result<String> {
@@ -256,67 +324,3 @@ impl Config {
         }
     }
 }
-
-impl FunctionConfig {
-    pub fn prefix(&self, annotations: &AnnotationSet) -> Option<String> {
-        if let Some(x) = annotations.atom("prefix") {
-            return x;
-        }
-        self.prefix.clone()
-    }
-
-    pub fn postfix(&self, annotations: &AnnotationSet) -> Option<String> {
-        if let Some(x) = annotations.atom("postfix") {
-            return x;
-        }
-        self.postfix.clone()
-    }
-}
-
-impl StructConfig {
-    pub fn derive_eq(&self, annotations: &AnnotationSet) -> bool {
-        if let Some(x) = annotations.bool("derive-eq") {
-            return x;
-        }
-        self.derive_eq
-    }
-    pub fn derive_neq(&self, annotations: &AnnotationSet) -> bool {
-        if let Some(x) = annotations.bool("derive-neq") {
-            return x;
-        }
-        self.derive_neq
-    }
-    pub fn derive_lt(&self, annotations: &AnnotationSet) -> bool {
-        if let Some(x) = annotations.bool("derive-lt") {
-            return x;
-        }
-        self.derive_lt
-    }
-    pub fn derive_lte(&self, annotations: &AnnotationSet) -> bool {
-        if let Some(x) = annotations.bool("derive-lte") {
-            return x;
-        }
-        self.derive_lte
-    }
-    pub fn derive_gt(&self, annotations: &AnnotationSet) -> bool {
-        if let Some(x) = annotations.bool("derive-gt") {
-            return x;
-        }
-        self.derive_gt
-    }
-    pub fn derive_gte(&self, annotations: &AnnotationSet) -> bool {
-        if let Some(x) = annotations.bool("derive-gte") {
-            return x;
-        }
-        self.derive_gte
-    }
-}
-
-impl EnumConfig {
-    pub fn add_sentinel(&self, annotations: &AnnotationSet) -> bool {
-        if let Some(x) = annotations.bool("add-sentinel") {
-            return x;
-        }
-        self.add_sentinel
-    }
-}
diff --git a/src/bindgen/items.rs b/src/bindgen/items.rs
index f80b526..7a974fd 100644
--- a/src/bindgen/items.rs
+++ b/src/bindgen/items.rs
@@ -39,6 +39,7 @@ pub enum PrimitiveType {
     Float,
     Double,
 }
+
 impl PrimitiveType {
     fn maybe(path: &str) -> Option<PrimitiveType> {
         match path {
@@ -83,6 +84,7 @@ impl PrimitiveType {
         true
     }
 }
+
 impl fmt::Display for PrimitiveType {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match self {
@@ -124,11 +126,12 @@ pub enum Type {
     Array(Box<Type>, u64),
     FuncPtr(Box<Type>, Vec<Type>),
 }
+
 impl Type {
-    pub fn convert(ty: &syn::Ty) -> ConvertResult<Option<Type>> {
+    pub fn load(ty: &syn::Ty) -> Result<Option<Type>, String> {
         let converted = match ty {
             &syn::Ty::Rptr(_, ref mut_ty) => {
-                let converted = try!(Type::convert(&mut_ty.ty));
+                let converted = Type::load(&mut_ty.ty)?;
 
                 let converted = match converted {
                     Some(converted) => converted,
@@ -141,7 +144,7 @@ impl Type {
                 }
             }
             &syn::Ty::Ptr(ref mut_ty) => {
-                let converted = try!(Type::convert(&mut_ty.ty));
+                let converted = Type::load(&mut_ty.ty)?;
 
                 let converted = match converted {
                     Some(converted) => converted,
@@ -154,7 +157,7 @@ impl Type {
                 }
             }
             &syn::Ty::Path(_, ref p) => {
-                let (name, generics) = try!(p.convert_to_generic_single_segment());
+                let (name, generics) = p.convert_to_generic_single_segment()?;
 
                 if name == "PhantomData" {
                     return Ok(None);
@@ -169,7 +172,7 @@ impl Type {
                 }
             }
             &syn::Ty::Array(ref ty, syn::ConstExpr::Lit(syn::Lit::Int(sz, _))) => {
-                let converted = try!(Type::convert(ty));
+                let converted = Type::load(ty)?;
 
                 let converted = match converted {
                     Some(converted) => converted,
@@ -179,9 +182,9 @@ impl Type {
                 Type::Array(Box::new(converted), sz)
             },
             &syn::Ty::BareFn(ref f) => {
-                let args = try!(f.inputs.iter()
-                                        .try_skip_map(|x| Type::convert(&x.ty)));
-                let ret = try!(f.output.as_type());
+                let args = f.inputs.iter()
+                                   .try_skip_map(|x| Type::load(&x.ty))?;
+                let ret = f.output.as_type()?;
 
                 Type::FuncPtr(Box::new(ret), args)
             },
@@ -281,6 +284,7 @@ impl Type {
         }
     }
 }
+
 impl Source for (String, Type) {
     fn write<F: Write>(&self, _config: &Config, out: &mut SourceWriter<F>) {
         cdecl::write_type(out, &self.1, &self.0);
@@ -297,14 +301,14 @@ pub struct Function {
 }
 
 impl Function {
-    pub fn convert(name: String,
-                   annotations: AnnotationSet,
-                   decl: &syn::FnDecl,
-                   extern_decl: bool) -> ConvertResult<Function>
+    pub fn load(name: String,
+                annotations: AnnotationSet,
+                decl: &syn::FnDecl,
+                extern_decl: bool) -> Result<Function, String>
     {
-        let args = try!(decl.inputs.iter()
-                                   .try_skip_map(|x| x.as_ident_and_type()));
-        let ret = try!(decl.output.as_type());
+        let args = decl.inputs.iter()
+                              .try_skip_map(|x| x.as_ident_and_type())?;
+        let ret = decl.output.as_type()?;
 
         Ok(Function {
             name: name,
@@ -335,6 +339,7 @@ impl Function {
         }
     }
 }
+
 impl Source for Function {
     fn write<F: Write>(&self, config: &Config, out: &mut SourceWriter<F>) {
         fn write_1<W: Write>(func: &Function, config: &Config, out: &mut SourceWriter<W>) {
@@ -352,6 +357,7 @@ impl Source for Function {
             }
             out.write(";");
         }
+
         fn write_2<W: Write>(func: &Function, config: &Config, out: &mut SourceWriter<W>) {
             let prefix = config.function.prefix(&func.annotations);
             let postfix = config.function.postfix(&func.annotations);
@@ -388,21 +394,21 @@ pub struct Struct {
 }
 
 impl Struct {
-    pub fn convert(name: String,
-                   annotations: AnnotationSet,
-                   decl: &syn::VariantData,
-                   generics: &syn::Generics) -> ConvertResult<Struct>
+    pub fn load(name: String,
+                annotations: AnnotationSet,
+                decl: &syn::VariantData,
+                generics: &syn::Generics) -> Result<Struct, String>
     {
         let fields = match decl {
             &syn::VariantData::Struct(ref fields) => {
-                try!(fields.iter()
-                           .try_skip_map(|x| x.as_ident_and_type()))
+                fields.iter()
+                      .try_skip_map(|x| x.as_ident_and_type())?
             }
             &syn::VariantData::Tuple(ref fields) => {
                 let mut out = Vec::new();
                 let mut current = 0;
                 for field in fields {
-                    if let Some(x) = try!(Type::convert(&field.ty)) {
+                    if let Some(x) = Type::load(&field.ty)? {
                         out.push((format!("{}", current), x));
                         current += 1;
                     }
@@ -457,6 +463,7 @@ impl Struct {
         }
     }
 }
+
 impl Source for Struct {
     fn write<F: Write>(&self, config: &Config, out: &mut SourceWriter<F>) {
         assert!(self.generic_params.is_empty());
@@ -548,6 +555,7 @@ impl OpaqueStruct {
         }
     }
 }
+
 impl Source for OpaqueStruct {
     fn write<F: Write>(&self, config: &Config, out: &mut SourceWriter<F>) {
         if config.language == Language::C {
@@ -560,6 +568,15 @@ impl Source for OpaqueStruct {
     }
 }
 
+#[derive(Debug, Copy, Clone, PartialEq)]
+pub enum Repr {
+    None,
+    C,
+    U8,
+    U16,
+    U32,
+}
+
 #[derive(Debug, Clone)]
 pub struct Enum {
     pub name: String,
@@ -569,10 +586,10 @@ pub struct Enum {
 }
 
 impl Enum {
-    pub fn convert(name: String,
-                   repr: Repr,
-                   annotations: AnnotationSet,
-                   variants: &Vec<syn::Variant>) -> ConvertResult<Enum>
+    pub fn load(name: String,
+                repr: Repr,
+                annotations: AnnotationSet,
+                variants: &Vec<syn::Variant>) -> Result<Enum, String>
     {
         if repr != Repr::U32 &&
            repr != Repr::U16 &&
@@ -637,6 +654,7 @@ impl Enum {
         }
     }
 }
+
 impl Source for Enum {
     fn write<F: Write>(&self, config: &Config, out: &mut SourceWriter<F>) {
         let size = match self.repr {
@@ -672,6 +690,9 @@ impl Source for Enum {
     }
 }
 
+/// A type alias that generates a copy of its aliasee with a new name. If the type
+/// alias has generic values, it monomorphosizes its aliasee. This is useful for
+/// presenting an interface that includes generic types.
 #[derive(Debug, Clone)]
 pub struct Specialization {
     pub name: String,
@@ -682,10 +703,10 @@ pub struct Specialization {
 }
 
 impl Specialization {
-    pub fn convert(name: String,
-                   annotations: AnnotationSet,
-                   generics: &syn::Generics,
-                   ty: &syn::Ty) -> ConvertResult<Specialization>
+    pub fn load(name: String,
+                annotations: AnnotationSet,
+                generics: &syn::Generics,
+                ty: &syn::Ty) -> Result<Specialization, String>
     {
         match ty {
             &syn::Ty::Path(ref _q, ref p) => {
@@ -693,7 +714,7 @@ impl Specialization {
                                                        .map(|x| x.ident.to_string())
                                                        .collect::<Vec<_>>();
 
-                let (path, generic_values) = try!(p.convert_to_generic_single_segment());
+                let (path, generic_values) = p.convert_to_generic_single_segment()?;
 
                 if PrimitiveType::maybe(&path).is_some() {
                     return Err(format!("can't specialize a primitive"));
@@ -720,7 +741,9 @@ impl Specialization {
         }
     }
 
-    pub fn specialize(&self, config: &Config, library: &Library) -> ConvertResult<Option<PathValue>> {
+    pub fn specialize(&self,
+                      config: &Config,
+                      library: &Library) -> Result<Option<PathValue>, String> {
         if self.generic_params.len() > 0 {
             return Ok(None);
         }
@@ -799,6 +822,7 @@ impl Specialization {
     }
 }
 
+/// A type alias that is represented as a C typedef
 #[derive(Debug, Clone)]
 pub struct Typedef {
     pub name: String,
@@ -807,10 +831,10 @@ pub struct Typedef {
 }
 
 impl Typedef {
-    pub fn convert(name: String,
-                   annotations: AnnotationSet,
-                   ty: &syn::Ty) -> ConvertResult<Typedef> {
-        if let Some(x) = try!(Type::convert(ty)) {
+    pub fn load(name: String,
+                annotations: AnnotationSet,
+                ty: &syn::Ty) -> Result<Typedef, String> {
+        if let Some(x) = Type::load(ty)? {
             Ok(Typedef {
                 name: name,
                 annotations: annotations,
@@ -825,6 +849,7 @@ impl Typedef {
         self.aliased.add_deps(library, out);
     }
 }
+
 impl Source for Typedef {
     fn write<F: Write>(&self, config: &Config, out: &mut SourceWriter<F>) {
         out.write("typedef ");
diff --git a/src/bindgen/library.rs b/src/bindgen/library.rs
index dce2238..952d95f 100644
--- a/src/bindgen/library.rs
+++ b/src/bindgen/library.rs
@@ -15,20 +15,10 @@ use bindgen::rust_lib;
 use bindgen::utilities::*;
 use bindgen::writer::{Source, SourceWriter};
 
-pub type ParseResult<'a> = Result<Library<'a>, String>;
-pub type ConvertResult<T> = Result<T, String>;
-pub type GenerateResult<T> = Result<T, String>;
-
-#[derive(Debug, Copy, Clone, PartialEq)]
-pub enum Repr {
-    None,
-    C,
-    U8,
-    U16,
-    U32,
-}
-
+/// A path ref is used to reference a path value
 pub type PathRef = String;
+
+/// A path value is any type of rust item besides a function
 #[derive(Debug, Clone)]
 pub enum PathValue {
     Enum(Enum),
@@ -37,6 +27,7 @@ pub enum PathValue {
     Typedef(Typedef),
     Specialization(Specialization),
 }
+
 impl PathValue {
     pub fn name(&self) -> &String {
         match self {
@@ -72,6 +63,7 @@ pub struct DependencyGraph {
     order: Vec<PathValue>,
     items: HashSet<PathRef>,
 }
+
 impl DependencyGraph {
     fn new() -> DependencyGraph {
         DependencyGraph {
@@ -111,19 +103,22 @@ impl<'a> Library<'a> {
     }
 
     /// Parse the specified crate or source file and load #[repr(C)] types for binding generation.
-    pub fn load_src(src: &path::Path, config: &'a Config) -> ParseResult<'a>
+    pub fn load_src(src: &path::Path,
+                    config: &'a Config) -> Result<Library<'a>, String>
     {
         let mut library = Library::blank("", config);
 
         rust_lib::parse_src(src, &mut |crate_name, items| {
-            library.parse_crate_mod(&crate_name, items);
+            library.load_from_crate_mod(&crate_name, items);
         })?;
 
         Ok(library)
     }
 
     /// Parse the specified crate or source file and load #[repr(C)] types for binding generation.
-    pub fn load_crate(crate_dir: &path::Path, bindings_crate_name: &str, config: &'a Config) -> ParseResult<'a>
+    pub fn load_crate(crate_dir: &path::Path,
+                      bindings_crate_name: &str,
+                      config: &'a Config) -> Result<Library<'a>, String>
     {
         let mut library = Library::blank(bindings_crate_name, config);
 
@@ -131,13 +126,13 @@ impl<'a> Library<'a> {
                             bindings_crate_name,
                             &config.expand,
                             &mut |crate_name, items| {
-            library.parse_crate_mod(&crate_name, items);
+            library.load_from_crate_mod(&crate_name, items);
         })?;
 
         Ok(library)
     }
 
-    fn parse_crate_mod(&mut self, crate_name: &str, items: &Vec<syn::Item>) {
+    fn load_from_crate_mod(&mut self, crate_name: &str, items: &Vec<syn::Item>) {
         for item in items {
             match item.node {
                 syn::ItemKind::ForeignMod(ref block) => {
@@ -163,7 +158,7 @@ impl<'a> Library<'a> {
                                     }
                                 };
 
-                                match Function::convert(foreign_item.ident.to_string(), annotations, decl, true) {
+                                match Function::load(foreign_item.ident.to_string(), annotations, decl, true) {
                                     Ok(func) => {
                                         info!("take {}::{}", crate_name, &foreign_item.ident);
 
@@ -198,7 +193,7 @@ impl<'a> Library<'a> {
                             }
                         };
 
-                        match Function::convert(item.ident.to_string(), annotations, decl, false) {
+                        match Function::load(item.ident.to_string(), annotations, decl, false) {
                             Ok(func) => {
                                 info!("take {}::{}", crate_name, &item.ident);
 
@@ -226,7 +221,7 @@ impl<'a> Library<'a> {
                     };
 
                     if item.is_repr_c() {
-                        match Struct::convert(struct_name.clone(), annotations.clone(), variant, generics) {
+                        match Struct::load(struct_name.clone(), annotations.clone(), variant, generics) {
                             Ok(st) => {
                                 info!("take {}::{}", crate_name, &item.ident);
                                 self.structs.insert(struct_name,
@@ -261,7 +256,7 @@ impl<'a> Library<'a> {
                         }
                     };
 
-                    match Enum::convert(enum_name.clone(), item.get_repr(), annotations.clone(), variants) {
+                    match Enum::load(enum_name.clone(), item.get_repr(), annotations.clone(), variants) {
                         Ok(en) => {
                             info!("take {}::{}", crate_name, &item.ident);
                             self.enums.insert(enum_name, en);
@@ -283,10 +278,10 @@ impl<'a> Library<'a> {
                         }
                     };
 
-                    let fail1 = match Specialization::convert(alias_name.clone(),
-                                                              annotations.clone(),
-                                                              generics,
-                                                              ty) {
+                    let fail1 = match Specialization::load(alias_name.clone(),
+                                                           annotations.clone(),
+                                                           generics,
+                                                           ty) {
                         Ok(spec) => {
                             info!("take {}::{}", crate_name, &item.ident);
                             self.specializations.insert(alias_name, spec);
@@ -301,7 +296,7 @@ impl<'a> Library<'a> {
                         continue;
                     }
 
-                    let fail2 = match Typedef::convert(alias_name.clone(), annotations, ty) {
+                    let fail2 = match Typedef::load(alias_name.clone(), annotations, ty) {
                         Ok(typedef) => {
                             info!("take {}::{}", crate_name, &item.ident);
                             self.typedefs.insert(alias_name, typedef);
@@ -359,8 +354,8 @@ impl<'a> Library<'a> {
     }
 
     /// Build a bindings file from this rust library.
-    pub fn generate(self) -> GenerateResult<BuiltBindings<'a>> {
-        let mut result = BuiltBindings::blank(self.config);
+    pub fn generate(self) -> Result<GeneratedBindings<'a>, String> {
+        let mut result = GeneratedBindings::blank(self.config);
 
         // Gather only the items that we need for this
         // `extern "c"` interface
@@ -370,7 +365,7 @@ impl<'a> Library<'a> {
         }
 
         // Copy the binding items in dependencies order
-        // into the BuiltBindings, specializing any type
+        // into the GeneratedBindings, specializing any type
         // aliases we encounter
         for dep in deps.order {
             match &dep {
@@ -429,18 +424,18 @@ impl<'a> Library<'a> {
     }
 }
 
-/// A BuiltBindings is a completed bindings file ready to be written.
+/// A GeneratedBindings is a completed bindings file ready to be written.
 #[derive(Debug, Clone)]
-pub struct BuiltBindings<'a> {
+pub struct GeneratedBindings<'a> {
     config: &'a Config,
 
     items: Vec<PathValue>,
     functions: Vec<Function>,
 }
 
-impl<'a> BuiltBindings<'a> {
-    fn blank(config: &'a Config) -> BuiltBindings<'a> {
-        BuiltBindings {
+impl<'a> GeneratedBindings<'a> {
+    fn blank(config: &'a Config) -> GeneratedBindings<'a> {
+        GeneratedBindings {
             config: config,
             items: Vec::new(),
             functions: Vec::new(),
diff --git a/src/bindgen/mod.rs b/src/bindgen/mod.rs
index bf80c2e..2d9a47d 100644
--- a/src/bindgen/mod.rs
+++ b/src/bindgen/mod.rs
@@ -1,3 +1,6 @@
+/// A helper macro for deriving deserialize for an enum to be used in toml-rs.
+/// This macro works be relying on an existing FromStr implementation for the
+/// desired type.
 macro_rules! deserialize_enum_str {
     ($name:ident) => {
         impl ::serde::Deserialize for $name {
diff --git a/src/bindgen/utilities.rs b/src/bindgen/utilities.rs
index 48d4515..fb7a336 100644
--- a/src/bindgen/utilities.rs
+++ b/src/bindgen/utilities.rs
@@ -1,28 +1,20 @@
 use syn::*;
 
 use bindgen::items::*;
-use bindgen::library::*;
-
-pub fn find_first_some<T>(slice: &[Option<T>]) -> Option<&T> {
-    for x in slice {
-        if let &Some(ref x) = x {
-            return Some(x);
-        }
-    }
-    return None;
-}
 
+/// Helper functions for dealing with iterators
 pub trait IterHelpers : Iterator {
     fn try_skip_map<F, T, E>(&mut self, f: F) -> Result<Vec<T>, E>
         where F: FnMut(&Self::Item) -> Result<Option<T>, E>;
 }
+
 impl<I> IterHelpers for I where I: Iterator {
     fn try_skip_map<F, T, E>(&mut self, mut f: F) -> Result<Vec<T>, E>
         where F: FnMut(&Self::Item) -> Result<Option<T>, E>
     {
         let mut out = Vec::new();
         while let Some(item) = self.next() {
-            if let Some(x) = try!(f(&item)) {
+            if let Some(x) = f(&item)? {
                 out.push(x);
             }
         }
@@ -30,6 +22,17 @@ impl<I> IterHelpers for I where I: Iterator {
     }
 }
 
+/// I'd like this to be in IterHelpers, but my generic foo isn't strong enough
+pub fn find_first_some<T>(slice: &[Option<T>]) -> Option<&T> {
+    for x in slice {
+        if let &Some(ref x) = x {
+            return Some(x);
+        }
+    }
+    return None;
+}
+
+/// Helper functions for getting attribute information from syn::{ForeignItem, Item}
 pub trait SynItemHelpers {
     fn has_attr(&self, target: MetaItem) -> bool;
     fn get_doc_attr(&self) -> String;
@@ -69,6 +72,7 @@ pub trait SynItemHelpers {
         Repr::None
     }
 }
+
 impl SynItemHelpers for Item {
     fn has_attr(&self, target: MetaItem) -> bool {
         return self.attrs
@@ -89,6 +93,7 @@ impl SynItemHelpers for Item {
         doc
     }
 }
+
 impl SynItemHelpers for ForeignItem {
     fn has_attr(&self, target: MetaItem) -> bool {
         return self.attrs
@@ -110,29 +115,34 @@ impl SynItemHelpers for ForeignItem {
     }
 }
 
+/// Helper function for accessing Abi information
 pub trait SynAbiHelpers {
     fn is_c(&self) -> bool;
 }
+
 impl SynAbiHelpers for Option<Abi> {
     fn is_c(&self) -> bool {
         self == &Some(Abi::Named(String::from("C")))
     }
 }
+
 impl SynAbiHelpers for Abi {
     fn is_c(&self) -> bool {
         self == &Abi::Named(String::from("C"))
     }
 }
 
+/// Helper function for loading a type from a syn::FnRetTy
 pub trait SynFnRetTyHelpers {
-    fn as_type(&self) -> ConvertResult<Type>;
+    fn as_type(&self) -> Result<Type, String>;
 }
+
 impl SynFnRetTyHelpers for FunctionRetTy {
-    fn as_type(&self) -> ConvertResult<Type> {
+    fn as_type(&self) -> Result<Type, String> {
         match self {
             &FunctionRetTy::Default => Ok(Type::Primitive(PrimitiveType::Void)),
             &FunctionRetTy::Ty(ref t) => {
-                if let Some(x) = try!(Type::convert(t)) {
+                if let Some(x) = Type::load(t)? {
                     Ok(x)
                 } else {
                     Ok(Type::Primitive(PrimitiveType::Void))
@@ -142,14 +152,16 @@ impl SynFnRetTyHelpers for FunctionRetTy {
     }
 }
 
+/// Helper function for loading an ident and type from a syn::FnArg
 pub trait SynFnArgHelpers {
-    fn as_ident_and_type(&self) -> ConvertResult<Option<(String, Type)>>;
+    fn as_ident_and_type(&self) -> Result<Option<(String, Type)>, String>;
 }
+
 impl SynFnArgHelpers for FnArg {
-    fn as_ident_and_type(&self) -> ConvertResult<Option<(String, Type)>> {
+    fn as_ident_and_type(&self) -> Result<Option<(String, Type)>, String> {
         match self {
             &FnArg::Captured(Pat::Ident(_, ref ident, _), ref ty) => {
-                if let Some(x) = try!(Type::convert(ty)) {
+                if let Some(x) = Type::load(ty)? {
                     Ok(Some((ident.to_string(), x)))
                 } else {
                     Ok(None)
@@ -160,13 +172,15 @@ impl SynFnArgHelpers for FnArg {
     }
 }
 
+/// Helper function for loading an ident and type from a syn::Field
 pub trait SynFieldHelpers {
-    fn as_ident_and_type(&self) -> ConvertResult<Option<(String, Type)>>;
+    fn as_ident_and_type(&self) -> Result<Option<(String, Type)>, String>;
 }
+
 impl SynFieldHelpers for Field {
-    fn as_ident_and_type(&self) -> ConvertResult<Option<(String, Type)>> {
-        let ident = try!(self.ident.as_ref().ok_or(format!("field is missing identifier"))).clone();
-        let converted_ty = try!(Type::convert(&self.ty));
+    fn as_ident_and_type(&self) -> Result<Option<(String, Type)>, String> {
+        let ident = self.ident.as_ref().ok_or(format!("field is missing identifier"))?.clone();
+        let converted_ty = Type::load(&self.ty)?;
 
         if let Some(x) = converted_ty {
             Ok(Some((ident.to_string(), x)))
@@ -176,11 +190,13 @@ impl SynFieldHelpers for Field {
     }
 }
 
+/// Helper function for loading a Path and generics from a syn::Path
 pub trait SynPathHelpers {
-    fn convert_to_generic_single_segment(&self) -> ConvertResult<(String, Vec<Type>)>;
+    fn convert_to_generic_single_segment(&self) -> Result<(String, Vec<Type>), String>;
 }
+
 impl SynPathHelpers for Path {
-    fn convert_to_generic_single_segment(&self) -> ConvertResult<(String, Vec<Type>)> {
+    fn convert_to_generic_single_segment(&self) -> Result<(String, Vec<Type>), String> {
         if self.segments.len() != 1 {
             return Err(format!("path contains more than one segment"));
         }
@@ -198,8 +214,8 @@ impl SynPathHelpers for Path {
                     return Err(format!("path generic parameter contains bindings, or lifetimes"));
                 }
 
-                try!(d.types.iter()
-                            .try_skip_map(|x| Type::convert(x)))
+                d.types.iter()
+                       .try_skip_map(|x| Type::load(x))?
             }
             &PathParameters::Parenthesized(_) => {
                 return Err(format!("path contains parentheses"));
diff --git a/src/bindgen/writer.rs b/src/bindgen/writer.rs
index 33ebd00..b7707d4 100644
--- a/src/bindgen/writer.rs
+++ b/src/bindgen/writer.rs
@@ -4,11 +4,16 @@ use std::io::Write;
 
 use bindgen::config::{Config, Braces};
 
+/// A type of way to format a list.
 pub enum ListType<'a> {
+    /// Join each adjacent item with a str.
     Join(&'a str),
+    /// End each item with a str.
     Cap(&'a str),
 }
 
+/// An empty file used for creating a null source writer and measuring line
+/// metrics for various code layouts.
 pub struct NullFile;
 impl Write for NullFile {
     fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
@@ -44,6 +49,8 @@ impl<'a, F: Write> SourceWriter<'a, F> {
         }
     }
 
+    /// Takes a function that writes source and returns the maximum line length
+    /// written.
     pub fn measure<T>(&self, func: T) -> usize
         where T: Fn(&mut MeasureWriter)
     {
diff --git a/src/main.rs b/src/main.rs
index 74ecfe9..7931339 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -52,19 +52,24 @@ fn main() {
                          .required(false))
                     .get_matches();
 
+    // Initialize logging
     match matches.occurrences_of("v") {
         0 => logging::WarnLogger::init().unwrap(),
         1 => logging::InfoLogger::init().unwrap(),
         _ => logging::TraceLogger::init().unwrap(),
     }
 
+    // Find the input directory
     let input = matches.value_of("INPUT").unwrap();
 
+    // Load any config specified or search in the input directory
     let mut config = match matches.value_of("config") {
         Some(c) => Config::from_file(c).unwrap(),
         None => Config::from_root_or_default(&input),
     };
 
+    // We allow specifying a language to override the config default. This is
+    // used by compile-tests.
     if let Some(lang) = matches.value_of("lang") {
         config.language = match lang {
             "c++"=> Language::Cxx,
@@ -76,6 +81,7 @@ fn main() {
         };
     }
 
+    // Load the library into memory
     let library = if Path::new(&input).is_dir() {
         let binding_crate = match matches.value_of("crate") {
             Some(binding_crate) => String::from(binding_crate),
@@ -105,6 +111,7 @@ fn main() {
         }
     };
 
+    // Generate a bindings file
     let built = match library.generate() {
         Ok(x) => x,
         Err(msg) => {
@@ -114,6 +121,7 @@ fn main() {
         },
     };
 
+    // Write the bindings file
     match matches.value_of("out") {
         Some(file) => {
             built.write_to_file(file);
-- 
GitLab