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
diff --git a/compiler/cpp/src/generate/t_py_generator.cc b/compiler/cpp/src/generate/t_py_generator.cc
index d63fec7..e52c2da 100644
--- a/compiler/cpp/src/generate/t_py_generator.cc
+++ b/compiler/cpp/src/generate/t_py_generator.cc
@@ -521,19 +521,6 @@
}
/**
- * 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.
*
* @param tstruct The struct definition
@@ -545,8 +532,6 @@
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();
@@ -586,12 +571,12 @@
// 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 51cfcb0..67a2125 100644
--- a/compiler/cpp/src/parse/t_field.h
+++ b/compiler/cpp/src/parse/t_field.h
@@ -122,6 +122,18 @@
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 a18b131..172a0f2 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 <algorithm>
#include <vector>
+#include <utility>
#include <string>
#include "t_type.h"
@@ -36,6 +38,8 @@
*/
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),
@@ -62,11 +66,19 @@
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<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;
}
- const std::vector<t_field*>& get_members() {
+ const members_type& get_members() {
return members_;
}
@@ -80,7 +92,7 @@
virtual std::string get_fingerprint_material() const {
std::string rv = "{";
- std::vector<t_field*>::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 @@
virtual void generate_fingerprint() {
t_type::generate_fingerprint();
- std::vector<t_field*>::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<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:
- std::vector<t_field*> 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 e077377..1848efd 100644
--- a/compiler/cpp/src/thrifty.yy
+++ b/compiler/cpp/src/thrifty.yy
@@ -830,11 +830,10 @@
{
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);
}
|
{