From ebb6cc4cf1345aee0f64e400a3d4c08a93cfe96c Mon Sep 17 00:00:00 2001 From: David Reiss Date: Thu, 9 Apr 2009 20:02:56 +0000 Subject: [PATCH] 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 --- compiler/cpp/src/generate/t_py_generator.cc | 19 +++++++++-- compiler/cpp/src/parse/t_field.h | 12 ------- compiler/cpp/src/parse/t_struct.h | 35 ++++++++++----------- compiler/cpp/src/thrifty.yy | 3 +- 4 files changed, 36 insertions(+), 33 deletions(-) diff --git a/compiler/cpp/src/generate/t_py_generator.cc b/compiler/cpp/src/generate/t_py_generator.cc index e52c2dad..d63fec75 100644 --- 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& members = tstruct->get_members(); vector::const_iterator m_iter; + vector 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 67a2125c..51cfcb03 100644 --- 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 172a0f28..a18b131c 100644 --- 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 #include -#include #include #include "t_type.h" @@ -38,8 +36,6 @@ class t_program; */ class t_struct : public t_type { public: - typedef std::vector 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 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& 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::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::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::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 members_; bool is_xception_; bool xsd_all_; diff --git a/compiler/cpp/src/thrifty.yy b/compiler/cpp/src/thrifty.yy index bf5408e3..2c16ca01 100644 --- 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); } | { -- 2.17.1