From d9d97c4bc7c4872431c1d95d6800feb9cea40140 Mon Sep 17 00:00:00 2001
From: Duy-Nguyen Ta <thduynguyen@gmail.com>
Date: Mon, 19 Dec 2016 17:47:30 -0500
Subject: [PATCH] Forward declare not only classes but their inheritance

This is needed for wrapping to Cython another project based on gtsam. The current scheme requires information about all parent classes. See updated comments in gtsam.h.
---
 cython/gtsam.h               | 12 ++++++++++++
 wrap/Class.h                 |  9 +++++++++
 wrap/ForwardDeclaration.h    |  7 +++++--
 wrap/Module.cpp              | 21 ++++++++++++++++-----
 wrap/TypeAttributesTable.cpp |  4 ++--
 5 files changed, 44 insertions(+), 9 deletions(-)

diff --git a/cython/gtsam.h b/cython/gtsam.h
index 16c6cdd95..4589efc4b 100644
--- a/cython/gtsam.h
+++ b/cython/gtsam.h
@@ -89,6 +89,18 @@
  *        - Add "void serializable()" to a class if you only want the class to be serialized as a
  *          part of a container (such as noisemodel). This version does not require a publicly
  *          accessible default constructor.
+ *   Forward declarations and class definitions for Cython:
+ *     - Need to specify the base class (both this forward class and base class are declared in an external cython header)
+ *       This is so Cython can generate proper inheritance.
+ *       Example when wrapping a gtsam-based project:
+ *          // forward declarations
+ *          virtual class gtsam::NonlinearFactor
+ *          virtual class gtsam::NoiseModelFactor : gtsam::NonlinearFactor
+ *          // class definition
+ *          #include <MyFactor.h>
+ *          virtual class MyFactor : gtsam::NoiseModelFactor {...};
+ *    - *DO NOT* re-define overriden function already declared in the external (forward-declared) base class
+ *        - This will cause an ambiguity problem in Cython pxd header file
  */
 
 /**
diff --git a/wrap/Class.h b/wrap/Class.h
index 3c9264d17..1a8092228 100644
--- a/wrap/Class.h
+++ b/wrap/Class.h
@@ -90,6 +90,15 @@ public:
           false), deconstructor(verbose), verbose_(verbose) {
   }
 
+  Class(const std::string& name, bool verbose = true)
+      : Qualified(name, Qualified::Category::CLASS),
+        parentClass(boost::none),
+        isVirtual(false),
+        isSerializable(false),
+        hasSerialization(false),
+        deconstructor(verbose),
+        verbose_(verbose) {}
+
   void assignParent(const Qualified& parent);
 
   boost::optional<std::string> qualifiedParent() const;
diff --git a/wrap/ForwardDeclaration.h b/wrap/ForwardDeclaration.h
index 5ec022ca4..127823e82 100644
--- a/wrap/ForwardDeclaration.h
+++ b/wrap/ForwardDeclaration.h
@@ -23,11 +23,14 @@
 
 namespace wrap {
 
+  class Class;
+
   struct ForwardDeclaration {
-    std::string name;
+    Class cls;
     bool isVirtual;
     ForwardDeclaration() : isVirtual(false) {}
-    ForwardDeclaration(const std::string& s) : name(s), isVirtual(false) {}
+    explicit ForwardDeclaration(const std::string& s) : cls(s), isVirtual(false) {}
+    std::string name() const { return cls.qualifiedName("::"); }
   };
 
 }
diff --git a/wrap/Module.cpp b/wrap/Module.cpp
index c2b3dbc07..622e26564 100644
--- a/wrap/Module.cpp
+++ b/wrap/Module.cpp
@@ -176,11 +176,17 @@ void Module::parseMarkup(const std::string& data) {
 
   // parse forward declaration
   ForwardDeclaration fwDec0, fwDec;
+  Class fwParentClass;
+  TypeGrammar className_g(fwDec.cls);
+  TypeGrammar classParent_g(fwParentClass);
+  Rule classParent_p = (':' >> classParent_g >> ';') //
+         [bl::bind(&Class::assignParent, bl::var(fwDec.cls),
+              bl::var(fwParentClass))][clear_a(fwParentClass)];
+
   Rule forward_declaration_p =
       !(str_p("virtual")[assign_a(fwDec.isVirtual, T)])
-      >> str_p("class") 
-      >> (*(basic.namespace_p >> str_p("::")) >> basic.className_p)[assign_a(fwDec.name)]
-      >> ch_p(';')
+      >> str_p("class") >> className_g
+      >> (classParent_p | ';')
       [push_back_a(forward_declarations, fwDec)]
       [assign_a(cls,cls0)] // also clear class to avoid partial parse
       [assign_a(fwDec, fwDec0)];
@@ -419,8 +425,13 @@ R"rawstr(def Vectorize(*args):
     return ret
 )rawstr";
 
+  // all classes include all forward declarations
+  std::vector<Class> allClasses = expandedClasses;
+  for(const ForwardDeclaration& fd: forward_declarations)
+    allClasses.push_back(fd.cls);
+
   for(const Class& cls: expandedClasses)
-    cls.emit_cython_pyx(pyxFile, expandedClasses);
+    cls.emit_cython_pyx(pyxFile, allClasses);
   pyxFile.oss << "\n";
   //... wrap global functions
   for(const GlobalFunctions::value_type& p: global_functions)
@@ -493,7 +504,7 @@ vector<Class> Module::ExpandTypedefInstantiations(const vector<Class>& classes,
 vector<string> Module::GenerateValidTypes(const vector<Class>& classes, const vector<ForwardDeclaration>& forwardDeclarations, const vector<TypedefPair>& typedefs) {
   vector<string> validTypes;
   for(const ForwardDeclaration& fwDec: forwardDeclarations) {
-    validTypes.push_back(fwDec.name);
+    validTypes.push_back(fwDec.name());
   }
   validTypes.push_back("void");
   validTypes.push_back("string");
diff --git a/wrap/TypeAttributesTable.cpp b/wrap/TypeAttributesTable.cpp
index 887261d19..ee98480c1 100644
--- a/wrap/TypeAttributesTable.cpp
+++ b/wrap/TypeAttributesTable.cpp
@@ -63,8 +63,8 @@ void TypeAttributesTable::addClasses(const vector<Class>& classes) {
 void TypeAttributesTable::addForwardDeclarations(
     const vector<ForwardDeclaration>& forwardDecls) {
   for(const ForwardDeclaration& fwDec: forwardDecls) {
-    if (!table_.insert(make_pair(fwDec.name, TypeAttributes(fwDec.isVirtual))).second)
-      throw DuplicateDefinition("class " + fwDec.name);
+    if (!table_.insert(make_pair(fwDec.name(), TypeAttributes(fwDec.isVirtual))).second)
+      throw DuplicateDefinition("forward defined class " + fwDec.name());
   }
 }