[
https://issues.apache.org/jira/browse/THRIFT-632?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12783780#action_12783780
]
David Reiss commented on THRIFT-632:
------------------------------------
{noformat}
- 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";
+ } else {
+ const vector<t_enum_value*>& enum_values =
((t_enum*)type)->get_constants();
+ vector<t_enum_value*>::const_iterator c_iter;
{noformat}
No need for an "else" since the "throw" transfers control. The code can just
continue after the "if".
Also, does this break the ability to use an integer as an enum constant. That
is going to be pretty annoying since it doesn't allow for a graceful upgrade.
{noformat}
pdebug("Const -> tok_const FieldType tok_identifier = ConstValue");
+// printf("first: %d last: %d\n", @2.first_line, @2.last_line);
+ //g_scope->resolve_const_value($5, $2);
if (g_parse_mode == PROGRAM) {
+ g_scope->resolve_const_value($5, $2);
{noformat}
Drop the debug code.
{noformat}
if (g_parse_mode == INCLUDES) {
+// printf("ignoring %s on line %d because we are in include mode!", $1,
yylineno);
// Ignore identifiers in include mode
{noformat}
Same.
{noformat}
+ } else {
+ // pwarning(1, "No enum value or constant found named \"%s\"\n",
get_identifier().c_str());
+ throw "No enum value or constant found named \"" +
const_val->get_identifier() + "\"!";
+ }
{noformat}
Move this up to the top of that if block (and reverse the test) so the common
path doesn't need to be indented.
Also, drop debug code.
{noformat}
+ } else {
+ // printf("setting enum for %s to %s\n",
const_val->get_identifier().c_str(), ttype->get_name().c_str());
+ const_val->set_enum((t_enum*)ttype);
+ }
{noformat}
Again, make this the "if" branch and the big block of code the "else" branch.
And remove debug code.
{noformat}
+ if (enum_ != NULL) {
+ t_enum_value* val = enum_->get_constant_by_name(get_identifier());
+ return val->get_value();
+ } else {
+ throw "have identifier \"" + get_identifier() + "\", but unset enum on
line!";
+ }
{noformat}
Reverse the branches.
{noformat}
+ void set_value(int val) {
+ has_value_ = true;
+ value_ = val;
+ }
{noformat}
Is this going to cause us to start generating explicit values for all enums? I
guess that is okay, but I'd prefer to see that as a separate commit, just in
case is causes some problem.
> Constants of enum types don't behave well
> -----------------------------------------
>
> Key: THRIFT-632
> URL: https://issues.apache.org/jira/browse/THRIFT-632
> Project: Thrift
> Issue Type: Bug
> Components: Compiler (Java)
> Affects Versions: 0.2
> Reporter: Bryan Duxbury
> Assignee: Bryan Duxbury
> Fix For: 0.2
>
> Attachments: thrift-632-v2.patch, thrift-632-v3.patch,
> thrift-632-v4.patch, thrift-632-v5.patch, thrift-632.patch
>
>
> It turns out that THRIFT-551 missed the case of constants defined of enum
> types.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
