THRIFT-445. Revert r760201 "THRIFT-236. Sort fields in id order during parsing"

git-svn-id: https://svn.apache.org/repos/asf/incubator/thrift/trunk@763786 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/compiler/cpp/src/generate/t_py_generator.cc b/compiler/cpp/src/generate/t_py_generator.cc
index e52c2da..d63fec7 100644 (file)
--- a/compiler/cpp/src/generate/t_py_generator.cc
+++ b/compiler/cpp/src/generate/t_py_generator.cc
@@ -520,6 +520,19 @@ void t_py_generator::generate_py_struct(t_struct* tstruct,
   generate_py_struct_definition(f_types_, tstruct, is_exception);
 }
 
+/**
+ * Comparator to sort fields in ascending order by key.
+ * Make this a functor instead of a function to help GCC inline it.
+ * The arguments are (const) references to const pointers to const t_fields.
+ * Unfortunately, we cannot declare it within the function.  Boo!
+ * http://www.open-std.org/jtc1/sc22/open/n2356/ (paragraph 9).
+ */
+struct FieldKeyCompare {
+  bool operator()(t_field const * const & a, t_field const * const & b) {
+    return a->get_key() < b->get_key();
+  }
+};
+
 /**
  * Generates a struct definition for a thrift data type.
  *
@@ -532,6 +545,8 @@ void t_py_generator::generate_py_struct_definition(ofstream& out,
 
   const vector<t_field*>& members = tstruct->get_members();
   vector<t_field*>::const_iterator m_iter;
+  vector<t_field*> sorted_members(members);
+  std::sort(sorted_members.begin(), sorted_members.end(), FieldKeyCompare());
 
   out <<
     "class " << tstruct->get_name();
@@ -571,12 +586,12 @@ void t_py_generator::generate_py_struct_definition(ofstream& out,
   // for structures with no members.
   // TODO(dreiss): Test encoding of structs where some inner structs
   // don't have thrift_spec.
-  if (members.empty() || (members[0]->get_key() >= 0)) {
+  if (sorted_members.empty() || (sorted_members[0]->get_key() >= 0)) {
     indent(out) << "thrift_spec = (" << endl;
     indent_up();
 
     int sorted_keys_pos = 0;
-    for (m_iter = members.begin(); m_iter != members.end(); ++m_iter) {
+    for (m_iter = sorted_members.begin(); m_iter != sorted_members.end(); ++m_iter) {
 
       for (; sorted_keys_pos != (*m_iter)->get_key(); sorted_keys_pos++) {
         indent(out) << "None, # " << sorted_keys_pos << endl;

diff --git a/compiler/cpp/src/parse/t_field.h b/compiler/cpp/src/parse/t_field.h
index 67a2125..51cfcb0 100644 (file)
--- a/compiler/cpp/src/parse/t_field.h
+++ b/compiler/cpp/src/parse/t_field.h
@@ -122,18 +122,6 @@ class t_field : public t_doc {
       type_->get_fingerprint_material();
   }
 
-  /**
-   * Comparator to sort fields in ascending order by key.
-   * Make this a functor instead of a function to help GCC inline it.
-   * The arguments are (const) references to const pointers to const t_fields.
-   */
-  struct key_compare {
-    bool operator()(t_field const * const & a, t_field const * const & b) {
-      return a->get_key() < b->get_key();
-    }
-  };
-
-
  private:
   t_type* type_;
   std::string name_;

diff --git a/compiler/cpp/src/parse/t_struct.h b/compiler/cpp/src/parse/t_struct.h
index 172a0f2..a18b131 100644 (file)
--- a/compiler/cpp/src/parse/t_struct.h
+++ b/compiler/cpp/src/parse/t_struct.h
@@ -20,9 +20,7 @@
 #ifndef T_STRUCT_H
 #define T_STRUCT_H
 
-#include <algorithm>
 #include <vector>
-#include <utility>
 #include <string>
 
 #include "t_type.h"
@@ -38,8 +36,6 @@ class t_program;
  */
 class t_struct : public t_type {
  public:
-  typedef std::vector<t_field*> members_type;
-
   t_struct(t_program* program) :
     t_type(program),
     is_xception_(false),
@@ -66,19 +62,11 @@ class t_struct : public t_type {
     return xsd_all_;
   }
 
-  bool append(t_field* elem) {
-    typedef members_type::iterator iter_type;
-    std::pair<iter_type, iter_type> bounds = std::equal_range(
-            members_.begin(), members_.end(), elem, t_field::key_compare()
-        );
-    if (bounds.first != bounds.second) {
-      return false;
-    }
-    members_.insert(bounds.second, elem);
-    return true;
+  void append(t_field* elem) {
+    members_.push_back(elem);
   }
 
-  const members_type& get_members() {
+  const std::vector<t_field*>& get_members() {
     return members_;
   }
 
@@ -92,7 +80,7 @@ class t_struct : public t_type {
 
   virtual std::string get_fingerprint_material() const {
     std::string rv = "{";
-    members_type::const_iterator m_iter;
+    std::vector<t_field*>::const_iterator m_iter;
     for (m_iter = members_.begin(); m_iter != members_.end(); ++m_iter) {
       rv += (*m_iter)->get_fingerprint_material();
       rv += ";";
@@ -103,15 +91,26 @@ class t_struct : public t_type {
 
   virtual void generate_fingerprint() {
     t_type::generate_fingerprint();
-    members_type::const_iterator m_iter;
+    std::vector<t_field*>::const_iterator m_iter;
     for (m_iter = members_.begin(); m_iter != members_.end(); ++m_iter) {
       (*m_iter)->get_type()->generate_fingerprint();
     }
   }
 
+  bool validate_field(t_field* field) {
+    int key = field->get_key();
+    std::vector<t_field*>::const_iterator m_iter;
+    for (m_iter = members_.begin(); m_iter != members_.end(); ++m_iter) {
+      if ((*m_iter)->get_key() == key) {
+        return false;
+      }
+    }
+    return true;
+  }
+
  private:
 
-  members_type members_;
+  std::vector<t_field*> members_;
   bool is_xception_;
 
   bool xsd_all_;

diff --git a/compiler/cpp/src/thrifty.yy b/compiler/cpp/src/thrifty.yy
index bf5408e..2c16ca0 100644 (file)
--- a/compiler/cpp/src/thrifty.yy
+++ b/compiler/cpp/src/thrifty.yy
@@ -830,10 +830,11 @@ FieldList:
     {
       pdebug("FieldList -> FieldList , Field");
       $$ = $1;
-      if (!($$->append($2))) {
+      if (!($$->validate_field($2))) {
         yyerror("Field identifier %d for \"%s\" has already been used", $2->get_key(), $2->get_name().c_str());
         exit(1);
       }
+      $$->append($2);
     }
 |
     {