You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@avro.apache.org by cu...@apache.org on 2011/07/01 22:03:20 UTC

svn commit: r1142058 - in /avro/branches/branch-1.5: ./ CHANGES.txt lang/c++/CMakeLists.txt lang/c++/impl/avrogencpp.cc lang/c++/jsonschemas/circulardep lang/c++/jsonschemas/recursive lang/c++/jsonschemas/reuse lang/c++/test/AvrogencppTests.cc

Author: cutting
Date: Fri Jul  1 20:03:19 2011
New Revision: 1142058

URL: http://svn.apache.org/viewvc?rev=1142058&view=rev
Log:
Merge -c 1142057 from trunk to 1.5 branch.  Fixes: AVRO-825.

Added:
    avro/branches/branch-1.5/lang/c++/jsonschemas/circulardep
      - copied unchanged from r1142057, avro/trunk/lang/c++/jsonschemas/circulardep
    avro/branches/branch-1.5/lang/c++/jsonschemas/recursive
      - copied unchanged from r1142057, avro/trunk/lang/c++/jsonschemas/recursive
    avro/branches/branch-1.5/lang/c++/jsonschemas/reuse
      - copied unchanged from r1142057, avro/trunk/lang/c++/jsonschemas/reuse
Modified:
    avro/branches/branch-1.5/   (props changed)
    avro/branches/branch-1.5/CHANGES.txt
    avro/branches/branch-1.5/lang/c++/CMakeLists.txt
    avro/branches/branch-1.5/lang/c++/impl/avrogencpp.cc
    avro/branches/branch-1.5/lang/c++/test/AvrogencppTests.cc

Propchange: avro/branches/branch-1.5/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Fri Jul  1 20:03:19 2011
@@ -1 +1 @@
-/avro/trunk:1075938,1075993,1078917,1079055,1079060,1079063,1079680,1083246,1085921,1086727,1086730,1086866,1087076,1087129,1087136,1087439-1087440,1087463,1087472,1087792,1089128,1089131,1089550,1094812,1095206-1095208,1095493,1095529,1095548,1095550,1096798,1097916,1097927,1097968,1097974,1099257,1102332,1102335,1124127,1124971,1129053,1129071,1129697-1129706,1129729,1130503,1136342,1141677,1141685,1141979-1141980
+/avro/trunk:1075938,1075993,1078917,1079055,1079060,1079063,1079680,1083246,1085921,1086727,1086730,1086866,1087076,1087129,1087136,1087439-1087440,1087463,1087472,1087792,1089128,1089131,1089550,1094812,1095206-1095208,1095493,1095529,1095548,1095550,1096798,1097916,1097927,1097968,1097974,1099257,1102332,1102335,1124127,1124971,1129053,1129071,1129697-1129706,1129729,1130503,1136342,1141677,1141685,1141979-1141980,1142057

Modified: avro/branches/branch-1.5/CHANGES.txt
URL: http://svn.apache.org/viewvc/avro/branches/branch-1.5/CHANGES.txt?rev=1142058&r1=1142057&r2=1142058&view=diff
==============================================================================
--- avro/branches/branch-1.5/CHANGES.txt (original)
+++ avro/branches/branch-1.5/CHANGES.txt Fri Jul  1 20:03:19 2011
@@ -51,6 +51,8 @@ Avro 1.5.2 (unreleased)
     errors when attempting to use a closed connection.
     (James Baldassari via cutting)
 
+    AVRO-825: C++: Fix bugs in codegen with recursive schemas. (thiru)
+
 Avro 1.5.1 (3 May 2011)
 
   NEW FEATURES

Modified: avro/branches/branch-1.5/lang/c++/CMakeLists.txt
URL: http://svn.apache.org/viewvc/avro/branches/branch-1.5/lang/c%2B%2B/CMakeLists.txt?rev=1142058&r1=1142057&r2=1142058&view=diff
==============================================================================
--- avro/branches/branch-1.5/lang/c++/CMakeLists.txt (original)
+++ avro/branches/branch-1.5/lang/c++/CMakeLists.txt Fri Jul  1 20:03:19 2011
@@ -108,6 +108,21 @@ add_custom_target (union_map_union_hh
         -o ${BUILD_DIRECTORY}/union_map_union.hh -n umu
     DEPENDS avrogencpp)
 
+add_custom_target (recursive_hh
+    COMMAND avrogencpp -i jsonschemas/recursive
+        -o ${BUILD_DIRECTORY}/recursive.hh -n rec
+    DEPENDS avrogencpp)
+
+add_custom_target (reuse_hh
+    COMMAND avrogencpp -i jsonschemas/reuse
+        -o ${BUILD_DIRECTORY}/reuse.hh -n reu
+    DEPENDS avrogencpp)
+
+add_custom_target (circulardep_hh
+    COMMAND avrogencpp -i jsonschemas/circulardep
+        -o ${BUILD_DIRECTORY}/circulardep.hh -n reu
+    DEPENDS avrogencpp)
+
 macro (test name)
     add_executable (${name} test/${name}.cc)
     target_link_libraries (${name} avrocpp ${Boost_LIBRARIES})
