Skip to content

Add moveable_types=forward_setter option for perfect forwarding setters #3292

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Jens-G merged 1 commit into from
Feb 8, 2026
Merged

Add moveable_types=forward_setter option for perfect forwarding setters #3292

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion .github/workflows/msvc.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,10 @@ jobs:
shell: pwsh
run: |
docker run -v c:\src\thrift:C:\Thrift -v "${env:THRIFT_BUILD_DIR}:C:\build" --rm -t "$($env:DOCKER_IMAGE):$($env:IMAGE_TAG)" c:\thrift\build\docker\msvc2017\build.bat
if ($LASTEXITCODE -ne 0) {
Write-Error "Container build failed with exit code $LASTEXITCODE"
exit $LASTEXITCODE
}

- name: Check test results
if: always()
Expand All @@ -117,7 +121,8 @@ jobs:
Write-Host "All tests passed"
}
} else {
Write-Warning "LastTest.log not found at $logPath"
Write-Error "LastTest.log not found at $logPath"
exit 1
}

- name: Upload LastTest log
Expand Down
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -453,3 +453,4 @@ compiler/cpp/tests/Testing/
compiler/cpp/tests/bin/
compiler/cpp/tests/*.a
compiler/cpp/tests/build/
compiler/cpp/build/
77 changes: 72 additions & 5 deletions compiler/cpp/src/thrift/generate/t_cpp_generator.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ class t_cpp_generator : public t_oop_generator {
gen_templates_ = false;
gen_templates_only_ = false;
gen_moveable_ = false;
gen_forward_setter_ = false;
gen_no_ostream_operators_ = false;
gen_no_skeleton_ = false;
gen_no_constructors_ = false;
Expand All @@ -90,6 +91,9 @@ class t_cpp_generator : public t_oop_generator {
gen_templates_only_ = (iter->second == "only");
} else if( iter->first.compare("moveable_types") == 0) {
gen_moveable_ = true;
if (iter->second.compare("forward_setter") == 0) {
gen_forward_setter_ = true;
}
} else if ( iter->first.compare("no_ostream_operators") == 0) {
gen_no_ostream_operators_ = true;
} else if ( iter->first.compare("no_skeleton") == 0) {
Expand Down Expand Up @@ -153,6 +157,7 @@ class t_cpp_generator : public t_oop_generator {
bool setters = true,
bool is_user_struct = false,
bool pointers = false);
void generate_struct_forward_setter_impls(std::ostream& out, t_struct* tstruct);
void generate_copy_constructor(std::ostream& out, t_struct* tstruct, bool is_exception);
void generate_move_constructor(std::ostream& out, t_struct* tstruct, bool is_exception);
void generate_default_constructor(std::ostream& out, t_struct* tstruct, bool is_exception);
Expand Down Expand Up @@ -361,6 +366,11 @@ class t_cpp_generator : public t_oop_generator {
*/
bool gen_moveable_;

/**
* True if we should generate setters with perfect forwarding for non-primitive types.
*/
bool gen_forward_setter_;

