THRIFT-632. java: Constants of enum types don't behave well
This patch causes constants of all types to be resolved differently by the compiler, and makes it so that constants of enum types contain a reference to the enum type so that code generators can produce the correct names.
git-svn-id: https://svn.apache.org/repos/asf/incubator/thrift/trunk@892358 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/compiler/cpp/src/generate/t_java_generator.cc b/compiler/cpp/src/generate/t_java_generator.cc
index 1282e2d..09d2fd1 100644
--- a/compiler/cpp/src/generate/t_java_generator.cc
+++ b/compiler/cpp/src/generate/t_java_generator.cc
@@ -367,11 +367,11 @@
f_enum << string() +
"import java.util.Map;\n" +
"import java.util.HashMap;\n" +
- "import org.apache.thrift.TEnum;" << endl;
+ "import org.apache.thrift.TEnum;" << endl << endl;
generate_java_doc(f_enum, tenum);
indent(f_enum) <<
- "public enum " << tenum->get_name() << " implements TEnum";
+ "public enum " << tenum->get_name() << " implements TEnum ";
scope_up(f_enum);
vector<t_enum_value*> constants = tenum->get_constants();
@@ -392,7 +392,7 @@
}
generate_java_doc(f_enum, *c_iter);
- indent(f_enum) << " " << (*c_iter)->get_name() << "(" << value << ")";
+ indent(f_enum) << (*c_iter)->get_name() << "(" << value << ")";
}
f_enum << ";" << endl << endl;
@@ -487,7 +487,7 @@
string v2 = render_const_value(out, name, type, value);
out << name << " = " << v2 << ";" << endl << endl;
} else if (type->is_enum()) {
- out << name << " = " << value->get_integer() << ";" << endl << endl;
+ out << name << " = " << render_const_value(out, name, type, value) << ";" << endl << endl;
} 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;
@@ -602,7 +602,7 @@
throw "compiler error: no const of base type " + t_base_type::t_base_name(tbase);
}
} else if (type->is_enum()) {
- render << value->get_integer();
+ render << type_name(type, false, false) << "." << value->get_identifier();
} else {
string t = tmp("tmp");
print_const_value(out, t, type, value, true);
diff --git a/compiler/cpp/src/main.cc b/compiler/cpp/src/main.cc
index 7a5d2d4..7eb4801 100644
--- a/compiler/cpp/src/main.cc
+++ b/compiler/cpp/src/main.cc
@@ -702,9 +702,24 @@
throw "compiler error: no const of base type " + t_base_type::t_base_name(tbase) + name;
}
} else if (type->is_enum()) {
- if (value->get_type() != t_const_value::CV_INTEGER) {
+ if (value->get_type() != t_const_value::CV_IDENTIFIER) {
throw "type error: const \"" + name + "\" was declared as enum";
}
+
+ const vector<t_enum_value*>& enum_values = ((t_enum*)type)->get_constants();
+ vector<t_enum_value*>::const_iterator c_iter;
+ bool found = false;
+ for (c_iter = enum_values.begin(); c_iter != enum_values.end(); ++c_iter) {
+ if ((*c_iter)->get_name() == value->get_identifier()) {
+ found = true;
+ break;
+ }
+ }
+ if (!found) {
+ throw "type error: const " + name + " was declared as type "
+ + type->get_name() + " which is an enum, but "
+ + value->get_identifier() + " is not a valid value for that enum";
+ }
} else if (type->is_struct() || type->is_xception()) {
if (value->get_type() != t_const_value::CV_MAP) {
throw "type error: const \"" + name + "\" was declared as struct/xception";
diff --git a/compiler/cpp/src/parse/t_const_value.h b/compiler/cpp/src/parse/t_const_value.h
index a7d6e31..5bfaeb2 100644
--- a/compiler/cpp/src/parse/t_const_value.h
+++ b/compiler/cpp/src/parse/t_const_value.h
@@ -20,7 +20,7 @@
#ifndef T_CONST_VALUE_H
#define T_CONST_VALUE_H
-#include "t_const.h"
+#include "t_enum.h"
#include <stdint.h>
#include <map>
#include <vector>
@@ -38,7 +38,8 @@
CV_DOUBLE,
CV_STRING,
CV_MAP,
- CV_LIST
+ CV_LIST,
+ CV_IDENTIFIER
};
t_const_value() {}
@@ -66,7 +67,15 @@
}
int64_t get_integer() const {
- return intVal_;
+ if (valType_ == CV_IDENTIFIER) {
+ if (enum_ == NULL) {
+ throw "have identifier \"" + get_identifier() + "\", but unset enum on line!";
+ }
+ t_enum_value* val = enum_->get_constant_by_name(get_identifier());
+ return val->get_value();
+ } else {
+ return intVal_;
+ }
}
void set_double(double val) {
@@ -102,6 +111,19 @@
return listVal_;
}
+ void set_identifier(std::string val) {
+ valType_ = CV_IDENTIFIER;
+ identifierVal_ = val;
+ }
+
+ std::string get_identifier() const {
+ return identifierVal_;
+ }
+
+ void set_enum(t_enum* tenum) {
+ enum_ = tenum;
+ }
+
t_const_value_type get_type() const {
return valType_;
}
@@ -112,6 +134,8 @@
std::string stringVal_;
int64_t intVal_;
double doubleVal_;
+ std::string identifierVal_;
+ t_enum* enum_;
t_const_value_type valType_;
diff --git a/compiler/cpp/src/parse/t_enum.h b/compiler/cpp/src/parse/t_enum.h
index 740f95c..45d1606 100644
--- a/compiler/cpp/src/parse/t_enum.h
+++ b/compiler/cpp/src/parse/t_enum.h
@@ -44,6 +44,28 @@
return constants_;
}
+ t_enum_value* get_constant_by_name(const std::string name) {
+ const std::vector<t_enum_value*>& enum_values = get_constants();
+ std::vector<t_enum_value*>::const_iterator c_iter;
+ for (c_iter = enum_values.begin(); c_iter != enum_values.end(); ++c_iter) {
+ if ((*c_iter)->get_name() == name) {
+ return *c_iter;
+ }
+ }
+ return NULL;
+ }
+
+ t_enum_value* get_constant_by_value(int64_t value) {
+ const std::vector<t_enum_value*>& enum_values = get_constants();
+ std::vector<t_enum_value*>::const_iterator c_iter;
+ for (c_iter = enum_values.begin(); c_iter != enum_values.end(); ++c_iter) {
+ if ((*c_iter)->get_value() == value) {
+ return *c_iter;
+ }
+ }
+ return NULL;
+ }
+
bool is_enum() const {
return true;
}
@@ -52,6 +74,19 @@
return "enum";
}
+ void resolve_values() {
+ const std::vector<t_enum_value*>& enum_values = get_constants();
+ std::vector<t_enum_value*>::const_iterator c_iter;
+ int lastValue = -1;
+ for (c_iter = enum_values.begin(); c_iter != enum_values.end(); ++c_iter) {
+ if (! (*c_iter)->has_value()) {
+ (*c_iter)->set_value(++lastValue);
+ } else {
+ lastValue = (*c_iter)->get_value();
+ }
+ }
+ }
+
private:
std::vector<t_enum_value*> constants_;
};
diff --git a/compiler/cpp/src/parse/t_enum_value.h b/compiler/cpp/src/parse/t_enum_value.h
index 68e905b..283a87e 100644
--- a/compiler/cpp/src/parse/t_enum_value.h
+++ b/compiler/cpp/src/parse/t_enum_value.h
@@ -55,6 +55,11 @@
return value_;
}
+ void set_value(int val) {
+ has_value_ = true;
+ value_ = val;
+ }
+
private:
std::string name_;
bool has_value_;
diff --git a/compiler/cpp/src/parse/t_scope.h b/compiler/cpp/src/parse/t_scope.h
index 122e325..d3f9d26 100644
--- a/compiler/cpp/src/parse/t_scope.h
+++ b/compiler/cpp/src/parse/t_scope.h
@@ -23,8 +23,14 @@
#include <map>
#include <string>
+#include <boost/lexical_cast.hpp>
#include "t_type.h"
#include "t_service.h"
+#include "t_const.h"
+#include "t_const_value.h"
+#include "t_base_type.h"
+#include "t_map.h"
+#include "t_list.h"
/**
* This represents a variable scope used for looking up predefined types and
@@ -70,6 +76,86 @@
}
}
+ void resolve_const_value(t_const_value* const_val, t_type* ttype) {
+ if (ttype->is_map()) {
+ const std::map<t_const_value*, t_const_value*>& map = const_val->get_map();
+ std::map<t_const_value*, t_const_value*>::const_iterator v_iter;
+ for (v_iter = map.begin(); v_iter != map.end(); ++v_iter) {
+ resolve_const_value(v_iter->first, ((t_map*)ttype)->get_key_type());
+ resolve_const_value(v_iter->second, ((t_map*)ttype)->get_val_type());
+ }
+ } else if (ttype->is_list() || ttype->is_set()) {
+ const std::vector<t_const_value*>& val = const_val->get_list();
+ std::vector<t_const_value*>::const_iterator v_iter;
+ for (v_iter = val.begin(); v_iter != val.end(); ++v_iter) {
+ resolve_const_value((*v_iter), ((t_list*)ttype)->get_elem_type());
+ }
+ } else if (ttype->is_struct()) {
+ t_struct* tstruct = (t_struct*)ttype;
+ const std::map<t_const_value*, t_const_value*>& map = const_val->get_map();
+ std::map<t_const_value*, t_const_value*>::const_iterator v_iter;
+ for (v_iter = map.begin(); v_iter != map.end(); ++v_iter) {
+ t_field* field = tstruct->get_field_by_name(v_iter->first->get_string());
+ resolve_const_value(v_iter->second, field->get_type());
+ }
+ } else if (const_val->get_type() == t_const_value::CV_IDENTIFIER) {
+ if (ttype->is_enum()) {
+ const_val->set_enum((t_enum*)ttype);
+ } else {
+ t_const* constant = get_constant(const_val->get_identifier());
+ if (constant == NULL) {
+ throw "No enum value or constant found named \"" + const_val->get_identifier() + "\"!";
+ }
+
+ if (constant->get_type()->is_base_type()) {
+ switch (((t_base_type*)constant->get_type())->get_base()) {
+ case t_base_type::TYPE_I16:
+ case t_base_type::TYPE_I32:
+ case t_base_type::TYPE_I64:
+ case t_base_type::TYPE_BOOL:
+ case t_base_type::TYPE_BYTE:
+ const_val->set_integer(constant->get_value()->get_integer());
+ break;
+ case t_base_type::TYPE_STRING:
+ const_val->set_string(constant->get_value()->get_string());
+ break;
+ case t_base_type::TYPE_DOUBLE:
+ const_val->set_double(constant->get_value()->get_double());
+ break;
+ case t_base_type::TYPE_VOID:
+ throw "Constants cannot be of type VOID";
+ }
+ } else if (constant->get_type()->is_map()) {
+ const std::map<t_const_value*, t_const_value*>& map = constant->get_value()->get_map();
+ std::map<t_const_value*, t_const_value*>::const_iterator v_iter;
+
+ const_val->set_map();
+ for (v_iter = map.begin(); v_iter != map.end(); ++v_iter) {
+ const_val->add_map(v_iter->first, v_iter->second);
+ }
+ } else if (constant->get_type()->is_list()) {
+ const std::vector<t_const_value*>& val = constant->get_value()->get_list();
+ std::vector<t_const_value*>::const_iterator v_iter;
+
+ const_val->set_list();
+ for (v_iter = val.begin(); v_iter != val.end(); ++v_iter) {
+ const_val->add_list(*v_iter);
+ }
+ }
+ }
+ } else if (ttype->is_enum()) {
+ // enum constant with non-identifier value. set the enum and find the
+ // value's name.
+ t_enum* tenum = (t_enum*)ttype;
+ t_enum_value* enum_value = tenum->get_constant_by_value(const_val->get_integer());
+ if (enum_value == NULL) {
+ throw "Couldn't find a named value in enum " + tenum->get_name() + " for value " + boost::lexical_cast<std::string>(const_val->get_integer());
+ }
+ const_val->set_identifier(enum_value->get_name());
+ const_val->set_enum(tenum);
+ }
+ }
+
private:
// Map of names to types
diff --git a/compiler/cpp/src/parse/t_struct.h b/compiler/cpp/src/parse/t_struct.h
index 76c24f2..aeee112 100644
--- a/compiler/cpp/src/parse/t_struct.h
+++ b/compiler/cpp/src/parse/t_struct.h
@@ -123,6 +123,16 @@
}
}
+ t_field* get_field_by_name(std::string field_name) {
+ members_type::const_iterator m_iter;
+ for (m_iter = members_in_id_order_.begin(); m_iter != members_in_id_order_.end(); ++m_iter) {
+ if ((*m_iter)->get_name() == field_name) {
+ return *m_iter;
+ }
+ }
+ return NULL;
+ }
+
private:
members_type members_;
diff --git a/compiler/cpp/src/thrifty.yy b/compiler/cpp/src/thrifty.yy
index 987dd8d..0fc90da 100644
--- a/compiler/cpp/src/thrifty.yy
+++ b/compiler/cpp/src/thrifty.yy
@@ -504,6 +504,7 @@
pdebug("Enum -> tok_enum tok_identifier { EnumDefList }");
$$ = $4;
$$->set_name($2);
+ $$->resolve_values();
}
EnumDefList:
@@ -583,6 +584,7 @@
{
pdebug("Const -> tok_const FieldType tok_identifier = ConstValue");
if (g_parse_mode == PROGRAM) {
+ g_scope->resolve_const_value($5, $2);
$$ = new t_const($2, $3, $5);
validate_const_type($$);
@@ -590,7 +592,6 @@
if (g_parent_scope != NULL) {
g_parent_scope->add_constant(g_parent_prefix + $3, $$);
}
-
} else {
$$ = NULL;
}
@@ -620,15 +621,8 @@
| tok_identifier
{
pdebug("ConstValue => tok_identifier");
- t_const* constant = g_scope->get_constant($1);
- if (constant != NULL) {
- $$ = constant->get_value();
- } else {
- if (g_parse_mode == PROGRAM) {
- pwarning(1, "Constant strings should be quoted: %s\n", $1);
- }
- $$ = new t_const_value($1);
- }
+ $$ = new t_const_value();
+ $$->set_identifier($1);
}
| ConstList
{
@@ -872,6 +866,7 @@
$$ = new t_field($4, $5, $2);
$$->set_req($3);
if ($6 != NULL) {
+ g_scope->resolve_const_value($6, $4);
validate_field_value($$, $6);
$$->set_value($6);
}