@@ -134,7 +149,8 @@ target_link_libraries (testgentest avroc
 
 add_executable (AvrogencppTests test/AvrogencppTests.cc)
 add_dependencies (AvrogencppTests bigrecord_hh bigrecord2_hh
-    union_array_union_hh union_map_union_hh)
+    union_array_union_hh union_map_union_hh recursive_hh reuse_hh
+    circulardep_hh)
 target_link_libraries (AvrogencppTests avrocpp ${BOOST_LIBRARIES})
 
 add_executable (DataFileTests test/DataFileTests.cc)

Modified: avro/branches/branch-1.5/lang/c++/impl/avrogencpp.cc
URL: http://svn.apache.org/viewvc/avro/branches/branch-1.5/lang/c%2B%2B/impl/avrogencpp.cc?rev=1142058&r1=1142057&r2=1142058&view=diff
==============================================================================
--- avro/branches/branch-1.5/lang/c++/impl/avrogencpp.cc (original)
+++ avro/branches/branch-1.5/lang/c++/impl/avrogencpp.cc Fri Jul  1 20:03:19 2011
@@ -20,6 +20,8 @@
 #include <sys/time.h>
 #include <iostream>
 #include <fstream>
+#include <map>
+#include <set>
 
 #include <boost/lexical_cast.hpp>
 #include <boost/program_options.hpp>
@@ -35,6 +37,8 @@
 using std::ostream;
 using std::ifstream;
 using std::ofstream;
+using std::map;
+using std::set;
 using std::string;
 using std::vector;
 using avro::NodePtr;
@@ -45,14 +49,40 @@ using boost::lexical_cast;
 using avro::ValidSchema;
 using avro::compileJsonSchema;
 
