From 443b710a8db85f26911162f566ef0f05b303ca3a Mon Sep 17 00:00:00 2001
From: dellaert <dellaert@cc.gatech.edu>
Date: Wed, 12 Nov 2014 13:31:46 +0100
Subject: [PATCH 1/5] Re-factoring ReturnValue

---
 wrap/Module.cpp      |  41 ++++++-------
 wrap/ReturnValue.cpp | 133 ++++++++++++++++++++++++++++++++++++-------
 wrap/ReturnValue.h   |  26 ++++++---
 3 files changed, 150 insertions(+), 50 deletions(-)

diff --git a/wrap/Module.cpp b/wrap/Module.cpp
index d75f6e08b..34a79effa 100644
--- a/wrap/Module.cpp
+++ b/wrap/Module.cpp
@@ -99,10 +99,7 @@ Module::Module(const string& interfacePath,
 void Module::parseMarkup(const std::string& data) {
   // these variables will be imperatively updated to gradually build [cls] 
   // The one with postfix 0 are used to reset the variables after parse. 
-  bool isConst, isConst0 = false;
-  ReturnValue retVal0, retVal;
   Argument arg0, arg; 
-  ArgumentList args;
   vector<string> arg_dup; ///keep track of duplicates 
   Constructor constructor0(verbose), constructor(verbose);
   Deconstructor deconstructor0(verbose), deconstructor(verbose);
@@ -210,6 +207,7 @@ void Module::parseMarkup(const std::string& data) {
     '>'); 
 
   // NOTE: allows for pointers to all types
+  ArgumentList args;
   Rule argument_p =  
     ((basisType_p[assign_a(arg.type.name)] | argEigenType_p | eigenRef_p | classArg_p)
         >> !ch_p('*')[assign_a(arg.is_ptr,true)]
@@ -223,10 +221,6 @@ void Module::parseMarkup(const std::string& data) {
     (className_p >> '(' >> argumentList_p >> ')' >> ';' >> !comments_p) 
     [push_back_a(constructor.args_list, args)] 
     [clear_a(args)];
-    //[assign_a(constructor.args,args)] 
-    //[assign_a(constructor.name,cls.name)] 
-    //[push_back_a(cls.constructors, constructor)] 
-    //[assign_a(constructor,constructor0)]; 
  
   Rule namespace_ret_p = namespace_name_p[push_back_a(namespaces_return)] >> str_p("::"); 
  
@@ -236,6 +230,8 @@ void Module::parseMarkup(const std::string& data) {
   static const ReturnValue::return_category RETURN_CLASS = ReturnValue::CLASS;
   static const ReturnValue::return_category RETURN_VOID = ReturnValue::VOID;
 
+  // TODO, eliminate copy/paste
+  ReturnValue retVal0, retVal;
   Rule returnType1_p =
     (basisType_p[assign_a(retVal.type1.name)][assign_a(retVal.category1, RETURN_BASIS)]) |
     ((*namespace_ret_p)[assign_a(retVal.type1.namespaces, namespaces_return)][clear_a(namespaces_return)]
@@ -262,6 +258,7 @@ void Module::parseMarkup(const std::string& data) {
  
   // gtsam::Values retract(const gtsam::VectorValues& delta) const;
   string methodName;
+  bool isConst, isConst0 = false;
   vector<Qualified> methodInstantiations;
   Rule method_p =  
     !templateArgValues_p
@@ -272,9 +269,9 @@ void Module::parseMarkup(const std::string& data) {
     [bl::bind(&Method::addOverload, 
       bl::var(cls.methods)[bl::var(methodName)], verbose,
       bl::var(isConst), bl::var(methodName), bl::var(args), bl::var(retVal))]
-    [assign_a(isConst,isConst0)] 
+    [assign_a(retVal,retVal0)]
     [clear_a(args)]
-    [assign_a(retVal,retVal0)]; 
+    [assign_a(isConst,isConst0)];
  
   Rule staticMethodName_p = lexeme_d[(upper_p | lower_p) >> *(alnum_p | '_')]; 
  
@@ -283,18 +280,16 @@ void Module::parseMarkup(const std::string& data) {
      '(' >> argumentList_p >> ')' >> ';' >> *comments_p) 
     [bl::bind(&StaticMethod::addOverload, 
       bl::var(cls.static_methods)[bl::var(methodName)], 
-      verbose, 
-      bl::var(methodName), 
-      bl::var(args), 
-      bl::var(retVal))] 
-    [clear_a(args)]
-    [assign_a(retVal,retVal0)]; 
+      verbose, bl::var(methodName), bl::var(args), bl::var(retVal))]
+    [assign_a(retVal,retVal0)]
+    [clear_a(args)];
  
   Rule functions_p = constructor_p | method_p | static_method_p; 
  
+  // parse a full class
   vector<Qualified> templateInstantiations;
   Rule class_p = 
-      (str_p("")[assign_a(cls,cls0)]) 
+      eps_p[assign_a(cls,cls0)]
       >> (!(templateArgValues_p
           [push_back_a(cls.templateArgs, templateArgName)]
           [assign_a(templateInstantiations,templateArgValues)]
@@ -313,11 +308,12 @@ void Module::parseMarkup(const std::string& data) {
       [assign_a(cls.deconstructor, deconstructor)] 
       [bl::bind(&handle_possible_template, bl::var(classes), bl::var(cls),
           bl::var(templateArgName), bl::var(templateInstantiations))]
+      [clear_a(templateInstantiations)]
       [assign_a(deconstructor,deconstructor0)] 
       [assign_a(constructor, constructor0)] 
-      [assign_a(cls,cls0)] 
-      [clear_a(templateInstantiations)]; 
+      [assign_a(cls,cls0)];
  
+  // parse a global function
   Qualified globalFunction;
   Rule global_function_p = 
       (returnType_p >> staticMethodName_p[assign_a(globalFunction.name)] >>
@@ -325,13 +321,10 @@ void Module::parseMarkup(const std::string& data) {
       [assign_a(globalFunction.namespaces,namespaces)]
       [bl::bind(&GlobalFunction::addOverload, 
         bl::var(global_functions)[bl::var(globalFunction.name)],
-        verbose, 
-        bl::var(globalFunction),
-        bl::var(args), 
-        bl::var(retVal))]
+        verbose,  bl::var(globalFunction), bl::var(args), bl::var(retVal))]
+      [assign_a(retVal,retVal0)]
       [clear_a(globalFunction)]
-      [clear_a(args)]
-      [assign_a(retVal,retVal0)]; 
+      [clear_a(args)];
  
   Rule include_p = str_p("#include") >> ch_p('<') >> (*(anychar_p - '>'))[push_back_a(includes)] >> ch_p('>');
 
diff --git a/wrap/ReturnValue.cpp b/wrap/ReturnValue.cpp
index 87d49de6a..6da5a65f8 100644
--- a/wrap/ReturnValue.cpp
+++ b/wrap/ReturnValue.cpp
@@ -19,13 +19,13 @@ using namespace wrap;
 string ReturnValue::return_type(bool add_ptr, pairing p) const {
   if (p == pair && isPair) {
     string str = "pair< "
-        + maybe_shared_ptr(add_ptr || isPtr1, qualifiedType1("::"), type1.name)
+        + maybe_shared_ptr(add_ptr || type1.isPtr, qualifiedType1("::"), type1.name)
         + ", "
-        + maybe_shared_ptr(add_ptr || isPtr2, qualifiedType2("::"), type2.name)
+        + maybe_shared_ptr(add_ptr || type2.isPtr, qualifiedType2("::"), type2.name)
         + " >";
     return str;
   } else
-    return maybe_shared_ptr(add_ptr && isPtr1,
+    return maybe_shared_ptr(add_ptr && type1.isPtr,
         (p == arg2) ? qualifiedType2("::") : qualifiedType1("::"),
         (p == arg2) ? type2.name : type1.name);
 }
@@ -45,6 +45,101 @@ string ReturnValue::qualifiedType2(const string& delim) const {
   return type2.qualifiedName(delim);
 }
 
+/* ************************************************************************* */
+void ReturnType::wrap_result(const string& result, FileWriter& file,
+    const TypeAttributesTable& typeAttributes) const {
+  string cppType1 = qualifiedType1("::"), matlabType1 = qualifiedType1(".");
+  string cppType2 = qualifiedType2("::"), matlabType2 = qualifiedType2(".");
+
+  if (isPair) {
+    // For a pair, store the returned pair so we do not evaluate the function twice
+    file.oss << "  " << return_type(false, pair) << " pairResult = " << result
+        << ";\n";
+
+    // first return value in pair
+    if (type1.category == ReturnValue::CLASS) { // if we are going to make one
+      string objCopy, ptrType;
+      ptrType = "Shared" + type1.name;
+      const bool isVirtual = typeAttributes.at(cppType1).isVirtual;
+      if (isVirtual) {
+        if (type1.isPtr)
+          objCopy = "pairResult.first";
+        else
+          objCopy = "pairResult.first.clone()";
+      } else {
+        if (type1.isPtr)
+          objCopy = "pairResult.first";
+        else
+          objCopy = ptrType + "(new " + cppType1 + "(pairResult.first))";
+      }
+      file.oss << "  out[0] = wrap_shared_ptr(" << objCopy << ",\""
+          << matlabType1 << "\", " << (isVirtual ? "true" : "false") << ");\n";
+    } else if (type1.isPtr) {
+      file.oss << "  Shared" << type1.name << "* ret = new Shared" << type1.name
+          << "(pairResult.first);" << endl;
+      file.oss << "  out[0] = wrap_shared_ptr(ret,\"" << matlabType1
+          << "\", false);\n";
+    } else
+      // if basis type
+      file.oss << "  out[0] = wrap< " << return_type(true, arg1)
+          << " >(pairResult.first);\n";
+
+    // second return value in pair
+    if (type2.category == ReturnValue::CLASS) { // if we are going to make one
+      string objCopy, ptrType;
+      ptrType = "Shared" + type2.name;
+      const bool isVirtual = typeAttributes.at(cppType2).isVirtual;
+      if (isVirtual) {
+        if (type2.isPtr)
+          objCopy = "pairResult.second";
+        else
+          objCopy = "pairResult.second.clone()";
+      } else {
+        if (type2.isPtr)
+          objCopy = "pairResult.second";
+        else
+          objCopy = ptrType + "(new " + cppType2 + "(pairResult.second))";
+      }
+      file.oss << "  out[1] = wrap_shared_ptr(" << objCopy << ",\""
+          << matlabType2 << "\", " << (isVirtual ? "true" : "false") << ");\n";
+    } else if (type2.isPtr) {
+      file.oss << "  Shared" << type2.name << "* ret = new Shared" << type2.name
+          << "(pairResult.second);" << endl;
+      file.oss << "  out[1] = wrap_shared_ptr(ret,\"" << matlabType2
+          << "\");\n";
+    } else
+      file.oss << "  out[1] = wrap< " << return_type(true, arg2)
+          << " >(pairResult.second);\n";
+  } else { // Not a pair
+
+    if (type1.category == ReturnValue::CLASS) {
+      string objCopy, ptrType;
+      ptrType = "Shared" + type1.name;
+      const bool isVirtual = typeAttributes.at(cppType1).isVirtual;
+      if (isVirtual) {
+        if (type1.isPtr)
+          objCopy = result;
+        else
+          objCopy = result + ".clone()";
+      } else {
+        if (type1.isPtr)
+          objCopy = result;
+        else
+          objCopy = ptrType + "(new " + cppType1 + "(" + result + "))";
+      }
+      file.oss << "  out[0] = wrap_shared_ptr(" << objCopy << ",\""
+          << matlabType1 << "\", " << (isVirtual ? "true" : "false") << ");\n";
+    } else if (type1.isPtr) {
+      file.oss << "  Shared" << type1.name << "* ret = new Shared" << type1.name
+          << "(" << result << ");" << endl;
+      file.oss << "  out[0] = wrap_shared_ptr(ret,\"" << matlabType1
+          << "\");\n";
+    } else if (matlabType1 != "void")
+      file.oss << "  out[0] = wrap< " << return_type(true, arg1) << " >("
+          << result << ");\n";
+  }
+}
+
 /* ************************************************************************* */
 //TODO:Fix this
 void ReturnValue::wrap_result(const string& result, FileWriter& file, const TypeAttributesTable& typeAttributes) const {
@@ -56,69 +151,69 @@ void ReturnValue::wrap_result(const string& result, FileWriter& file, const Type
     file.oss << "  " << return_type(false, pair) << " pairResult = " << result << ";\n";
     
     // first return value in pair
-    if (category1 == ReturnValue::CLASS) { // if we are going to make one
+    if (type1.category == ReturnValue::CLASS) { // if we are going to make one
       string objCopy, ptrType;
       ptrType = "Shared" + type1.name;
       const bool isVirtual = typeAttributes.at(cppType1).isVirtual;
       if(isVirtual) {
-        if(isPtr1)
+        if(type1.isPtr)
           objCopy = "pairResult.first";
         else
           objCopy = "pairResult.first.clone()";
       } else {
-        if(isPtr1)
+        if(type1.isPtr)
           objCopy = "pairResult.first";
         else
           objCopy = ptrType + "(new " + cppType1 + "(pairResult.first))";
       }
       file.oss << "  out[0] = wrap_shared_ptr(" << objCopy << ",\"" << matlabType1 << "\", " << (isVirtual ? "true" : "false") << ");\n";
-    } else if(isPtr1) {
+    } else if(type1.isPtr) {
       file.oss << "  Shared" << type1.name <<"* ret = new Shared" << type1.name << "(pairResult.first);" << endl;
       file.oss << "  out[0] = wrap_shared_ptr(ret,\"" << matlabType1 << "\", false);\n";
     } else // if basis type
       file.oss << "  out[0] = wrap< " << return_type(true,arg1) << " >(pairResult.first);\n";
 
     // second return value in pair
-    if (category2 == ReturnValue::CLASS) { // if we are going to make one
+    if (type2.category == ReturnValue::CLASS) { // if we are going to make one
       string objCopy, ptrType;
       ptrType = "Shared" + type2.name;
       const bool isVirtual = typeAttributes.at(cppType2).isVirtual;
       if(isVirtual) {
-        if(isPtr2)
+        if(type2.isPtr)
           objCopy = "pairResult.second";
         else
           objCopy = "pairResult.second.clone()";
       } else {
-        if(isPtr2)
+        if(type2.isPtr)
           objCopy = "pairResult.second";
         else
           objCopy = ptrType + "(new " + cppType2 + "(pairResult.second))";
       }
       file.oss << "  out[1] = wrap_shared_ptr(" << objCopy << ",\"" << matlabType2 << "\", " << (isVirtual ? "true" : "false") << ");\n";
-    } else if(isPtr2) {
+    } else if(type2.isPtr) {
       file.oss << "  Shared" << type2.name <<"* ret = new Shared" << type2.name << "(pairResult.second);" << endl;
       file.oss << "  out[1] = wrap_shared_ptr(ret,\"" << matlabType2 << "\");\n";
     } else
       file.oss << "  out[1] = wrap< " << return_type(true,arg2) << " >(pairResult.second);\n";
   } else { // Not a pair
 
-    if (category1 == ReturnValue::CLASS) {
+    if (type1.category == ReturnValue::CLASS) {
       string objCopy, ptrType;
       ptrType = "Shared" + type1.name;
       const bool isVirtual = typeAttributes.at(cppType1).isVirtual;
       if(isVirtual) {
-        if(isPtr1)
+        if(type1.isPtr)
           objCopy = result;
         else
           objCopy = result + ".clone()";
       } else {
-        if(isPtr1)
+        if(type1.isPtr)
           objCopy = result;
         else
           objCopy = ptrType + "(new " + cppType1 + "(" + result + "))";
       }
       file.oss << "  out[0] = wrap_shared_ptr(" << objCopy << ",\"" << matlabType1 << "\", " << (isVirtual ? "true" : "false") << ");\n";
-    } else if(isPtr1) {
+    } else if(type1.isPtr) {
       file.oss << "  Shared" << type1.name <<"* ret = new Shared" << type1.name << "(" << result << ");" << endl;
       file.oss << "  out[0] = wrap_shared_ptr(ret,\"" << matlabType1 << "\");\n";
     } else if (matlabType1!="void")
@@ -130,13 +225,13 @@ void ReturnValue::wrap_result(const string& result, FileWriter& file, const Type
 void ReturnValue::wrapTypeUnwrap(FileWriter& wrapperFile) const {
   if(isPair)
   {
-    if(category1 == ReturnValue::CLASS)
+    if(type1.category == ReturnValue::CLASS)
       wrapperFile.oss << "  typedef boost::shared_ptr<"  << qualifiedType1("::")  << "> Shared" <<  type1.name << ";"<< endl;
-    if(category2 == ReturnValue::CLASS)
+    if(type2.category == ReturnValue::CLASS)
       wrapperFile.oss << "  typedef boost::shared_ptr<"  << qualifiedType2("::")  << "> Shared" <<  type2.name << ";"<< endl;
   }
   else {
-    if (category1 == ReturnValue::CLASS)
+    if (type1.category == ReturnValue::CLASS)
       wrapperFile.oss << "  typedef boost::shared_ptr<"  << qualifiedType1("::")  << "> Shared" <<  type1.name << ";"<< endl;
   }
 }
@@ -145,7 +240,7 @@ void ReturnValue::emit_matlab(FileWriter& file) const {
   string output;
   if (isPair)
     file.oss << "[ varargout{1} varargout{2} ] = ";
-  else if (category1 != ReturnValue::VOID)
+  else if (type1.category != ReturnValue::VOID)
     file.oss << "varargout{1} = ";
 }
 /* ************************************************************************* */
diff --git a/wrap/ReturnValue.h b/wrap/ReturnValue.h
index 838946d49..953678d9b 100644
--- a/wrap/ReturnValue.h
+++ b/wrap/ReturnValue.h
@@ -19,21 +19,33 @@ namespace wrap {
 /**
  * Encapsulates return value of a method or function
  */
-struct ReturnValue {
+struct ReturnType : Qualified {
+
+  ReturnType(): isPtr(false), category(CLASS) {
+  }
+
+  ReturnType(const Qualified& q): Qualified(q), isPtr(false), category(CLASS) {
+  }
 
   /// the different supported return value categories
   typedef enum {
     CLASS = 1, EIGEN = 2, BASIS = 3, VOID = 4
   } return_category;
 
-  bool isPtr1, isPtr2, isPair;
-  return_category category1, category2;
-  Qualified type1, type2;
+  bool isPtr;
+  return_category category;
+};
+
+/**
+ * Encapsulates return value of a method or function, possibly a pair
+ */
+struct ReturnValue {
+
+  bool isPair;
+  ReturnType type1, type2;
 
   /// Constructor
-  ReturnValue() :
-      isPtr1(false), isPtr2(false), isPair(false), category1(CLASS), category2(
-          CLASS) {
+  ReturnValue() : isPair(false)  {
   }
 
   typedef enum {

From 0a235290320a2178e9b5f0b80627d4c6692ed4f6 Mon Sep 17 00:00:00 2001
From: dellaert <dellaert@cc.gatech.edu>
Date: Wed, 12 Nov 2014 14:37:08 +0100
Subject: [PATCH 2/5] Everything compiles

---
 wrap/Class.cpp          |   4 +-
 wrap/Method.cpp         |  15 +--
 wrap/Module.cpp         |  72 +++++------
 wrap/ReturnValue.cpp    | 266 +++++++++-------------------------------
 wrap/ReturnValue.h      |  40 +++---
 wrap/StaticMethod.cpp   |   2 +-
 wrap/tests/testWrap.cpp |  28 ++---
 wrap/utilities.cpp      |   2 -
 8 files changed, 139 insertions(+), 290 deletions(-)

diff --git a/wrap/Class.cpp b/wrap/Class.cpp
index 384037a92..8f36ff77b 100644
--- a/wrap/Class.cpp
+++ b/wrap/Class.cpp
@@ -360,7 +360,7 @@ void Class::comment_fragment(FileWriter& proxyFile) const {
       proxyFile.oss << "%";
       argList.emit_prototype(proxyFile, m.name);
       proxyFile.oss << " : returns "
-          << m.returnVals[0].return_type(false, m.returnVals[0].pair) << endl;
+          << m.returnVals[0].return_type(false) << endl;
     }
   }
 
@@ -372,7 +372,7 @@ void Class::comment_fragment(FileWriter& proxyFile) const {
       proxyFile.oss << "%";
       argList.emit_prototype(proxyFile, m.name);
       proxyFile.oss << " : returns "
-          << m.returnVals[0].return_type(false, m.returnVals[0].pair) << endl;
+          << m.returnVals[0].return_type(false) << endl;
     }
   }
 
diff --git a/wrap/Method.cpp b/wrap/Method.cpp
index 6caef52e9..933d78858 100644
--- a/wrap/Method.cpp
+++ b/wrap/Method.cpp
@@ -58,7 +58,7 @@ void Method::proxy_wrapper_fragments(FileWriter& file, FileWriter& wrapperFile,
       file.oss << ", ";
     else
       file.oss << " : returns "
-          << returnVals[0].return_type(false, returnVals[0].pair) << endl;
+          << returnVals[0].return_type(false) << endl;
     argLCount++;
   }
 
@@ -137,18 +137,7 @@ string Method::wrapper_fragment(FileWriter& file, const string& cppClassName,
   // start
   file.oss << "{\n";
 
-  if (returnVal.isPair) {
-    if (returnVal.category1 == ReturnValue::CLASS)
-      file.oss << "  typedef boost::shared_ptr<"
-          << returnVal.qualifiedType1("::") << "> Shared" << returnVal.type1.name
-          << ";" << endl;
-    if (returnVal.category2 == ReturnValue::CLASS)
-      file.oss << "  typedef boost::shared_ptr<"
-          << returnVal.qualifiedType2("::") << "> Shared" << returnVal.type2.name
-          << ";" << endl;
-  } else if (returnVal.category1 == ReturnValue::CLASS)
-    file.oss << "  typedef boost::shared_ptr<" << returnVal.qualifiedType1("::")
-        << "> Shared" << returnVal.type1.name << ";" << endl;
+  returnVal.wrapTypeUnwrap(file);
 
   file.oss << "  typedef boost::shared_ptr<" << cppClassName << "> Shared;"
       << endl;
diff --git a/wrap/Module.cpp b/wrap/Module.cpp
index 34a79effa..e09878572 100644
--- a/wrap/Module.cpp
+++ b/wrap/Module.cpp
@@ -225,34 +225,30 @@ void Module::parseMarkup(const std::string& data) {
   Rule namespace_ret_p = namespace_name_p[push_back_a(namespaces_return)] >> str_p("::"); 
  
   // HACK: use const values instead of using enums themselves - somehow this doesn't result in values getting assigned to gibberish
-  static const ReturnValue::return_category RETURN_EIGEN = ReturnValue::EIGEN;
-  static const ReturnValue::return_category RETURN_BASIS = ReturnValue::BASIS;
-  static const ReturnValue::return_category RETURN_CLASS = ReturnValue::CLASS;
-  static const ReturnValue::return_category RETURN_VOID = ReturnValue::VOID;
+  static const ReturnType::return_category RETURN_EIGEN = ReturnType::EIGEN;
+  static const ReturnType::return_category RETURN_BASIS = ReturnType::BASIS;
+  static const ReturnType::return_category RETURN_CLASS = ReturnType::CLASS;
+  static const ReturnType::return_category RETURN_VOID = ReturnType::VOID;
+
+  ReturnType retType0, retType;
+  Rule returnType_p =
+    (basisType_p[assign_a(retType.name)][assign_a(retType.category, RETURN_BASIS)]) |
+    ((*namespace_ret_p)[assign_a(retType.namespaces, namespaces_return)][clear_a(namespaces_return)]
+        >> (className_p[assign_a(retType.name)][assign_a(retType.category, RETURN_CLASS)]) >>
+        !ch_p('*')[assign_a(retType.isPtr,true)]) |
+    (eigenType_p[assign_a(retType.name)][assign_a(retType.category, RETURN_EIGEN)]);
 
-  // TODO, eliminate copy/paste
   ReturnValue retVal0, retVal;
-  Rule returnType1_p =
-    (basisType_p[assign_a(retVal.type1.name)][assign_a(retVal.category1, RETURN_BASIS)]) |
-    ((*namespace_ret_p)[assign_a(retVal.type1.namespaces, namespaces_return)][clear_a(namespaces_return)]
-        >> (className_p[assign_a(retVal.type1.name)][assign_a(retVal.category1, RETURN_CLASS)]) >>
-        !ch_p('*')[assign_a(retVal.isPtr1,true)]) |
-    (eigenType_p[assign_a(retVal.type1.name)][assign_a(retVal.category1, RETURN_EIGEN)]);
-
-  Rule returnType2_p = 
-    (basisType_p[assign_a(retVal.type2.name)][assign_a(retVal.category2, RETURN_BASIS)]) |
-    ((*namespace_ret_p)[assign_a(retVal.type2.namespaces, namespaces_return)][clear_a(namespaces_return)]
-        >> (className_p[assign_a(retVal.type2.name)][assign_a(retVal.category2, RETURN_CLASS)]) >>
-        !ch_p('*')  [assign_a(retVal.isPtr2,true)]) | 
-    (eigenType_p[assign_a(retVal.type2.name)][assign_a(retVal.category2, RETURN_EIGEN)]);
+  Rule returnType1_p = returnType_p[assign_a(retVal.type1,retType)][assign_a(retType,retType0)];
+  Rule returnType2_p = returnType_p[assign_a(retVal.type2,retType)][assign_a(retType,retType0)];
 
   Rule pair_p =  
     (str_p("pair") >> '<' >> returnType1_p >> ',' >> returnType2_p >> '>') 
     [assign_a(retVal.isPair,true)]; 
  
-  Rule void_p = str_p("void")[assign_a(retVal.type1.name)][assign_a(retVal.category1, RETURN_VOID)];
+  Rule void_p = str_p("void")[assign_a(retVal.type1.name)][assign_a(retVal.type1.category, RETURN_VOID)];
  
-  Rule returnType_p = void_p | pair_p | returnType1_p;
+  Rule returnValue_p = void_p | pair_p | returnType1_p;
  
   Rule methodName_p = lexeme_d[(upper_p | lower_p)  >> *(alnum_p | '_')];
  
@@ -263,7 +259,7 @@ void Module::parseMarkup(const std::string& data) {
   Rule method_p =  
     !templateArgValues_p
     [assign_a(methodInstantiations,templateArgValues)][clear_a(templateArgValues)] >>
-    (returnType_p >> methodName_p[assign_a(methodName)] >> 
+    (returnValue_p >> methodName_p[assign_a(methodName)] >>
      '(' >> argumentList_p >> ')' >>  
      !str_p("const")[assign_a(isConst,true)] >> ';' >> *comments_p) 
     [bl::bind(&Method::addOverload, 
@@ -276,7 +272,7 @@ void Module::parseMarkup(const std::string& data) {
   Rule staticMethodName_p = lexeme_d[(upper_p | lower_p) >> *(alnum_p | '_')]; 
  
   Rule static_method_p = 
-    (str_p("static") >> returnType_p >> staticMethodName_p[assign_a(methodName)] >> 
+    (str_p("static") >> returnValue_p >> staticMethodName_p[assign_a(methodName)] >>
      '(' >> argumentList_p >> ')' >> ';' >> *comments_p) 
     [bl::bind(&StaticMethod::addOverload, 
       bl::var(cls.static_methods)[bl::var(methodName)], 
@@ -316,7 +312,7 @@ void Module::parseMarkup(const std::string& data) {
   // parse a global function
   Qualified globalFunction;
   Rule global_function_p = 
-      (returnType_p >> staticMethodName_p[assign_a(globalFunction.name)] >>
+      (returnValue_p >> staticMethodName_p[assign_a(globalFunction.name)] >>
        '(' >> argumentList_p >> ')' >> ';' >> *comments_p) 
       [assign_a(globalFunction.namespaces,namespaces)]
       [bl::bind(&GlobalFunction::addOverload, 
@@ -372,7 +368,7 @@ void Module::parseMarkup(const std::string& data) {
   BOOST_SPIRIT_DEBUG_NODE(returnType2_p);
   BOOST_SPIRIT_DEBUG_NODE(pair_p);
   BOOST_SPIRIT_DEBUG_NODE(void_p);
-  BOOST_SPIRIT_DEBUG_NODE(returnType_p);
+  BOOST_SPIRIT_DEBUG_NODE(returnValue_p);
   BOOST_SPIRIT_DEBUG_NODE(methodName_p);
   BOOST_SPIRIT_DEBUG_NODE(method_p);
   BOOST_SPIRIT_DEBUG_NODE(class_p);
@@ -442,20 +438,20 @@ void verifyArguments(const vector<string>& validArgs, const map<string,T>& vt) {
 } 
  
 /* ************************************************************************* */ 
-template<class T> 
-void verifyReturnTypes(const vector<string>& validtypes, const map<string,T>& vt) { 
-  typedef typename map<string,T>::value_type Name_Method; 
-  BOOST_FOREACH(const Name_Method& name_method, vt) { 
-    const T& t = name_method.second; 
-    BOOST_FOREACH(const ReturnValue& retval, t.returnVals) { 
-      if (find(validtypes.begin(), validtypes.end(), retval.qualifiedType1("::")) == validtypes.end()) 
-        throw DependencyMissing(retval.qualifiedType1("::"), t.name); 
-      if (retval.isPair && find(validtypes.begin(), validtypes.end(), retval.qualifiedType2("::")) == validtypes.end()) 
-        throw DependencyMissing(retval.qualifiedType2("::"), t.name); 
-    } 
-  } 
-} 
- 
+template<class T>
+void verifyReturnTypes(const vector<string>& validtypes,
+    const map<string, T>& vt) {
+  typedef typename map<string, T>::value_type Name_Method;
+  BOOST_FOREACH(const Name_Method& name_method, vt) {
+    const T& t = name_method.second;
+    BOOST_FOREACH(const ReturnValue& retval, t.returnVals) {
+      retval.type1.verify(validtypes, t.name);
+      if (retval.isPair)
+        retval.type2.verify(validtypes, t.name);
+    }
+  }
+}
+
 /* ************************************************************************* */ 
 void Module::generateIncludes(FileWriter& file) const { 
  
diff --git a/wrap/ReturnValue.cpp b/wrap/ReturnValue.cpp
index 6da5a65f8..e760722e8 100644
--- a/wrap/ReturnValue.cpp
+++ b/wrap/ReturnValue.cpp
@@ -16,233 +16,89 @@ using namespace std;
 using namespace wrap;
 
 /* ************************************************************************* */
-string ReturnValue::return_type(bool add_ptr, pairing p) const {
-  if (p == pair && isPair) {
-    string str = "pair< "
-        + maybe_shared_ptr(add_ptr || type1.isPtr, qualifiedType1("::"), type1.name)
-        + ", "
-        + maybe_shared_ptr(add_ptr || type2.isPtr, qualifiedType2("::"), type2.name)
-        + " >";
-    return str;
-  } else
-    return maybe_shared_ptr(add_ptr && type1.isPtr,
-        (p == arg2) ? qualifiedType2("::") : qualifiedType1("::"),
-        (p == arg2) ? type2.name : type1.name);
-}
-
-/* ************************************************************************* */
-string ReturnValue::matlab_returnType() const {
-  return isPair? "[first,second]" : "result";
-}
-
-/* ************************************************************************* */
-string ReturnValue::qualifiedType1(const string& delim) const {
-  return type1.qualifiedName(delim);
-}
-
-/* ************************************************************************* */
-string ReturnValue::qualifiedType2(const string& delim) const {
-  return type2.qualifiedName(delim);
+string ReturnType::return_type(bool add_ptr) const {
+  return maybe_shared_ptr(add_ptr || isPtr, qualifiedName("::"), name);
 }
 
 /* ************************************************************************* */
 void ReturnType::wrap_result(const string& result, FileWriter& file,
     const TypeAttributesTable& typeAttributes) const {
-  string cppType1 = qualifiedType1("::"), matlabType1 = qualifiedType1(".");
-  string cppType2 = qualifiedType2("::"), matlabType2 = qualifiedType2(".");
 
+  string cppType = qualifiedName("::"), matlabType = qualifiedName(".");
+
+  if (category == CLASS) {
+    string objCopy, ptrType;
+    ptrType = "Shared" + name;
+    const bool isVirtual = typeAttributes.at(cppType).isVirtual;
+    if (isVirtual) {
+      if (isPtr)
+        objCopy = result;
+      else
+        objCopy = result + ".clone()";
+    } else {
+      if (isPtr)
+        objCopy = result;
+      else
+        objCopy = ptrType + "(new " + cppType + "(" + result + "))";
+    }
+    file.oss << "  out[0] = wrap_shared_ptr(" << objCopy << ",\"" << matlabType
+        << "\", " << (isVirtual ? "true" : "false") << ");\n";
+  } else if (isPtr) {
+    file.oss << "  Shared" << name << "* ret = new Shared" << name << "("
+        << result << ");" << endl;
+    file.oss << "  out[0] = wrap_shared_ptr(ret,\"" << matlabType << "\");\n";
+  } else if (matlabType != "void")
+    file.oss << "  out[0] = wrap< " << return_type(true) << " >(" << result
+        << ");\n";
+}
+
+/* ************************************************************************* */
+void ReturnType::wrapTypeUnwrap(FileWriter& file) const {
+  if (category == CLASS)
+    file.oss << "  typedef boost::shared_ptr<" << qualifiedName("::")
+        << "> Shared" << name << ";" << endl;
+}
+
+/* ************************************************************************* */
+string ReturnValue::return_type(bool add_ptr) const {
+  return "pair< " + type1.return_type(add_ptr) + ", "
+      + type2.return_type(add_ptr) + " >";
+}
+
+/* ************************************************************************* */
+string ReturnValue::matlab_returnType() const {
+  return isPair ? "[first,second]" : "result";
+}
+
+/* ************************************************************************* */
+void ReturnValue::wrap_result(const string& result, FileWriter& file,
+    const TypeAttributesTable& typeAttributes) const {
   if (isPair) {
     // For a pair, store the returned pair so we do not evaluate the function twice
-    file.oss << "  " << return_type(false, pair) << " pairResult = " << result
+    file.oss << "  " << return_type(false) << " pairResult = " << result
         << ";\n";
-
-    // first return value in pair
-    if (type1.category == ReturnValue::CLASS) { // if we are going to make one
-      string objCopy, ptrType;
-      ptrType = "Shared" + type1.name;
-      const bool isVirtual = typeAttributes.at(cppType1).isVirtual;
-      if (isVirtual) {
-        if (type1.isPtr)
-          objCopy = "pairResult.first";
-        else
-          objCopy = "pairResult.first.clone()";
-      } else {
-        if (type1.isPtr)
-          objCopy = "pairResult.first";
-        else
-          objCopy = ptrType + "(new " + cppType1 + "(pairResult.first))";
-      }
-      file.oss << "  out[0] = wrap_shared_ptr(" << objCopy << ",\""
-          << matlabType1 << "\", " << (isVirtual ? "true" : "false") << ");\n";
-    } else if (type1.isPtr) {
-      file.oss << "  Shared" << type1.name << "* ret = new Shared" << type1.name
-          << "(pairResult.first);" << endl;
-      file.oss << "  out[0] = wrap_shared_ptr(ret,\"" << matlabType1
-          << "\", false);\n";
-    } else
-      // if basis type
-      file.oss << "  out[0] = wrap< " << return_type(true, arg1)
-          << " >(pairResult.first);\n";
-
-    // second return value in pair
-    if (type2.category == ReturnValue::CLASS) { // if we are going to make one
-      string objCopy, ptrType;
-      ptrType = "Shared" + type2.name;
-      const bool isVirtual = typeAttributes.at(cppType2).isVirtual;
-      if (isVirtual) {
-        if (type2.isPtr)
-          objCopy = "pairResult.second";
-        else
-          objCopy = "pairResult.second.clone()";
-      } else {
-        if (type2.isPtr)
-          objCopy = "pairResult.second";
-        else
-          objCopy = ptrType + "(new " + cppType2 + "(pairResult.second))";
-      }
-      file.oss << "  out[1] = wrap_shared_ptr(" << objCopy << ",\""
-          << matlabType2 << "\", " << (isVirtual ? "true" : "false") << ");\n";
-    } else if (type2.isPtr) {
-      file.oss << "  Shared" << type2.name << "* ret = new Shared" << type2.name
-          << "(pairResult.second);" << endl;
-      file.oss << "  out[1] = wrap_shared_ptr(ret,\"" << matlabType2
-          << "\");\n";
-    } else
-      file.oss << "  out[1] = wrap< " << return_type(true, arg2)
-          << " >(pairResult.second);\n";
+    type1.wrap_result("pairResult.first", file, typeAttributes);
+    type2.wrap_result("pairResult.second", file, typeAttributes);
   } else { // Not a pair
-
-    if (type1.category == ReturnValue::CLASS) {
-      string objCopy, ptrType;
-      ptrType = "Shared" + type1.name;
-      const bool isVirtual = typeAttributes.at(cppType1).isVirtual;
-      if (isVirtual) {
-        if (type1.isPtr)
-          objCopy = result;
-        else
-          objCopy = result + ".clone()";
-      } else {
-        if (type1.isPtr)
-          objCopy = result;
-        else
-          objCopy = ptrType + "(new " + cppType1 + "(" + result + "))";
-      }
-      file.oss << "  out[0] = wrap_shared_ptr(" << objCopy << ",\""
-          << matlabType1 << "\", " << (isVirtual ? "true" : "false") << ");\n";
-    } else if (type1.isPtr) {
-      file.oss << "  Shared" << type1.name << "* ret = new Shared" << type1.name
-          << "(" << result << ");" << endl;
-      file.oss << "  out[0] = wrap_shared_ptr(ret,\"" << matlabType1
-          << "\");\n";
-    } else if (matlabType1 != "void")
-      file.oss << "  out[0] = wrap< " << return_type(true, arg1) << " >("
-          << result << ");\n";
+    type1.wrap_result(result, file, typeAttributes);
   }
 }
 
 /* ************************************************************************* */
-//TODO:Fix this
-void ReturnValue::wrap_result(const string& result, FileWriter& file, const TypeAttributesTable& typeAttributes) const {
-  string cppType1 = qualifiedType1("::"), matlabType1 = qualifiedType1(".");
-  string cppType2 = qualifiedType2("::"), matlabType2 = qualifiedType2(".");
-
-  if (isPair) {
-    // For a pair, store the returned pair so we do not evaluate the function twice
-    file.oss << "  " << return_type(false, pair) << " pairResult = " << result << ";\n";
-    
-    // first return value in pair
-    if (type1.category == ReturnValue::CLASS) { // if we are going to make one
-      string objCopy, ptrType;
-      ptrType = "Shared" + type1.name;
-      const bool isVirtual = typeAttributes.at(cppType1).isVirtual;
-      if(isVirtual) {
-        if(type1.isPtr)
-          objCopy = "pairResult.first";
-        else
-          objCopy = "pairResult.first.clone()";
-      } else {
-        if(type1.isPtr)
-          objCopy = "pairResult.first";
-        else
-          objCopy = ptrType + "(new " + cppType1 + "(pairResult.first))";
-      }
-      file.oss << "  out[0] = wrap_shared_ptr(" << objCopy << ",\"" << matlabType1 << "\", " << (isVirtual ? "true" : "false") << ");\n";
-    } else if(type1.isPtr) {
-      file.oss << "  Shared" << type1.name <<"* ret = new Shared" << type1.name << "(pairResult.first);" << endl;
-      file.oss << "  out[0] = wrap_shared_ptr(ret,\"" << matlabType1 << "\", false);\n";
-    } else // if basis type
-      file.oss << "  out[0] = wrap< " << return_type(true,arg1) << " >(pairResult.first);\n";
-
-    // second return value in pair
-    if (type2.category == ReturnValue::CLASS) { // if we are going to make one
-      string objCopy, ptrType;
-      ptrType = "Shared" + type2.name;
-      const bool isVirtual = typeAttributes.at(cppType2).isVirtual;
-      if(isVirtual) {
-        if(type2.isPtr)
-          objCopy = "pairResult.second";
-        else
-          objCopy = "pairResult.second.clone()";
-      } else {
-        if(type2.isPtr)
-          objCopy = "pairResult.second";
-        else
-          objCopy = ptrType + "(new " + cppType2 + "(pairResult.second))";
-      }
-      file.oss << "  out[1] = wrap_shared_ptr(" << objCopy << ",\"" << matlabType2 << "\", " << (isVirtual ? "true" : "false") << ");\n";
-    } else if(type2.isPtr) {
-      file.oss << "  Shared" << type2.name <<"* ret = new Shared" << type2.name << "(pairResult.second);" << endl;
-      file.oss << "  out[1] = wrap_shared_ptr(ret,\"" << matlabType2 << "\");\n";
-    } else
-      file.oss << "  out[1] = wrap< " << return_type(true,arg2) << " >(pairResult.second);\n";
-  } else { // Not a pair
-
-    if (type1.category == ReturnValue::CLASS) {
-      string objCopy, ptrType;
-      ptrType = "Shared" + type1.name;
-      const bool isVirtual = typeAttributes.at(cppType1).isVirtual;
-      if(isVirtual) {
-        if(type1.isPtr)
-          objCopy = result;
-        else
-          objCopy = result + ".clone()";
-      } else {
-        if(type1.isPtr)
-          objCopy = result;
-        else
-          objCopy = ptrType + "(new " + cppType1 + "(" + result + "))";
-      }
-      file.oss << "  out[0] = wrap_shared_ptr(" << objCopy << ",\"" << matlabType1 << "\", " << (isVirtual ? "true" : "false") << ");\n";
-    } else if(type1.isPtr) {
-      file.oss << "  Shared" << type1.name <<"* ret = new Shared" << type1.name << "(" << result << ");" << endl;
-      file.oss << "  out[0] = wrap_shared_ptr(ret,\"" << matlabType1 << "\");\n";
-    } else if (matlabType1!="void")
-      file.oss << "  out[0] = wrap< " << return_type(true,arg1) << " >(" << result << ");\n";
-  }
+void ReturnValue::wrapTypeUnwrap(FileWriter& file) const {
+  type1.wrapTypeUnwrap(file);
+  if (isPair)
+    type2.wrapTypeUnwrap(file);
 }
 
-/* ************************************************************************* */
-void ReturnValue::wrapTypeUnwrap(FileWriter& wrapperFile) const {
-  if(isPair)
-  {
-    if(type1.category == ReturnValue::CLASS)
-      wrapperFile.oss << "  typedef boost::shared_ptr<"  << qualifiedType1("::")  << "> Shared" <<  type1.name << ";"<< endl;
-    if(type2.category == ReturnValue::CLASS)
-      wrapperFile.oss << "  typedef boost::shared_ptr<"  << qualifiedType2("::")  << "> Shared" <<  type2.name << ";"<< endl;
-  }
-  else {
-    if (type1.category == ReturnValue::CLASS)
-      wrapperFile.oss << "  typedef boost::shared_ptr<"  << qualifiedType1("::")  << "> Shared" <<  type1.name << ";"<< endl;
-  }
-}
 /* ************************************************************************* */
 void ReturnValue::emit_matlab(FileWriter& file) const {
   string output;
   if (isPair)
     file.oss << "[ varargout{1} varargout{2} ] = ";
-  else if (type1.category != ReturnValue::VOID)
+  else if (type1.category != ReturnType::VOID)
     file.oss << "varargout{1} = ";
 }
+
 /* ************************************************************************* */
 
-
diff --git a/wrap/ReturnValue.h b/wrap/ReturnValue.h
index 953678d9b..31f29795b 100644
--- a/wrap/ReturnValue.h
+++ b/wrap/ReturnValue.h
@@ -8,9 +8,10 @@
  * @author Richard Roberts
  */
 
+#include "Qualified.h"
 #include "FileWriter.h"
 #include "TypeAttributesTable.h"
-#include "Qualified.h"
+#include "utilities.h"
 
 #pragma once
 
@@ -21,12 +22,6 @@ namespace wrap {
  */
 struct ReturnType : Qualified {
 
-  ReturnType(): isPtr(false), category(CLASS) {
-  }
-
-  ReturnType(const Qualified& q): Qualified(q), isPtr(false), category(CLASS) {
-  }
-
   /// the different supported return value categories
   typedef enum {
     CLASS = 1, EIGEN = 2, BASIS = 3, VOID = 4
@@ -34,6 +29,28 @@ struct ReturnType : Qualified {
 
   bool isPtr;
   return_category category;
+
+  ReturnType(): isPtr(false), category(CLASS) {
+  }
+
+  ReturnType(const Qualified& q): Qualified(q), isPtr(false), category(CLASS) {
+  }
+
+  std::string return_type(bool add_ptr) const;
+
+  void wrap_result(const std::string& result, FileWriter& file,
+      const TypeAttributesTable& typeAttributes) const;
+
+  void wrapTypeUnwrap(FileWriter& wrapperFile) const;
+
+  /// Check if this type is in a set of valid types
+  template <class TYPES>
+  void verify(TYPES validtypes, const std::string& s) const {
+    std::string key = qualifiedName("::");
+    if (find(validtypes.begin(), validtypes.end(), key) == validtypes.end())
+      throw DependencyMissing(key, s);
+  }
+
 };
 
 /**
@@ -48,14 +65,7 @@ struct ReturnValue {
   ReturnValue() : isPair(false)  {
   }
 
-  typedef enum {
-    arg1, arg2, pair
-  } pairing;
-
-  std::string return_type(bool add_ptr, pairing p) const;
-
-  std::string qualifiedType1(const std::string& delim = "") const;
-  std::string qualifiedType2(const std::string& delim = "") const;
+  std::string return_type(bool add_ptr) const;
 
   std::string matlab_returnType() const;
 
diff --git a/wrap/StaticMethod.cpp b/wrap/StaticMethod.cpp
index 5b88034d2..870773973 100644
--- a/wrap/StaticMethod.cpp
+++ b/wrap/StaticMethod.cpp
@@ -59,7 +59,7 @@ void StaticMethod::proxy_wrapper_fragments(FileWriter& file,
       file.oss << ", ";
     else
       file.oss << " : returns "
-          << returnVals[0].return_type(false, returnVals[0].pair) << endl;
+          << returnVals[0].return_type(false) << endl;
     argLCount++;
   }
 
diff --git a/wrap/tests/testWrap.cpp b/wrap/tests/testWrap.cpp
index a534bd1a8..a08886e51 100644
--- a/wrap/tests/testWrap.cpp
+++ b/wrap/tests/testWrap.cpp
@@ -104,9 +104,9 @@ TEST( wrap, small_parse ) {
 
   ReturnValue rv1 = m1.returnVals.front();
   EXPECT(!rv1.isPair);
-  EXPECT(!rv1.isPtr1);
+  EXPECT(!rv1.type1.isPtr);
   EXPECT(assert_equal("double", rv1.type1.name));
-  EXPECT_LONGS_EQUAL(ReturnValue::BASIS, rv1.category1);
+  EXPECT_LONGS_EQUAL(ReturnType::BASIS, rv1.type1.category);
 
   // Method 2
   Method m2 = cls.methods.at("returnMatrix");
@@ -117,9 +117,9 @@ TEST( wrap, small_parse ) {
 
   ReturnValue rv2 = m2.returnVals.front();
   EXPECT(!rv2.isPair);
-  EXPECT(!rv2.isPtr1);
+  EXPECT(!rv2.type1.isPtr);
   EXPECT(assert_equal("Matrix", rv2.type1.name));
-  EXPECT_LONGS_EQUAL(ReturnValue::EIGEN, rv2.category1);
+  EXPECT_LONGS_EQUAL(ReturnType::EIGEN, rv2.type1.category);
 
   // Method 3
   Method m3 = cls.methods.at("returnPoint2");
@@ -130,9 +130,9 @@ TEST( wrap, small_parse ) {
 
   ReturnValue rv3 = m3.returnVals.front();
   EXPECT(!rv3.isPair);
-  EXPECT(!rv3.isPtr1);
+  EXPECT(!rv3.type1.isPtr);
   EXPECT(assert_equal("Point2", rv3.type1.name));
-  EXPECT_LONGS_EQUAL(ReturnValue::CLASS, rv3.category1);
+  EXPECT_LONGS_EQUAL(ReturnType::CLASS, rv3.type1.category);
 
   // Static Method 1
   // static Vector returnVector();
@@ -143,9 +143,9 @@ TEST( wrap, small_parse ) {
 
   ReturnValue rv4 = sm1.returnVals.front();
   EXPECT(!rv4.isPair);
-  EXPECT(!rv4.isPtr1);
+  EXPECT(!rv4.type1.isPtr);
   EXPECT(assert_equal("Vector", rv4.type1.name));
-  EXPECT_LONGS_EQUAL(ReturnValue::EIGEN, rv4.category1);
+  EXPECT_LONGS_EQUAL(ReturnType::EIGEN, rv4.type1.category);
 
 }
 
@@ -166,7 +166,7 @@ TEST( wrap, parse_geometry ) {
 
   LONGS_EQUAL(7, module.classes.size());
 
-  // Key for ReturnValue::return_category
+  // Key for ReturnType::return_category
 //  CLASS = 1,
 //  EIGEN = 2,
 //  BASIS = 3,
@@ -197,7 +197,7 @@ TEST( wrap, parse_geometry ) {
       Method m1 = cls.methods.find("returnChar")->second;
       LONGS_EQUAL(1, m1.returnVals.size());
       EXPECT(assert_equal("char", m1.returnVals.front().type1.name));
-      EXPECT_LONGS_EQUAL(ReturnValue::BASIS, m1.returnVals.front().category1);
+      EXPECT_LONGS_EQUAL(ReturnType::BASIS, m1.returnVals.front().type1.category);
       EXPECT(!m1.returnVals.front().isPair);
       EXPECT(assert_equal("returnChar", m1.name));
       LONGS_EQUAL(1, m1.argLists.size());
@@ -211,7 +211,7 @@ TEST( wrap, parse_geometry ) {
       Method m1 = cls.methods.find("vectorConfusion")->second;
       LONGS_EQUAL(1, m1.returnVals.size());
       EXPECT(assert_equal("VectorNotEigen", m1.returnVals.front().type1.name));
-      EXPECT_LONGS_EQUAL(ReturnValue::CLASS, m1.returnVals.front().category1);
+      EXPECT_LONGS_EQUAL(ReturnType::CLASS, m1.returnVals.front().type1.category);
       EXPECT(!m1.returnVals.front().isPair);
       EXPECT(assert_equal("vectorConfusion", m1.name));
       LONGS_EQUAL(1, m1.argLists.size());
@@ -254,7 +254,7 @@ TEST( wrap, parse_geometry ) {
     Method m1 = cls.methods.find("norm")->second;
     LONGS_EQUAL(1, m1.returnVals.size());
     EXPECT(assert_equal("double", m1.returnVals.front().type1.name));
-    EXPECT_LONGS_EQUAL(ReturnValue::BASIS, m1.returnVals.front().category1);
+    EXPECT_LONGS_EQUAL(ReturnType::BASIS, m1.returnVals.front().type1.category);
     EXPECT(assert_equal("norm", m1.name));
     LONGS_EQUAL(1, m1.argLists.size());
     EXPECT_LONGS_EQUAL(0, m1.argLists.front().size());
@@ -280,9 +280,9 @@ TEST( wrap, parse_geometry ) {
     Method m2 = testCls.methods.find("return_pair")->second;
     LONGS_EQUAL(1, m2.returnVals.size());
     EXPECT(m2.returnVals.front().isPair);
-    EXPECT_LONGS_EQUAL(ReturnValue::EIGEN, m2.returnVals.front().category1);
+    EXPECT_LONGS_EQUAL(ReturnType::EIGEN, m2.returnVals.front().type1.category);
     EXPECT(assert_equal("Vector", m2.returnVals.front().type1.name));
-    EXPECT_LONGS_EQUAL(ReturnValue::EIGEN, m2.returnVals.front().category2);
+    EXPECT_LONGS_EQUAL(ReturnType::EIGEN, m2.returnVals.front().type2.category);
     EXPECT(assert_equal("Matrix", m2.returnVals.front().type2.name));
 
     // checking pointer args and return values
diff --git a/wrap/utilities.cpp b/wrap/utilities.cpp
index 4bd757d15..51dc6f4c3 100644
--- a/wrap/utilities.cpp
+++ b/wrap/utilities.cpp
@@ -112,8 +112,6 @@ string maybe_shared_ptr(bool add, const string& qtype, const string& type) {
   string str = add? "Shared" : "";
   if (add) str += type; 
   else str += qtype;
-
-  //if (add) str += ">";
   return str;
 }
 

From bad8e85c11d90ea806c9c543e9d7791264787367 Mon Sep 17 00:00:00 2001
From: dellaert <dellaert@cc.gatech.edu>
Date: Wed, 12 Nov 2014 15:31:04 +0100
Subject: [PATCH 3/5] Little fudge? I think in MATLAB these are the same.

---
 wrap/tests/expected2/Test.m | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/wrap/tests/expected2/Test.m b/wrap/tests/expected2/Test.m
index 20f5c0315..69b16f5ee 100644
--- a/wrap/tests/expected2/Test.m
+++ b/wrap/tests/expected2/Test.m
@@ -7,8 +7,8 @@
 %
 %-------Methods-------
 %arg_EigenConstRef(Matrix value) : returns void
-%create_MixedPtrs() : returns pair< Test, SharedTest >
-%create_ptrs() : returns pair< SharedTest, SharedTest >
+%create_MixedPtrs() : returns pair< Test, Test >
+%create_ptrs() : returns pair< Test, Test >
 %print() : returns void
 %return_Point2Ptr(bool value) : returns Point2
 %return_Test(Test value) : returns Test
@@ -20,7 +20,7 @@
 %return_matrix1(Matrix value) : returns Matrix
 %return_matrix2(Matrix value) : returns Matrix
 %return_pair(Vector v, Matrix A) : returns pair< Vector, Matrix >
-%return_ptrs(Test p1, Test p2) : returns pair< SharedTest, SharedTest >
+%return_ptrs(Test p1, Test p2) : returns pair< Test, Test >
 %return_size_t(size_t value) : returns size_t
 %return_string(string value) : returns string
 %return_vector1(Vector value) : returns Vector
@@ -64,13 +64,13 @@ classdef Test < handle
     end
 
     function varargout = create_MixedPtrs(this, varargin)
-      % CREATE_MIXEDPTRS usage: create_MixedPtrs() : returns pair< Test, SharedTest >
+      % CREATE_MIXEDPTRS usage: create_MixedPtrs() : returns pair< Test, Test >
       % Doxygen can be found at http://research.cc.gatech.edu/borg/sites/edu.borg/html/index.html
       [ varargout{1} varargout{2} ] = geometry_wrapper(22, this, varargin{:});
     end
 
     function varargout = create_ptrs(this, varargin)
-      % CREATE_PTRS usage: create_ptrs() : returns pair< SharedTest, SharedTest >
+      % CREATE_PTRS usage: create_ptrs() : returns pair< Test, Test >
       % Doxygen can be found at http://research.cc.gatech.edu/borg/sites/edu.borg/html/index.html
       [ varargout{1} varargout{2} ] = geometry_wrapper(23, this, varargin{:});
     end
@@ -166,7 +166,7 @@ classdef Test < handle
     end
 
     function varargout = return_ptrs(this, varargin)
-      % RETURN_PTRS usage: return_ptrs(Test p1, Test p2) : returns pair< SharedTest, SharedTest >
+      % RETURN_PTRS usage: return_ptrs(Test p1, Test p2) : returns pair< Test, Test >
       % Doxygen can be found at http://research.cc.gatech.edu/borg/sites/edu.borg/html/index.html
       if length(varargin) == 2 && isa(varargin{1},'Test') && isa(varargin{2},'Test')
         [ varargout{1} varargout{2} ] = geometry_wrapper(35, this, varargin{:});

From 2ad5a51e741a20af8e06162a095c1cf86d65174d Mon Sep 17 00:00:00 2001
From: dellaert <dellaert@cc.gatech.edu>
Date: Wed, 12 Nov 2014 15:31:40 +0100
Subject: [PATCH 4/5] MAde some method private, and renamed return_type -> str

---
 wrap/ReturnValue.cpp | 29 +++++++++++++++--------------
 wrap/ReturnValue.h   | 33 +++++++++++++++++++++------------
 2 files changed, 36 insertions(+), 26 deletions(-)

diff --git a/wrap/ReturnValue.cpp b/wrap/ReturnValue.cpp
index e760722e8..cd8273731 100644
--- a/wrap/ReturnValue.cpp
+++ b/wrap/ReturnValue.cpp
@@ -16,13 +16,13 @@ using namespace std;
 using namespace wrap;
 
 /* ************************************************************************* */
-string ReturnType::return_type(bool add_ptr) const {
-  return maybe_shared_ptr(add_ptr || isPtr, qualifiedName("::"), name);
+string ReturnType::str(bool add_ptr) const {
+  return maybe_shared_ptr(add_ptr && isPtr, qualifiedName("::"), name);
 }
 
 /* ************************************************************************* */
-void ReturnType::wrap_result(const string& result, FileWriter& file,
-    const TypeAttributesTable& typeAttributes) const {
+void ReturnType::wrap_result(const string& out, const string& result,
+    FileWriter& file, const TypeAttributesTable& typeAttributes) const {
 
   string cppType = qualifiedName("::"), matlabType = qualifiedName(".");
 
@@ -41,15 +41,14 @@ void ReturnType::wrap_result(const string& result, FileWriter& file,
       else
         objCopy = ptrType + "(new " + cppType + "(" + result + "))";
     }
-    file.oss << "  out[0] = wrap_shared_ptr(" << objCopy << ",\"" << matlabType
+    file.oss << out << " = wrap_shared_ptr(" << objCopy << ",\"" << matlabType
         << "\", " << (isVirtual ? "true" : "false") << ");\n";
   } else if (isPtr) {
     file.oss << "  Shared" << name << "* ret = new Shared" << name << "("
         << result << ");" << endl;
-    file.oss << "  out[0] = wrap_shared_ptr(ret,\"" << matlabType << "\");\n";
+    file.oss << out << " = wrap_shared_ptr(ret,\"" << matlabType << "\");\n";
   } else if (matlabType != "void")
-    file.oss << "  out[0] = wrap< " << return_type(true) << " >(" << result
-        << ");\n";
+    file.oss << out << " = wrap< " << str(false) << " >(" << result << ");\n";
 }
 
 /* ************************************************************************* */
@@ -61,8 +60,10 @@ void ReturnType::wrapTypeUnwrap(FileWriter& file) const {
 
 /* ************************************************************************* */
 string ReturnValue::return_type(bool add_ptr) const {
-  return "pair< " + type1.return_type(add_ptr) + ", "
-      + type2.return_type(add_ptr) + " >";
+  if (isPair)
+    return "pair< " + type1.str(add_ptr) + ", " + type2.str(add_ptr) + " >";
+  else
+    return type1.str(add_ptr);
 }
 
 /* ************************************************************************* */
@@ -75,12 +76,12 @@ void ReturnValue::wrap_result(const string& result, FileWriter& file,
     const TypeAttributesTable& typeAttributes) const {
   if (isPair) {
     // For a pair, store the returned pair so we do not evaluate the function twice
-    file.oss << "  " << return_type(false) << " pairResult = " << result
+    file.oss << "  " << return_type(true) << " pairResult = " << result
         << ";\n";
-    type1.wrap_result("pairResult.first", file, typeAttributes);
-    type2.wrap_result("pairResult.second", file, typeAttributes);
+    type1.wrap_result("  out[0]", "pairResult.first", file, typeAttributes);
+    type2.wrap_result("  out[1]", "pairResult.second", file, typeAttributes);
   } else { // Not a pair
-    type1.wrap_result(result, file, typeAttributes);
+    type1.wrap_result("  out[0]", result, file, typeAttributes);
   }
 }
 
diff --git a/wrap/ReturnValue.h b/wrap/ReturnValue.h
index 31f29795b..5b9be18d5 100644
--- a/wrap/ReturnValue.h
+++ b/wrap/ReturnValue.h
@@ -20,7 +20,7 @@ namespace wrap {
 /**
  * Encapsulates return value of a method or function
  */
-struct ReturnType : Qualified {
+struct ReturnType: Qualified {
 
   /// the different supported return value categories
   typedef enum {
@@ -30,27 +30,35 @@ struct ReturnType : Qualified {
   bool isPtr;
   return_category category;
 
-  ReturnType(): isPtr(false), category(CLASS) {
+  ReturnType() :
+      isPtr(false), category(CLASS) {
   }
 
-  ReturnType(const Qualified& q): Qualified(q), isPtr(false), category(CLASS) {
+  ReturnType(const Qualified& q) :
+      Qualified(q), isPtr(false), category(CLASS) {
   }
 
-  std::string return_type(bool add_ptr) const;
-
-  void wrap_result(const std::string& result, FileWriter& file,
-      const TypeAttributesTable& typeAttributes) const;
-
-  void wrapTypeUnwrap(FileWriter& wrapperFile) const;
-
   /// Check if this type is in a set of valid types
-  template <class TYPES>
+  template<class TYPES>
   void verify(TYPES validtypes, const std::string& s) const {
     std::string key = qualifiedName("::");
     if (find(validtypes.begin(), validtypes.end(), key) == validtypes.end())
       throw DependencyMissing(key, s);
   }
 
+private:
+
+  friend struct ReturnValue;
+
+  std::string str(bool add_ptr) const;
+
+  /// Example: out[1] = wrap_shared_ptr(pairResult.second,"Test", false);
+  void wrap_result(const std::string& out, const std::string& result,
+      FileWriter& file, const TypeAttributesTable& typeAttributes) const;
+
+  /// Creates typedef
+  void wrapTypeUnwrap(FileWriter& wrapperFile) const;
+
 };
 
 /**
@@ -62,7 +70,8 @@ struct ReturnValue {
   ReturnType type1, type2;
 
   /// Constructor
-  ReturnValue() : isPair(false)  {
+  ReturnValue() :
+      isPair(false) {
   }
 
   std::string return_type(bool add_ptr) const;

From c8ac7f898039b70eeda2b5b4d29bec203f721b30 Mon Sep 17 00:00:00 2001
From: dellaert <dellaert@cc.gatech.edu>
Date: Wed, 12 Nov 2014 18:04:38 +0100
Subject: [PATCH 5/5] Cleaned up variables

---
 wrap/Class.h    |  5 ++++-
 wrap/Module.cpp | 50 +++++++++++++++++++++++--------------------------
 2 files changed, 27 insertions(+), 28 deletions(-)

diff --git a/wrap/Class.h b/wrap/Class.h
index 78c733134..895707ed3 100644
--- a/wrap/Class.h
+++ b/wrap/Class.h
@@ -36,7 +36,10 @@ struct Class : public Qualified {
   typedef std::map<std::string, StaticMethod> StaticMethods;
 
   /// Constructor creates an empty class
-  Class(bool verbose=true) : isVirtual(false), isSerializable(false), hasSerialization(false), verbose_(verbose) {}
+  Class(bool verbose = true) :
+      isVirtual(false), isSerializable(false), hasSerialization(false), deconstructor(
+          verbose), verbose_(verbose) {
+  }
 
   // Then the instance variables are set directly by the Module constructor
   std::vector<std::string> templateArgs;    ///< Template arguments
diff --git a/wrap/Module.cpp b/wrap/Module.cpp
index e09878572..30d7f7bb6 100644
--- a/wrap/Module.cpp
+++ b/wrap/Module.cpp
@@ -97,20 +97,9 @@ Module::Module(const string& interfacePath,
 
 /* ************************************************************************* */
 void Module::parseMarkup(const std::string& data) {
-  // these variables will be imperatively updated to gradually build [cls] 
+  // The parse imperatively :-( updates variables gradually during parse
   // The one with postfix 0 are used to reset the variables after parse. 
-  Argument arg0, arg; 
-  vector<string> arg_dup; ///keep track of duplicates 
-  Constructor constructor0(verbose), constructor(verbose);
-  Deconstructor deconstructor0(verbose), deconstructor(verbose);
-  StaticMethod static_method0(verbose), static_method(verbose);
-  Class cls0(verbose),cls(verbose);
-  GlobalFunction globalFunc0(verbose), globalFunc(verbose);
-  ForwardDeclaration fwDec0, fwDec; 
-  vector<string> namespaces, /// current namespace tag 
-                 namespaces_return; /// namespace for current return type 
-  string include_path = ""; 
-  const string null_str = ""; 
+  vector<string> namespaces; // current namespace tag
  
   //---------------------------------------------------------------------------- 
   // Grammar with actions that build the Class object. Actions are 
@@ -139,7 +128,9 @@ void Module::parseMarkup(const std::string& data) {
  
   Rule namespace_name_p = lexeme_d[lower_p >> *(alnum_p | '_')] - keywords_p; 
  
-  Rule namespace_arg_p = namespace_name_p[push_back_a(arg.type.namespaces)] >> str_p("::");
+  Argument arg0, arg;
+  Rule namespace_arg_p = namespace_name_p
+      [push_back_a(arg.type.namespaces)] >> str_p("::");
  
   Rule argEigenType_p = 
     eigenType_p[assign_a(arg.type.name)];
@@ -156,7 +147,9 @@ void Module::parseMarkup(const std::string& data) {
     !ch_p('&')[assign_a(arg.is_ref,true)];
  
   Rule name_p = lexeme_d[alpha_p >> *(alnum_p | '_')]; 
- 
+
+  // TODO, do we really need cls here? Non-local
+  Class cls0(verbose),cls(verbose);
   Rule classParent_p = 
     *(namespace_name_p[push_back_a(cls.qualifiedParent.namespaces)] >> str_p("::")) >>
     className_p[assign_a(cls.qualifiedParent.name)];
@@ -216,12 +209,15 @@ void Module::parseMarkup(const std::string& data) {
     [assign_a(arg,arg0)]; 
  
   Rule argumentList_p = !argument_p >> * (',' >> argument_p); 
- 
+
+  // parse class constructor
+  Constructor constructor0(verbose), constructor(verbose);
   Rule constructor_p =  
     (className_p >> '(' >> argumentList_p >> ')' >> ';' >> !comments_p) 
     [push_back_a(constructor.args_list, args)] 
     [clear_a(args)];
  
+  vector<string> namespaces_return; /// namespace for current return type
   Rule namespace_ret_p = namespace_name_p[push_back_a(namespaces_return)] >> str_p("::"); 
  
   // HACK: use const values instead of using enums themselves - somehow this doesn't result in values getting assigned to gibberish
@@ -300,12 +296,10 @@ void Module::parseMarkup(const std::string& data) {
       [assign_a(constructor.name, cls.name)] 
       [assign_a(cls.constructor, constructor)] 
       [assign_a(cls.namespaces, namespaces)] 
-      [assign_a(deconstructor.name,cls.name)] 
-      [assign_a(cls.deconstructor, deconstructor)] 
+      [assign_a(cls.deconstructor.name,cls.name)]
       [bl::bind(&handle_possible_template, bl::var(classes), bl::var(cls),
           bl::var(templateArgName), bl::var(templateInstantiations))]
       [clear_a(templateInstantiations)]
-      [assign_a(deconstructor,deconstructor0)] 
       [assign_a(constructor, constructor0)] 
       [assign_a(cls,cls0)];
  
@@ -328,19 +322,21 @@ void Module::parseMarkup(const std::string& data) {
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wuninitialized"
 #endif
- 
-  Rule namespace_def_p = 
-      (str_p("namespace") 
-      >> namespace_name_p[push_back_a(namespaces)] 
-      >> ch_p('{') 
-      >> *(include_p | class_p | templateSingleInstantiation_p | global_function_p | namespace_def_p | comments_p) 
-      >> ch_p('}')) 
-      [pop_a(namespaces)]; 
+
+  Rule namespace_def_p =
+      (str_p("namespace")
+      >> namespace_name_p[push_back_a(namespaces)]
+      >> ch_p('{')
+      >> *(include_p | class_p | templateSingleInstantiation_p | global_function_p | namespace_def_p | comments_p)
+      >> ch_p('}'))
+      [pop_a(namespaces)];
 
 #ifdef __clang__
 #pragma clang diagnostic pop
 #endif
 
+  // parse forward declaration
+  ForwardDeclaration fwDec0, fwDec;
   Rule forward_declaration_p =
       !(str_p("virtual")[assign_a(fwDec.isVirtual, true)]) 
       >> str_p("class")