add private_optional support (and wire up tests/CI)

.github/workflows/cmake.yml

@@ -38,3 +38,11 @@ jobs:
         run: |
           cd cmake_build
           ctest -j$(nproc)
+
+      - name: Upload LastTest log
+        if: always()
+        uses: actions/upload-artifact@v4
+        with:
+          name: cmake-LastTest-log
+          path: cmake_build/Testing/Temporary/LastTest.log
+          if-no-files-found: warn

build/docker/msvc2017/build-compiler.bat

@@ -29,13 +29,14 @@ cmake^
   -DBOOST_ROOT=C:\Libraries\boost_1_89_0^
   -DBOOST_LIBRARYDIR=C:\Libraries\boost_1_89_0\lib64-msvc-14.1^
   -DBUILD_LIBRARIES=OFF^
+  -DBUILD_TESTING=ON^
   -DCMAKE_BUILD_TYPE=Release^
   -DCMAKE_INSTALL_PREFIX=C:\install^
   -DWITH_MT=ON^
   c:\thrift || EXIT /B
 
 :: Build
-cmake --build . --target thrift-compiler --config Release || EXIT /B
+cmake --build . --target thrift-compiler thrift_compiler_tests --config Release || EXIT /B
 
 :: Test
 ctest -C Release || EXIT /B

compiler/cpp/CMakeLists.txt

@@ -143,4 +143,5 @@ install(TARGETS thrift-compiler DESTINATION bin)
 
 if(BUILD_TESTING)
     add_subdirectory(test)
+    add_subdirectory(tests)
 endif()

compiler/cpp/src/thrift/generate/t_cpp_generator.cc