+struct PendingSetterGetter {
+    string structName;
+    string type;
+    string name;
+    size_t idx;
+
+    PendingSetterGetter(const string& sn, const string& t,
+        const string& n, size_t i) :
+        structName(sn), type(t), name(n), idx(i) { }
+};
+
+struct PendingConstructor {
+    string structName;
+    string memberName;
+    bool initMember;
+    PendingConstructor(const string& sn, const string& n, bool im) :
+        structName(sn), memberName(n), initMember(im) { }
+};
+
 class CodeGen {
     size_t unionNumber_;
     std::ostream& os_;
+    bool inNamespace_;
     const std::string ns_;
     const std::string headerFile_;
     const std::string schemaFile_;
     boost::mt19937 random_;
 
+    vector<PendingSetterGetter> pendingGettersAndSetters;
+    vector<PendingConstructor> pendingConstructors;
+
+    map<NodePtr, string> done;
+    set<NodePtr> doing;
+
     std::string guard();
     std::string fullname(const string& name) const;
     std::string generateEnumType(const NodePtr& n);
@@ -61,6 +91,8 @@ class CodeGen {
     std::string unionName();
     std::string generateUnionType(const NodePtr& n);
     std::string generateType(const NodePtr& n);
+    std::string generateDeclaration(const NodePtr& n);
+    std::string doGenerateType(const NodePtr& n);
     void generateEnumTraits(const NodePtr& n);
     void generateTraits(const NodePtr& n);
     void generateRecordTraits(const NodePtr& n);
@@ -69,7 +101,7 @@ class CodeGen {
 public:
     CodeGen(std::ostream& os, std::string& ns,
         std::string& schemaFile, std::string& headerFile) :
-        unionNumber_(0), os_(os), ns_(ns),
+        unionNumber_(0), os_(os), inNamespace_(false), ns_(ns),
         schemaFile_(schemaFile), headerFile_(headerFile),
         random_(::time(0)) { }
     void generate(const ValidSchema& schema);
@@ -110,7 +142,7 @@ string CodeGen::cppTypeOf(const NodePtr&
         return "bool";
     case avro::AVRO_RECORD:
     case avro::AVRO_ENUM:
-        return fullname(n->name());
+        return inNamespace_ ? n->name() : fullname(n->name());
     case avro::AVRO_ARRAY:
         return "std::vector<" + cppTypeOf(n->leafAt(0)) + " >";
     case avro::AVRO_MAP:
@@ -167,6 +199,11 @@ string CodeGen::generateRecordType(const
         types.push_back(generateType(n->leafAt(i)));
     }
 
+    map<NodePtr, string>::const_iterator it = done.find(n);
+    if (it != done.end()) {
+        return it->second;
+    }
+
     os_ << "struct " << n->name() << " {\n";
     for (int i = 0; i < c; ++i) {
         os_ << "    " << types[i]
@@ -201,18 +238,73 @@ string CodeGen::unionName()
     return s + "_Union__" + boost::lexical_cast<string>(unionNumber_++) + "__";
 }
 
+static void generateGetterAndSetter(ostream& os,
+    const string& structName, const string& type, const string& name,
+    size_t idx)
+{
+    string sn = " " + structName + "::";
+
+    os << "inline\n";
+
+    os << type << sn << "get_" << name << "() const {\n"
+        << "    if (idx_ != " << idx << ") {\n"
+        << "        throw avro::Exception(\"Invalid type for "
+            << "union\");\n"
+        << "    }\n"
+        << "    return boost::any_cast<" << type << " >(value_);\n"
+        << "}\n\n";
+
+    os << "void" << sn << "set_" << name
+        << "(const " << type << "& v) {\n"
+        << "    idx_ = " << idx << ";\n"
+        << "    value_ = v;\n"
+        << "}\n\n";
+}
+
+static void generateConstructor(ostream& os,
+    const string& structName, bool initMember,
+    const string& type) {
+    os << "inline " << structName  << "::" << structName << "() : idx_(0)";
+    if (initMember) {
+        os << ", value_(" << type << "())";
+    }
+    os << " { }\n";
+}
+
+/**
+ * Generates a type for union and emits the code.
+ * Since unions can encounter names that are not fully defined yet,
+ * such names must be declared and the inline functions deferred until all
+ * types are fully defined.
+ */
 string CodeGen::generateUnionType(const NodePtr& n)
 {
     size_t c = n->leaves();
     vector<string> types;
     vector<string> names;
-    for (size_t i = 0; i < c; ++i) {
-        const NodePtr& nn = n->leafAt(i);
-        types.push_back(generateType(nn));
-        names.push_back(cppNameOf(nn));
+
+    set<NodePtr>::const_iterator it = doing.find(n);
+    if (it != doing.end()) {
+        for (size_t i = 0; i < c; ++i) {
+            const NodePtr& nn = n->leafAt(i);
+            types.push_back(generateDeclaration(nn));
+            names.push_back(cppNameOf(nn));
+        }
+    } else {
+        doing.insert(n);
+        for (size_t i = 0; i < c; ++i) {
+            const NodePtr& nn = n->leafAt(i);
+            types.push_back(generateType(nn));
+            names.push_back(cppNameOf(nn));
+        }
+        doing.erase(n);
+    }
+    if (done.find(n) != done.end()) {
+        return done[n];
     }
 
-    string result = unionName();
+    const string result = unionName();
+
     os_ << "struct " << result << " {\n"
         << "private:\n"
         << "    size_t idx_;\n"
@@ -227,35 +319,42 @@ string CodeGen::generateUnionType(const 
                 << "        idx_ = " << i << ";\n"
                 << "        value_ = boost::any();\n"
                 << "    }\n";
-            continue;
-        } 
-        string type = types[i];
-        string name = names[i];
-        os_ << "    " << type << " get_" << name << "() const {\n"
-            << "        if (idx_ != " << i << ") {\n"
-            << "            throw avro::Exception(\"Invalid type for "
-                << "union\");\n"
-            << "        }\n"
-            << "        return boost::any_cast<" << type << " >(value_);\n"
-            << "    }\n";
-
-        os_ << "    void set_" << name << "(const " << type << "& v) {\n"
-            << "        idx_ = " << i << ";\n"
-            << "        value_ = v;\n"
-            << "    }\n";
-    }
-    os_ << "    " << result << "() : idx_(0) {\n";
-    if (n->leafAt(0)->type() != avro::AVRO_NULL) {
-        os_ << "        value_ = " << types[0] << "();\n";
+        } else {
+            const string& type = types[i];
+            const string& name = names[i];
+            os_ << "    " << type << " get_" << name << "() const;\n"
+                   "    void set_" << name << "(const " << type << "& v);\n";
+            pendingGettersAndSetters.push_back(
+                PendingSetterGetter(result, type, name, i));
+        }
     }
-    os_ << "    }\n";
+
+    os_ << "    " << result << "();\n";
+    pendingConstructors.push_back(PendingConstructor(result, types[0],
+        n->leafAt(0)->type() != avro::AVRO_NULL));
     os_ << "};\n\n";
     
     return result;
 }
 
+/**
+ * Returns the type for the given schema node and emits code to os.
+ */
 string CodeGen::generateType(const NodePtr& n)
 {
+    NodePtr nn = (n->type() == avro::AVRO_SYMBOLIC) ?  resolveSymbol(n) : n;
+
+    map<NodePtr, string>::const_iterator it = done.find(nn);
+    if (it != done.end()) {
+        return it->second;
+    }
+    string result = doGenerateType(nn);
+    done[nn] = result;
+    return result;
+}
+
+string CodeGen::doGenerateType(const NodePtr& n)
+{
     switch (n->type()) {
     case avro::AVRO_STRING:
     case avro::AVRO_BYTES:
@@ -268,17 +367,46 @@ string CodeGen::generateType(const NodeP
     case avro::AVRO_FIXED:
         return cppTypeOf(n);
     case avro::AVRO_ARRAY:
+        return "std::vector<" + generateType(n->leafAt(0)) + " >";
     case avro::AVRO_MAP:
-        generateType(n->leafAt(n->type() == avro::AVRO_ARRAY ? 0 : 1));
-        return cppTypeOf(n);
+        return "std::map<std::string, " + generateType(n->leafAt(1)) + " >";
     case avro::AVRO_RECORD:
         return generateRecordType(n);
     case avro::AVRO_ENUM:
         return generateEnumType(n);
     case avro::AVRO_UNION:
         return generateUnionType(n);
-    case avro::AVRO_SYMBOLIC:
-        return cppTypeOf(resolveSymbol(n));
+    }
+    return "$Undefuned$";
+}
+
+string CodeGen::generateDeclaration(const NodePtr& n)
+{
+    NodePtr nn = (n->type() == avro::AVRO_SYMBOLIC) ?  resolveSymbol(n) : n;
+    switch (nn->type()) {
+    case avro::AVRO_STRING:
+    case avro::AVRO_BYTES:
+    case avro::AVRO_INT:
+    case avro::AVRO_LONG:
+    case avro::AVRO_FLOAT:
+    case avro::AVRO_DOUBLE:
+    case avro::AVRO_BOOL:
+    case avro::AVRO_NULL:
+    case avro::AVRO_FIXED:
+        return cppTypeOf(nn);
+    case avro::AVRO_ARRAY:
+        return "std::vector<" + generateDeclaration(nn->leafAt(0)) + " >";
+    case avro::AVRO_MAP:
+        return "std::map<std::string, " +
+            generateDeclaration(nn->leafAt(1)) + " >";
+    case avro::AVRO_RECORD:
+        os_ << "struct " << cppTypeOf(nn) << ";\n";
+        return cppTypeOf(nn);
+    case avro::AVRO_ENUM:
+        return generateEnumType(nn);
+    case avro::AVRO_UNION:
+        // FIXME: When can this happen?
+        return generateUnionType(nn);
     }
     return "$Undefuned$";
 }
@@ -331,7 +459,7 @@ void CodeGen::generateUnionTraits(const 
         generateTraits(nn);
     }
 
-    string name = unionName();
+    string name = done[n];
     string fn = fullname(name);
 
     os_ << "template<> struct codec_traits<" << fn << "> {\n"
@@ -458,12 +586,28 @@ void CodeGen::generate(const ValidSchema
 
     if (! ns_.empty()) {
         os_ << "namespace " << ns_ << " {\n";
+        inNamespace_ = true;
     }
 
     const NodePtr& root = schema.root();
     generateType(root);
 
+    for (vector<PendingSetterGetter>::const_iterator it =
+        pendingGettersAndSetters.begin();
+        it != pendingGettersAndSetters.end(); ++it) {
+        generateGetterAndSetter(os_, it->structName, it->type, it->name,
+            it->idx);
+    }
+
+    for (vector<PendingConstructor>::const_iterator it =
+        pendingConstructors.begin();
+        it != pendingConstructors.end(); ++it) {
+        generateConstructor(os_, it->structName,
+            it->initMember, it->memberName);
+    }
+
     if (! ns_.empty()) {
+        inNamespace_ = false;
         os_ << "}\n";
     }
 
@@ -477,6 +621,7 @@ void CodeGen::generate(const ValidSchema
 
     os_ << "#endif\n";
     os_.flush();
+
 }
 
 namespace po = boost::program_options;

Modified: avro/branches/branch-1.5/lang/c++/test/AvrogencppTests.cc
URL: http://svn.apache.org/viewvc/avro/branches/branch-1.5/lang/c%2B%2B/test/AvrogencppTests.cc?rev=1142058&r1=1142057&r2=1142058&view=diff
==============================================================================
--- avro/branches/branch-1.5/lang/c++/test/AvrogencppTests.cc (original)
+++ avro/branches/branch-1.5/lang/c++/test/AvrogencppTests.cc Fri Jul  1 20:03:19 2011
@@ -20,6 +20,9 @@
 #include "bigrecord2.hh"
 #include "union_array_union.hh"
 #include "union_map_union.hh"
+#include "recursive.hh"
+#include "circulardep.hh"
+#include "reuse.hh"
 #include "Compiler.hh"
 
 #include <fstream>