thrift-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From David Reiss <dre...@facebook.com>
Subject Re: Java optional field bug?
Date Mon, 13 Jul 2009 23:24:30 GMT
Makes sense to me.  Bryan?

Chengdu Huang wrote:
> The change is really simple.  I'm attaching the patch and pasting it below:
> 
> Index: compiler/cpp/src/generate/t_java_generator.cc
> ===================================================================
> --- compiler/cpp/src/generate/t_java_generator.cc       (revision 792770)
> +++ compiler/cpp/src/generate/t_java_generator.cc       (working copy)
> @@ -1096,7 +1096,7 @@
>          indent() << "if (this." << (*f_iter)->get_name() << " !=
> null) {" << endl;
>        indent_up();
>      }
> -    bool optional = bean_style_ && (*f_iter)->get_req() == t_field::T_OPTIONAL;
> +    bool optional = (*f_iter)->get_req() == t_field::T_OPTIONAL;
>      if (optional) {
>        indent(out) << "if (" << generate_isset_check((*f_iter)) <<
")
> {" << endl;
>        indent_up();
> 
> Chengdu
> 
> On Thu, Jul 2, 2009 at 10:00 AM, David Reiss<dreiss@facebook.com> wrote:
>> Yeah, we never got around to implementing optional for the normal
>> Java generated code.  If you submit a patch for it, it would probably
>> be accepted.
>>
>> --David
>>
>> Bryan Duxbury wrote:
>>> This is not a "bug" per se. It's a known fact that it does this.
>>>
>>> For anything beyond trivial uses, I think you want to use the beans-
>>> style generator. I would personally go so far as to say that we
>>> shouldn't even have the default generator, but that's just me.
>>>
>>> -Bryan
>>>
>>> On Jul 1, 2009, at 5:15 PM, Chengdu Huang wrote:
>>>
>>>> I have a simple thrift file:
>>>>
>>>> struct UserDataContent {
>>>>     1: i32 id,
>>>>     2: optional i32 value,
>>>> }
>>>>
>>>> Java file generated by "thrift --gen java " doesn't seem to respect
>>>> the "optional" keyword:
>>>>
>>>>   public void write(TProtocol oprot) throws TException {
>>>>     validate();
>>>>
>>>>     oprot.writeStructBegin(STRUCT_DESC);
>>>>     oprot.writeFieldBegin(ID_FIELD_DESC);
>>>>     oprot.writeI32(this.id);
>>>>     oprot.writeFieldEnd();
>>>>     oprot.writeFieldBegin(VALUE_FIELD_DESC);
>>>>     oprot.writeI32(this.value);
>>>>     oprot.writeFieldEnd();
>>>>     oprot.writeFieldStop();
>>>>     oprot.writeStructEnd();
>>>>   }
>>>>
>>>> However, java file generated using "thrift --gen java:beans" looks
>>>> correct to me:
>>>>  public void write(TProtocol oprot) throws TException {
>>>>     validate();
>>>>
>>>>     oprot.writeStructBegin(STRUCT_DESC);
>>>>     oprot.writeFieldBegin(ID_FIELD_DESC);
>>>>     oprot.writeI32(this.id);
>>>>     oprot.writeFieldEnd();
>>>>     if (isSetValue()) {
>>>>       oprot.writeFieldBegin(VALUE_FIELD_DESC);
>>>>       oprot.writeI32(this.value);
>>>>       oprot.writeFieldEnd();
>>>>     }
>>>>     oprot.writeFieldStop();
>>>>     oprot.writeStructEnd();
>>>>   }
>>>>
>>>> My question is that why "normal" java style files should be different
>>>> from "beans" style files in terms of handling optional fields?
>>>>
>>>> The change is introduced in rev665308 (t_java_generator.cc:1105).
>>>>
>>>> Thanks,
>>>> Chengdu

Mime
View raw message