@@ -67,6 +67,7 @@ class t_cpp_generator : public t_oop_generator {
     gen_no_ostream_operators_ = false;
     gen_no_skeleton_ = false;
     gen_no_constructors_ = false;
+    gen_private_optional_ = false;
     has_members_ = false;
 
     for( iter = parsed_options.begin(); iter != parsed_options.end(); ++iter) {
@@ -91,6 +92,8 @@ class t_cpp_generator : public t_oop_generator {
         gen_no_skeleton_ = true;
       } else if ( iter->first.compare("no_constructors") == 0) {
         gen_no_constructors_ = true;
+      } else if ( iter->first.compare("private_optional") == 0) {
+        gen_private_optional_ = true;
       } else {
         throw "unknown option cpp:" + iter->first;
       }
@@ -161,6 +164,7 @@ class t_cpp_generator : public t_oop_generator {
   void generate_struct_writer(std::ostream& out, t_struct* tstruct, bool pointers = false);
   void generate_struct_result_writer(std::ostream& out, t_struct* tstruct, bool pointers = false);
   void generate_struct_swap(std::ostream& out, t_struct* tstruct);
+  void generate_struct_swap_decl(std::ostream& out, t_struct* tstruct);
   void generate_struct_print_method(std::ostream& out, t_struct* tstruct);
   void generate_exception_what_method(std::ostream& out, t_struct* tstruct);
 
@@ -384,6 +388,11 @@ class t_cpp_generator : public t_oop_generator {
    */
   bool gen_no_constructors_;
 
+  /**
+   * True if we should generate optional fields as private members with getters.
+   */
+  bool gen_private_optional_;
+
   /**
    * True if thrift has member(s)
    */
@@ -1315,12 +1324,26 @@ void t_cpp_generator::generate_struct_declaration(ostream& out,
   }
 
   // Declare all fields
-  for (m_iter = members.begin(); m_iter != members.end(); ++m_iter) {
-    generate_java_doc(out, *m_iter);
-    indent(out) << declare_field(*m_iter,
-                                 !pointers && gen_no_constructors_,
-                                 (pointers && !(*m_iter)->get_type()->is_xception()),
-                                 !read) << '\n';
+  if (gen_private_optional_ && !pointers) {
+    // When private_optional is enabled, declare non-optional fields first in public section
+    for (m_iter = members.begin(); m_iter != members.end(); ++m_iter) {
+      if ((*m_iter)->get_req() != t_field::T_OPTIONAL) {
+        generate_java_doc(out, *m_iter);
+        indent(out) << declare_field(*m_iter,
+                                     gen_no_constructors_,
+                                     false,
+                                     !read) << '\n';
+      }
+    }
+  } else {
+    // Default behavior: all fields in public section
+    for (m_iter = members.begin(); m_iter != members.end(); ++m_iter) {
+      generate_java_doc(out, *m_iter);
+      indent(out) << declare_field(*m_iter,
+                                   !pointers && gen_no_constructors_,
+                                   (pointers && !(*m_iter)->get_type()->is_xception()),
+                                   !read) << '\n';
+    }
   }
 
   // Add the __isset data member if we need it, using the definition from above
@@ -1343,6 +1366,19 @@ void t_cpp_generator::generate_struct_declaration(ostream& out,
       out << " val);" << '\n';
     }
   }
+
+  // Generate getter methods when private_optional is enabled
+  if (gen_private_optional_ && !pointers) {
+    for (m_iter = members.begin(); m_iter != members.end(); ++m_iter) {
+      std::string field_type = type_name((*m_iter)->get_type());
+      if (is_reference((*m_iter))) {
+        field_type = "::std::shared_ptr<" + field_type + ">";
+      }
+      // Const getter only
+      out << '\n' << indent() << "const " << field_type << "& __get_" << (*m_iter)->get_name() 
+          << "() const { return " << (*m_iter)->get_name() << "; }" << '\n';
+    }
+  }
   out << '\n';
 
   if (!pointers) {
@@ -1405,18 +1441,55 @@ void t_cpp_generator::generate_struct_declaration(ostream& out,
     out << ";" << '\n';
   }
 
+  // Generate private section for optional fields when private_optional is enabled
+  if (gen_private_optional_ && !pointers) {
+    bool has_optional_fields = false;
+    for (m_iter = members.begin(); m_iter != members.end(); ++m_iter) {
+      if ((*m_iter)->get_req() == t_field::T_OPTIONAL) {
+        has_optional_fields = true;
+        break;
+      }
+    }
+
+    if (has_optional_fields) {
+      indent_down();
+      out << '\n' << indent() << " private:" << '\n';
+      indent_up();
+
+      // Declare optional fields in private section
+      for (m_iter = members.begin(); m_iter != members.end(); ++m_iter) {
+        if ((*m_iter)->get_req() == t_field::T_OPTIONAL) {
+          generate_java_doc(out, *m_iter);
+          indent(out) << declare_field(*m_iter,
+                                       gen_no_constructors_,
+                                       false,
+                                       !read) << '\n';
+        }
+      }
+    }
+  }
+
+  // When private_optional is enabled, optional members may be private.
+  // The generated namespace-scope swap() needs friend access.
+  if (swap && gen_private_optional_) {
+    indent(out) << "friend ";
+    generate_struct_swap_decl(out, tstruct);
+  }
+
+  // When private_optional is enabled, optional members may be private.
+  // The generated namespace-scope operator<< needs friend access.
+  if (is_user_struct && gen_private_optional_) {
+    indent(out) << "friend ";
+    generate_struct_ostream_operator_decl(out, tstruct);
+  }
+
   indent_down();
   indent(out) << "};" << '\n' << '\n';
 
   if (swap) {
     // Generate a namespace-scope swap() function
-    if (tstruct->get_name() == "a" || tstruct->get_name() == "b") {
-      out << indent() << "void swap(" << tstruct->get_name() << " &a1, " << tstruct->get_name()
-          << " &a2) noexcept;" << '\n' << '\n';
-    } else {
-       out << indent() << "void swap(" << tstruct->get_name() << " &a, " << tstruct->get_name()
-           << " &b) noexcept;" << '\n' << '\n';
-    }
+    out << indent();
+    generate_struct_swap_decl(out, tstruct);
   }
 
   if (is_user_struct) {
@@ -1798,6 +1871,17 @@ void t_cpp_generator::generate_struct_swap(ostream& out, t_struct* tstruct) {
   out << '\n';
 }
 
+void t_cpp_generator::generate_struct_swap_decl(std::ostream& out, t_struct* tstruct) {
+  if (tstruct->get_name() == "a" || tstruct->get_name() == "b") {
+    out << "void swap(" << tstruct->get_name() << " &a1, " << tstruct->get_name()
+        << " &a2) noexcept;";
+  } else {
+    out << "void swap(" << tstruct->get_name() << " &a, " << tstruct->get_name()
+        << " &b) noexcept;";
+  }
+  out << '\n' << '\n';
+}
+
 void t_cpp_generator::generate_struct_ostream_operator_decl(std::ostream& out, t_struct* tstruct) {
   out << "std::ostream& operator<<(std::ostream& out, const "
       << tstruct->get_name()