Skip to content

Commit

Permalink
THRIFT-4513: Fixing java thrift compiler to generate constants in sta…
Browse files Browse the repository at this point in the history
…ble order.

See https://issues.apache.org/jira/browse/THRIFT-4513

- Implementing deep comparison in t_const_value to allow stability of generated code with complex map keys.
  • Loading branch information
romanoid committed Mar 13, 2018
1 parent da9a9ba commit 9e3944d
Show file tree
Hide file tree
Showing 35 changed files with 248 additions and 187 deletions.
4 changes: 2 additions & 2 deletions compiler/cpp/src/thrift/audit/t_audit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,8 @@ bool compare_defaults(t_const_value* newStructDefault, t_const_value* oldStructD
}
case t_const_value::CV_MAP:
{
const std::map<t_const_value*, t_const_value*> newMap = newStructDefault->get_map();
const std::map<t_const_value*, t_const_value*> oldMap = oldStructDefault->get_map();
const std::map<t_const_value*, t_const_value*, t_const_value::value_compare> newMap = newStructDefault->get_map();
const std::map<t_const_value*, t_const_value*, t_const_value::value_compare> oldMap = oldStructDefault->get_map();

bool defaultValuesCompare = (oldMap.size() == newMap.size());

Expand Down
8 changes: 4 additions & 4 deletions compiler/cpp/src/thrift/generate/t_as3_generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -471,8 +471,8 @@ void t_as3_generator::print_const_value(std::ofstream& out,
} else if (type->is_struct() || type->is_xception()) {
const vector<t_field*>& fields = ((t_struct*)type)->get_members();
vector<t_field*>::const_iterator f_iter;
const map<t_const_value*, t_const_value*>& val = value->get_map();
map<t_const_value*, t_const_value*>::const_iterator v_iter;
const map<t_const_value*, t_const_value*, t_const_value::value_compare>& val = value->get_map();
map<t_const_value*, t_const_value*, t_const_value::value_compare>::const_iterator v_iter;
out << name << ":" << type_name(type) << " = new " << type_name(type, false, true) << "();"
<< endl;
if (!in_static) {
Expand Down Expand Up @@ -516,8 +516,8 @@ void t_as3_generator::print_const_value(std::ofstream& out,
}
t_type* ktype = ((t_map*)type)->get_key_type();
t_type* vtype = ((t_map*)type)->get_val_type();
const map<t_const_value*, t_const_value*>& val = value->get_map();
map<t_const_value*, t_const_value*>::const_iterator v_iter;
const map<t_const_value*, t_const_value*, t_const_value::value_compare>& val = value->get_map();
map<t_const_value*, t_const_value*, t_const_value::value_compare>::const_iterator v_iter;
for (v_iter = val.begin(); v_iter != val.end(); ++v_iter) {
string key = render_const_value(out, name, ktype, v_iter->first);
string val = render_const_value(out, name, vtype, v_iter->second);
Expand Down
8 changes: 4 additions & 4 deletions compiler/cpp/src/thrift/generate/t_c_glib_generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -997,8 +997,8 @@ void t_c_glib_generator::generate_const_initializer(string name,
if (type->is_struct() || type->is_xception()) {
const vector<t_field*>& fields = ((t_struct*)type)->get_members();
vector<t_field*>::const_iterator f_iter;
const map<t_const_value*, t_const_value*>& val = value->get_map();
map<t_const_value*, t_const_value*>::const_iterator v_iter;
const map<t_const_value*, t_const_value*, t_const_value::value_compare>& val = value->get_map();
map<t_const_value*, t_const_value*, t_const_value::value_compare>::const_iterator v_iter;
ostringstream initializers;

// initialize any constants that may be referenced by this initializer
Expand Down Expand Up @@ -1171,8 +1171,8 @@ void t_c_glib_generator::generate_const_initializer(string name,
} else if (type->is_map()) {
t_type* ktype = ((t_map*)type)->get_key_type();
t_type* vtype = ((t_map*)type)->get_val_type();
const map<t_const_value*, t_const_value*>& val = value->get_map();
map<t_const_value*, t_const_value*>::const_iterator v_iter;
const map<t_const_value*, t_const_value*, t_const_value::value_compare>& val = value->get_map();
map<t_const_value*, t_const_value*, t_const_value::value_compare>::const_iterator v_iter;
ostringstream initializers;
ostringstream appenders;

Expand Down
18 changes: 9 additions & 9 deletions compiler/cpp/src/thrift/generate/t_cl_generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class t_cl_generator : public t_oop_generator {
system_prefix = "thrift-gen-";

std::map<std::string, std::string>::const_iterator iter;

for(iter = parsed_options.begin(); iter != parsed_options.end(); ++iter) {
if(iter->first.compare("no_asd") == 0) {
no_asd = true;
Expand All @@ -65,7 +65,7 @@ class t_cl_generator : public t_oop_generator {
throw "unknown option cl:" + iter->first;
}
}

out_dir_base_ = "gen-cl";
copy_options_ = option_string;
}
Expand Down Expand Up @@ -112,7 +112,7 @@ class t_cl_generator : public t_oop_generator {
std::ofstream f_vars_;

std::string copy_options_;

bool no_asd;
std::string system_prefix;
};
Expand Down Expand Up @@ -310,8 +310,8 @@ string t_cl_generator::render_const_value(t_type* type, t_const_value* value) {

const vector<t_field*>& fields = ((t_struct*)type)->get_members();
vector<t_field*>::const_iterator f_iter;
const map<t_const_value*, t_const_value*>& val = value->get_map();
map<t_const_value*, t_const_value*>::const_iterator v_iter;
const map<t_const_value*, t_const_value*, t_const_value::value_compare>& val = value->get_map();
map<t_const_value*, t_const_value*, t_const_value::value_compare>::const_iterator v_iter;

for (v_iter = val.begin(); v_iter != val.end(); ++v_iter) {
t_type* field_type = NULL;
Expand All @@ -336,8 +336,8 @@ string t_cl_generator::render_const_value(t_type* type, t_const_value* value) {
t_type* vtype = ((t_map*)type)->get_val_type();
out << "(thrift:map ";
indent_up();
const map<t_const_value*, t_const_value*>& val = value->get_map();
map<t_const_value*, t_const_value*>::const_iterator v_iter;
const map<t_const_value*, t_const_value*, t_const_value::value_compare>& val = value->get_map();
map<t_const_value*, t_const_value*, t_const_value::value_compare>::const_iterator v_iter;
for (v_iter = val.begin(); v_iter != val.end(); ++v_iter) {
out << endl << indent()
<< "(cl:cons " << render_const_value(ktype, v_iter->first) << " "
Expand Down Expand Up @@ -494,7 +494,7 @@ string t_cl_generator::typespec(t_type *t) {
return type_name(t);
} else if (t->is_map()) {
t_map *m = (t_map*) t;
return "(thrift:map " + typespec(m->get_key_type()) + " " +
return "(thrift:map " + typespec(m->get_key_type()) + " " +
typespec(m->get_val_type()) + ")";
} else if (t->is_struct() || t->is_xception()) {
return "(struct " + prefix(type_name(t)) + ")";
Expand Down Expand Up @@ -530,7 +530,7 @@ string t_cl_generator::argument_list(t_struct* tstruct) {
typespec((*f_iter)->get_type()) << " " <<
(*f_iter)->get_key() << ")";


}
res << ")";
return res.str();
Expand Down
16 changes: 8 additions & 8 deletions compiler/cpp/src/thrift/generate/t_cocoa_generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2733,8 +2733,8 @@ void t_cocoa_generator::print_const_value(ostream& out,
indent(out);
const vector<t_field*>& fields = ((t_struct*)type)->get_members();
vector<t_field*>::const_iterator f_iter;
const map<t_const_value*, t_const_value*>& val = value->get_map();
map<t_const_value*, t_const_value*>::const_iterator v_iter;
const map<t_const_value*, t_const_value*, t_const_value::value_compare>& val = value->get_map();
map<t_const_value*, t_const_value*, t_const_value::value_compare>::const_iterator v_iter;
if (defval)
out << type_name(type) << " ";
out << name << " = [" << type_name(type, true) << " new];"
Expand All @@ -2758,8 +2758,8 @@ void t_cocoa_generator::print_const_value(ostream& out,
indent(mapout);
t_type* ktype = ((t_map*)type)->get_key_type();
t_type* vtype = ((t_map*)type)->get_val_type();
const map<t_const_value*, t_const_value*>& val = value->get_map();
map<t_const_value*, t_const_value*>::const_iterator v_iter;
const map<t_const_value*, t_const_value*, t_const_value::value_compare>& val = value->get_map();
map<t_const_value*, t_const_value*, t_const_value::value_compare>::const_iterator v_iter;
if (defval)
mapout << type_name(type) << " ";
mapout << name << " = @{";
Expand Down Expand Up @@ -2904,8 +2904,8 @@ string t_cocoa_generator::render_const_value(string name,
} else if (type->is_struct() || type->is_xception()) {
const vector<t_field*>& fields = ((t_struct*)type)->get_members();
vector<t_field*>::const_iterator f_iter;
const map<t_const_value*, t_const_value*>& val = value->get_map();
map<t_const_value*, t_const_value*>::const_iterator v_iter;
const map<t_const_value*, t_const_value*, t_const_value::value_compare>& val = value->get_map();
map<t_const_value*, t_const_value*, t_const_value::value_compare>::const_iterator v_iter;
if (val.size() > 0)
render << "[[" << type_name(type, true) << " alloc] initWith";
else
Expand Down Expand Up @@ -2937,8 +2937,8 @@ string t_cocoa_generator::render_const_value(string name,
render << "[[NSDictionary alloc] initWithObjectsAndKeys: ";
t_type* ktype = ((t_map*)type)->get_key_type();
t_type* vtype = ((t_map*)type)->get_val_type();
const map<t_const_value*, t_const_value*>& val = value->get_map();
map<t_const_value*, t_const_value*>::const_iterator v_iter;
const map<t_const_value*, t_const_value*, t_const_value::value_compare>& val = value->get_map();
map<t_const_value*, t_const_value*, t_const_value::value_compare>::const_iterator v_iter;
bool first = true;
for (v_iter = val.begin(); v_iter != val.end(); ++v_iter) {
string key = render_const_value(name, ktype, v_iter->first, true);
Expand Down
10 changes: 5 additions & 5 deletions compiler/cpp/src/thrift/generate/t_cpp_generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -703,8 +703,8 @@ void t_cpp_generator::print_const_value(ofstream& out,
} else if (type->is_struct() || type->is_xception()) {
const vector<t_field*>& fields = ((t_struct*)type)->get_members();
vector<t_field*>::const_iterator f_iter;
const map<t_const_value*, t_const_value*>& val = value->get_map();
map<t_const_value*, t_const_value*>::const_iterator v_iter;
const map<t_const_value*, t_const_value*, t_const_value::value_compare>& val = value->get_map();
map<t_const_value*, t_const_value*, t_const_value::value_compare>::const_iterator v_iter;
bool is_nonrequired_field = false;
for (v_iter = val.begin(); v_iter != val.end(); ++v_iter) {
t_type* field_type = NULL;
Expand All @@ -728,8 +728,8 @@ void t_cpp_generator::print_const_value(ofstream& out,
} else if (type->is_map()) {
t_type* ktype = ((t_map*)type)->get_key_type();
t_type* vtype = ((t_map*)type)->get_val_type();
const map<t_const_value*, t_const_value*>& val = value->get_map();
map<t_const_value*, t_const_value*>::const_iterator v_iter;
const map<t_const_value*, t_const_value*, t_const_value::value_compare>& val = value->get_map();
map<t_const_value*, t_const_value*, t_const_value::value_compare>::const_iterator v_iter;
for (v_iter = val.begin(); v_iter != val.end(); ++v_iter) {
string key = render_const_value(out, name, ktype, v_iter->first);
string val = render_const_value(out, name, vtype, v_iter->second);
Expand Down Expand Up @@ -1796,7 +1796,7 @@ void t_cpp_generator::generate_service(t_service* tservice) {
if (!gen_no_skeleton_) {
generate_service_async_skeleton(tservice);
}

}

f_header_ << "#ifdef _MSC_VER\n"
Expand Down
10 changes: 5 additions & 5 deletions compiler/cpp/src/thrift/generate/t_csharp_generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -511,8 +511,8 @@ void t_csharp_generator::print_const_def_value(std::ofstream& out,
if (type->is_struct() || type->is_xception()) {
const vector<t_field*>& fields = ((t_struct*)type)->get_members();
vector<t_field*>::const_iterator f_iter;
const map<t_const_value*, t_const_value*>& val = value->get_map();
map<t_const_value*, t_const_value*>::const_iterator v_iter;
const map<t_const_value*, t_const_value*, t_const_value::value_compare>& val = value->get_map();
map<t_const_value*, t_const_value*, t_const_value::value_compare>::const_iterator v_iter;
prepare_member_name_mapping((t_struct*)type);
for (v_iter = val.begin(); v_iter != val.end(); ++v_iter) {
t_field* field = NULL;
Expand All @@ -532,8 +532,8 @@ void t_csharp_generator::print_const_def_value(std::ofstream& out,
} else if (type->is_map()) {
t_type* ktype = ((t_map*)type)->get_key_type();
t_type* vtype = ((t_map*)type)->get_val_type();
const map<t_const_value*, t_const_value*>& val = value->get_map();
map<t_const_value*, t_const_value*>::const_iterator v_iter;
const map<t_const_value*, t_const_value*, t_const_value::value_compare>& val = value->get_map();
map<t_const_value*, t_const_value*, t_const_value::value_compare>::const_iterator v_iter;
for (v_iter = val.begin(); v_iter != val.end(); ++v_iter) {
string key = render_const_value(out, name, ktype, v_iter->first);
string val = render_const_value(out, name, vtype, v_iter->second);
Expand Down Expand Up @@ -988,7 +988,7 @@ void t_csharp_generator::generate_csharp_struct_reader(ofstream& out, t_struct*
indent_up();
out << indent()
<< "throw new TProtocolException(TProtocolException.INVALID_DATA, "
<< "\"required field " << prop_name((*f_iter)) << " not set\");"
<< "\"required field " << prop_name((*f_iter)) << " not set\");"
<< endl;
indent_down();
}
Expand Down
8 changes: 4 additions & 4 deletions compiler/cpp/src/thrift/generate/t_d_generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -557,8 +557,8 @@ class t_d_generator : public t_oop_generator {

const vector<t_field*>& fields = ((t_struct*)type)->get_members();
vector<t_field*>::const_iterator f_iter;
const map<t_const_value*, t_const_value*>& val = value->get_map();
map<t_const_value*, t_const_value*>::const_iterator v_iter;
const map<t_const_value*, t_const_value*, t_const_value::value_compare>& val = value->get_map();
map<t_const_value*, t_const_value*, t_const_value::value_compare>::const_iterator v_iter;
for (v_iter = val.begin(); v_iter != val.end(); ++v_iter) {
t_type* field_type = NULL;
for (f_iter = fields.begin(); f_iter != fields.end(); ++f_iter) {
Expand All @@ -576,8 +576,8 @@ class t_d_generator : public t_oop_generator {
} else if (type->is_map()) {
t_type* ktype = ((t_map*)type)->get_key_type();
t_type* vtype = ((t_map*)type)->get_val_type();
const map<t_const_value*, t_const_value*>& val = value->get_map();
map<t_const_value*, t_const_value*>::const_iterator v_iter;
const map<t_const_value*, t_const_value*, t_const_value::value_compare>& val = value->get_map();
map<t_const_value*, t_const_value*, t_const_value::value_compare>::const_iterator v_iter;
for (v_iter = val.begin(); v_iter != val.end(); ++v_iter) {
string key = render_const_value(ktype, v_iter->first);
string val = render_const_value(vtype, v_iter->second);
Expand Down
8 changes: 4 additions & 4 deletions compiler/cpp/src/thrift/generate/t_dart_generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -594,8 +594,8 @@ void t_dart_generator::print_const_value(std::ofstream& out,
} else if (type->is_struct() || type->is_xception()) {
const vector<t_field*>& fields = ((t_struct*)type)->get_members();
vector<t_field*>::const_iterator f_iter;
const map<t_const_value*, t_const_value*>& val = value->get_map();
map<t_const_value*, t_const_value*>::const_iterator v_iter;
const map<t_const_value*, t_const_value*, t_const_value::value_compare>& val = value->get_map();
map<t_const_value*, t_const_value*, t_const_value::value_compare>::const_iterator v_iter;
out << type_name(type) << " " << name << " = new " << type_name(type) << "()";
indent_up();
for (v_iter = val.begin(); v_iter != val.end(); ++v_iter) {
Expand Down Expand Up @@ -623,8 +623,8 @@ void t_dart_generator::print_const_value(std::ofstream& out,

t_type* ktype = ((t_map*)type)->get_key_type();
t_type* vtype = ((t_map*)type)->get_val_type();
const map<t_const_value*, t_const_value*>& val = value->get_map();
map<t_const_value*, t_const_value*>::const_iterator v_iter;
const map<t_const_value*, t_const_value*, t_const_value::value_compare>& val = value->get_map();
map<t_const_value*, t_const_value*, t_const_value::value_compare>::const_iterator v_iter;

for (v_iter = val.begin(); v_iter != val.end(); ++v_iter) {
string key = render_const_value(out, name, ktype, v_iter->first);
Expand Down
Loading

0 comments on commit 9e3944d

Please sign in to comment.