Add test coverage for private_optional + template_streamop combined; fix generator bugs (#14)
* Add test coverage for private_optional + template_streamop combined, fix generator bugs
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: zsy056 <1074382+zsy056@users.noreply.github.com>
diff --git a/compiler/cpp/src/thrift/generate/t_cpp_generator.cc b/compiler/cpp/src/thrift/generate/t_cpp_generator.cc
index acc40c0..c7702a5 100644
--- a/compiler/cpp/src/thrift/generate/t_cpp_generator.cc
+++ b/compiler/cpp/src/thrift/generate/t_cpp_generator.cc
@@ -1618,7 +1618,11 @@
generate_struct_swap_decl(out, tstruct);
}
- if (is_user_struct) {
+ // When both private_optional and template_streamop are enabled, the friend
+ // function template declared inside the class body is sufficient (it is
+ // findable via ADL). Emitting a second namespace-scope declaration would
+ // place the 'friend' keyword outside a class, which is ill-formed.
+ if (is_user_struct && !(gen_private_optional_ && gen_template_streamop_)) {
generate_struct_ostream_operator_decl(out, tstruct);
}
}
@@ -2139,13 +2143,16 @@
void generate_optional_field_value(std::ostream& out, const t_field* field, bool use_printto) {
out << "; (__isset." << field->get_name() << " ? ";
if (use_printto) {
- // For printTo, call directly without wrapping in (out ...)
+ // printTo() returns void. Both ternary branches must have the same type, so
+ // cast the false-branch to void as well. The non-printTo path below does not
+ // need the cast because both of its branches return the same stream reference.
out << "printTo(out, " << field->get_name() << ")";
+ out << " : (void)(out << \"<null>\"))";
} else {
- // For to_string, need to wrap with (out << ...)
+ // Both branches return std::ostream&, so no cast is needed.
out << "(out << to_string(" << field->get_name() << "))";
+ out << " : (out << \"<null>\"))";
}
- out << " : (out << \"<null>\"))";
}
void generate_field_value(std::ostream& out, const t_field* field, bool use_printto) {
diff --git a/test/cpp/CMakeLists.txt b/test/cpp/CMakeLists.txt
index 6fdc95e..47602a1 100644
--- a/test/cpp/CMakeLists.txt
+++ b/test/cpp/CMakeLists.txt
@@ -187,6 +187,29 @@
target_link_libraries(TemplateStreamOpTest thrift)
add_test(NAME TemplateStreamOpTest COMMAND TemplateStreamOpTest)
+# PrivateOptionalTemplateStreamOpTest - tests private_optional and template_streamop together
+set(privateopttemplstreamoptestgencpp_SOURCES
+ gen-cpp-private-templatestreamop/gen-cpp/ThriftTest_types.cpp
+ gen-cpp-private-templatestreamop/gen-cpp/ThriftTest_constants.cpp
+ src/ThriftTest_extras.cpp
+)
+add_library(privateopttemplstreamoptestgencpp STATIC ${privateopttemplstreamoptestgencpp_SOURCES})
+target_include_directories(privateopttemplstreamoptestgencpp BEFORE PRIVATE
+ "${CMAKE_CURRENT_BINARY_DIR}/gen-cpp-private-templatestreamop"
+ "${CMAKE_CURRENT_BINARY_DIR}"
+ "${PROJECT_SOURCE_DIR}/lib/cpp/src"
+)
+target_link_libraries(privateopttemplstreamoptestgencpp thrift)
+
+add_executable(PrivateOptionalTemplateStreamOpTest src/PrivateOptionalTemplateStreamOpTest.cpp)
+target_include_directories(PrivateOptionalTemplateStreamOpTest BEFORE PRIVATE
+ "${CMAKE_CURRENT_BINARY_DIR}/gen-cpp-private-templatestreamop/gen-cpp"
+ "${CMAKE_CURRENT_BINARY_DIR}/gen-cpp-private-templatestreamop"
+)
+target_link_libraries(PrivateOptionalTemplateStreamOpTest privateopttemplstreamoptestgencpp ${Boost_LIBRARIES})
+target_link_libraries(PrivateOptionalTemplateStreamOpTest thrift)
+add_test(NAME PrivateOptionalTemplateStreamOpTest COMMAND PrivateOptionalTemplateStreamOpTest)
+
#
# Common thrift code generation rules
#
@@ -223,6 +246,13 @@
WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
)
+# Generate ThriftTest with private_optional,template_streamop options for PrivateOptionalTemplateStreamOpTest
+add_custom_command(OUTPUT gen-cpp-private-templatestreamop/gen-cpp/ThriftTest_types.cpp gen-cpp-private-templatestreamop/gen-cpp/ThriftTest_types.h gen-cpp-private-templatestreamop/gen-cpp/ThriftTest_constants.cpp
+ COMMAND ${CMAKE_COMMAND} -E make_directory ${CMAKE_CURRENT_BINARY_DIR}/gen-cpp-private-templatestreamop
+ COMMAND ${THRIFT_COMPILER} --gen cpp:private_optional,template_streamop -o gen-cpp-private-templatestreamop ${PROJECT_SOURCE_DIR}/test/ThriftTest.thrift
+ WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
+)
+
add_custom_command(OUTPUT gen-cpp/Service.cpp
COMMAND ${THRIFT_COMPILER} --gen cpp ${PROJECT_SOURCE_DIR}/test/StressTest.thrift
)
diff --git a/test/cpp/Makefile.am b/test/cpp/Makefile.am
index 720a4b3..a17a3fb 100644
--- a/test/cpp/Makefile.am
+++ b/test/cpp/Makefile.am
@@ -31,7 +31,10 @@
gen-cpp-enumclass/ThriftTest_constants.cpp \
gen-cpp-templatestreamop/ThriftTest_types.cpp \
gen-cpp-templatestreamop/ThriftTest_types.tcc \
- gen-cpp-templatestreamop/ThriftTest_constants.cpp
+ gen-cpp-templatestreamop/ThriftTest_constants.cpp \
+ gen-cpp-private-templatestreamop/ThriftTest_types.cpp \
+ gen-cpp-private-templatestreamop/ThriftTest_types.tcc \
+ gen-cpp-private-templatestreamop/ThriftTest_constants.cpp
noinst_LTLIBRARIES = libtestgencpp.la libstresstestgencpp.la
nodist_libtestgencpp_la_SOURCES = \
@@ -55,7 +58,8 @@
libforwardsettertestgencpp.la \
libprivateoptonaltestgencpp.la \
libenumclasstestgencpp.la \
- libtemplatestreamoptestgencpp.la
+ libtemplatestreamoptestgencpp.la \
+ libprivateopttemplstreamoptestgencpp.la
nodist_libforwardsettertestgencpp_la_SOURCES = \
gen-cpp-forward/ThriftTest_types.cpp \
@@ -95,6 +99,16 @@
libtemplatestreamoptestgencpp_la_LIBADD = $(top_builddir)/lib/cpp/libthrift.la
+nodist_libprivateopttemplstreamoptestgencpp_la_SOURCES = \
+ gen-cpp-private-templatestreamop/ThriftTest_types.cpp \
+ gen-cpp-private-templatestreamop/ThriftTest_types.h \
+ gen-cpp-private-templatestreamop/ThriftTest_types.tcc \
+ gen-cpp-private-templatestreamop/ThriftTest_constants.cpp \
+ gen-cpp-private-templatestreamop/ThriftTest_constants.h \
+ src/ThriftTest_extras.cpp
+
+libprivateopttemplstreamoptestgencpp_la_LIBADD = $(top_builddir)/lib/cpp/libthrift.la
+
nodist_libstresstestgencpp_la_SOURCES = \
gen-cpp/StressTest_types.h \
gen-cpp/Service.cpp \
@@ -112,7 +126,8 @@
ForwardSetterTest \
PrivateOptionalTest \
EnumClassTest \
- TemplateStreamOpTest
+ TemplateStreamOpTest \
+ PrivateOptionalTemplateStreamOpTest
# we currently do not run the testsuite, stop c++ server issue
# TESTS = \
@@ -185,6 +200,14 @@
libtemplatestreamoptestgencpp.la \
$(top_builddir)/lib/cpp/libthrift.la
+PrivateOptionalTemplateStreamOpTest_SOURCES = \
+ src/PrivateOptionalTemplateStreamOpTest.cpp
+
+PrivateOptionalTemplateStreamOpTest_CPPFLAGS = -Igen-cpp-private-templatestreamop $(AM_CPPFLAGS)
+PrivateOptionalTemplateStreamOpTest_LDADD = \
+ libprivateopttemplstreamoptestgencpp.la \
+ $(top_builddir)/lib/cpp/libthrift.la
+
#
# Common thrift code generation rules
#
@@ -211,6 +234,11 @@
$(MKDIR_P) gen-cpp-templatestreamop
$(THRIFT) --gen cpp:template_streamop -out gen-cpp-templatestreamop $<
+# Generate ThriftTest with private_optional,template_streamop options
+gen-cpp-private-templatestreamop/ThriftTest_types.cpp gen-cpp-private-templatestreamop/ThriftTest_types.h gen-cpp-private-templatestreamop/ThriftTest_types.tcc gen-cpp-private-templatestreamop/ThriftTest_constants.cpp: $(top_srcdir)/test/ThriftTest.thrift $(THRIFT)
+ $(MKDIR_P) gen-cpp-private-templatestreamop
+ $(THRIFT) --gen cpp:private_optional,template_streamop -out gen-cpp-private-templatestreamop $<
+
gen-cpp/Service.cpp: $(top_srcdir)/test/StressTest.thrift $(THRIFT)
$(THRIFT) --gen cpp $<
@@ -222,7 +250,7 @@
AM_LDFLAGS = $(BOOST_LDFLAGS) $(LIBEVENT_LDFLAGS) $(ZLIB_LIBS)
clean-local:
- $(RM) -r gen-cpp/ gen-cpp-forward/ gen-cpp-private/ gen-cpp-enumclass/ gen-cpp-templatestreamop/
+ $(RM) -r gen-cpp/ gen-cpp-forward/ gen-cpp-private/ gen-cpp-enumclass/ gen-cpp-templatestreamop/ gen-cpp-private-templatestreamop/
style-local:
$(CPPSTYLE_CMD)
@@ -238,4 +266,5 @@
src/ForwardSetterTest.cpp \
src/PrivateOptionalTest.cpp \
src/EnumClassTest.cpp \
- src/TemplateStreamOpTest.cpp
+ src/TemplateStreamOpTest.cpp \
+ src/PrivateOptionalTemplateStreamOpTest.cpp
diff --git a/test/cpp/src/PrivateOptionalTemplateStreamOpTest.cpp b/test/cpp/src/PrivateOptionalTemplateStreamOpTest.cpp
new file mode 100644
index 0000000..634cce7
--- /dev/null
+++ b/test/cpp/src/PrivateOptionalTemplateStreamOpTest.cpp
@@ -0,0 +1,174 @@
+/*
+ * 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.
+ */
+
+/**
+ * Test file to verify that generated code compiles and works correctly when
+ * both private_optional and template_streamop options are enabled together.
+ */
+
+#include <iostream>
+#include <sstream>
+#include <string>
+#include <cassert>
+#include <type_traits>
+#include <cstring>
+#include <cstdio>
+#include <cstdint>
+
+// Include generated thrift types with private_optional,template_streamop options
+#include "ThriftTest_types.h"
+#include <thrift/TToString.h>
+
+using namespace thrift::test;
+
+// SFINAE helpers to verify field accessibility at compile time.
+// Each requires a separate template because C++ template parameters cannot be
+// string literals, so each field name gets its own specialisation.
+template<typename T, typename = void>
+struct has_public_aa : std::false_type {};
+
+template<typename T>
+struct has_public_aa<T, decltype(void(std::declval<T>().aa))> : std::true_type {};
+
+// SFINAE test to check if required field 'ab' is directly accessible
+template<typename T, typename = void>
+struct has_public_ab : std::false_type {};
+
+template<typename T>
+struct has_public_ab<T, decltype(void(std::declval<T>().ab))> : std::true_type {};
+
+// Custom minimal stream - verifies that template_streamop works with non-std streams
+class MinimalStream {
+private:
+ std::string buf_;
+public:
+ MinimalStream& operator<<(const std::string& s) { buf_ += s; return *this; }
+ MinimalStream& operator<<(const char* s) { buf_ += s; return *this; }
+ MinimalStream& operator<<(char c) { buf_ += c; return *this; }
+ MinimalStream& operator<<(int32_t i) { buf_ += std::to_string(i); return *this; }
+ MinimalStream& operator<<(int64_t i) { buf_ += std::to_string(i); return *this; }
+ MinimalStream& operator<<(uint32_t i) { buf_ += std::to_string(i); return *this; }
+ MinimalStream& operator<<(uint64_t i) { buf_ += std::to_string(i); return *this; }
+ MinimalStream& operator<<(double d) {
+ char tmp[64];
+ std::snprintf(tmp, sizeof(tmp), "%g", d);
+ buf_ += tmp;
+ return *this;
+ }
+ MinimalStream& operator<<(bool b) { buf_ += (b ? "true" : "false"); return *this; }
+ const std::string& str() const { return buf_; }
+};
+
+int main() {
+ std::cout << "Testing private_optional + template_streamop combined..." << std::endl;
+
+ // Compile-time: optional field 'aa' in StructB must be private
+ static_assert(!has_public_aa<StructB>::value,
+ "Optional field 'aa' in StructB should be private with private_optional");
+ std::cout << " ✓ Compile-time: optional field 'aa' is private in StructB" << std::endl;
+
+ // Compile-time: required field 'ab' in StructB must remain public
+ static_assert(has_public_ab<StructB>::value,
+ "Required field 'ab' in StructB should remain public");
+ std::cout << " ✓ Compile-time: required field 'ab' is public in StructB" << std::endl;
+
+ // Test 1: private_optional getters/setters work
+ {
+ Xtruct x;
+ x.__set_string_thing("hello");
+ x.__set_i32_thing(42);
+ assert(x.__get_string_thing() == "hello");
+ assert(x.__get_i32_thing() == 42);
+ std::cout << " ✓ private_optional getters/setters work on Xtruct" << std::endl;
+ }
+
+ // Test 2: template_streamop with std::ostringstream
+ {
+ Xtruct x;
+ x.__set_string_thing("stream test");
+ x.__set_i32_thing(99);
+
+ std::ostringstream oss;
+ oss << x;
+ std::string result = oss.str();
+
+ assert(!result.empty());
+ assert(result.find("stream test") != std::string::npos);
+ assert(result.find("99") != std::string::npos);
+ std::cout << " ✓ template_streamop works with std::ostringstream" << std::endl;
+ }
+
+ // Test 3: template_streamop with custom MinimalStream
+ {
+ Xtruct x;
+ x.__set_string_thing("minimal stream");
+ x.__set_i32_thing(7);
+
+ MinimalStream ms;
+ ms << x;
+ std::string result = ms.str();
+
+ assert(!result.empty());
+ assert(result.find("minimal stream") != std::string::npos);
+ assert(result.find("7") != std::string::npos);
+ std::cout << " ✓ template_streamop works with custom MinimalStream" << std::endl;
+ }
+
+ // Test 4: Stream a struct whose optional fields were set via setters (combines both options)
+ {
+ StructB sb;
+ StructA sa;
+ sa.__set_s("optional value");
+
+ // private_optional: must use setter, not direct assignment
+ sb.__set_aa(sa);
+ // required field: direct access is still fine
+ sb.ab = sa;
+
+ // template_streamop: stream the struct to both stream types
+ std::ostringstream oss;
+ oss << sb;
+ std::string oss_result = oss.str();
+
+ MinimalStream ms;
+ ms << sb;
+ std::string ms_result = ms.str();
+
+ assert(!oss_result.empty());
+ assert(!ms_result.empty());
+ assert(oss_result.find("optional value") != std::string::npos);
+ assert(ms_result.find("optional value") != std::string::npos);
+ std::cout << " ✓ Combined: StructB with private optional fields streams correctly" << std::endl;
+ }
+
+ // Test 5: to_string works with combined options
+ {
+ Xtruct x;
+ x.__set_string_thing("to_string test");
+ x.__set_i32_thing(123);
+
+ std::string s = apache::thrift::to_string(x);
+ assert(!s.empty());
+ assert(s.find("to_string test") != std::string::npos);
+ std::cout << " ✓ to_string works with combined options" << std::endl;
+ }
+
+ std::cout << "\n✅ All private_optional + template_streamop combined tests passed!" << std::endl;
+ return 0;
+}