svn commit: r1236529 - in /thrift/trunk: compiler/cpp/src/generate/t_cpp_generator.cc lib/cpp/test/OptionalRequiredTest.cpp test/OptionalRequiredTest.thrift

[jf...@apache.org]

Author: jfarrell
Date: Fri Jan 27 04:48:26 2012
New Revision: 1236529

URL: http://svn.apache.org/viewvc?rev=1236529&view=rev
Log:
THRIFT-1394:Treatment of optional fields is not consistent between C++ and Java
Client: cpp
Patch: Diwaker Gupta

In CPP, all optional fields are guarded by the isset helper struct. On Java, however, the generated code takes advantage of nullable types: for containers, structs, exceptions, enums, and, notably, strings, the generator elides explicit use of an "isset" bit vector and instead emits checks of the form "field null". This leads to varying behavior between the two languages: an optional string field with a default value will have {{isset[fieldid]}} false on C, but the equivalent test in Java will be true.


Modified:
    thrift/trunk/compiler/cpp/src/generate/t_cpp_generator.cc
    thrift/trunk/lib/cpp/test/OptionalRequiredTest.cpp
    thrift/trunk/test/OptionalRequiredTest.thrift

Modified: thrift/trunk/compiler/cpp/src/generate/t_cpp_generator.cc
URL: http://svn.apache.org/viewvc/thrift/trunk/compiler/cpp/src/generate/t_cpp_generator.cc?rev=1236529&r1=1236528&r2=1236529&view=diff
==============================================================================
--- thrift/trunk/compiler/cpp/src/generate/t_cpp_generator.cc (original)
+++ thrift/trunk/compiler/cpp/src/generate/t_cpp_generator.cc Fri Jan 27 04:48:26 2012
@@ -815,13 +815,14 @@ void t_cpp_generator::generate_struct_de
       if ((*m_iter)->get_req() == t_field::T_REQUIRED) {
         continue;
       }
+      string isSet = ((*m_iter)->get_value() != NULL) ? "true" : "false";
       if (first) {
         first = false;
         out <<
-          ": " << (*m_iter)->get_name() << "(false)";
+          ": " << (*m_iter)->get_name() << "(" << isSet << ")";
       } else {
         out <<
-          ", " << (*m_iter)->get_name() << "(false)";
+          ", " << (*m_iter)->get_name() << "(" << isSet << ")";
       }
     }
     out << " {}" << endl;

Modified: thrift/trunk/lib/cpp/test/OptionalRequiredTest.cpp
URL: http://svn.apache.org/viewvc/thrift/trunk/lib/cpp/test/OptionalRequiredTest.cpp?rev=1236529&r1=1236528&r2=1236529&view=diff
==============================================================================
--- thrift/trunk/lib/cpp/test/OptionalRequiredTest.cpp (original)
+++ thrift/trunk/lib/cpp/test/OptionalRequiredTest.cpp Fri Jan 27 04:48:26 2012
@@ -242,5 +242,21 @@ int main() {
     assert(t1 != t2);
   }
 
+  {
+    OptionalDefault t1, t2;
+    cout << ThriftDebugString(t1) << endl;
+    assert(t1.__isset.opt_int == true);
+    assert(t1.__isset.opt_str == true);
+    assert(t1.opt_int == t2.opt_int);
+    assert(t1.opt_str == t2.opt_str);
+
+    write_to_read(t1, t2);
+    cout << ThriftDebugString(t2) << endl;
+    assert(t2.__isset.opt_int == true);
+    assert(t2.__isset.opt_str == true);
+    assert(t1.opt_int == t2.opt_int);
+    assert(t1.opt_str == t2.opt_str);
+  }
+
   return 0;
 }

Modified: thrift/trunk/test/OptionalRequiredTest.thrift
URL: http://svn.apache.org/viewvc/thrift/trunk/test/OptionalRequiredTest.thrift?rev=1236529&r1=1236528&r2=1236529&view=diff
==============================================================================
--- thrift/trunk/test/OptionalRequiredTest.thrift (original)
+++ thrift/trunk/test/OptionalRequiredTest.thrift Fri Jan 27 04:48:26 2012
@@ -49,6 +49,11 @@ struct Tricky3 {
   1: required i16 im_required;
 }
 
+struct OptionalDefault {
+  1: optional i16 opt_int = 1234;
+  2: optional string opt_str = "default";
+}
+
 struct Complex {
   1:          i16 cp_default;
   2: required i16 cp_required;