diff --git a/ThriftLibrary.cmake b/ThriftLibrary.cmake index 0e0a2dbb56c..2a4bbfb9b84 100644 --- a/ThriftLibrary.cmake +++ b/ThriftLibrary.cmake @@ -117,7 +117,7 @@ endmacro() # #include_prefix # ) # add_library(somelib ...) -# target_link_libraries(somelibe ${file_name}-${language} ...) +# target_link_libraries(somelib ${file_name}-${language} ...) # macro(thrift_library @@ -167,15 +167,21 @@ endmacro() # # thrift_generate # This is used to codegen thrift files using the thrift compiler +# Supports library names that differ from the file name (to handle two libraries +# with the same filename on disk (in different folders)) # Params: -# @file_name - The name of tge thrift file +# @file_name - Input file name. Will be used for naming the CMake +# target if TARGET_NAME_BASE is not specified. # @services - A list of services that are declared in the thrift file # @language - The generator to use (cpp, cpp2 or py3) # @options - Extra options to pass to the generator # @output_path - The directory where the thrift file lives +# @include_prefix - Prefix to use for thrift includes in generated sources +# @TARGET_NAME_BASE - (optional) - name used for target instead of real filename +# @THRIFT_INCLUDE_DIRECTORIES - (optional) path to thrift include directories # # Output: -# file-language-target - A custom target to add a dependenct +# file-language-target - A custom target to add a dependency # ${file-language-HEADERS} - The generated Header Files. # ${file-language-SOURCES} - The generated Source Files. # @@ -186,7 +192,6 @@ endmacro() # bypass_source_check(${file_language-SOURCES}) # This will prevent cmake from complaining about missing source files # - macro(thrift_generate file_name services @@ -198,43 +203,55 @@ macro(thrift_generate ) cmake_parse_arguments(THRIFT_GENERATE # Prefix "" # Options - "" # One Value args + "TARGET_NAME_BASE" # One Value args "THRIFT_INCLUDE_DIRECTORIES" # Multi-value args "${ARGN}") + set(source_file_name ${file_name}) + set(target_file_name ${file_name}) set(thrift_include_directories) foreach(dir ${THRIFT_GENERATE_THRIFT_INCLUDE_DIRECTORIES}) list(APPEND thrift_include_directories "-I" "${dir}") endforeach() + if(DEFINED THRIFT_GENERATE_TARGET_NAME_BASE + AND NOT THRIFT_GENERATE_TARGET_NAME_BASE STREQUAL "") + set(target_file_name ${THRIFT_GENERATE_TARGET_NAME_BASE}) + endif() - set("${file_name}-${language}-HEADERS" - ${output_path}/gen-${language}/${file_name}_constants.h - ${output_path}/gen-${language}/${file_name}_data.h - ${output_path}/gen-${language}/${file_name}_metadata.h - ${output_path}/gen-${language}/${file_name}_types.h - ${output_path}/gen-${language}/${file_name}_types.tcc + set("${target_file_name}-${language}-HEADERS" + ${output_path}/gen-${language}/${source_file_name}_constants.h + ${output_path}/gen-${language}/${source_file_name}_data.h + ${output_path}/gen-${language}/${source_file_name}_metadata.h + ${output_path}/gen-${language}/${source_file_name}_types.h + ${output_path}/gen-${language}/${source_file_name}_types.tcc ) - set("${file_name}-${language}-SOURCES" - ${output_path}/gen-${language}/${file_name}_constants.cpp - ${output_path}/gen-${language}/${file_name}_data.cpp - ${output_path}/gen-${language}/${file_name}_types.cpp + set("${target_file_name}-${language}-SOURCES" + ${output_path}/gen-${language}/${source_file_name}_constants.cpp + ${output_path}/gen-${language}/${source_file_name}_data.cpp + ${output_path}/gen-${language}/${source_file_name}_types.cpp ) + if("${options}" MATCHES "layouts") + set("${target_file_name}-${language}-SOURCES" + ${${target_file_name}-${language}-SOURCES} + ${output_path}/gen-${language}/${source_file_name}_layouts.cpp + ) + endif() if(NOT "${options}" MATCHES "no_metadata") - set("${file_name}-${language}-SOURCES" - ${${file_name}-${language}-SOURCES} - ${output_path}/gen-${language}/${file_name}_metadata.cpp + set("${target_file_name}-${language}-SOURCES" + ${${target_file_name}-${language}-SOURCES} + ${output_path}/gen-${language}/${source_file_name}_metadata.cpp ) endif() foreach(service ${services}) - set("${file_name}-${language}-HEADERS" - ${${file_name}-${language}-HEADERS} + set("${target_file_name}-${language}-HEADERS" + ${${source_file_name}-${language}-HEADERS} ${output_path}/gen-${language}/${service}.h ${output_path}/gen-${language}/${service}.tcc ${output_path}/gen-${language}/${service}AsyncClient.h ${output_path}/gen-${language}/${service}_custom_protocol.h ) - set("${file_name}-${language}-SOURCES" - ${${file_name}-${language}-SOURCES} + set("${target_file_name}-${language}-SOURCES" + ${${source_file_name}-${language}-SOURCES} ${output_path}/gen-${language}/${service}.cpp ${output_path}/gen-${language}/${service}AsyncClient.cpp ) @@ -252,25 +269,25 @@ macro(thrift_generate set(gen_language "mstch_cpp2") elseif("${language}" STREQUAL "py3") set(gen_language "mstch_py3") - file(WRITE "${output_path}/gen-${language}/${file_name}/__init__.py") + file(WRITE "${output_path}/gen-${language}/${source_file_name}/__init__.py") endif() add_custom_command( - OUTPUT ${${file_name}-${language}-HEADERS} - ${${file_name}-${language}-SOURCES} + OUTPUT ${${target_file_name}-${language}-HEADERS} + ${${target_file_name}-${language}-SOURCES} COMMAND ${THRIFT1} --gen "${gen_language}:${options}${include_prefix_text}" -o ${output_path} ${thrift_include_directories} - "${file_path}/${file_name}.thrift" + "${file_path}/${source_file_name}.thrift" DEPENDS ${THRIFT1} - "${file_path}/${file_name}.thrift" - COMMENT "Generating ${file_name} files. Output: ${output_path}" + "${file_path}/${source_file_name}.thrift" + COMMENT "Generating ${target_file_name} files. Output: ${output_path}" ) add_custom_target( - ${file_name}-${language}-target ALL + ${target_file_name}-${language}-target ALL DEPENDS ${${language}-${language}-HEADERS} - ${${file_name}-${language}-SOURCES} + ${${target_file_name}-${language}-SOURCES} ) install( DIRECTORY gen-${language} diff --git a/thrift/lib/cpp2/CMakeLists.txt b/thrift/lib/cpp2/CMakeLists.txt index c1c18cf989f..7eab6f3f274 100644 --- a/thrift/lib/cpp2/CMakeLists.txt +++ b/thrift/lib/cpp2/CMakeLists.txt @@ -15,6 +15,10 @@ # Set the cpp2 directory set(LIB_CPP2_HOME ${CMAKE_CURRENT_SOURCE_DIR}) +if(enable_tests) + add_subdirectory(protocol/test) +endif(enable_tests) +add_subdirectory(test) ####### # CMAKE variables only have local/subdirectory scope diff --git a/thrift/lib/cpp2/protocol/test/BinaryProtocolTest.cpp b/thrift/lib/cpp2/protocol/test/BinaryProtocolTest.cpp index 17e06287fe0..48b9d2ab946 100644 --- a/thrift/lib/cpp2/protocol/test/BinaryProtocolTest.cpp +++ b/thrift/lib/cpp2/protocol/test/BinaryProtocolTest.cpp @@ -45,18 +45,25 @@ bool makeInvalidBool() { return *reinterpret_cast("\x42"); } -TEST_F(BinaryProtocolTest, writeInvalidBool) { - auto w = BinaryProtocolWriter(); +void testWriteInvalidBool() { + auto w = BinaryProtocolWriter(); auto q = folly::IOBufQueue(); w.setOutput(&q); // writeBool should either fail CHECK or write a valid bool. - EXPECT_DEATH( - { - w.writeBool(makeInvalidBool()); + + w.writeBool(makeInvalidBool()); auto s = std::string(); q.appendToString(s); // Die on success. - CHECK(s != std::string(1, '\0')) << "invalid bool value"; + + CHECK(s != std::string(1, '\0')) << "invalid bool value"; + + +} +TEST_F(BinaryProtocolTest, writeInvalidBool) { + EXPECT_DEATH( + { + testWriteInvalidBool(); }, "invalid bool value"); } diff --git a/thrift/lib/cpp2/protocol/test/CMakeLists.txt b/thrift/lib/cpp2/protocol/test/CMakeLists.txt new file mode 100644 index 00000000000..82bc59842a5 --- /dev/null +++ b/thrift/lib/cpp2/protocol/test/CMakeLists.txt @@ -0,0 +1,60 @@ +# Copyright (c) Meta Platforms, Inc. and affiliates and Contributors. All rights reserved. +# +# Licensed 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(GoogleTest) + +# Generate required thrift dependencies +foreach(thrift_file Module CompactProtocolTestStructs) + thrift_generate( + "${thrift_file}" #file_name + "" #services + "cpp2" #language + "frozen2,layouts" #options + "${CMAKE_CURRENT_SOURCE_DIR}" #file_path + "${CMAKE_CURRENT_BINARY_DIR}" #output_path + "thrift/lib/cpp2/protocol/test" #include_prefix + THRIFT_INCLUDE_DIRECTORIES ${THRIFT_SOURCE_DIR} + ) + set_source_files_properties(${thrift_file} PROPERTIES GENERATED TRUE) + set(GENERATED_THRIFT_SOURCES ${GENERATED_THRIFT_SOURCES} ${${thrift_file}-cpp2-SOURCES}) +endforeach() + +# Test files to compile +set(PROTOCOL_TESTS + BinaryProtocolTest + CompactProtocolTest + CompactV1ProtocolTest + ContainerSkippingTest + DebugProtocolTest + F14RoundTripTest + InterfaceTest + JSONProtocolTest + ProtocolReaderStructReadStateTest + ProtocolTest + SimpleJSONProtocolTest + TraitsTest) + +# Build each test independently +foreach(test_name ${PROTOCOL_TESTS}) + add_executable(${test_name} + ${test_name}.cpp + ${GENERATED_THRIFT_SOURCES}) + target_compile_options(${test_name} PUBLIC + -Wno-shadow-compatible-local) + target_link_libraries(${test_name} + GTest::gtest_main + thriftcpp2 + Folly::folly + ) + gtest_discover_tests(${test_name}) +endforeach() diff --git a/thrift/lib/cpp2/protocol/test/CompactProtocolTest.cpp b/thrift/lib/cpp2/protocol/test/CompactProtocolTest.cpp index e6a8614d55e..69d26033e87 100644 --- a/thrift/lib/cpp2/protocol/test/CompactProtocolTest.cpp +++ b/thrift/lib/cpp2/protocol/test/CompactProtocolTest.cpp @@ -31,20 +31,24 @@ bool makeInvalidBool() { return *reinterpret_cast("\x42"); } +void writeInvalidBoolCheck() { + auto w = CompactProtocolWriter(); + auto q = folly::IOBufQueue(); + w.setOutput(&q); + w.writeBool(makeInvalidBool()); + auto s = std::string(); + q.appendToString(s); + // Die on success. + CHECK(s != std::string(1, '\1') && s != std::string(1, '\2')) + << "invalid bool value"; +} + TEST(CompactProtocolTest, writeInvalidBool) { - auto w = CompactProtocolWriter(); - auto q = folly::IOBufQueue(); - w.setOutput(&q); // writeBool should either throw or write a valid bool. The exact value may // depend on the build mode because the optimizer can make use of the UB. EXPECT_DEATH( { - w.writeBool(makeInvalidBool()); - auto s = std::string(); - q.appendToString(s); - // Die on success. - CHECK(s != std::string(1, '\1') && s != std::string(1, '\2')) - << "invalid bool value"; + writeInvalidBoolCheck(); }, "invalid bool value"); } diff --git a/thrift/lib/cpp2/protocol/test/ContainerSkippingTest.cpp b/thrift/lib/cpp2/protocol/test/ContainerSkippingTest.cpp index cbd2b861cd5..7ac410e6b14 100644 --- a/thrift/lib/cpp2/protocol/test/ContainerSkippingTest.cpp +++ b/thrift/lib/cpp2/protocol/test/ContainerSkippingTest.cpp @@ -21,6 +21,7 @@ #include #include #include +#include namespace apache::thrift::test { diff --git a/thrift/lib/cpp2/test/CMakeLists.txt b/thrift/lib/cpp2/test/CMakeLists.txt new file mode 100644 index 00000000000..03602b18854 --- /dev/null +++ b/thrift/lib/cpp2/test/CMakeLists.txt @@ -0,0 +1,90 @@ +# Copyright (c) Meta Platforms, Inc. and affiliates and Contributors. All rights reserved. +# +# Licensed 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. + +# This CMakeLists.txt takes care of building a selection of benchmarks and tests in this folder. +# Currently, it only builds ProtocolBench with its dependencies + +# obtain paths to other generated thrift files from the global property space +get_property(protocol-cpp2-SOURCES GLOBAL PROPERTY + protocol-cpp2-SOURCES) +get_property(protocol_detail-cpp2-SOURCES GLOBAL PROPERTY + protocol_detail-cpp2-SOURCES) +get_property(frozen-cpp2-SOURCES GLOBAL PROPERTY + frozen-cpp2-SOURCES) +get_property(field_mask-cpp2-SOURCES GLOBAL PROPERTY + field_mask-cpp2-SOURCES) + +# generate thrift files for this benchmark +thrift_generate( + "ProtocolBenchData" #file_name + "" #services + "cpp2" #language + "frozen2,layouts" #options + "${CMAKE_CURRENT_SOURCE_DIR}" #file_path + "${CMAKE_CURRENT_BINARY_DIR}" #output_path + "thrift/lib/cpp2/test" #include_prefix + THRIFT_INCLUDE_DIRECTORIES ${THRIFT_SOURCE_DIR} +) + +set(GENERATED_THRIFT_SOURCES ${ProtocolBenchData-cpp2-SOURCES} + ${protocol-cpp2-SOURCES} + ${protocol_detail-cpp2-SOURCES} + ${field_mask-cpp2-SOURCES} + ${frozen-cpp2-SOURCES}) +set(SOURCE_FILES ProtocolBench.cpp) +set(ADDITIONAL_SOURCE_FILES ../../thrift/detail/protocol.cpp ../../thrift/TypeToMaskAdapter.cpp) + +# generate conformance thrift files +foreach(conf_name "protocol" "type" "any" "serialization" "patch_data" "conformance") + thrift_generate( + "${conf_name}" #file_name + "" #services + "cpp2" #language + "frozen2" #options + "${CMAKE_CURRENT_SOURCE_DIR}/../../../conformance/if" #file_path + "${CMAKE_CURRENT_BINARY_DIR}/../../../conformance/if" #output_path + "thrift/conformance/if" #include_prefix + THRIFT_INCLUDE_DIRECTORIES ${THRIFT_SOURCE_DIR} + TARGET_NAME_BASE "${conf_name}conformance" + ) + set(GENERATED_THRIFT_SOURCES + ${GENERATED_THRIFT_SOURCES} + ${${conf_name}conformance-cpp2-SOURCES}) +endforeach() + +foreach(GENERATED ${GENERATED_THRIFT_SOURCES}) + set_source_files_properties(${GENERATED} PROPERTIES GENERATED TRUE) +endforeach() + +# build the executable +add_executable(ProtocolBench + ${SOURCE_FILES} + ${ADDITIONAL_SOURCE_FILES} + ${GENERATED_THRIFT_SOURCES} +) +target_link_libraries(ProtocolBench + thriftcpp2 + Folly::folly + Folly::follybenchmark + xxhash +) + +install( + TARGETS ProtocolBench + EXPORT fbthrift-exports + RUNTIME DESTINATION ${BIN_INSTALL_DIR} + LIBRARY DESTINATION ${LIB_INSTALL_DIR} + ARCHIVE DESTINATION ${LIB_INSTALL_DIR} +) + diff --git a/thrift/lib/cpp2/test/Structs.h b/thrift/lib/cpp2/test/Structs.h index 65ae697ae8d..8312eac6fb2 100644 --- a/thrift/lib/cpp2/test/Structs.h +++ b/thrift/lib/cpp2/test/Structs.h @@ -36,5 +36,7 @@ void populateMap(F&& getter) { } } +#if defined __has_include && __has_include() #include +#endif #include diff --git a/thrift/lib/thrift/CMakeLists.txt b/thrift/lib/thrift/CMakeLists.txt index 0e2ba2bc4ed..bb797f1eaae 100644 --- a/thrift/lib/thrift/CMakeLists.txt +++ b/thrift/lib/thrift/CMakeLists.txt @@ -133,6 +133,40 @@ thrift_generate( THRIFT_INCLUDE_DIRECTORIES ${THRIFT_SOURCE_DIR} ) +thrift_generate( + "protocol" #file_name + "" #services + "cpp2" #language + "" #options + "${CMAKE_CURRENT_SOURCE_DIR}" #file_path + "${CMAKE_CURRENT_BINARY_DIR}" #output_path + "thrift/lib/thrift" #include_prefix + THRIFT_INCLUDE_DIRECTORIES ${THRIFT_SOURCE_DIR} +) + +thrift_generate( + "protocol_detail" #file_name + "" #services + "cpp2" #language + "" #options + "${CMAKE_CURRENT_SOURCE_DIR}" #file_path + "${CMAKE_CURRENT_BINARY_DIR}" #output_path + "thrift/lib/thrift" #include_prefix + THRIFT_INCLUDE_DIRECTORIES ${THRIFT_SOURCE_DIR} +) + +thrift_generate( + "field_mask" #file_name + "" #services + "cpp2" #language + "no_metadata" #options + "${CMAKE_CURRENT_SOURCE_DIR}" #file_path + "${CMAKE_CURRENT_BINARY_DIR}" #output_path + "thrift/lib/thrift" #include_prefix + THRIFT_INCLUDE_DIRECTORIES ${THRIFT_SOURCE_DIR} +) + + set_property(GLOBAL PROPERTY any_rep-cpp2-SOURCES ${any_rep-cpp2-SOURCES}) set_property(GLOBAL PROPERTY RpcMetadata-cpp2-SOURCES ${RpcMetadata-cpp2-SOURCES}) @@ -147,3 +181,9 @@ set_property(GLOBAL PROPERTY type_rep-cpp2-SOURCES ${type_rep-cpp2-SOURCES}) set_property(GLOBAL PROPERTY type-cpp2-SOURCES ${type-cpp2-SOURCES}) set_property(GLOBAL PROPERTY serverdbginfo-cpp2-SOURCES ${serverdbginfo-cpp2-SOURCES}) +set_property(GLOBAL PROPERTY protocol-cpp2-SOURCES + ${protocol-cpp2-SOURCES}) +set_property(GLOBAL PROPERTY protocol_detail-cpp2-SOURCES + ${protocol_detail-cpp2-SOURCES}) +set_property(GLOBAL PROPERTY field_mask-cpp2-SOURCES + ${field_mask-cpp2-SOURCES})