From 566a987dacfda8a85cdc55c5a0577ff6453da9d4 Mon Sep 17 00:00:00 2001 From: David Reiss Date: Mon, 30 Mar 2009 22:24:30 +0000 Subject: [PATCH] THRIFT-236. Sort fields in id order during parsing This effectively eliminates the notion of "IDL-order". All structs have their fields sorted in increasing order of field id. git-svn-id: https://svn.apache.org/repos/asf/incubator/thrift/trunk@760201 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, 33 insertions(+), 36 deletions(-) diff --git a/compiler/cpp/src/generate/t_py_generator.cc b/compiler/cpp/src/generate/t_py_generator.cc index d63fec75f..e52c2dad0 100644 --- a/compiler/cpp/src/generate/t_py_generator.cc +++ b/compiler/cpp/src/generate/t_py_generator.cc @@ -520,19 +520,6 @@ 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. * @@ -545,8 +532,6 @@ 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(); @@ -586,12 +571,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 (sorted_members.empty() || (sorted_members[0]->get_key() >= 0)) { + if (members.empty() || (members[0]->get_key() >= 0)) { indent(out) << "thrift_spec = (" << endl; indent_up(); int sorted_keys_pos = 0; - for (m_iter = sorted_members.begin(); m_iter != sorted_members.end(); ++m_iter) { + for (m_iter = members.begin(); m_iter != 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 51cfcb031..67a2125ce 100644 --- a/compiler/cpp/src/parse/t_field.h +++ b/compiler/cpp/src/parse/t_field.h @@ -122,6 +122,18 @@ 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 a18b131c5..172a0f289 100644 --- a/compiler/cpp/src/parse/t_struct.h +++ b/compiler/cpp/src/parse/t_struct.h @@ -20,7 +20,9 @@ #ifndef T_STRUCT_H #define T_STRUCT_H +#include #include +#include #include #include "t_type.h" @@ -36,6 +38,8 @@ 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), @@ -62,11 +66,19 @@ class t_struct : public t_type { return xsd_all_; } - void append(t_field* elem) { - members_.push_back(elem); + 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; } - const std::vector& get_members() { + const members_type& get_members() { return members_; } @@ -80,7 +92,7 @@ class t_struct : public t_type { virtual std::string get_fingerprint_material() const { std::string rv = "{"; - std::vector::const_iterator m_iter; + members_type::const_iterator m_iter; for (m_iter = members_.begin(); m_iter != members_.end(); ++m_iter) { rv += (*m_iter)->get_fingerprint_material(); rv += ";"; @@ -91,26 +103,15 @@ class t_struct : public t_type { virtual void generate_fingerprint() { t_type::generate_fingerprint(); - std::vector::const_iterator m_iter; + members_type::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: - std::vector members_; + members_type members_; bool is_xception_; bool xsd_all_; diff --git a/compiler/cpp/src/thrifty.yy b/compiler/cpp/src/thrifty.yy index e07737795..1848efd87 100644 --- a/compiler/cpp/src/thrifty.yy +++ b/compiler/cpp/src/thrifty.yy @@ -830,11 +830,10 @@ FieldList: { pdebug("FieldList -> FieldList , Field"); $$ = $1; - if (!($$->validate_field($2))) { + if (!($$->append($2))) { yyerror("Field identifier %d for \"%s\" has already been used", $2->get_key(), $2->get_name().c_str()); exit(1); } - $$->append($2); } | {