/**
* True if we should generate ostream definitions
*/
Expand Down Expand Up @@ -452,7 +462,7 @@ void t_cpp_generator::init_generator() {
string f_types_impl_name = get_out_dir() + program_name_ + "_types.cpp";
f_types_impl_.open(f_types_impl_name.c_str());

if (gen_templates_) {
if (gen_templates_ || gen_forward_setter_) {
// If we don't open the stream, it appears to just discard data,
// which is fine.
string f_types_tcc_name = get_out_dir() + program_name_ + "_types.tcc";
Expand Down Expand Up @@ -542,7 +552,7 @@ void t_cpp_generator::close_generator() {
// Include the types.tcc file from the types header file,
// so clients don't have to explicitly include the tcc file.
// TODO(simpkins): Make this a separate option.
if (gen_templates_) {
if (gen_templates_ || gen_forward_setter_) {
f_types_ << "#include \"" << get_include_prefix(*get_program()) << program_name_
<< "_types.tcc\"" << '\n' << '\n';
}
Expand Down Expand Up @@ -971,6 +981,12 @@ void t_cpp_generator::generate_cpp_struct(t_struct* tstruct, bool is_exception)
std::ostream& out = (gen_templates_ ? f_types_tcc_ : f_types_impl_);
generate_struct_reader(out, tstruct);
generate_struct_writer(out, tstruct);

// Generate forward setter template implementations in .tcc file
if (gen_forward_setter_) {
generate_struct_forward_setter_impls(f_types_tcc_, tstruct);
}

generate_struct_swap(f_types_impl_, tstruct);
if (!gen_no_default_operators_) {
generate_equality_operator(f_types_impl_, tstruct);
Expand Down Expand Up @@ -1392,9 +1408,15 @@ void t_cpp_generator::generate_struct_declaration(ostream& out,
<< type_name((*m_iter)->get_type(), false, false) << ">";
out << " val);" << '\n';
} else {
out << '\n' << indent() << "void __set_" << (*m_iter)->get_name() << "("
<< type_name((*m_iter)->get_type(), false, true);
out << " val);" << '\n';
// Use template for perfect forwarding with forward_setter on complex types
if (gen_forward_setter_ && is_complex_type((*m_iter)->get_type())) {
out << '\n' << indent() << "template <typename T_>\n";
out << indent() << "void __set_" << (*m_iter)->get_name() << "(T_&& val);" << '\n';
} else {
out << '\n' << indent() << "void __set_" << (*m_iter)->get_name() << "("
<< type_name((*m_iter)->get_type(), false, true);
out << " val);" << '\n';
}
}
}

Expand Down Expand Up @@ -1559,6 +1581,11 @@ void t_cpp_generator::generate_struct_definition(ostream& out,
// Create a setter function for each field
if (setters) {
for (m_iter = members.begin(); m_iter != members.end(); ++m_iter) {
// Skip implementation for forwarding setters (they're inline in header)
if (gen_forward_setter_ && !is_reference((*m_iter)) && is_complex_type((*m_iter)->get_type())) {
continue;
}

if (is_reference((*m_iter))) {
out << '\n' << indent() << "void " << tstruct->get_name() << "::__set_"
<< (*m_iter)->get_name() << "(::std::shared_ptr<"
Expand Down Expand Up @@ -1588,6 +1615,44 @@ void t_cpp_generator::generate_struct_definition(ostream& out,
out << '\n';
}

/**
* Generates template setter implementations for forward_setter mode.
* These are output to the .tcc file.
*
* @param out Stream to write to
* @param tstruct The struct
*/
void t_cpp_generator::generate_struct_forward_setter_impls(ostream& out, t_struct* tstruct) {
if (!gen_forward_setter_) {
return;
}

const vector<t_field*>& members = tstruct->get_members();
vector<t_field*>::const_iterator m_iter;

for (m_iter = members.begin(); m_iter != members.end(); ++m_iter) {
// Only generate implementations for complex types with forward_setter
if (is_reference((*m_iter)) || !is_complex_type((*m_iter)->get_type())) {
continue;
}

out << '\n' << indent() << "template <typename T_>\n";
out << indent() << "void " << tstruct->get_name() << "::__set_"
<< (*m_iter)->get_name() << "(T_&& val) {" << '\n';
indent_up();
out << indent() << "this->" << (*m_iter)->get_name() << " = ::std::forward<T_>(val);" << '\n';

// assume all fields are required except optional fields.
// for optional fields change __isset.name to true
bool is_optional = (*m_iter)->get_req() == t_field::T_OPTIONAL;
if (is_optional) {
out << indent() << "__isset." << (*m_iter)->get_name() << " = true;" << '\n';
}
indent_down();
out << indent() << "}" << '\n';
}
}

/**
* Makes a helper function to gen a struct reader.
*
Expand Down Expand Up @@ -4988,6 +5053,8 @@ THRIFT_REGISTER_GENERATOR(
" When 'pure_enums=enum_class', generate C++ 11 enum class.\n"
" include_prefix: Use full include paths in generated files.\n"
" moveable_types: Generate move constructors and assignment operators.\n"
" When 'moveable_types=forward_setter', also generate setters\n"
" with perfect forwarding for non-primitive types.\n"
" no_ostream_operators:\n"
" Omit generation of ostream definitions.\n"
" no_skeleton: Omits generation of skeleton.\n")
113 changes: 0 additions & 113 deletions compiler/cpp/tests/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,118 +194,5 @@ else()
)
endif()

# Compile-check generated C++ output for fixture thrift files.
# This ensures the generator output is compileable (no link step required).
# Note: These checks require Boost headers and are optional.
if(TARGET thrift-compiler)
# Try to find Boost for the compile checks
find_package(Boost QUIET)
set(_compile_check_boost_include_dirs "")
if(Boost_FOUND)
set(_compile_check_boost_include_dirs ${Boost_INCLUDE_DIRS})
elseif(DEFINED BOOST_ROOT)
if(EXISTS "${BOOST_ROOT}/include/boost")
set(_compile_check_boost_include_dirs "${BOOST_ROOT}/include")
elseif(EXISTS "${BOOST_ROOT}/boost")
set(_compile_check_boost_include_dirs "${BOOST_ROOT}")
endif()
endif()

# Only create compile-check targets if Boost is available
if(NOT _compile_check_boost_include_dirs STREQUAL "")
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}"
${_compile_check_boost_include_dirs}
)

set(_enum_class_thrift
"${CMAKE_CURRENT_SOURCE_DIR}/cpp/test_enum_class.thrift"
)
set(_enum_class_gen_out_dir
"${CMAKE_CURRENT_BINARY_DIR}/generated-enum-class"
)
set(_enum_class_gen_cpp_dir
"${_enum_class_gen_out_dir}/gen-cpp"
)
set(_enum_class_types_cpp
"${_enum_class_gen_cpp_dir}/test_enum_class_types.cpp"
)

add_custom_command(
OUTPUT "${_enum_class_types_cpp}"
COMMAND ${CMAKE_COMMAND} -E make_directory "${_enum_class_gen_out_dir}"
COMMAND ${CMAKE_COMMAND} -E chdir "${_enum_class_gen_out_dir}"
$<TARGET_FILE:thrift-compiler>
--gen cpp:pure_enums=enum_class
-o "${_enum_class_gen_out_dir}"
"${_enum_class_thrift}"
DEPENDS thrift-compiler "${_enum_class_thrift}"
VERBATIM
)

set_source_files_properties(
"${_enum_class_types_cpp}"
PROPERTIES GENERATED TRUE
)

add_library(thrift_compiler_generated_enum_class STATIC
"${_enum_class_types_cpp}"
)

target_include_directories(thrift_compiler_generated_enum_class PRIVATE
"${_enum_class_gen_cpp_dir}"
"${THRIFT_COMPILER_SOURCE_DIR}/../../lib/cpp/src"
"${CMAKE_CURRENT_BINARY_DIR}"
"${CMAKE_BINARY_DIR}"
${_compile_check_boost_include_dirs}
)

# Build the compile-check as part of the standard test build.
add_dependencies(thrift_compiler_tests thrift_compiler_generated_private_optional)
add_dependencies(thrift_compiler_tests thrift_compiler_generated_enum_class)
else()
message(STATUS "Skipping generated code compile-checks (Boost headers not found)")
endif()
else()
message(STATUS "Skipping generated code compile-checks (no thrift-compiler target)")
endif()

enable_testing()
add_test(NAME ThriftCompilerTests COMMAND thrift_compiler_tests)
Loading
Loading