## cpp: add `private_optional` support (and wire up tests/CI)
Add a new `cpp:private_optional` generator option for C++ that emits optional fields as private members and provides const getters, enabling stricter encapsulation while preserving access for generated helpers.
To keep the feature stable and exercised in automation, add fixture-based compiler tests and the minimal build/CI wiring required for those tests to build and run in the workflow (including MSVC).
### Example generated code (behavior change only, from `TestStruct`)
#### Default (no `cpp:private_optional`): optional fields stay public
```cpp
public:
int32_t required_field;
int32_t optional_field;
std::string optional_string;
```
With cpp:private_optional: optional fields become private + const getters
```cpp
public:
int32_t required_field;
const int32_t& __get_optional_field() const { return optional_field; }
const std::string& __get_optional_string() const { return optional_string; }
private:
int32_t optional_field;
std::string optional_string;
friend void swap(TestStruct &a, TestStruct &b) noexcept;
friend std::ostream& operator<<(std::ostream& out, const TestStruct& obj);
```
diff --git a/compiler/cpp/CMakeLists.txt b/compiler/cpp/CMakeLists.txt
index eeef203..2f5cb7a 100644
--- a/compiler/cpp/CMakeLists.txt
+++ b/compiler/cpp/CMakeLists.txt
@@ -143,4 +143,5 @@
if(BUILD_TESTING)
add_subdirectory(test)
+ add_subdirectory(tests)
endif()
diff --git a/compiler/cpp/src/thrift/generate/t_cpp_generator.cc b/compiler/cpp/src/thrift/generate/t_cpp_generator.cc
index 013628d..f6a15b3 100644
--- a/compiler/cpp/src/thrift/generate/t_cpp_generator.cc
+++ b/compiler/cpp/src/thrift/generate/t_cpp_generator.cc
@@ -67,6 +67,7 @@
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 @@
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 @@
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);
@@ -385,6 +389,11 @@
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)
*/
bool has_members_;
@@ -1315,12 +1324,26 @@
}
// 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 @@
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 @@
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 @@
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()
diff --git a/compiler/cpp/tests/CMakeLists.txt b/compiler/cpp/tests/CMakeLists.txt
index f990895..77c1524 100644
--- a/compiler/cpp/tests/CMakeLists.txt
+++ b/compiler/cpp/tests/CMakeLists.txt
@@ -41,21 +41,23 @@
find_package(FLEX REQUIRED)
find_package(BISON REQUIRED)
-# create directory for thrifty and thriftl
-file(MAKE_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/thrift/)
+if(NOT TARGET parse)
+ # create directory for thrifty and thriftl
+ file(MAKE_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/thrift/)
-# Create flex and bison files and build the lib parse static library
-BISON_TARGET(thrifty ${THRIFT_COMPILER_SOURCE_DIR}/src/thrift/thrifty.yy ${CMAKE_CURRENT_BINARY_DIR}/thrift/thrifty.cc COMPILE_FLAGS "--file-prefix-map=${CMAKE_BINARY_DIR}='' --file-prefix-map=${CMAKE_SOURCE_DIR}=''")
-FLEX_TARGET(thriftl ${THRIFT_COMPILER_SOURCE_DIR}/src/thrift/thriftl.ll ${CMAKE_CURRENT_BINARY_DIR}/thrift/thriftl.cc)
-ADD_FLEX_BISON_DEPENDENCY(thriftl thrifty)
+ # Create flex and bison files and build the lib parse static library
+ BISON_TARGET(thrifty ${THRIFT_COMPILER_SOURCE_DIR}/src/thrift/thrifty.yy ${CMAKE_CURRENT_BINARY_DIR}/thrift/thrifty.cc COMPILE_FLAGS "--file-prefix-map=${CMAKE_BINARY_DIR}='' --file-prefix-map=${CMAKE_SOURCE_DIR}=''")
+ FLEX_TARGET(thriftl ${THRIFT_COMPILER_SOURCE_DIR}/src/thrift/thriftl.ll ${CMAKE_CURRENT_BINARY_DIR}/thrift/thriftl.cc)
+ ADD_FLEX_BISON_DEPENDENCY(thriftl thrifty)
-set(parse_SOURCES
- ${CMAKE_CURRENT_BINARY_DIR}/thrift/thrifty.cc
- ${CMAKE_CURRENT_BINARY_DIR}/thrift/thriftl.cc
- ${CMAKE_CURRENT_BINARY_DIR}/thrift/thrifty.hh
-)
+ set(parse_SOURCES
+ ${CMAKE_CURRENT_BINARY_DIR}/thrift/thrifty.cc
+ ${CMAKE_CURRENT_BINARY_DIR}/thrift/thriftl.cc
+ ${CMAKE_CURRENT_BINARY_DIR}/thrift/thrifty.hh
+ )
-add_library(parse STATIC ${parse_SOURCES})
+ add_library(parse STATIC ${parse_SOURCES})
+endif()
# Thrift compiler tests
set(thrift_compiler_tests
@@ -72,6 +74,8 @@
)
set(thrift_compiler_SOURCES
+ ${CMAKE_CURRENT_SOURCE_DIR}/thrift_test_globals.cc
+ ${CMAKE_CURRENT_SOURCE_DIR}/thrift_test_parser_support.cc
${THRIFT_COMPILER_SOURCE_DIR}/src/thrift/logging.cc # we use logging instead of main to avoid breaking compillation (2 main v)
${THRIFT_COMPILER_SOURCE_DIR}/src/thrift/audit/t_audit.cpp
${THRIFT_COMPILER_SOURCE_DIR}/src/thrift/common.cc
@@ -91,10 +95,11 @@
option(${enabler} ${description} ${initial})
if(${enabler})
list(APPEND thrift_compiler_SOURCES ${src})
- file(GLOB thrift_compiler_tests_SOURCES
+ file(GLOB temp_test_sources
"${CMAKE_CURRENT_SOURCE_DIR}/${name}/*.c*"
"${CMAKE_CURRENT_SOURCE_DIR}/${name}/*.thrift"
)
+ list(APPEND thrift_compiler_tests_SOURCES ${temp_test_sources})
endif()
endmacro()
@@ -106,14 +111,14 @@
list(APPEND "${THRIFT_COMPILER_SOURCE_DIR}/src/thrift/generate/${name}_validator_generator.h")
option(${enabler} ${description} ${initial})
if(${enabler})
- list(APPEND thrift-compiler_SOURCES ${src})
+ list(APPEND thrift_compiler_SOURCES ${src})
endif()
endmacro()
# The following compiler with unit tests can be enabled or disabled
THRIFT_ADD_COMPILER(c_glib "Enable compiler for C with Glib" OFF)
THRIFT_ADD_COMPILER(cl "Enable compiler for Common LISP" OFF)
-THRIFT_ADD_COMPILER(cpp "Enable compiler for C++" OFF)
+THRIFT_ADD_COMPILER(cpp "Enable compiler for C++" ON)
THRIFT_ADD_COMPILER(d "Enable compiler for D" OFF)
THRIFT_ADD_COMPILER(dart "Enable compiler for Dart" OFF)
THRIFT_ADD_COMPILER(delphi "Enable compiler for Delphi" OFF)
@@ -142,11 +147,19 @@
# The following compiler can be enabled or disabled by enabling or disabling certain languages
THRIFT_ADD_VALIDATOR_COMPILER(go "Enable validator compiler for Go" ON)
-# Thrift is looking for include files in the src directory
-# we also add the current binary directory for generated files
-include_directories(${CMAKE_CURRENT_BINARY_DIR} ${THRIFT_COMPILER_SOURCE_DIR}/src ${CMAKE_CURRENT_SOURCE_DIR}/catch)
+# OCaml tests include the implementation .cc directly, so compiling it into the
+# thrift_compiler lib would cause duplicate definitions (LNK2005).
+list(REMOVE_ITEM thrift_compiler_SOURCES
+ "${THRIFT_COMPILER_SOURCE_DIR}/src/thrift/generate/t_ocaml_generator.cc"
+)
-add_library(thrift_compiler ${thrift_compiler_SOURCES})
+# Thrift is looking for include files in the src directory
+# We also add:
+# - the current binary directory for locally generated files (standalone tests build)
+# - the top-level binary directory for generated files when reusing targets from the parent build
+include_directories(${CMAKE_CURRENT_BINARY_DIR} ${CMAKE_BINARY_DIR} ${THRIFT_COMPILER_SOURCE_DIR}/src ${CMAKE_CURRENT_SOURCE_DIR}/catch)
+
+add_library(thrift_compiler STATIC ${thrift_compiler_SOURCES})
#link parse lib to thrift_compiler lib
target_link_libraries(thrift_compiler parse)
@@ -162,7 +175,93 @@
set_target_properties(thrift_compiler_tests PROPERTIES RUNTIME_OUTPUT_DIRECTORY bin/)
set_target_properties(thrift_compiler_tests PROPERTIES OUTPUT_NAME thrift_compiler_tests)
-target_link_libraries(thrift_compiler_tests thrift_compiler)
+# Ensure generator registration translation units are linked in.
+# Many generators register themselves via static initialization; linkers may
+# otherwise discard those objects from static libraries.
+if(MSVC)
+ target_link_libraries(thrift_compiler_tests PRIVATE thrift_compiler)
+ target_link_options(thrift_compiler_tests PRIVATE
+ "/WHOLEARCHIVE:$<TARGET_LINKER_FILE:thrift_compiler>"
+ )
+elseif(APPLE)
+ target_link_libraries(thrift_compiler_tests PRIVATE thrift_compiler)
+ target_link_options(thrift_compiler_tests PRIVATE
+ "-Wl,-force_load,$<TARGET_LINKER_FILE:thrift_compiler>"
+ )
+else()
+ target_link_libraries(thrift_compiler_tests PRIVATE
+ "-Wl,--whole-archive" thrift_compiler "-Wl,--no-whole-archive"
+ )
+endif()
+
+# Compile-check generated C++ output for the fixture thrift file with private_optional enabled.
+# This ensures the generator output is compileable (no link step required).
+if(TARGET thrift-compiler)
+ # Generated C++ includes Thrift runtime headers which may require Boost.
+ # Only enable the compile-check when Boost headers are available.
+ set(_private_optional_boost_include_dirs "")
+ find_package(Boost QUIET)
+ if(Boost_FOUND)
+ set(_private_optional_boost_include_dirs ${Boost_INCLUDE_DIRS})
+ elseif(DEFINED BOOST_ROOT)
+ if(EXISTS "${BOOST_ROOT}/include/boost")
+ set(_private_optional_boost_include_dirs "${BOOST_ROOT}/include")
+ elseif(EXISTS "${BOOST_ROOT}/boost")
+ set(_private_optional_boost_include_dirs "${BOOST_ROOT}")
+ endif()
+ endif()
+
+ if(_private_optional_boost_include_dirs STREQUAL "")
+ message(STATUS "Skipping generated private_optional compile-check (Boost headers not found)")
+ else()
+ set(_private_optional_thrift
+ "${CMAKE_CURRENT_SOURCE_DIR}/cpp/test_private_optional.thrift"
+ )
+ set(_private_optional_gen_out_dir
+ "${CMAKE_CURRENT_BINARY_DIR}/generated-private-optional"
+ )
+ set(_private_optional_gen_cpp_dir
+ "${_private_optional_gen_out_dir}/gen-cpp"
+ )
+ set(_private_optional_types_cpp
+ "${_private_optional_gen_cpp_dir}/test_private_optional_types.cpp"
+ )
+
+ add_custom_command(
+ OUTPUT "${_private_optional_types_cpp}"
+ COMMAND ${CMAKE_COMMAND} -E make_directory "${_private_optional_gen_out_dir}"
+ COMMAND ${CMAKE_COMMAND} -E chdir "${_private_optional_gen_out_dir}"
+ $<TARGET_FILE:thrift-compiler>
+ --gen cpp:private_optional
+ -o "${_private_optional_gen_out_dir}"
+ "${_private_optional_thrift}"
+ DEPENDS thrift-compiler "${_private_optional_thrift}"
+ VERBATIM
+ )
+
+ set_source_files_properties(
+ "${_private_optional_types_cpp}"
+ PROPERTIES GENERATED TRUE
+ )
+
+ add_library(thrift_compiler_generated_private_optional STATIC
+ "${_private_optional_types_cpp}"
+ )
+
+ target_include_directories(thrift_compiler_generated_private_optional PRIVATE
+ "${_private_optional_gen_cpp_dir}"
+ "${THRIFT_COMPILER_SOURCE_DIR}/../../lib/cpp/src"
+ "${CMAKE_CURRENT_BINARY_DIR}"
+ "${CMAKE_BINARY_DIR}"
+ ${_private_optional_boost_include_dirs}
+ )
+
+ # Build the compile-check as part of the standard test build.
+ add_dependencies(thrift_compiler_tests thrift_compiler_generated_private_optional)
+ endif()
+else()
+ message(STATUS "Skipping generated private_optional compile-check (no thrift-compiler target)")
+endif()
enable_testing()
-add_test(NAME ThriftTests COMMAND thrift_compiler_tests)
+add_test(NAME ThriftCompilerTests COMMAND thrift_compiler_tests)
diff --git a/compiler/cpp/tests/cpp/expected_TestStruct_default.txt b/compiler/cpp/tests/cpp/expected_TestStruct_default.txt
new file mode 100644
index 0000000..bce1685
--- /dev/null
+++ b/compiler/cpp/tests/cpp/expected_TestStruct_default.txt
@@ -0,0 +1,52 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+class TestStruct : public virtual ::apache::thrift::TBase {
+ public:
+
+ TestStruct(const TestStruct&);
+ TestStruct& operator=(const TestStruct&);
+ TestStruct() noexcept;
+
+ virtual ~TestStruct() noexcept;
+ int32_t required_field;
+ int32_t optional_field;
+ int32_t default_field;
+ std::string optional_string;
+
+ _TestStruct__isset __isset;
+
+ void __set_required_field(const int32_t val);
+
+ void __set_optional_field(const int32_t val);
+
+ void __set_default_field(const int32_t val);
+
+ void __set_optional_string(const std::string& val);
+
+ bool operator == (const TestStruct & rhs) const;
+ bool operator != (const TestStruct &rhs) const {
+ return !(*this == rhs);
+ }
+
+ bool operator < (const TestStruct & ) const;
+
+ uint32_t read(::apache::thrift::protocol::TProtocol* iprot) override;
+ uint32_t write(::apache::thrift::protocol::TProtocol* oprot) const override;
+
+ virtual void printTo(std::ostream& out) const;
+};
diff --git a/compiler/cpp/tests/cpp/expected_TestStruct_private_optional.txt b/compiler/cpp/tests/cpp/expected_TestStruct_private_optional.txt
new file mode 100644
index 0000000..7c42909
--- /dev/null
+++ b/compiler/cpp/tests/cpp/expected_TestStruct_private_optional.txt
@@ -0,0 +1,66 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+class TestStruct : public virtual ::apache::thrift::TBase {
+ public:
+
+ TestStruct(const TestStruct&);
+ TestStruct& operator=(const TestStruct&);
+ TestStruct() noexcept;
+
+ virtual ~TestStruct() noexcept;
+ int32_t required_field;
+ int32_t default_field;
+
+ _TestStruct__isset __isset;
+
+ void __set_required_field(const int32_t val);
+
+ void __set_optional_field(const int32_t val);
+
+ void __set_default_field(const int32_t val);
+
+ void __set_optional_string(const std::string& val);
+
+ const int32_t& __get_required_field() const { return required_field; }
+
+ const int32_t& __get_optional_field() const { return optional_field; }
+
+ const int32_t& __get_default_field() const { return default_field; }
+
+ const std::string& __get_optional_string() const { return optional_string; }
+
+ bool operator == (const TestStruct & rhs) const;
+ bool operator != (const TestStruct &rhs) const {
+ return !(*this == rhs);
+ }
+
+ bool operator < (const TestStruct & ) const;
+
+ uint32_t read(::apache::thrift::protocol::TProtocol* iprot) override;
+ uint32_t write(::apache::thrift::protocol::TProtocol* oprot) const override;
+
+ virtual void printTo(std::ostream& out) const;
+
+ private:
+ int32_t optional_field;
+ std::string optional_string;
+
+ friend void swap(TestStruct &a, TestStruct &b) noexcept;
+
+ friend std::ostream& operator<<(std::ostream& out, const TestStruct& obj);
+};
diff --git a/compiler/cpp/tests/cpp/t_cpp_generator_private_optional_tests.cc b/compiler/cpp/tests/cpp/t_cpp_generator_private_optional_tests.cc
new file mode 100644
index 0000000..9f48229
--- /dev/null
+++ b/compiler/cpp/tests/cpp/t_cpp_generator_private_optional_tests.cc
@@ -0,0 +1,230 @@
+// Licensed to the Apache Software Foundation(ASF) under one
+// or more contributor license agreements.See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "../catch/catch.hpp"
+#include <thrift/parse/t_program.h>
+#include <thrift/generate/t_generator.h>
+#include <thrift/generate/t_generator_registry.h>
+#include <thrift/globals.h>
+#include <thrift/main.h>
+#include <fstream>
+#include <memory>
+#include <string>
+#include <sstream>
+
+#include <algorithm>
+
+using std::string;
+using std::map;
+using std::ifstream;
+using std::ofstream;
+using std::ostringstream;
+
+// Provided by compiler/cpp/tests/thrift_test_parser_support.cc
+extern std::string g_curdir;
+extern std::string g_curpath;
+
+// Helper function to read file content
+string read_file(const string& filename) {
+ ifstream file(filename);
+ if (!file.is_open()) {
+ return "";
+ }
+ ostringstream ss;
+ ss << file.rdbuf();
+ return ss.str();
+}
+
+static string source_dir() {
+ string file = __FILE__;
+ std::replace(file.begin(), file.end(), '\\', '/');
+ size_t slash = file.rfind('/');
+ return (slash == string::npos) ? string(".") : file.substr(0, slash);
+}
+
+static string join_path(const string& a, const string& b) {
+ if (a.empty()) {
+ return b;
+ }
+ if (a.back() == '/' || a.back() == '\\') {
+ return a + b;
+ }
+ return a + "/" + b;
+}
+
+static string normalize_for_compare(string s) {
+ s.erase(std::remove(s.begin(), s.end(), '\r'), s.end());
+
+ std::istringstream in(s);
+ std::ostringstream out;
+ string line;
+ bool in_block_comment = false;
+ bool first = true;
+ while (std::getline(in, line)) {
+ while (!line.empty() && (line.back() == ' ' || line.back() == '\t')) {
+ line.pop_back();
+ }
+
+ const auto first_non_ws = line.find_first_not_of(" \t");
+ if (first_non_ws == string::npos) {
+ continue;
+ }
+
+ const string trimmed = line.substr(first_non_ws);
+
+ if (in_block_comment) {
+ if (trimmed.find("*/") != string::npos) {
+ in_block_comment = false;
+ }
+ continue;
+ }
+
+ if (trimmed.size() >= 2 && trimmed.compare(0, 2, "//") == 0) {
+ continue;
+ }
+
+ if (trimmed.size() >= 2 && trimmed.compare(0, 2, "/*") == 0) {
+ if (trimmed.find("*/") == string::npos) {
+ in_block_comment = true;
+ }
+ continue;
+ }
+
+ if (!first) {
+ out << '\n';
+ }
+ first = false;
+ out << line;
+ }
+
+ return out.str();
+}
+
+// Helper function to extract class definition from generated header
+string extract_class_definition(const string& content, const string& class_name) {
+ size_t class_start = content.find("class " + class_name + " :");
+ if (class_start == string::npos) {
+ return "";
+ }
+
+ size_t class_end = content.find("};", class_start);
+ if (class_end == string::npos) {
+ return "";
+ }
+
+ return content.substr(class_start, class_end - class_start + 2);
+}
+
+static void parse_thrift_for_test(t_program* program) {
+ REQUIRE(program != nullptr);
+
+ // These globals are used by the parser; see thrift/globals.h.
+ g_program = program;
+ g_scope = program->scope();
+ g_parent_scope = nullptr;
+ g_parent_prefix = program->get_name() + ".";
+
+ g_curpath = program->get_path();
+ g_curdir = directory_name(g_curpath);
+
+ // Pass 1: scan includes (even if none) to match the compiler behavior.
+ g_parse_mode = INCLUDES;
+ yylineno = 1;
+ yyin = std::fopen(g_curpath.c_str(), "r");
+ REQUIRE(yyin != nullptr);
+ REQUIRE(yyparse() == 0);
+ std::fclose(yyin);
+ yyin = nullptr;
+
+ // Pass 2: parse program.
+ g_parse_mode = PROGRAM;
+ yylineno = 1;
+ yyin = std::fopen(g_curpath.c_str(), "r");
+ REQUIRE(yyin != nullptr);
+ REQUIRE(yyparse() == 0);
+ std::fclose(yyin);
+ yyin = nullptr;
+}
+
+TEST_CASE("t_cpp_generator default behavior generates all public fields", "[functional]")
+{
+ string path = join_path(source_dir(), "test_private_optional.thrift");
+ string name = "test_private_optional";
+ map<string, string> parsed_options = {}; // No private_optional flag
+ string option_string = "";
+
+ std::unique_ptr<t_program> program(new t_program(path, name));
+ parse_thrift_for_test(program.get());
+
+ std::unique_ptr<t_generator> gen(
+ t_generator_registry::get_generator(program.get(), "cpp", parsed_options, option_string));
+ REQUIRE(gen != nullptr);
+
+ // Generate code
+ REQUIRE_NOTHROW(gen->generate_program());
+
+ // Read generated output
+ string generated_file = "gen-cpp/test_private_optional_types.h";
+ string generated_content = read_file(generated_file);
+ REQUIRE(!generated_content.empty());
+
+ // Compare generated class definition against the expected fixture.
+ string class_def = extract_class_definition(generated_content, "TestStruct");
+ REQUIRE(!class_def.empty());
+
+ string expected_path = join_path(source_dir(), "expected_TestStruct_default.txt");
+ string expected_content = read_file(expected_path);
+ REQUIRE(!expected_content.empty());
+
+ REQUIRE(normalize_for_compare(class_def) == normalize_for_compare(expected_content));
+
+}
+
+TEST_CASE("t_cpp_generator with private_optional generates private optional fields", "[functional]")
+{
+ string path = join_path(source_dir(), "test_private_optional.thrift");
+ string name = "test_private_optional";
+ map<string, string> parsed_options = {{"private_optional", ""}};
+ string option_string = "";
+
+ std::unique_ptr<t_program> program(new t_program(path, name));
+ parse_thrift_for_test(program.get());
+
+ std::unique_ptr<t_generator> gen(
+ t_generator_registry::get_generator(program.get(), "cpp", parsed_options, option_string));
+ REQUIRE(gen != nullptr);
+
+ // Generate code
+ REQUIRE_NOTHROW(gen->generate_program());
+
+ // Read generated output
+ string generated_file = "gen-cpp/test_private_optional_types.h";
+ string generated_content = read_file(generated_file);
+ REQUIRE(!generated_content.empty());
+
+ // Extract class definition
+ string class_def = extract_class_definition(generated_content, "TestStruct");
+ REQUIRE(!class_def.empty());
+
+ // Compare generated class definition against the expected fixture.
+ string expected_path = join_path(source_dir(), "expected_TestStruct_private_optional.txt");
+ string expected_content = read_file(expected_path);
+ REQUIRE(!expected_content.empty());
+
+ REQUIRE(normalize_for_compare(class_def) == normalize_for_compare(expected_content));
+
+}
diff --git a/compiler/cpp/tests/cpp/test_private_optional.thrift b/compiler/cpp/tests/cpp/test_private_optional.thrift
new file mode 100644
index 0000000..8801838
--- /dev/null
+++ b/compiler/cpp/tests/cpp/test_private_optional.thrift
@@ -0,0 +1,27 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+namespace cpp test.private_optional
+
+struct TestStruct {
+ 1: required i32 required_field;
+ 2: optional i32 optional_field;
+ 3: i32 default_field;
+ 4: optional string optional_string;
+}
diff --git a/compiler/cpp/tests/ocaml/snapshot_exception_types_i.cc b/compiler/cpp/tests/ocaml/snapshot_exception_types_i.hpp
similarity index 100%
rename from compiler/cpp/tests/ocaml/snapshot_exception_types_i.cc
rename to compiler/cpp/tests/ocaml/snapshot_exception_types_i.hpp
diff --git a/compiler/cpp/tests/ocaml/snapshot_service_handle_ex.cc b/compiler/cpp/tests/ocaml/snapshot_service_handle_ex.hpp
similarity index 100%
rename from compiler/cpp/tests/ocaml/snapshot_service_handle_ex.cc
rename to compiler/cpp/tests/ocaml/snapshot_service_handle_ex.hpp
diff --git a/compiler/cpp/tests/ocaml/t_ocaml_generator_tests.cc b/compiler/cpp/tests/ocaml/t_ocaml_generator_tests.cc
index ea788fc..a09b8fa 100644
--- a/compiler/cpp/tests/ocaml/t_ocaml_generator_tests.cc
+++ b/compiler/cpp/tests/ocaml/t_ocaml_generator_tests.cc
@@ -83,7 +83,7 @@
});
{
- #include "snapshot_exception_types_i.cc"
+ #include "snapshot_exception_types_i.hpp"
REQUIRE( snapshot == errors_gen.types_i() );
}
@@ -105,7 +105,7 @@
});
{
- #include "snapshot_service_handle_ex.cc"
+ #include "snapshot_service_handle_ex.hpp"
REQUIRE( snapshot == service_gen.service() );
}
}
diff --git a/compiler/cpp/tests/tests_main.cc b/compiler/cpp/tests/tests_main.cc
index 21d09b9..16cba7e 100644
--- a/compiler/cpp/tests/tests_main.cc
+++ b/compiler/cpp/tests/tests_main.cc
@@ -15,5 +15,15 @@
// specific language governing permissions and limitations
// under the License.
-#define CATCH_CONFIG_MAIN
+#define CATCH_CONFIG_NO_POSIX_SIGNALS
+#define CATCH_CONFIG_RUNNER
#include "catch/catch.hpp"
+
+#include "thrift/common.h"
+
+int main(int argc, char* argv[]) {
+ initGlobals();
+ int result = Catch::Session().run(argc, argv);
+ clearGlobals();
+ return result;
+}
diff --git a/compiler/cpp/tests/thrift_test_globals.cc b/compiler/cpp/tests/thrift_test_globals.cc
new file mode 100644
index 0000000..a195958
--- /dev/null
+++ b/compiler/cpp/tests/thrift_test_globals.cc
@@ -0,0 +1,45 @@
+// Licensed to the Apache Software Foundation(ASF) under one
+// or more contributor license agreements.See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+// Minimal global definitions needed when building compiler tests without src/thrift/main.cc
+
+#include "thrift/globals.h"
+
+#include <string>
+#include <vector>
+
+// Additional globals normally defined in src/thrift/main.cc
+t_program* g_program = nullptr;
+t_scope* g_scope = nullptr;
+t_scope* g_parent_scope = nullptr;
+std::string g_parent_prefix;
+PARSE_MODE g_parse_mode = PROGRAM;
+
+int g_strict = 127;
+
+char* g_time_str = nullptr;
+char* g_doctext = nullptr;
+char* g_program_doctext_candidate = nullptr;
+
+int g_allow_neg_field_keys = 0;
+int g_allow_64bit_consts = 0;
+
+std::string g_curdir;
+std::string g_curpath;
+std::vector<std::string> g_incl_searchpath;
+
+bool g_return_failure = false;
diff --git a/compiler/cpp/tests/thrift_test_parser_support.cc b/compiler/cpp/tests/thrift_test_parser_support.cc
new file mode 100644
index 0000000..c70902d
--- /dev/null
+++ b/compiler/cpp/tests/thrift_test_parser_support.cc
@@ -0,0 +1,149 @@
+// Licensed to the Apache Software Foundation(ASF) under one
+// or more contributor license agreements.See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+// Minimal parser support for compiler unit tests.
+//
+// The Bison grammar (thrifty.yy) references a set of functions and globals that
+// are normally provided by src/thrift/main.cc. The compiler unit tests build
+// does not compile main.cc (it would conflict with the Catch2 main), but some
+// tests still need to parse .thrift files.
+//
+// This file provides lightweight implementations sufficient for unit tests.
+
+
+#include "thrift/globals.h"
+#include "thrift/main.h"
+
+#include <algorithm>
+#include <cstdarg>
+#include <cstdio>
+#include <cstdlib>
+#include <stdexcept>
+#include <string>
+#include <vector>
+
+// Provided by compiler/cpp/tests/thrift_test_globals.cc (not declared in public headers)
+extern std::string g_curdir;
+extern std::string g_curpath;
+extern std::vector<std::string> g_incl_searchpath;
+
+// Error reporting used by the parser.
+void yyerror(const char* fmt, ...) {
+ std::fprintf(stderr, "[ERROR:%s:%d] ", g_curpath.c_str(), yylineno);
+ va_list args;
+ va_start(args, fmt);
+ std::vfprintf(stderr, fmt, args);
+ va_end(args);
+ std::fprintf(stderr, "\n");
+
+ throw std::runtime_error("thrift parser error");
+}
+
+// Simplified helpers referenced by the grammar.
+std::string program_name(std::string filename) {
+ filename.erase(std::remove(filename.begin(), filename.end(), '\\'), filename.end());
+ std::string::size_type slash = filename.rfind('/');
+ if (slash != std::string::npos) {
+ filename = filename.substr(slash + 1);
+ }
+ std::string::size_type dot = filename.rfind('.');
+ if (dot != std::string::npos) {
+ filename = filename.substr(0, dot);
+ }
+ return filename;
+}
+
+std::string directory_name(std::string filename) {
+ std::replace(filename.begin(), filename.end(), '\\', '/');
+ std::string::size_type slash = filename.rfind('/');
+ if (slash == std::string::npos) {
+ return ".";
+ }
+ return filename.substr(0, slash);
+}
+
+std::string include_file(std::string filename) {
+ // Unit tests only parse fixtures without includes. Provide a best-effort
+ // resolution to satisfy the linker and any unexpected includes.
+ if (!filename.empty() && (filename[0] == '/' || filename.find(":/") != std::string::npos)) {
+ return filename;
+ }
+ // Search current dir first.
+ if (!g_curdir.empty()) {
+ return g_curdir + "/" + filename;
+ }
+ return filename;
+}
+
+void clear_doctext() {
+ if (g_doctext != nullptr) {
+ std::free(g_doctext);
+ g_doctext = nullptr;
+ }
+}
+
+char* clean_up_doctext(char* doctext) {
+ // Keep behavior minimal for unit tests.
+ return doctext;
+}
+
+void declare_valid_program_doctext() {
+ if ((g_program_doctext_candidate != nullptr) && (g_program_doctext_status == STILL_CANDIDATE)) {
+ g_program_doctext_status = ABSOLUTELY_SURE;
+ } else {
+ g_program_doctext_status = NO_PROGRAM_DOCTEXT;
+ }
+}
+
+void validate_simple_identifier(const char* identifier) {
+ if (identifier == nullptr) {
+ return;
+ }
+ const std::string name(identifier);
+ if (name.find('.') != std::string::npos) {
+ yyerror("Identifier %s can't have a dot.", identifier);
+ }
+}
+
+void validate_const_type(t_const* /*c*/) {
+ // Not needed for current unit tests.
+}
+
+void validate_field_value(t_field* /*field*/, t_const_value* /*cv*/) {
+ // Not needed for current unit tests.
+}
+
+bool validate_throws(t_struct* /*throws*/) {
+ return true;
+}
+
+void check_for_list_of_bytes(t_type* /*list_elem_type*/) {
+ // Not needed for current unit tests.
+}
+
+void emit_byte_type_warning() {
+ // Not needed for current unit tests.
+}
+
+void error_unsupported_namespace_decl(const char* old_form, const char* new_form) {
+ // Treat as fatal in unit tests.
+ if (new_form == nullptr) {
+ yyerror("Unsupported declaration '%s_namespace'", old_form);
+ } else {
+ yyerror("Unsupported declaration '%s'", old_form);
+ }
+}