diff --git a/ci/travis_before_script_cpp.sh b/ci/travis_before_script_cpp.sh index a7f1d275634..65171a73557 100755 --- a/ci/travis_before_script_cpp.sh +++ b/ci/travis_before_script_cpp.sh @@ -90,12 +90,12 @@ fi if [ $TRAVIS_OS_NAME == "linux" ]; then cmake $CMAKE_COMMON_FLAGS \ $CMAKE_LINUX_FLAGS \ - -DARROW_CXXFLAGS="-Wconversion -Wno-sign-conversion -Werror" \ + -DBUILD_WARNING_LEVEL=CHECKIN \ $ARROW_CPP_DIR else cmake $CMAKE_COMMON_FLAGS \ $CMAKE_OSX_FLAGS \ - -DARROW_CXXFLAGS=-Werror \ + -DBUILD_WARNING_LEVEL=CHECKIN \ $ARROW_CPP_DIR fi diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt index ad99970e9b2..a190f229deb 100644 --- a/cpp/CMakeLists.txt +++ b/cpp/CMakeLists.txt @@ -223,8 +223,6 @@ else() set(ARROW_BOOST_HEADER_ONLY 1) endif() -include(BuildUtils) - ############################################################ # Compiler flags ############################################################ @@ -248,6 +246,9 @@ include(SetupCxxFlags) add_custom_target(arrow_dependencies) +include(BuildUtils) +enable_testing() + include(ThirdpartyToolchain) # Add common flags @@ -354,153 +355,6 @@ if (PARQUET_BUILD_SHARED) endif() endif() -############################################################ -# Benchmarking -############################################################ -# Add a new micro benchmark, with or without an executable that should be built. -# If benchmarks are enabled then they will be run along side unit tests with ctest. -# 'make runbenchmark' and 'make unittest' to build/run only benchmark or unittests, -# respectively. -# -# REL_BENCHMARK_NAME is the name of the benchmark app. It may be a single component -# (e.g. monotime-benchmark) or contain additional components (e.g. -# net/net_util-benchmark). Either way, the last component must be a globally -# unique name. - -# The benchmark will registered as unit test with ctest with a label -# of 'benchmark'. -# -# Arguments after the test name will be passed to set_tests_properties(). -function(ADD_ARROW_BENCHMARK REL_BENCHMARK_NAME) - if(NO_BENCHMARKS) - return() - endif() - get_filename_component(BENCHMARK_NAME ${REL_BENCHMARK_NAME} NAME_WE) - - if(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/${REL_BENCHMARK_NAME}.cc) - # This benchmark has a corresponding .cc file, set it up as an executable. - set(BENCHMARK_PATH "${EXECUTABLE_OUTPUT_PATH}/${BENCHMARK_NAME}") - add_executable(${BENCHMARK_NAME} "${REL_BENCHMARK_NAME}.cc") - target_link_libraries(${BENCHMARK_NAME} ${ARROW_BENCHMARK_LINK_LIBS}) - add_dependencies(runbenchmark ${BENCHMARK_NAME}) - set(NO_COLOR "--color_print=false") - else() - # No executable, just invoke the benchmark (probably a script) directly. - set(BENCHMARK_PATH ${CMAKE_CURRENT_SOURCE_DIR}/${REL_BENCHMARK_NAME}) - set(NO_COLOR "") - endif() - - add_test(${BENCHMARK_NAME} - ${BUILD_SUPPORT_DIR}/run-test.sh ${CMAKE_BINARY_DIR} benchmark ${BENCHMARK_PATH} ${NO_COLOR}) - set_tests_properties(${BENCHMARK_NAME} PROPERTIES LABELS "benchmark") - if(ARGN) - set_tests_properties(${BENCHMARK_NAME} PROPERTIES ${ARGN}) - endif() -endfunction() - -# A wrapper for add_dependencies() that is compatible with NO_BENCHMARKS. -function(ADD_ARROW_BENCHMARK_DEPENDENCIES REL_BENCHMARK_NAME) - if(NO_BENCHMARKS) - return() - endif() - get_filename_component(BENCMARK_NAME ${REL_BENCHMARK_NAME} NAME_WE) - - add_dependencies(${BENCHMARK_NAME} ${ARGN}) -endfunction() - -# A wrapper for target_link_libraries() that is compatible with NO_BENCHMARKS. -function(ARROW_BENCHMARK_LINK_LIBRARIES REL_BENCHMARK_NAME) - if(NO_BENCHMARKS) - return() - endif() - get_filename_component(BENCHMARK_NAME ${REL_BENCHMARK_NAME} NAME_WE) - - target_link_libraries(${BENCHMARK_NAME} ${ARGN}) -endfunction() - - -############################################################ -# Testing -############################################################ -# Add a new test case, with or without an executable that should be built. -# -# REL_TEST_NAME is the name of the test. It may be a single component -# (e.g. monotime-test) or contain additional components (e.g. -# net/net_util-test). Either way, the last component must be a globally -# unique name. -# -# The unit test is added with a label of "unittest" to support filtering with -# ctest. -# -# Arguments after the test name will be passed to set_tests_properties(). -function(ADD_ARROW_TEST REL_TEST_NAME) - set(options) - set(single_value_args) - set(multi_value_args STATIC_LINK_LIBS) - cmake_parse_arguments(ARG "${options}" "${one_value_args}" "${multi_value_args}" ${ARGN}) - if(ARG_UNPARSED_ARGUMENTS) - message(SEND_ERROR "Error: unrecognized arguments: ${ARG_UNPARSED_ARGUMENTS}") - endif() - - if(NO_TESTS OR NOT ARROW_BUILD_STATIC) - return() - endif() - get_filename_component(TEST_NAME ${REL_TEST_NAME} NAME_WE) - - if(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/${REL_TEST_NAME}.cc) - # This test has a corresponding .cc file, set it up as an executable. - set(TEST_PATH "${EXECUTABLE_OUTPUT_PATH}/${TEST_NAME}") - add_executable(${TEST_NAME} "${REL_TEST_NAME}.cc") - - if (ARG_STATIC_LINK_LIBS) - # Customize link libraries - target_link_libraries(${TEST_NAME} ${ARG_STATIC_LINK_LIBS}) - else() - target_link_libraries(${TEST_NAME} ${ARROW_TEST_LINK_LIBS}) - endif() - add_dependencies(unittest ${TEST_NAME}) - else() - # No executable, just invoke the test (probably a script) directly. - set(TEST_PATH ${CMAKE_CURRENT_SOURCE_DIR}/${REL_TEST_NAME}) - endif() - - if (ARROW_TEST_MEMCHECK) - SET_PROPERTY(TARGET ${TEST_NAME} - APPEND_STRING PROPERTY - COMPILE_FLAGS " -DARROW_VALGRIND") - add_test(${TEST_NAME} - bash -c "cd ${EXECUTABLE_OUTPUT_PATH}; valgrind --tool=memcheck --leak-check=full --leak-check-heuristics=stdstring --error-exitcode=1 ${TEST_PATH}") - elseif(MSVC) - add_test(${TEST_NAME} ${TEST_PATH}) - else() - add_test(${TEST_NAME} - ${BUILD_SUPPORT_DIR}/run-test.sh ${CMAKE_BINARY_DIR} test ${TEST_PATH}) - endif() - set_tests_properties(${TEST_NAME} PROPERTIES LABELS "unittest") -endfunction() - -# A wrapper for add_dependencies() that is compatible with NO_TESTS. -function(ADD_ARROW_TEST_DEPENDENCIES REL_TEST_NAME) - if(NO_TESTS) - return() - endif() - get_filename_component(TEST_NAME ${REL_TEST_NAME} NAME_WE) - - add_dependencies(${TEST_NAME} ${ARGN}) -endfunction() - -# A wrapper for target_link_libraries() that is compatible with NO_TESTS. -function(ARROW_TEST_LINK_LIBRARIES REL_TEST_NAME) - if(NO_TESTS) - return() - endif() - get_filename_component(TEST_NAME ${REL_TEST_NAME} NAME_WE) - - target_link_libraries(${TEST_NAME} ${ARGN}) -endfunction() - -enable_testing() - ############################################################ # "make ctags" target ############################################################ @@ -733,121 +587,6 @@ if(NOT WIN32 AND ARROW_PLASMA) endif() add_subdirectory(src/arrow) -add_subdirectory(src/arrow/io) - -set(ARROW_SRCS - src/arrow/array.cc - src/arrow/buffer.cc - src/arrow/builder.cc - src/arrow/compare.cc - src/arrow/memory_pool.cc - src/arrow/pretty_print.cc - src/arrow/status.cc - src/arrow/table.cc - src/arrow/tensor.cc - src/arrow/type.cc - src/arrow/visitor.cc - - src/arrow/compute/cast.cc - src/arrow/compute/context.cc - - src/arrow/io/file.cc - src/arrow/io/interfaces.cc - src/arrow/io/memory.cc - - src/arrow/util/bit-util.cc - src/arrow/util/compression.cc - src/arrow/util/cpu-info.cc - src/arrow/util/decimal.cc - src/arrow/util/key_value_metadata.cc -) - -if (ARROW_COMPUTE) - add_subdirectory(src/arrow/compute) - set(ARROW_SRCS ${ARROW_SRCS} - src/arrow/compute/cast.cc - src/arrow/compute/context.cc - ) -endif() - -if (ARROW_GPU) - # IPC extensions required to build the GPU library - set(ARROW_IPC ON) - add_subdirectory(src/arrow/gpu) -endif() - -if (ARROW_IPC) - add_subdirectory(src/arrow/ipc) - add_dependencies(arrow_dependencies metadata_fbs) -endif() - -if (ARROW_WITH_BROTLI) - add_definitions(-DARROW_WITH_BROTLI) - SET(ARROW_SRCS src/arrow/util/compression_brotli.cc ${ARROW_SRCS}) -endif() - -if (ARROW_WITH_LZ4) - add_definitions(-DARROW_WITH_LZ4) - SET(ARROW_SRCS src/arrow/util/compression_lz4.cc ${ARROW_SRCS}) -endif() - -if (ARROW_WITH_SNAPPY) - add_definitions(-DARROW_WITH_SNAPPY) - SET(ARROW_SRCS src/arrow/util/compression_snappy.cc ${ARROW_SRCS}) -endif() - -if (ARROW_WITH_ZLIB) - add_definitions(-DARROW_WITH_ZLIB) - SET(ARROW_SRCS src/arrow/util/compression_zlib.cc ${ARROW_SRCS}) -endif() - -if (ARROW_WITH_ZSTD) - add_definitions(-DARROW_WITH_ZSTD) - SET(ARROW_SRCS src/arrow/util/compression_zstd.cc ${ARROW_SRCS}) -endif() - -if (NOT ARROW_BOOST_HEADER_ONLY) - set(ARROW_SRCS ${ARROW_SRCS} - src/arrow/io/hdfs.cc - src/arrow/io/hdfs-internal.cc - ) -endif() - -if (ARROW_IPC) - set(ARROW_SRCS ${ARROW_SRCS} - src/arrow/ipc/dictionary.cc - src/arrow/ipc/feather.cc - src/arrow/ipc/json.cc - src/arrow/ipc/json-internal.cc - src/arrow/ipc/message.cc - src/arrow/ipc/metadata-internal.cc - src/arrow/ipc/reader.cc - src/arrow/ipc/writer.cc - ) -endif() - - -if(NOT APPLE AND NOT MSVC) - # Localize thirdparty symbols using a linker version script. This hides them - # from the client application. The OS X linker does not support the - # version-script option. - set(ARROW_SHARED_LINK_FLAGS "-Wl,--version-script=${CMAKE_CURRENT_SOURCE_DIR}/src/arrow/symbols.map") -endif() - -set(ARROW_ALL_SRCS - ${ARROW_SRCS}) - -ADD_ARROW_LIB(arrow - SOURCES ${ARROW_ALL_SRCS} - DEPENDENCIES arrow_dependencies - SHARED_LINK_FLAGS ${ARROW_SHARED_LINK_FLAGS} - SHARED_LINK_LIBS ${ARROW_LINK_LIBS} - SHARED_PRIVATE_LINK_LIBS ${ARROW_SHARED_PRIVATE_LINK_LIBS} - STATIC_LINK_LIBS ${ARROW_STATIC_LINK_LIBS} - STATIC_PRIVATE_LINK_LIBS ${ARROW_STATIC_PRIVATE_LINK_LIBS} -) - -add_subdirectory(src/arrow/util) if(ARROW_PYTHON) find_package(PythonLibsNew REQUIRED) diff --git a/cpp/README.md b/cpp/README.md index 4a515079ddc..6e29e6f78f4 100644 --- a/cpp/README.md +++ b/cpp/README.md @@ -238,6 +238,13 @@ build failures by running the following checks before submitting your pull reque # before running it. make format # requires clang-format is installed +We run our CI builds with more compiler warnings enabled for the Clang +compiler. Please run CMake with + +`-DBUILD_WARNING_LEVEL=CHECKIN` + +to avoid failures due to compiler warnings. + Note that the clang-tidy target may take a while to run. You might consider running clang-tidy separately on the files you have added/changed before invoking the make target to reduce iteration time. Also, it might generate warnings diff --git a/cpp/cmake_modules/BuildUtils.cmake b/cpp/cmake_modules/BuildUtils.cmake index 8f92d73baea..e398dc1827c 100644 --- a/cpp/cmake_modules/BuildUtils.cmake +++ b/cpp/cmake_modules/BuildUtils.cmake @@ -188,3 +188,149 @@ function(ADD_ARROW_LIB LIB_NAME) endif() endfunction() + + +############################################################ +# Benchmarking +############################################################ +# Add a new micro benchmark, with or without an executable that should be built. +# If benchmarks are enabled then they will be run along side unit tests with ctest. +# 'make runbenchmark' and 'make unittest' to build/run only benchmark or unittests, +# respectively. +# +# REL_BENCHMARK_NAME is the name of the benchmark app. It may be a single component +# (e.g. monotime-benchmark) or contain additional components (e.g. +# net/net_util-benchmark). Either way, the last component must be a globally +# unique name. + +# The benchmark will registered as unit test with ctest with a label +# of 'benchmark'. +# +# Arguments after the test name will be passed to set_tests_properties(). +function(ADD_ARROW_BENCHMARK REL_BENCHMARK_NAME) + if(NO_BENCHMARKS) + return() + endif() + get_filename_component(BENCHMARK_NAME ${REL_BENCHMARK_NAME} NAME_WE) + + if(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/${REL_BENCHMARK_NAME}.cc) + # This benchmark has a corresponding .cc file, set it up as an executable. + set(BENCHMARK_PATH "${EXECUTABLE_OUTPUT_PATH}/${BENCHMARK_NAME}") + add_executable(${BENCHMARK_NAME} "${REL_BENCHMARK_NAME}.cc") + target_link_libraries(${BENCHMARK_NAME} ${ARROW_BENCHMARK_LINK_LIBS}) + add_dependencies(runbenchmark ${BENCHMARK_NAME}) + set(NO_COLOR "--color_print=false") + else() + # No executable, just invoke the benchmark (probably a script) directly. + set(BENCHMARK_PATH ${CMAKE_CURRENT_SOURCE_DIR}/${REL_BENCHMARK_NAME}) + set(NO_COLOR "") + endif() + + add_test(${BENCHMARK_NAME} + ${BUILD_SUPPORT_DIR}/run-test.sh ${CMAKE_BINARY_DIR} benchmark ${BENCHMARK_PATH} ${NO_COLOR}) + set_tests_properties(${BENCHMARK_NAME} PROPERTIES LABELS "benchmark") + if(ARGN) + set_tests_properties(${BENCHMARK_NAME} PROPERTIES ${ARGN}) + endif() +endfunction() + +# A wrapper for add_dependencies() that is compatible with NO_BENCHMARKS. +function(ADD_ARROW_BENCHMARK_DEPENDENCIES REL_BENCHMARK_NAME) + if(NO_BENCHMARKS) + return() + endif() + get_filename_component(BENCMARK_NAME ${REL_BENCHMARK_NAME} NAME_WE) + + add_dependencies(${BENCHMARK_NAME} ${ARGN}) +endfunction() + +# A wrapper for target_link_libraries() that is compatible with NO_BENCHMARKS. +function(ARROW_BENCHMARK_LINK_LIBRARIES REL_BENCHMARK_NAME) + if(NO_BENCHMARKS) + return() + endif() + get_filename_component(BENCHMARK_NAME ${REL_BENCHMARK_NAME} NAME_WE) + + target_link_libraries(${BENCHMARK_NAME} ${ARGN}) +endfunction() + + +############################################################ +# Testing +############################################################ +# Add a new test case, with or without an executable that should be built. +# +# REL_TEST_NAME is the name of the test. It may be a single component +# (e.g. monotime-test) or contain additional components (e.g. +# net/net_util-test). Either way, the last component must be a globally +# unique name. +# +# The unit test is added with a label of "unittest" to support filtering with +# ctest. +# +# Arguments after the test name will be passed to set_tests_properties(). +function(ADD_ARROW_TEST REL_TEST_NAME) + set(options) + set(single_value_args) + set(multi_value_args STATIC_LINK_LIBS) + cmake_parse_arguments(ARG "${options}" "${one_value_args}" "${multi_value_args}" ${ARGN}) + if(ARG_UNPARSED_ARGUMENTS) + message(SEND_ERROR "Error: unrecognized arguments: ${ARG_UNPARSED_ARGUMENTS}") + endif() + + if(NO_TESTS OR NOT ARROW_BUILD_STATIC) + return() + endif() + get_filename_component(TEST_NAME ${REL_TEST_NAME} NAME_WE) + + if(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/${REL_TEST_NAME}.cc) + # This test has a corresponding .cc file, set it up as an executable. + set(TEST_PATH "${EXECUTABLE_OUTPUT_PATH}/${TEST_NAME}") + add_executable(${TEST_NAME} "${REL_TEST_NAME}.cc") + + if (ARG_STATIC_LINK_LIBS) + # Customize link libraries + target_link_libraries(${TEST_NAME} ${ARG_STATIC_LINK_LIBS}) + else() + target_link_libraries(${TEST_NAME} ${ARROW_TEST_LINK_LIBS}) + endif() + add_dependencies(unittest ${TEST_NAME}) + else() + # No executable, just invoke the test (probably a script) directly. + set(TEST_PATH ${CMAKE_CURRENT_SOURCE_DIR}/${REL_TEST_NAME}) + endif() + + if (ARROW_TEST_MEMCHECK) + SET_PROPERTY(TARGET ${TEST_NAME} + APPEND_STRING PROPERTY + COMPILE_FLAGS " -DARROW_VALGRIND") + add_test(${TEST_NAME} + bash -c "cd ${EXECUTABLE_OUTPUT_PATH}; valgrind --tool=memcheck --leak-check=full --leak-check-heuristics=stdstring --error-exitcode=1 ${TEST_PATH}") + elseif(MSVC) + add_test(${TEST_NAME} ${TEST_PATH}) + else() + add_test(${TEST_NAME} + ${BUILD_SUPPORT_DIR}/run-test.sh ${CMAKE_BINARY_DIR} test ${TEST_PATH}) + endif() + set_tests_properties(${TEST_NAME} PROPERTIES LABELS "unittest") +endfunction() + +# A wrapper for add_dependencies() that is compatible with NO_TESTS. +function(ADD_ARROW_TEST_DEPENDENCIES REL_TEST_NAME) + if(NO_TESTS) + return() + endif() + get_filename_component(TEST_NAME ${REL_TEST_NAME} NAME_WE) + + add_dependencies(${TEST_NAME} ${ARGN}) +endfunction() + +# A wrapper for target_link_libraries() that is compatible with NO_TESTS. +function(ARROW_TEST_LINK_LIBRARIES REL_TEST_NAME) + if(NO_TESTS) + return() + endif() + get_filename_component(TEST_NAME ${REL_TEST_NAME} NAME_WE) + + target_link_libraries(${TEST_NAME} ${ARGN}) +endfunction() diff --git a/cpp/cmake_modules/SetupCxxFlags.cmake b/cpp/cmake_modules/SetupCxxFlags.cmake index 1f4c898cfcf..77bfac83555 100644 --- a/cpp/cmake_modules/SetupCxxFlags.cmake +++ b/cpp/cmake_modules/SetupCxxFlags.cmake @@ -70,17 +70,17 @@ if ("${UPPERCASE_BUILD_WARNING_LEVEL}" STREQUAL "CHECKIN") set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} /WX") elseif ("${COMPILER_FAMILY}" STREQUAL "clang") set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -Weverything -Wno-c++98-compat \ - -Wno-c++98-compat-pedantic -Wno-deprecated -Wno-weak-vtables -Wno-padded \ - -Wno-unused-parameter -Wno-undef -Wno-documentation-deprecated-sync \ - -Wno-shadow -Wno-switch-enum -Wno-documentation -Wno-exit-time-destructors \ - -Wno-global-constructors -Wno-weak-template-vtables -Wno-undefined-reinterpret-cast \ - -Wno-implicit-fallthrough -Wno-old-style-cast -Wno-unreachable-code-return \ - -Wno-float-equal -Wno-missing-prototypes -Wno-non-virtual-dtor \ - -Wno-unused-macros -Wno-covered-switch-default -Wno-unreachable-code-break \ - -Wno-extra-semi -Wno-cast-align -Wno-vla-extension -Wno-shift-sign-overflow \ - -Wno-used-but-marked-unused -Wno-missing-variable-declarations \ - -Wno-gnu-zero-variadic-macro-arguments -Wconversion -Wno-sign-conversion \ - -Wno-disabled-macro-expansion -Wc++11-narrowing -Wnarrowing -Wno-shorten-64-to-32") +-Wno-c++98-compat-pedantic -Wno-deprecated -Wno-weak-vtables -Wno-padded \ +-Wno-unused-parameter -Wno-undef \ +-Wno-shadow -Wno-switch-enum -Wno-exit-time-destructors \ +-Wno-global-constructors -Wno-weak-template-vtables -Wno-undefined-reinterpret-cast \ +-Wno-implicit-fallthrough -Wno-unreachable-code-return \ +-Wno-float-equal -Wno-missing-prototypes \ +-Wno-old-style-cast -Wno-covered-switch-default \ +-Wno-cast-align -Wno-vla-extension -Wno-shift-sign-overflow \ +-Wno-used-but-marked-unused -Wno-missing-variable-declarations \ +-Wno-gnu-zero-variadic-macro-arguments -Wconversion -Wno-sign-conversion \ +-Wno-disabled-macro-expansion -Wno-shorten-64-to-32") # Version numbers where warnings are introduced if ("${COMPILER_VERSION}" VERSION_GREATER "3.3") @@ -98,11 +98,11 @@ if ("${UPPERCASE_BUILD_WARNING_LEVEL}" STREQUAL "CHECKIN") endif() # Treat all compiler warnings as errors - set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -Wunknown-warning-option -Werror") + set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -Wno-unknown-warning-option -Werror") elseif ("${COMPILER_FAMILY}" STREQUAL "gcc") set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -Wall -Wconversion -Wno-sign-conversion") # Treat all compiler warnings as errors - set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -Wunknown-warning-option -Werror") + set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -Wno-unknown-warning-option -Werror") else() message(FATAL_ERROR "Unknown compiler. Version info:\n${COMPILER_VERSION_FULL}") endif() diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index 194156cc656..98186d01415 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -126,6 +126,7 @@ endif() set(Boost_DEBUG TRUE) set(Boost_USE_MULTITHREADED ON) set(Boost_ADDITIONAL_VERSIONS + "1.65.0" "1.65" "1.64.0" "1.64" "1.63.0" "1.63" "1.62.0" "1.61" diff --git a/cpp/src/arrow/CMakeLists.txt b/cpp/src/arrow/CMakeLists.txt index 6963b11e3ff..5c903314181 100644 --- a/cpp/src/arrow/CMakeLists.txt +++ b/cpp/src/arrow/CMakeLists.txt @@ -15,6 +15,121 @@ # specific language governing permissions and limitations # under the License. +set(ARROW_SRCS + array.cc + buffer.cc + builder.cc + compare.cc + memory_pool.cc + pretty_print.cc + status.cc + table.cc + tensor.cc + type.cc + visitor.cc + + io/file.cc + io/interfaces.cc + io/memory.cc + + util/bit-util.cc + util/compression.cc + util/cpu-info.cc + util/decimal.cc + util/key_value_metadata.cc +) + +if ("${COMPILER_FAMILY}" STREQUAL "clang") + set_property(SOURCE io/file.cc + APPEND_STRING + PROPERTY COMPILE_FLAGS + " -Wno-unused-macros ") +endif() + +if (ARROW_COMPUTE) + add_subdirectory(compute) + set(ARROW_SRCS ${ARROW_SRCS} + compute/cast.cc + compute/context.cc + ) +endif() + +if (ARROW_GPU) + # IPC extensions required to build the GPU library + set(ARROW_IPC ON) + add_subdirectory(gpu) +endif() + +if (ARROW_IPC) + add_subdirectory(ipc) + add_dependencies(arrow_dependencies metadata_fbs) +endif() + +if (ARROW_WITH_BROTLI) + add_definitions(-DARROW_WITH_BROTLI) + SET(ARROW_SRCS util/compression_brotli.cc ${ARROW_SRCS}) +endif() + +if (ARROW_WITH_LZ4) + add_definitions(-DARROW_WITH_LZ4) + SET(ARROW_SRCS util/compression_lz4.cc ${ARROW_SRCS}) +endif() + +if (ARROW_WITH_SNAPPY) + add_definitions(-DARROW_WITH_SNAPPY) + SET(ARROW_SRCS util/compression_snappy.cc ${ARROW_SRCS}) +endif() + +if (ARROW_WITH_ZLIB) + add_definitions(-DARROW_WITH_ZLIB) + SET(ARROW_SRCS util/compression_zlib.cc ${ARROW_SRCS}) +endif() + +if (ARROW_WITH_ZSTD) + add_definitions(-DARROW_WITH_ZSTD) + SET(ARROW_SRCS util/compression_zstd.cc ${ARROW_SRCS}) +endif() + +if (NOT ARROW_BOOST_HEADER_ONLY) + set(ARROW_SRCS ${ARROW_SRCS} + io/hdfs.cc + io/hdfs-internal.cc + ) +endif() + +if (ARROW_IPC) + set(ARROW_SRCS ${ARROW_SRCS} + ipc/dictionary.cc + ipc/feather.cc + ipc/json.cc + ipc/json-internal.cc + ipc/message.cc + ipc/metadata-internal.cc + ipc/reader.cc + ipc/writer.cc + ) +endif() + +if(NOT APPLE AND NOT MSVC) + # Localize thirdparty symbols using a linker version script. This hides them + # from the client application. The OS X linker does not support the + # version-script option. + set(ARROW_SHARED_LINK_FLAGS "-Wl,--version-script=${CMAKE_CURRENT_SOURCE_DIR}/symbols.map") +endif() + +set(ARROW_ALL_SRCS + ${ARROW_SRCS}) + +ADD_ARROW_LIB(arrow + SOURCES ${ARROW_ALL_SRCS} + DEPENDENCIES arrow_dependencies + SHARED_LINK_FLAGS ${ARROW_SHARED_LINK_FLAGS} + SHARED_LINK_LIBS ${ARROW_LINK_LIBS} + SHARED_PRIVATE_LINK_LIBS ${ARROW_SHARED_PRIVATE_LINK_LIBS} + STATIC_LINK_LIBS ${ARROW_STATIC_LINK_LIBS} + STATIC_PRIVATE_LINK_LIBS ${ARROW_STATIC_PRIVATE_LINK_LIBS} +) + # Headers: top level install(FILES allocator.h @@ -60,3 +175,6 @@ ADD_ARROW_TEST(tensor-test) ADD_ARROW_BENCHMARK(builder-benchmark) ADD_ARROW_BENCHMARK(column-benchmark) + +add_subdirectory(io) +add_subdirectory(util) diff --git a/cpp/src/arrow/array-test.cc b/cpp/src/arrow/array-test.cc index 4ea9248f3e4..4ecf0f92b2a 100644 --- a/cpp/src/arrow/array-test.cc +++ b/cpp/src/arrow/array-test.cc @@ -283,7 +283,7 @@ class TestPrimitiveBuilder : public TestBuilder { #define PINT_DECL(CapType, c_type, LOWER, UPPER) \ struct P##CapType { \ - PTYPE_DECL(CapType, c_type); \ + PTYPE_DECL(CapType, c_type) \ static void draw(int64_t N, vector* draws) { \ test::randint(N, LOWER, UPPER, draws); \ } \ @@ -291,7 +291,7 @@ class TestPrimitiveBuilder : public TestBuilder { #define PFLOAT_DECL(CapType, c_type, LOWER, UPPER) \ struct P##CapType { \ - PTYPE_DECL(CapType, c_type); \ + PTYPE_DECL(CapType, c_type) \ static void draw(int64_t N, vector* draws) { \ test::random_real(N, 0, LOWER, UPPER, draws); \ } \ @@ -311,7 +311,7 @@ PFLOAT_DECL(Float, float, -1000, 1000); PFLOAT_DECL(Double, double, -1000, 1000); struct PBoolean { - PTYPE_DECL(Boolean, uint8_t); + PTYPE_DECL(Boolean, uint8_t) }; template <> @@ -378,8 +378,6 @@ TYPED_TEST_CASE(TestPrimitiveBuilder, Primitives); #define DECL_TYPE() typedef typename TestFixture::Type Type; -#define DECL_ARRAYTYPE() typedef typename TestFixture::ArrayType ArrayType; - TYPED_TEST(TestPrimitiveBuilder, TestInit) { DECL_TYPE(); diff --git a/cpp/src/arrow/array.cc b/cpp/src/arrow/array.cc index 80188a14a95..92e8d0f0fa8 100644 --- a/cpp/src/arrow/array.cc +++ b/cpp/src/arrow/array.cc @@ -194,7 +194,8 @@ ListArray::ListArray(const std::shared_ptr& type, int64_t length, SetData(internal_data); } -Status ListArray::FromArrays(const Array& offsets, const Array& values, MemoryPool* pool, +Status ListArray::FromArrays(const Array& offsets, const Array& values, + MemoryPool* ARROW_ARG_UNUSED(pool), std::shared_ptr* out) { if (ARROW_PREDICT_FALSE(offsets.length() == 0)) { return Status::Invalid("List offsets must have non-zero length"); @@ -239,9 +240,6 @@ std::shared_ptr ListArray::values() const { return values_; } // ---------------------------------------------------------------------- // String and binary -static std::shared_ptr kBinary = std::make_shared(); -static std::shared_ptr kString = std::make_shared(); - BinaryArray::BinaryArray(const std::shared_ptr& data) { DCHECK_EQ(data->type->id(), Type::BINARY); SetData(data); @@ -261,8 +259,8 @@ BinaryArray::BinaryArray(int64_t length, const std::shared_ptr& value_of const std::shared_ptr& data, const std::shared_ptr& null_bitmap, int64_t null_count, int64_t offset) - : BinaryArray(kBinary, length, value_offsets, data, null_bitmap, null_count, offset) { -} + : BinaryArray(binary(), length, value_offsets, data, null_bitmap, null_count, + offset) {} BinaryArray::BinaryArray(const std::shared_ptr& type, int64_t length, const std::shared_ptr& value_offsets, @@ -283,8 +281,7 @@ StringArray::StringArray(int64_t length, const std::shared_ptr& value_of const std::shared_ptr& data, const std::shared_ptr& null_bitmap, int64_t null_count, int64_t offset) - : BinaryArray(kString, length, value_offsets, data, null_bitmap, null_count, offset) { -} + : BinaryArray(utf8(), length, value_offsets, data, null_bitmap, null_count, offset) {} // ---------------------------------------------------------------------- // Fixed width binary @@ -437,13 +434,13 @@ Status Array::Accept(ArrayVisitor* visitor) const { namespace internal { struct ValidateVisitor { - Status Visit(const NullArray& array) { return Status::OK(); } + Status Visit(const NullArray&) { return Status::OK(); } - Status Visit(const PrimitiveArray& array) { return Status::OK(); } + Status Visit(const PrimitiveArray&) { return Status::OK(); } - Status Visit(const DecimalArray& array) { return Status::OK(); } + Status Visit(const DecimalArray&) { return Status::OK(); } - Status Visit(const BinaryArray& array) { + Status Visit(const BinaryArray&) { // TODO(wesm): what to do here? return Status::OK(); } @@ -585,7 +582,7 @@ class ArrayDataWrapper { : data_(data), out_(out) {} template - Status Visit(const T& type) { + Status Visit(const T&) { using ArrayType = typename TypeTraits::ArrayType; *out_ = std::make_shared(data_); return Status::OK(); diff --git a/cpp/src/arrow/array.h b/cpp/src/arrow/array.h index e801b3586df..ee29d950682 100644 --- a/cpp/src/arrow/array.h +++ b/cpp/src/arrow/array.h @@ -674,7 +674,7 @@ ARROW_EXPORT Status ValidateArray(const Array& array); #ifndef ARROW_NO_DEPRECATED_API -// \deprecated Since 0.7.0 +// \note Deprecated since 0.7.0 /// Create new arrays for logical types that are backed by primitive arrays. ARROW_EXPORT diff --git a/cpp/src/arrow/buffer.cc b/cpp/src/arrow/buffer.cc index cf533eb3be4..e308ed26049 100644 --- a/cpp/src/arrow/buffer.cc +++ b/cpp/src/arrow/buffer.cc @@ -132,6 +132,8 @@ Status AllocateBuffer(MemoryPool* pool, const int64_t size, return Status::OK(); } +#ifndef ARROW_NO_DEPRECATED_API + Status AllocateBuffer(MemoryPool* pool, const int64_t size, std::shared_ptr* out) { std::shared_ptr buffer; @@ -140,6 +142,8 @@ Status AllocateBuffer(MemoryPool* pool, const int64_t size, return Status::OK(); } +#endif + Status AllocateResizableBuffer(MemoryPool* pool, const int64_t size, std::shared_ptr* out) { auto buffer = std::make_shared(pool); diff --git a/cpp/src/arrow/compare.cc b/cpp/src/arrow/compare.cc index 2aeb03b8b9a..515b8f62f56 100644 --- a/cpp/src/arrow/compare.cc +++ b/cpp/src/arrow/compare.cc @@ -584,7 +584,7 @@ class TypeEqualsVisitor { typename std::enable_if::value || std::is_base_of::value, Status>::type - Visit(const T& type) { + Visit(const T&) { result_ = true; return Status::OK(); } diff --git a/cpp/src/arrow/compute/cast.h b/cpp/src/arrow/compute/cast.h index 081cdd90832..7a07512b2ad 100644 --- a/cpp/src/arrow/compute/cast.h +++ b/cpp/src/arrow/compute/cast.h @@ -46,11 +46,11 @@ Status GetCastFunction(const DataType& in_type, const std::shared_ptr& const CastOptions& options, std::unique_ptr* kernel); /// \brief Cast from one array type to another -/// \param[in] context -/// \param[in] array -/// \param[in] to_type -/// \param[in] options -/// \param[out] out +/// \param[in] context the FunctionContext +/// \param[in] array array to cast +/// \param[in] to_type type to cast to +/// \param[in] options casting options +/// \param[out] out resulting array /// /// \since 0.7.0 /// \note API not yet finalized diff --git a/cpp/src/arrow/io/file.h b/cpp/src/arrow/io/file.h index 1b1bbe0e495..4fb09634a2b 100644 --- a/cpp/src/arrow/io/file.h +++ b/cpp/src/arrow/io/file.h @@ -86,7 +86,7 @@ class ARROW_EXPORT ReadableFile : public RandomAccessFile { /// \param[in] pool a MemoryPool for memory allocations /// \param[out] file ReadableFile instance /// Open file with one's own memory pool for memory allocations - static Status Open(const std::string& path, MemoryPool* memory_pool, + static Status Open(const std::string& path, MemoryPool* pool, std::shared_ptr* file); Status Close() override; diff --git a/cpp/src/arrow/io/interfaces.h b/cpp/src/arrow/io/interfaces.h index 11441794e18..2c5b351e208 100644 --- a/cpp/src/arrow/io/interfaces.h +++ b/cpp/src/arrow/io/interfaces.h @@ -86,11 +86,14 @@ class ARROW_EXPORT FileInterface { class ARROW_EXPORT Seekable { public: + virtual ~Seekable() = default; virtual Status Seek(int64_t position) = 0; }; class ARROW_EXPORT Writeable { public: + virtual ~Writeable() = default; + virtual Status Write(const uint8_t* data, int64_t nbytes) = 0; /// \brief Flush buffered bytes, if any @@ -101,6 +104,8 @@ class ARROW_EXPORT Writeable { class ARROW_EXPORT Readable { public: + virtual ~Readable() = default; + virtual Status Read(int64_t nbytes, int64_t* bytes_read, uint8_t* out) = 0; // Does not copy if not necessary diff --git a/cpp/src/arrow/ipc/feather.cc b/cpp/src/arrow/ipc/feather.cc index 31dc0e73e0e..9d244f11544 100644 --- a/cpp/src/arrow/ipc/feather.cc +++ b/cpp/src/arrow/ipc/feather.cc @@ -477,7 +477,6 @@ fbs::Type ToFlatbufferType(Type::type type) { return fbs::Type_INT64; default: DCHECK(false) << "Cannot reach this code"; - break; } // prevent compiler warning return fbs::Type_MIN; @@ -632,19 +631,19 @@ class TableWriter::TableWriterImpl : public ArrayVisitor { #define VISIT_PRIMITIVE(TYPE) \ Status Visit(const TYPE& values) override { return WritePrimitiveValues(values); } - VISIT_PRIMITIVE(BooleanArray); - VISIT_PRIMITIVE(Int8Array); - VISIT_PRIMITIVE(Int16Array); - VISIT_PRIMITIVE(Int32Array); - VISIT_PRIMITIVE(Int64Array); - VISIT_PRIMITIVE(UInt8Array); - VISIT_PRIMITIVE(UInt16Array); - VISIT_PRIMITIVE(UInt32Array); - VISIT_PRIMITIVE(UInt64Array); - VISIT_PRIMITIVE(FloatArray); - VISIT_PRIMITIVE(DoubleArray); - VISIT_PRIMITIVE(BinaryArray); - VISIT_PRIMITIVE(StringArray); + VISIT_PRIMITIVE(BooleanArray) + VISIT_PRIMITIVE(Int8Array) + VISIT_PRIMITIVE(Int16Array) + VISIT_PRIMITIVE(Int32Array) + VISIT_PRIMITIVE(Int64Array) + VISIT_PRIMITIVE(UInt8Array) + VISIT_PRIMITIVE(UInt16Array) + VISIT_PRIMITIVE(UInt32Array) + VISIT_PRIMITIVE(UInt64Array) + VISIT_PRIMITIVE(FloatArray) + VISIT_PRIMITIVE(DoubleArray) + VISIT_PRIMITIVE(BinaryArray) + VISIT_PRIMITIVE(StringArray) #undef VISIT_PRIMITIVE diff --git a/cpp/src/arrow/ipc/message.h b/cpp/src/arrow/ipc/message.h index 4bc4384f64e..522b3bdcdbb 100644 --- a/cpp/src/arrow/ipc/message.h +++ b/cpp/src/arrow/ipc/message.h @@ -82,11 +82,10 @@ class ARROW_EXPORT Message { static Status ReadFrom(const std::shared_ptr& metadata, io::InputStream* stream, std::unique_ptr* out); - /// \brief Write length-prefixed metadata and body to output stream + /// \brief Return true if message type and contents are equal /// - /// \param[in] file output stream to write to - /// \param[out] output_length the number of bytes written - /// \return Status + /// \param other another message + /// \return true if contents equal bool Equals(const Message& other) const; /// \brief the Message metadata diff --git a/cpp/src/arrow/ipc/reader.h b/cpp/src/arrow/ipc/reader.h index 54174f9ca17..e90dc1e5234 100644 --- a/cpp/src/arrow/ipc/reader.h +++ b/cpp/src/arrow/ipc/reader.h @@ -54,8 +54,8 @@ class ARROW_EXPORT RecordBatchStreamReader : public RecordBatchReader { /// Create batch reader from generic MessageReader /// - /// \param(in) message_reader a MessageReader implementation - /// \param(out) out the created RecordBatchReader object + /// \param[in] message_reader a MessageReader implementation + /// \param[out] out the created RecordBatchReader object /// \return Status static Status Open(std::unique_ptr message_reader, std::shared_ptr* out); @@ -72,9 +72,9 @@ class ARROW_EXPORT RecordBatchStreamReader : public RecordBatchReader { /// \brief Record batch stream reader from InputStream /// - /// \param(in) stream an input stream instance. Must stay alive throughout + /// \param[in] stream an input stream instance. Must stay alive throughout /// lifetime of stream reader - /// \param(out) out the created RecordBatchStreamReader object + /// \param[out] out the created RecordBatchStreamReader object /// \return Status static Status Open(io::InputStream* stream, std::shared_ptr* out); @@ -112,8 +112,8 @@ class ARROW_EXPORT RecordBatchFileReader { /// metadata footer). The metadata must have been written with memory offsets /// relative to the start of the containing file /// - /// @param file: the data source - /// @param footer_offset: the position of the end of the Arrow "file" + /// @param file the data source + /// @param footer_offset the position of the end of the Arrow "file" static Status Open(io::RandomAccessFile* file, int64_t footer_offset, std::shared_ptr* reader); @@ -138,8 +138,8 @@ class ARROW_EXPORT RecordBatchFileReader { /// Read a record batch from the file. Does not copy memory if the input /// source supports zero-copy. /// - /// \param(in) i the index of the record batch to return - /// \param(out) batch the read batch + /// \param[in] i the index of the record batch to return + /// \param[out] batch the read batch /// \return Status Status ReadRecordBatch(int i, std::shared_ptr* batch); @@ -165,20 +165,19 @@ Status ReadSchema(io::InputStream* stream, std::shared_ptr* out); /// Read record batch as encapsulated IPC message with metadata size prefix and /// header /// -/// \param(in) schema the record batch schema -/// \param(in) offset the file location of the start of the message -/// \param(in) file the file where the batch is located -/// \param(out) out the read record batch +/// \param[in] schema the record batch schema +/// \param[in] stream the file where the batch is located +/// \param[out] out the read record batch ARROW_EXPORT Status ReadRecordBatch(const std::shared_ptr& schema, io::InputStream* stream, std::shared_ptr* out); /// \brief Read record batch from file given metadata and schema /// -/// \param(in) metadata a Message containing the record batch metadata -/// \param(in) schema the record batch schema -/// \param(in) file a random access file -/// \param(out) out the read record batch +/// \param[in] metadata a Message containing the record batch metadata +/// \param[in] schema the record batch schema +/// \param[in] file a random access file +/// \param[out] out the read record batch ARROW_EXPORT Status ReadRecordBatch(const Buffer& metadata, const std::shared_ptr& schema, io::RandomAccessFile* file, std::shared_ptr* out); @@ -186,7 +185,7 @@ Status ReadRecordBatch(const Buffer& metadata, const std::shared_ptr& sc /// \brief Read record batch from fully encapulated Message /// /// \param[in] message a message instance containing metadata and body -/// \param[in] schema +/// \param[in] schema the record batch schema /// \param[out] out the resulting RecordBatch /// \return Status ARROW_EXPORT @@ -195,11 +194,11 @@ Status ReadRecordBatch(const Message& message, const std::shared_ptr& sc /// Read record batch from file given metadata and schema /// -/// \param(in) metadata a Message containing the record batch metadata -/// \param(in) schema the record batch schema -/// \param(in) file a random access file -/// \param(in) max_recursion_depth the maximum permitted nesting depth -/// \param(out) out the read record batch +/// \param[in] metadata a Message containing the record batch metadata +/// \param[in] schema the record batch schema +/// \param[in] file a random access file +/// \param[in] max_recursion_depth the maximum permitted nesting depth +/// \param[out] out the read record batch ARROW_EXPORT Status ReadRecordBatch(const Buffer& metadata, const std::shared_ptr& schema, int max_recursion_depth, io::RandomAccessFile* file, @@ -207,9 +206,9 @@ Status ReadRecordBatch(const Buffer& metadata, const std::shared_ptr& sc /// EXPERIMENTAL: Read arrow::Tensor as encapsulated IPC message in file /// -/// \param(in) offset the file location of the start of the message -/// \param(in) file the file where the batch is located -/// \param(out) out the read tensor +/// \param[in] offset the file location of the start of the message +/// \param[in] file the file where the batch is located +/// \param[out] out the read tensor ARROW_EXPORT Status ReadTensor(int64_t offset, io::RandomAccessFile* file, std::shared_ptr* out); diff --git a/cpp/src/arrow/ipc/writer.cc b/cpp/src/arrow/ipc/writer.cc index 9f557f6659e..9531fd77a24 100644 --- a/cpp/src/arrow/ipc/writer.cc +++ b/cpp/src/arrow/ipc/writer.cc @@ -324,24 +324,24 @@ class RecordBatchSerializer : public ArrayVisitor { #define VISIT_FIXED_WIDTH(TYPE) \ Status Visit(const TYPE& array) override { return VisitFixedWidth(array); } - VISIT_FIXED_WIDTH(Int8Array); - VISIT_FIXED_WIDTH(Int16Array); - VISIT_FIXED_WIDTH(Int32Array); - VISIT_FIXED_WIDTH(Int64Array); - VISIT_FIXED_WIDTH(UInt8Array); - VISIT_FIXED_WIDTH(UInt16Array); - VISIT_FIXED_WIDTH(UInt32Array); - VISIT_FIXED_WIDTH(UInt64Array); - VISIT_FIXED_WIDTH(HalfFloatArray); - VISIT_FIXED_WIDTH(FloatArray); - VISIT_FIXED_WIDTH(DoubleArray); - VISIT_FIXED_WIDTH(Date32Array); - VISIT_FIXED_WIDTH(Date64Array); - VISIT_FIXED_WIDTH(TimestampArray); - VISIT_FIXED_WIDTH(Time32Array); - VISIT_FIXED_WIDTH(Time64Array); - VISIT_FIXED_WIDTH(FixedSizeBinaryArray); - VISIT_FIXED_WIDTH(DecimalArray); + VISIT_FIXED_WIDTH(Int8Array) + VISIT_FIXED_WIDTH(Int16Array) + VISIT_FIXED_WIDTH(Int32Array) + VISIT_FIXED_WIDTH(Int64Array) + VISIT_FIXED_WIDTH(UInt8Array) + VISIT_FIXED_WIDTH(UInt16Array) + VISIT_FIXED_WIDTH(UInt32Array) + VISIT_FIXED_WIDTH(UInt64Array) + VISIT_FIXED_WIDTH(HalfFloatArray) + VISIT_FIXED_WIDTH(FloatArray) + VISIT_FIXED_WIDTH(DoubleArray) + VISIT_FIXED_WIDTH(Date32Array) + VISIT_FIXED_WIDTH(Date64Array) + VISIT_FIXED_WIDTH(TimestampArray) + VISIT_FIXED_WIDTH(Time32Array) + VISIT_FIXED_WIDTH(Time64Array) + VISIT_FIXED_WIDTH(FixedSizeBinaryArray) + VISIT_FIXED_WIDTH(DecimalArray) #undef VISIT_FIXED_WIDTH diff --git a/cpp/src/arrow/ipc/writer.h b/cpp/src/arrow/ipc/writer.h index df03f4f1381..89d3f5f2684 100644 --- a/cpp/src/arrow/ipc/writer.h +++ b/cpp/src/arrow/ipc/writer.h @@ -61,7 +61,7 @@ class ARROW_EXPORT RecordBatchWriter { virtual Status WriteRecordBatch(const RecordBatch& batch, bool allow_64bit = false) = 0; /// \brief Write possibly-chunked table by creating sequence of record batches - /// \param[in] table + /// \param[in] table table to write /// \return Status Status WriteTable(const Table& table); @@ -87,9 +87,9 @@ class ARROW_EXPORT RecordBatchStreamWriter : public RecordBatchWriter { /// Create a new writer from stream sink and schema. User is responsible for /// closing the actual OutputStream. /// - /// \param(in) sink output stream to write to - /// \param(in) schema the schema of the record batches to be written - /// \param(out) out the created stream writer + /// \param[in] sink output stream to write to + /// \param[in] schema the schema of the record batches to be written + /// \param[out] out the created stream writer /// \return Status static Status Open(io::OutputStream* sink, const std::shared_ptr& schema, std::shared_ptr* out); @@ -121,9 +121,9 @@ class ARROW_EXPORT RecordBatchFileWriter : public RecordBatchStreamWriter { /// Create a new writer from stream sink and schema /// - /// \param(in) sink output stream to write to - /// \param(in) schema the schema of the record batches to be written - /// \param(out) out the created stream writer + /// \param[in] sink output stream to write to + /// \param[in] schema the schema of the record batches to be written + /// \param[out] out the created stream writer /// \return Status static Status Open(io::OutputStream* sink, const std::shared_ptr& schema, std::shared_ptr* out); @@ -155,13 +155,13 @@ class ARROW_EXPORT RecordBatchFileWriter : public RecordBatchStreamWriter { /// to the end of the body and end of the metadata / data header (suffixed by /// the header size) is returned in out-variables /// -/// \param(in) buffer_start_offset the start offset to use in the buffer metadata, +/// \param[in] buffer_start_offset the start offset to use in the buffer metadata, /// default should be 0 -/// \param(in) allow_64bit permit field lengths exceeding INT32_MAX. May not be +/// \param[in] allow_64bit permit field lengths exceeding INT32_MAX. May not be /// readable by other Arrow implementations -/// \param(out) metadata_length: the size of the length-prefixed flatbuffer +/// \param[out] metadata_length the size of the length-prefixed flatbuffer /// including padding to a 64-byte boundary -/// \param(out) body_length: the size of the contiguous buffer block plus +/// \param[out] body_length the size of the contiguous buffer block plus /// padding bytes /// \return Status /// @@ -198,7 +198,7 @@ Status SerializeRecordBatch(const RecordBatch& batch, MemoryPool* pool, /// \brief Serialize schema using stream writer as a sequence of one or more /// IPC messages /// -/// \param[in] scheam the schema to write +/// \param[in] schema the schema to write /// \param[in] pool a MemoryPool to allocate memory from /// \param[out] out the serialized schema /// \return Status diff --git a/cpp/src/arrow/pretty_print.cc b/cpp/src/arrow/pretty_print.cc index cc1acf4befd..31b93442402 100644 --- a/cpp/src/arrow/pretty_print.cc +++ b/cpp/src/arrow/pretty_print.cc @@ -199,7 +199,7 @@ class ArrayPrinter : public PrettyPrinter { } } - Status Visit(const NullArray& array) { return Status::OK(); } + Status Visit(const NullArray&) { return Status::OK(); } template typename std::enable_if::value || @@ -213,7 +213,7 @@ class ArrayPrinter : public PrettyPrinter { return Status::OK(); } - Status Visit(const IntervalArray& array) { return Status::NotImplemented("interval"); } + Status Visit(const IntervalArray&) { return Status::NotImplemented("interval"); } Status WriteValidityBitmap(const Array& array); diff --git a/cpp/src/arrow/python/arrow_to_python.h b/cpp/src/arrow/python/arrow_to_python.h index 3650c058efd..7509f30eb4e 100644 --- a/cpp/src/arrow/python/arrow_to_python.h +++ b/cpp/src/arrow/python/arrow_to_python.h @@ -54,7 +54,7 @@ Status ReadSerializedObject(io::RandomAccessFile* src, SerializedPyObject* out); /// _serialize_callback method for serialization and a _deserialize_callback /// method for deserialization. If context is None, no custom serialization /// will be attempted. -/// \param[in] object +/// \param[in] object object to deserialize /// \param[in] base a Python object holding the underlying data that any NumPy /// arrays will reference, to avoid premature deallocation /// \param[out] out the returned object diff --git a/cpp/src/arrow/python/numpy_to_arrow.cc b/cpp/src/arrow/python/numpy_to_arrow.cc index 7151c94c0db..8845ee7838e 100644 --- a/cpp/src/arrow/python/numpy_to_arrow.cc +++ b/cpp/src/arrow/python/numpy_to_arrow.cc @@ -149,7 +149,7 @@ Status CheckFlatNumpyArray(PyArrayObject* numpy_array, int np_type) { /// can fit /// /// \param[in] offset starting offset for appending -/// \param[out] values_consumed ending offset where we stopped appending. Will +/// \param[out] end_offset ending offset where we stopped appending. Will /// be length of arr if fully consumed /// \param[out] have_bytes true if we encountered any PyBytes object static Status AppendObjectStrings(PyArrayObject* arr, PyArrayObject* mask, int64_t offset, diff --git a/cpp/src/arrow/python/numpy_to_arrow.h b/cpp/src/arrow/python/numpy_to_arrow.h index 4a70b4bc533..4a870fff9fc 100644 --- a/cpp/src/arrow/python/numpy_to_arrow.h +++ b/cpp/src/arrow/python/numpy_to_arrow.h @@ -42,7 +42,7 @@ namespace py { /// \param[in] pool Memory pool for any memory allocations /// \param[in] ao an ndarray with the array data /// \param[in] mo an ndarray with a null mask (True is null), optional -/// \param[in] type +/// \param[in] type a specific type to cast to, may be null /// \param[out] out a ChunkedArray, to accommodate chunked output ARROW_EXPORT Status NdarrayToArrow(MemoryPool* pool, PyObject* ao, PyObject* mo, diff --git a/cpp/src/arrow/python/python_to_arrow.cc b/cpp/src/arrow/python/python_to_arrow.cc index c57091fc069..9ba78213b2c 100644 --- a/cpp/src/arrow/python/python_to_arrow.cc +++ b/cpp/src/arrow/python/python_to_arrow.cc @@ -297,12 +297,11 @@ class DictBuilder { /// Construct an Arrow StructArray representing the dictionary. /// Contains a field "keys" for the keys and "vals" for the values. - - /// \param list_data + /// \param val_list_data /// List containing the data from nested lists in the value /// list of the dictionary /// - /// \param dict_data + /// \param val_dict_data /// List containing the data from nested dictionaries in the /// value list of the dictionary Status Finish(const Array* key_tuple_data, const Array* key_dict_data, @@ -543,7 +542,11 @@ Status SerializeSequences(PyObject* context, std::vector sequences, ScopedRef iterator(PyObject_GetIter(sequence)); RETURN_IF_PYERROR(); ScopedRef item; - while (item.reset(PyIter_Next(iterator.get())), item.get()) { + while (true) { + item.reset(PyIter_Next(iterator.get())); + if (!item.get()) { + break; + } RETURN_NOT_OK(Append(context, item.get(), &builder, &sublists, &subtuples, &subdicts, &subsets, tensors_out)); } diff --git a/cpp/src/arrow/python/util/datetime.h b/cpp/src/arrow/python/util/datetime.h index de751510151..4ebef720f7a 100644 --- a/cpp/src/arrow/python/util/datetime.h +++ b/cpp/src/arrow/python/util/datetime.h @@ -70,11 +70,14 @@ static inline Status PyTime_from_int(int64_t val, const TimeUnit::type unit, } static inline int64_t PyDate_to_ms(PyDateTime_Date* pydate) { - struct tm date = {0}; + struct tm date; + memset(&date, 0, sizeof(struct tm)); date.tm_year = PyDateTime_GET_YEAR(pydate) - 1900; date.tm_mon = PyDateTime_GET_MONTH(pydate) - 1; date.tm_mday = PyDateTime_GET_DAY(pydate); - struct tm epoch = {0}; + struct tm epoch; + memset(&epoch, 0, sizeof(struct tm)); + epoch.tm_year = 70; epoch.tm_mday = 1; #ifdef _MSC_VER @@ -88,7 +91,8 @@ static inline int64_t PyDate_to_ms(PyDateTime_Date* pydate) { } static inline int64_t PyDateTime_to_us(PyDateTime_DateTime* pydatetime) { - struct tm datetime = {0}; + struct tm datetime; + memset(&datetime, 0, sizeof(struct tm)); datetime.tm_year = PyDateTime_GET_YEAR(pydatetime) - 1900; datetime.tm_mon = PyDateTime_GET_MONTH(pydatetime) - 1; datetime.tm_mday = PyDateTime_GET_DAY(pydatetime); @@ -96,7 +100,8 @@ static inline int64_t PyDateTime_to_us(PyDateTime_DateTime* pydatetime) { datetime.tm_min = PyDateTime_DATE_GET_MINUTE(pydatetime); datetime.tm_sec = PyDateTime_DATE_GET_SECOND(pydatetime); int us = PyDateTime_DATE_GET_MICROSECOND(pydatetime); - struct tm epoch = {0}; + struct tm epoch; + memset(&epoch, 0, sizeof(struct tm)); epoch.tm_year = 70; epoch.tm_mday = 1; #ifdef _MSC_VER diff --git a/cpp/src/arrow/table.cc b/cpp/src/arrow/table.cc index 3d3ecd2734e..d0bbe7e0ec9 100644 --- a/cpp/src/arrow/table.cc +++ b/cpp/src/arrow/table.cc @@ -153,13 +153,6 @@ Status Column::ValidateData() { // ---------------------------------------------------------------------- // RecordBatch methods -void AssertBatchValid(const RecordBatch& batch) { - Status s = batch.Validate(); - if (!s.ok()) { - DCHECK(false) << s.ToString(); - } -} - RecordBatch::RecordBatch(const std::shared_ptr& schema, int64_t num_rows) : schema_(schema), num_rows_(num_rows) { boxed_columns_.resize(schema->num_fields()); diff --git a/cpp/src/arrow/table.h b/cpp/src/arrow/table.h index a66772e5a71..85fa2341a82 100644 --- a/cpp/src/arrow/table.h +++ b/cpp/src/arrow/table.h @@ -113,7 +113,7 @@ class ARROW_EXPORT Column { /// sequence of fields, each a contiguous Arrow array class ARROW_EXPORT RecordBatch { public: - /// \param[in] schema + /// \param[in] schema The record batch schema /// \param[in] num_rows length of fields in the record batch. Each array /// should have the same length as num_rows /// \param[in] columns the record batch fields as vector of arrays @@ -209,16 +209,16 @@ class ARROW_EXPORT Table { public: /// \brief Construct Table from schema and columns /// If columns is zero-length, the table's number of rows is zero - /// \param schema - /// \param columns - /// \param number of rows in table, -1 (default) to infer from columns + /// \param schema The table schema (column types) + /// \param columns The table's columns + /// \param num_rows number of rows in table, -1 (default) to infer from columns Table(const std::shared_ptr& schema, const std::vector>& columns, int64_t num_rows = -1); /// \brief Construct Table from schema and arrays - /// \param schema - /// \param arrays - /// \param number of rows in table, -1 (default) to infer from columns + /// \param schema The table schema (column types) + /// \param arrays The table's columns as arrays + /// \param num_rows number of rows in table, -1 (default) to infer from columns Table(const std::shared_ptr& schema, const std::vector>& arrays, int64_t num_rows = -1); @@ -231,7 +231,7 @@ class ARROW_EXPORT Table { /// \return the table's schema std::shared_ptr schema() const { return schema_; } - /// \param[i] i column index, does not boundscheck + /// \param[in] i column index, does not boundscheck /// \return the i-th column std::shared_ptr column(int i) const { return columns_[i]; } @@ -283,7 +283,7 @@ class ARROW_EXPORT RecordBatchReader { /// Read the next record batch in the stream. Return nullptr for batch when /// reaching end of stream /// - /// \param(out) batch the next loaded batch, nullptr at end of stream + /// \param[out] batch the next loaded batch, nullptr at end of stream /// \return Status virtual Status ReadNext(std::shared_ptr* batch) = 0; diff --git a/cpp/src/arrow/type-test.cc b/cpp/src/arrow/type-test.cc index 710344ab0d2..3242fadd50d 100644 --- a/cpp/src/arrow/type-test.cc +++ b/cpp/src/arrow/type-test.cc @@ -68,8 +68,7 @@ TEST(TestField, TestAddMetadata) { new KeyValueMetadata({"foo", "bar"}, {"bizz", "buzz"})); auto f0 = field("f0", int32()); auto f1 = field("f0", int32(), true, metadata); - std::shared_ptr f2; - ASSERT_OK(f0->AddMetadata(metadata, &f2)); + std::shared_ptr f2 = f0->AddMetadata(metadata); ASSERT_FALSE(f2->Equals(*f0)); ASSERT_TRUE(f2->Equals(*f1)); @@ -184,8 +183,7 @@ TEST_F(TestSchema, TestAddMetadata) { auto metadata = std::shared_ptr( new KeyValueMetadata({"foo", "bar"}, {"bizz", "buzz"})); auto schema = std::make_shared(fields); - std::shared_ptr new_schema; - ASSERT_OK(schema->AddMetadata(metadata, &new_schema)); + std::shared_ptr new_schema = schema->AddMetadata(metadata); ASSERT_TRUE(metadata->Equals(*new_schema->metadata())); // Not copied diff --git a/cpp/src/arrow/type.cc b/cpp/src/arrow/type.cc index 87bb73b1e7c..b9e314440f5 100644 --- a/cpp/src/arrow/type.cc +++ b/cpp/src/arrow/type.cc @@ -36,12 +36,16 @@ std::shared_ptr Field::AddMetadata( return std::make_shared(name_, type_, nullable_, metadata); } +#ifndef ARROW_NO_DEPRECATED_API + Status Field::AddMetadata(const std::shared_ptr& metadata, std::shared_ptr* out) const { *out = AddMetadata(metadata); return Status::OK(); } +#endif + std::shared_ptr Field::RemoveMetadata() const { return std::make_shared(name_, type_, nullable_); } @@ -307,12 +311,16 @@ std::shared_ptr Schema::AddMetadata( return std::make_shared(fields_, metadata); } +#ifndef ARROW_NO_DEPRECATED_API + Status Schema::AddMetadata(const std::shared_ptr& metadata, std::shared_ptr* out) const { *out = AddMetadata(metadata); return Status::OK(); } +#endif + std::shared_ptr Schema::metadata() const { return metadata_; } std::shared_ptr Schema::RemoveMetadata() const { diff --git a/cpp/src/arrow/type.h b/cpp/src/arrow/type.h index 7630f48151b..c1da6e27043 100644 --- a/cpp/src/arrow/type.h +++ b/cpp/src/arrow/type.h @@ -227,9 +227,11 @@ class ARROW_EXPORT Field { std::shared_ptr metadata() const { return metadata_; } - /// \deprecated +#ifndef ARROW_NO_DEPRECATED_API + /// \note Deprecated since 0.8.0 Status AddMetadata(const std::shared_ptr& metadata, std::shared_ptr* out) const; +#endif std::shared_ptr AddMetadata( const std::shared_ptr& metadata) const; @@ -749,9 +751,11 @@ class ARROW_EXPORT Schema { std::shared_ptr* out) const; Status RemoveField(int i, std::shared_ptr* out) const; - /// \deprecated +#ifndef ARROW_NO_DEPRECATED_API + /// \note Deprecated since 0.8.0 Status AddMetadata(const std::shared_ptr& metadata, std::shared_ptr* out) const; +#endif /// \brief Replace key-value metadata with new metadata /// diff --git a/cpp/src/arrow/util/compression_brotli.cc b/cpp/src/arrow/util/compression_brotli.cc index 1aaec11d9ca..196c19a660b 100644 --- a/cpp/src/arrow/util/compression_brotli.cc +++ b/cpp/src/arrow/util/compression_brotli.cc @@ -25,6 +25,7 @@ #include #include "arrow/status.h" +#include "arrow/util/macros.h" namespace arrow { @@ -41,7 +42,8 @@ Status BrotliCodec::Decompress(int64_t input_len, const uint8_t* input, return Status::OK(); } -int64_t BrotliCodec::MaxCompressedLen(int64_t input_len, const uint8_t* input) { +int64_t BrotliCodec::MaxCompressedLen(int64_t input_len, + const uint8_t* ARROW_ARG_UNUSED(input)) { return BrotliEncoderMaxCompressedSize(input_len); } diff --git a/cpp/src/arrow/util/compression_lz4.cc b/cpp/src/arrow/util/compression_lz4.cc index cda40ad8c3a..001edeb01b6 100644 --- a/cpp/src/arrow/util/compression_lz4.cc +++ b/cpp/src/arrow/util/compression_lz4.cc @@ -22,6 +22,7 @@ #include #include "arrow/status.h" +#include "arrow/util/macros.h" namespace arrow { @@ -39,7 +40,8 @@ Status Lz4Codec::Decompress(int64_t input_len, const uint8_t* input, int64_t out return Status::OK(); } -int64_t Lz4Codec::MaxCompressedLen(int64_t input_len, const uint8_t* input) { +int64_t Lz4Codec::MaxCompressedLen(int64_t input_len, + const uint8_t* ARROW_ARG_UNUSED(input)) { return LZ4_compressBound(static_cast(input_len)); } diff --git a/cpp/src/arrow/util/compression_snappy.cc b/cpp/src/arrow/util/compression_snappy.cc index e284bd4358d..2edaef7cf4b 100644 --- a/cpp/src/arrow/util/compression_snappy.cc +++ b/cpp/src/arrow/util/compression_snappy.cc @@ -33,7 +33,8 @@ namespace arrow { // Snappy implementation Status SnappyCodec::Decompress(int64_t input_len, const uint8_t* input, - int64_t output_len, uint8_t* output_buffer) { + int64_t ARROW_ARG_UNUSED(output_len), + uint8_t* output_buffer) { if (!snappy::RawUncompress(reinterpret_cast(input), static_cast(input_len), reinterpret_cast(output_buffer))) { @@ -42,13 +43,14 @@ Status SnappyCodec::Decompress(int64_t input_len, const uint8_t* input, return Status::OK(); } -int64_t SnappyCodec::MaxCompressedLen(int64_t input_len, const uint8_t* input) { +int64_t SnappyCodec::MaxCompressedLen(int64_t input_len, + const uint8_t* ARROW_ARG_UNUSED(input)) { return snappy::MaxCompressedLength(input_len); } Status SnappyCodec::Compress(int64_t input_len, const uint8_t* input, - int64_t output_buffer_len, uint8_t* output_buffer, - int64_t* output_length) { + int64_t ARROW_ARG_UNUSED(output_buffer_len), + uint8_t* output_buffer, int64_t* output_length) { size_t output_len; snappy::RawCompress(reinterpret_cast(input), static_cast(input_len), diff --git a/cpp/src/arrow/util/compression_zlib.cc b/cpp/src/arrow/util/compression_zlib.cc index 0656fd64b5c..3a520240ec6 100644 --- a/cpp/src/arrow/util/compression_zlib.cc +++ b/cpp/src/arrow/util/compression_zlib.cc @@ -165,7 +165,7 @@ class GZipCodec::GZipCodecImpl { return Status::OK(); } - int64_t MaxCompressedLen(int64_t input_length, const uint8_t* input) { + int64_t MaxCompressedLen(int64_t input_length, const uint8_t* ARROW_ARG_UNUSED(input)) { // Most be in compression mode if (!compressor_initialized_) { Status s = InitCompressor(); diff --git a/cpp/src/arrow/util/compression_zstd.cc b/cpp/src/arrow/util/compression_zstd.cc index d19ac43449b..20306f48efb 100644 --- a/cpp/src/arrow/util/compression_zstd.cc +++ b/cpp/src/arrow/util/compression_zstd.cc @@ -23,6 +23,7 @@ #include #include "arrow/status.h" +#include "arrow/util/macros.h" using std::size_t; @@ -42,7 +43,8 @@ Status ZSTDCodec::Decompress(int64_t input_len, const uint8_t* input, int64_t ou return Status::OK(); } -int64_t ZSTDCodec::MaxCompressedLen(int64_t input_len, const uint8_t* input) { +int64_t ZSTDCodec::MaxCompressedLen(int64_t input_len, + const uint8_t* ARROW_ARG_UNUSED(input)) { return ZSTD_compressBound(input_len); } diff --git a/cpp/src/arrow/util/cpu-info.cc b/cpp/src/arrow/util/cpu-info.cc index 639f02e482b..822fcaea74e 100644 --- a/cpp/src/arrow/util/cpu-info.cc +++ b/cpp/src/arrow/util/cpu-info.cc @@ -76,6 +76,8 @@ static struct { }; static const int64_t num_flags = sizeof(flag_mappings) / sizeof(flag_mappings[0]); +namespace { + // Helper function to parse for hardware flags. // values contains a list of space-seperated flags. check to see if the flags we // care about are present. @@ -90,6 +92,8 @@ int64_t ParseCPUFlags(const string& values) { return flags; } +} // namespace + #ifdef _WIN32 bool RetrieveCacheSize(int64_t* cache_sizes) { if (!cache_sizes) { diff --git a/cpp/src/arrow/util/decimal.h b/cpp/src/arrow/util/decimal.h index a0dea09216a..58496a874f1 100644 --- a/cpp/src/arrow/util/decimal.h +++ b/cpp/src/arrow/util/decimal.h @@ -75,8 +75,8 @@ class ARROW_EXPORT Decimal128 { /// -21 / 5 -> -4, -1 /// 21 / -5 -> -4, 1 /// -21 / -5 -> 4, -1 - /// @param right the number to divide by - /// @param remainder the remainder after the division + /// \param divisor the number to divide by + /// \param remainder the remainder after the division Status Divide(const Decimal128& divisor, Decimal128* result, Decimal128* remainder) const; diff --git a/cpp/src/arrow/util/hash-util.h b/cpp/src/arrow/util/hash-util.h index d5fb212f33a..3bba07bb20f 100644 --- a/cpp/src/arrow/util/hash-util.h +++ b/cpp/src/arrow/util/hash-util.h @@ -199,7 +199,7 @@ class HashUtil { static uint32_t FnvHash64to32(const void* data, int32_t bytes, uint32_t hash) { // IMPALA-2270: this function should never be used for zero-byte inputs. DCHECK_GT(bytes, 0); - uint64_t hash_u64 = hash | ((uint64_t)hash << 32); + uint64_t hash_u64 = hash | (static_cast(hash) << 32); hash_u64 = FnvHash64(data, bytes, hash_u64); return static_cast((hash_u64 >> 32) ^ (hash_u64 & 0xFFFFFFFF)); } diff --git a/cpp/src/arrow/util/logging.h b/cpp/src/arrow/util/logging.h index b7e2ceea9f3..40a51cb5569 100644 --- a/cpp/src/arrow/util/logging.h +++ b/cpp/src/arrow/util/logging.h @@ -90,7 +90,7 @@ namespace internal { class NullLog { public: template - NullLog& operator<<(const T& t) { + NullLog& operator<<(const T& ARROW_ARG_UNUSED(t)) { return *this; } }; diff --git a/cpp/src/arrow/util/macros.h b/cpp/src/arrow/util/macros.h index 2f1db0924a1..a5f6e57079a 100644 --- a/cpp/src/arrow/util/macros.h +++ b/cpp/src/arrow/util/macros.h @@ -26,7 +26,7 @@ #endif #define ARROW_UNUSED(x) (void)x - +#define ARROW_ARG_UNUSED(x) // // GCC can be told that a certain branch is not likely to be taken (for // instance, a CHECK failure), and use that information in static analysis. diff --git a/cpp/src/arrow/util/sse-util.h b/cpp/src/arrow/util/sse-util.h index a0ec8a2e939..32ac43f919b 100644 --- a/cpp/src/arrow/util/sse-util.h +++ b/cpp/src/arrow/util/sse-util.h @@ -176,27 +176,27 @@ static inline int SSE4_cmpestri(__m128i str1, int len1, __m128i str2, int len2) return 0; } -static inline uint32_t SSE4_crc32_u8(uint32_t crc, uint8_t v) { +static inline uint32_t SSE4_crc32_u8(uint32_t, uint8_t) { DCHECK(false) << "CPU doesn't support SSE 4.2"; return 0; } -static inline uint32_t SSE4_crc32_u16(uint32_t crc, uint16_t v) { +static inline uint32_t SSE4_crc32_u16(uint32_t, uint16_t) { DCHECK(false) << "CPU doesn't support SSE 4.2"; return 0; } -static inline uint32_t SSE4_crc32_u32(uint32_t crc, uint32_t v) { +static inline uint32_t SSE4_crc32_u32(uint32_t, uint32_t) { DCHECK(false) << "CPU doesn't support SSE 4.2"; return 0; } -static inline uint32_t SSE4_crc32_u64(uint32_t crc, uint64_t v) { +static inline uint32_t SSE4_crc32_u64(uint32_t, uint64_t) { DCHECK(false) << "CPU doesn't support SSE 4.2"; return 0; } -static inline int64_t POPCNT_popcnt_u64(uint64_t a) { +static inline int64_t POPCNT_popcnt_u64(uint64_t) { DCHECK(false) << "CPU doesn't support SSE 4.2"; return 0; } @@ -205,27 +205,27 @@ static inline int64_t POPCNT_popcnt_u64(uint64_t a) { #else -static inline uint32_t SSE4_crc32_u8(uint32_t crc, uint8_t v) { +static inline uint32_t SSE4_crc32_u8(uint32_t, uint8_t) { DCHECK(false) << "SSE support is not enabled"; return 0; } -static inline uint32_t SSE4_crc32_u16(uint32_t crc, uint16_t v) { +static inline uint32_t SSE4_crc32_u16(uint32_t, uint16_t) { DCHECK(false) << "SSE support is not enabled"; return 0; } -static inline uint32_t SSE4_crc32_u32(uint32_t crc, uint32_t v) { +static inline uint32_t SSE4_crc32_u32(uint32_t, uint32_t) { DCHECK(false) << "SSE support is not enabled"; return 0; } -static inline uint32_t SSE4_crc32_u64(uint32_t crc, uint64_t v) { +static inline uint32_t SSE4_crc32_u64(uint32_t, uint64_t) { DCHECK(false) << "SSE support is not enabled"; return 0; } -static inline int64_t POPCNT_popcnt_u64(uint64_t a) { +static inline int64_t POPCNT_popcnt_u64(uint64_t) { DCHECK(false) << "SSE support is not enabled"; return 0; } diff --git a/cpp/src/arrow/visitor.cc b/cpp/src/arrow/visitor.cc index 53a0fee0aae..a7b01b0f631 100644 --- a/cpp/src/arrow/visitor.cc +++ b/cpp/src/arrow/visitor.cc @@ -56,10 +56,7 @@ ARRAY_VISITOR_DEFAULT(ListArray); ARRAY_VISITOR_DEFAULT(StructArray); ARRAY_VISITOR_DEFAULT(UnionArray); ARRAY_VISITOR_DEFAULT(DictionaryArray); - -Status ArrayVisitor::Visit(const DecimalArray& array) { - return Status::NotImplemented("decimal"); -} +ARRAY_VISITOR_DEFAULT(DecimalArray); #undef ARRAY_VISITOR_DEFAULT diff --git a/cpp/src/plasma/CMakeLists.txt b/cpp/src/plasma/CMakeLists.txt index ed1762b32e5..c9339545fca 100644 --- a/cpp/src/plasma/CMakeLists.txt +++ b/cpp/src/plasma/CMakeLists.txt @@ -94,7 +94,14 @@ if ("${COMPILER_FAMILY}" STREQUAL "clang") set_property(SOURCE malloc.cc APPEND_STRING PROPERTY COMPILE_FLAGS - " -Wno-parentheses-equality -Wno-shorten-64-to-32") + " -Wno-parentheses-equality \ +-Wno-shorten-64-to-32 \ +-Wno-unused-macros ") + + set_property(SOURCE thirdparty/xxhash.cc + APPEND_STRING + PROPERTY COMPILE_FLAGS + "-Wno-unused-macros") endif() if ("${COMPILER_FAMILY}" STREQUAL "gcc") diff --git a/cpp/src/plasma/client.cc b/cpp/src/plasma/client.cc index 5e28d4f2af7..3f99fe04708 100644 --- a/cpp/src/plasma/client.cc +++ b/cpp/src/plasma/client.cc @@ -286,7 +286,6 @@ Status PlasmaClient::Get(const ObjectID* object_ids, int64_t num_objects, /// calls will not do anything. The client will only send a message to the store /// releasing the object when the client is truly done with the object. /// -/// @param conn The plasma connection. /// @param object_id The object ID to attempt to release. Status PlasmaClient::PerformRelease(const ObjectID& object_id) { // Decrement the count of the number of instances of this object that are @@ -401,7 +400,8 @@ static inline bool compute_object_hash_parallel(XXH64_state_t* hash_state, } } - XXH64_update(hash_state, (unsigned char*)threadhash, sizeof(threadhash)); + XXH64_update(hash_state, reinterpret_cast(threadhash), + sizeof(threadhash)); return true; } @@ -409,12 +409,14 @@ static uint64_t compute_object_hash(const ObjectBuffer& obj_buffer) { XXH64_state_t hash_state; XXH64_reset(&hash_state, XXH64_DEFAULT_SEED); if (obj_buffer.data_size >= kBytesInMB) { - compute_object_hash_parallel(&hash_state, (unsigned char*)obj_buffer.data, + compute_object_hash_parallel(&hash_state, + reinterpret_cast(obj_buffer.data), obj_buffer.data_size); } else { - XXH64_update(&hash_state, (unsigned char*)obj_buffer.data, obj_buffer.data_size); + XXH64_update(&hash_state, reinterpret_cast(obj_buffer.data), + obj_buffer.data_size); } - XXH64_update(&hash_state, (unsigned char*)obj_buffer.metadata, + XXH64_update(&hash_state, reinterpret_cast(obj_buffer.metadata), obj_buffer.metadata_size); return XXH64_digest(&hash_state); } @@ -548,8 +550,6 @@ Status PlasmaClient::Disconnect() { return Status::OK(); } -#define h_addr h_addr_list[0] - Status PlasmaClient::Transfer(const char* address, int port, const ObjectID& object_id) { return SendDataRequest(manager_conn_, object_id, address, port); } diff --git a/cpp/src/plasma/client.h b/cpp/src/plasma/client.h index 50ec55f5ec8..145942441c9 100644 --- a/cpp/src/plasma/client.h +++ b/cpp/src/plasma/client.h @@ -82,15 +82,15 @@ class ARROW_EXPORT PlasmaClient { /// Connect to the local plasma store and plasma manager. Return /// the resulting connection. /// - /// @param store_socket_name The name of the UNIX domain socket to use to + /// \param store_socket_name The name of the UNIX domain socket to use to /// connect to the Plasma store. - /// @param manager_socket_name The name of the UNIX domain socket to use to + /// \param manager_socket_name The name of the UNIX domain socket to use to /// connect to the local Plasma manager. If this is "", then this /// function will not connect to a manager. - /// @param release_delay Number of released objects that are kept around + /// \param release_delay Number of released objects that are kept around /// and not evicted to avoid too many munmaps. - /// @param num_retries number of attempts to connect to IPC socket, default 50 - /// @return The return status. + /// \param num_retries number of attempts to connect to IPC socket, default 50 + /// \return The return status. Status Connect(const std::string& store_socket_name, const std::string& manager_socket_name, int release_delay, int num_retries = -1); @@ -98,17 +98,17 @@ class ARROW_EXPORT PlasmaClient { /// Create an object in the Plasma Store. Any metadata for this object must be /// be passed in when the object is created. /// - /// @param object_id The ID to use for the newly created object. - /// @param data_size The size in bytes of the space to be allocated for this + /// \param object_id The ID to use for the newly created object. + /// \param data_size The size in bytes of the space to be allocated for this /// object's /// data (this does not include space used for metadata). - /// @param metadata The object's metadata. If there is no metadata, this + /// \param metadata The object's metadata. If there is no metadata, this /// pointer /// should be NULL. - /// @param metadata_size The size in bytes of the metadata. If there is no + /// \param metadata_size The size in bytes of the metadata. If there is no /// metadata, this should be 0. - /// @param data The address of the newly created object will be written here. - /// @return The return status. + /// \param data The address of the newly created object will be written here. + /// \return The return status. Status Create(const ObjectID& object_id, int64_t data_size, uint8_t* metadata, int64_t metadata_size, uint8_t** data); @@ -119,14 +119,14 @@ class ARROW_EXPORT PlasmaClient { /// but /// the caller should not release objects that were not retrieved. /// - /// @param object_ids The IDs of the objects to get. - /// @param num_object_ids The number of object IDs to get. - /// @param timeout_ms The amount of time in milliseconds to wait before this + /// \param object_ids The IDs of the objects to get. + /// \param num_objects The number of object IDs to get. + /// \param timeout_ms The amount of time in milliseconds to wait before this /// request times out. If this value is -1, then no timeout is set. - /// @param object_buffers An array where the results will be stored. If the + /// \param object_buffers An array where the results will be stored. If the /// data /// size field is -1, then the object was not retrieved. - /// @return The return status. + /// \return The return status. Status Get(const ObjectID* object_ids, int64_t num_objects, int64_t timeout_ms, ObjectBuffer* object_buffers); @@ -136,8 +136,8 @@ class ARROW_EXPORT PlasmaClient { /// the address returned by Get is no longer valid. This should be called /// once for each call to Get (with the same object ID). /// - /// @param object_id The ID of the object that is no longer needed. - /// @return The return status. + /// \param object_id The ID of the object that is no longer needed. + /// \return The return status. Status Release(const ObjectID& object_id); /// Check if the object store contains a particular object and the object has @@ -146,18 +146,18 @@ class ARROW_EXPORT PlasmaClient { /// @todo: We may want to indicate if the object has been created but not /// sealed. /// - /// @param object_id The ID of the object whose presence we are checking. - /// @param has_object The function will write true at this address if + /// \param object_id The ID of the object whose presence we are checking. + /// \param has_object The function will write true at this address if /// the object is present and false if it is not present. - /// @return The return status. + /// \return The return status. Status Contains(const ObjectID& object_id, bool* has_object); /// Seal an object in the object store. The object will be immutable after /// this /// call. /// - /// @param object_id The ID of the object to seal. - /// @return The return status. + /// \param object_id The ID of the object to seal. + /// \return The return status. Status Seal(const ObjectID& object_id); /// Delete an object from the object store. This currently assumes that the @@ -166,52 +166,51 @@ class ARROW_EXPORT PlasmaClient { /// @todo We may want to allow the deletion of objects that are not present or /// haven't been sealed. /// - /// @param object_id The ID of the object to delete. - /// @return The return status. + /// \param object_id The ID of the object to delete. + /// \return The return status. Status Delete(const ObjectID& object_id); /// Delete objects until we have freed up num_bytes bytes or there are no more /// released objects that can be deleted. /// - /// @param num_bytes The number of bytes to try to free up. - /// @param num_bytes_evicted Out parameter for total number of bytes of space + /// \param num_bytes The number of bytes to try to free up. + /// \param num_bytes_evicted Out parameter for total number of bytes of space /// retrieved. - /// @return The return status. + /// \return The return status. Status Evict(int64_t num_bytes, int64_t& num_bytes_evicted); /// Compute the hash of an object in the object store. /// - /// @param conn The object containing the connection state. - /// @param object_id The ID of the object we want to hash. - /// @param digest A pointer at which to return the hash digest of the object. + /// \param object_id The ID of the object we want to hash. + /// \param digest A pointer at which to return the hash digest of the object. /// The pointer must have at least kDigestSize bytes allocated. - /// @return The return status. + /// \return The return status. Status Hash(const ObjectID& object_id, uint8_t* digest); /// Subscribe to notifications when objects are sealed in the object store. /// Whenever an object is sealed, a message will be written to the client /// socket that is returned by this method. /// - /// @param fd Out parameter for the file descriptor the client should use to + /// \param fd Out parameter for the file descriptor the client should use to /// read notifications /// from the object store about sealed objects. - /// @return The return status. + /// \return The return status. Status Subscribe(int* fd); /// Receive next object notification for this client if Subscribe has been called. /// - /// @param fd The file descriptor we are reading the notification from. - /// @param object_id Out parameter, the object_id of the object that was sealed. - /// @param data_size Out parameter, the data size of the object that was sealed. - /// @param metadata_size Out parameter, the metadata size of the object that was sealed. - /// @return The return status. + /// \param fd The file descriptor we are reading the notification from. + /// \param object_id Out parameter, the object_id of the object that was sealed. + /// \param data_size Out parameter, the data size of the object that was sealed. + /// \param metadata_size Out parameter, the metadata size of the object that was sealed. + /// \return The return status. Status GetNotification(int fd, ObjectID* object_id, int64_t* data_size, int64_t* metadata_size); /// Disconnect from the local plasma instance, including the local store and /// manager. /// - /// @return The return status. + /// \return The return status. Status Disconnect(); /// Attempt to initiate the transfer of some objects from remote Plasma @@ -236,17 +235,17 @@ class ARROW_EXPORT PlasmaClient { /// This method is idempotent in the sense that it is ok to call it multiple /// times. /// - /// @param num_object_ids The number of object IDs fetch is being called on. - /// @param object_ids The IDs of the objects that fetch is being called on. - /// @return The return status. + /// \param num_object_ids The number of object IDs fetch is being called on. + /// \param object_ids The IDs of the objects that fetch is being called on. + /// \return The return status. Status Fetch(int num_object_ids, const ObjectID* object_ids); /// Wait for (1) a specified number of objects to be available (sealed) in the /// local Plasma Store or in a remote Plasma Store, or (2) for a timeout to /// expire. This is a blocking call. /// - /// @param num_object_requests Size of the object_requests array. - /// @param object_requests Object event array. Each element contains a request + /// \param num_object_requests Size of the object_requests array. + /// \param object_requests Object event array. Each element contains a request /// for a particular object_id. The type of request is specified in the /// "type" field. /// - A PLASMA_QUERY_LOCAL request is satisfied when object_id becomes @@ -260,36 +259,34 @@ class ARROW_EXPORT PlasmaClient { /// available either at the local Plasma Store or on a remote Plasma /// Store. In this case, the functions sets the "status" field to /// ObjectStatus_Local or ObjectStatus_Remote. - /// @param num_ready_objects The number of requests in object_requests array + /// \param num_ready_objects The number of requests in object_requests array /// that /// must be satisfied before the function returns, unless it timeouts. /// The num_ready_objects should be no larger than num_object_requests. - /// @param timeout_ms Timeout value in milliseconds. If this timeout expires + /// \param timeout_ms Timeout value in milliseconds. If this timeout expires /// before min_num_ready_objects of requests are satisfied, the /// function /// returns. - /// @param num_objects_ready Out parameter for number of satisfied requests in + /// \param num_objects_ready Out parameter for number of satisfied requests in /// the object_requests list. If the returned number is less than /// min_num_ready_objects this means that timeout expired. - /// @return The return status. + /// \return The return status. Status Wait(int64_t num_object_requests, ObjectRequest* object_requests, int num_ready_objects, int64_t timeout_ms, int* num_objects_ready); /// Transfer local object to a different plasma manager. /// - /// @param conn The object containing the connection state. - /// @param addr IP address of the plasma manager we are transfering to. - /// @param port Port of the plasma manager we are transfering to. - /// @object_id ObjectID of the object we are transfering. - /// @return The return status. + /// \param addr IP address of the plasma manager we are transfering to. + /// \param port Port of the plasma manager we are transfering to. + /// \param object_id ObjectID of the object we are transfering. + /// \return The return status. Status Transfer(const char* addr, int port, const ObjectID& object_id); /// Return the status of a given object. This method may query the object /// table. /// - /// @param conn The object containing the connection state. - /// @param object_id The ID of the object whose status we query. - /// @param object_status Out parameter for object status. Can take the + /// \param object_id The ID of the object whose status we query. + /// \param object_status Out parameter for object status. Can take the /// following values. /// - PLASMA_CLIENT_LOCAL, if object is stored in the local Plasma /// Store. @@ -300,13 +297,12 @@ class ARROW_EXPORT PlasmaClient { /// Plasma Store. /// - PLASMA_CLIENT_DOES_NOT_EXIST, if the object doesn’t exist in the /// system. - /// @return The return status. + /// \return The return status. Status Info(const ObjectID& object_id, int* object_status); /// Get the file descriptor for the socket connection to the plasma manager. /// - /// @param conn The plasma connection. - /// @return The file descriptor for the manager connection. If there is no + /// \return The file descriptor for the manager connection. If there is no /// connection to the manager, this is -1. int get_manager_fd(); diff --git a/cpp/src/plasma/events.h b/cpp/src/plasma/events.h index 42914848645..419c2ebfba7 100644 --- a/cpp/src/plasma/events.h +++ b/cpp/src/plasma/events.h @@ -66,7 +66,6 @@ class EventLoop { /// Remove a file event handler from the event loop. /// /// @param fd The file descriptor of the event handler. - /// @return Void. void RemoveFileEvent(int fd); /// Register a handler that will be called after a time slice of @@ -84,8 +83,6 @@ class EventLoop { int RemoveTimer(int64_t timer_id); /// \brief Run the event loop. - /// - /// @return Void. void Start(); /// \brief Stop the event loop diff --git a/cpp/src/plasma/eviction_policy.h b/cpp/src/plasma/eviction_policy.h index dd1c873466e..de33dabcbaf 100644 --- a/cpp/src/plasma/eviction_policy.h +++ b/cpp/src/plasma/eviction_policy.h @@ -70,7 +70,6 @@ class EvictionPolicy { /// cache. /// /// @param object_id The object ID of the object that was created. - /// @return Void. void object_created(const ObjectID& object_id); /// This method will be called when the Plasma store needs more space, perhaps @@ -94,7 +93,6 @@ class EvictionPolicy { /// @param object_id The ID of the object that is now being used. /// @param objects_to_evict The object IDs that were chosen for eviction will /// be stored into this vector. - /// @return Void. void begin_object_access(const ObjectID& object_id, std::vector* objects_to_evict); @@ -106,7 +104,6 @@ class EvictionPolicy { /// @param object_id The ID of the object that is no longer being used. /// @param objects_to_evict The object IDs that were chosen for eviction will /// be stored into this vector. - /// @return Void. void end_object_access(const ObjectID& object_id, std::vector* objects_to_evict); diff --git a/cpp/src/plasma/fling.cc b/cpp/src/plasma/fling.cc index 79da4f43a19..14db32085c0 100644 --- a/cpp/src/plasma/fling.cc +++ b/cpp/src/plasma/fling.cc @@ -64,8 +64,9 @@ int recv_fd(int conn) { for (struct cmsghdr* header = CMSG_FIRSTHDR(&msg); header != NULL; header = CMSG_NXTHDR(&msg, header)) if (header->cmsg_level == SOL_SOCKET && header->cmsg_type == SCM_RIGHTS) { - ssize_t count = - (header->cmsg_len - (CMSG_DATA(header) - (unsigned char*)header)) / sizeof(int); + ssize_t count = (header->cmsg_len - + (CMSG_DATA(header) - reinterpret_cast(header))) / + sizeof(int); for (int i = 0; i < count; ++i) { int fd = (reinterpret_cast(CMSG_DATA(header)))[i]; if (found_fd == -1) { diff --git a/cpp/src/plasma/io.cc b/cpp/src/plasma/io.cc index 9bb43399082..fc0010b0631 100644 --- a/cpp/src/plasma/io.cc +++ b/cpp/src/plasma/io.cc @@ -26,10 +26,6 @@ using arrow::Status; -/* Number of times we try binding to a socket. */ -#define NUM_BIND_ATTEMPTS 5 -#define BIND_TIMEOUT_MS 100 - /* Number of times we try connecting to a socket. */ #define NUM_CONNECT_ATTEMPTS 50 #define CONNECT_TIMEOUT_MS 100 @@ -134,7 +130,8 @@ int bind_ipc_sock(const std::string& pathname, bool shall_listen) { } strncpy(socket_address.sun_path, pathname.c_str(), pathname.size() + 1); - if (bind(socket_fd, (struct sockaddr*)&socket_address, sizeof(socket_address)) != 0) { + if (bind(socket_fd, reinterpret_cast(&socket_address), + sizeof(socket_address)) != 0) { ARROW_LOG(ERROR) << "Bind failed for pathname " << pathname; close(socket_fd); return -1; @@ -197,8 +194,8 @@ int connect_ipc_sock(const std::string& pathname) { } strncpy(socket_address.sun_path, pathname.c_str(), pathname.size() + 1); - if (connect(socket_fd, (struct sockaddr*)&socket_address, sizeof(socket_address)) != - 0) { + if (connect(socket_fd, reinterpret_cast(&socket_address), + sizeof(socket_address)) != 0) { close(socket_fd); return -1; } diff --git a/cpp/src/plasma/protocol.cc b/cpp/src/plasma/protocol.cc index 305e3d5d9ce..2261b6a624a 100644 --- a/cpp/src/plasma/protocol.cc +++ b/cpp/src/plasma/protocol.cc @@ -79,8 +79,9 @@ Status SendCreateReply(int sock, ObjectID object_id, PlasmaObject* object, PlasmaObjectSpec plasma_object(object->handle.store_fd, object->handle.mmap_size, object->data_offset, object->data_size, object->metadata_offset, object->metadata_size); - auto message = CreatePlasmaCreateReply(fbb, fbb.CreateString(object_id.binary()), - &plasma_object, (PlasmaError)error_code); + auto message = + CreatePlasmaCreateReply(fbb, fbb.CreateString(object_id.binary()), &plasma_object, + static_cast(error_code)); return PlasmaSend(sock, MessageType_PlasmaCreateReply, &fbb, message); } @@ -123,7 +124,7 @@ Status ReadSealRequest(uint8_t* data, size_t size, ObjectID* object_id, Status SendSealReply(int sock, ObjectID object_id, int error) { flatbuffers::FlatBufferBuilder fbb; auto message = CreatePlasmaSealReply(fbb, fbb.CreateString(object_id.binary()), - (PlasmaError)error); + static_cast(error)); return PlasmaSend(sock, MessageType_PlasmaSealReply, &fbb, message); } @@ -154,7 +155,7 @@ Status ReadReleaseRequest(uint8_t* data, size_t size, ObjectID* object_id) { Status SendReleaseReply(int sock, ObjectID object_id, int error) { flatbuffers::FlatBufferBuilder fbb; auto message = CreatePlasmaReleaseReply(fbb, fbb.CreateString(object_id.binary()), - (PlasmaError)error); + static_cast(error)); return PlasmaSend(sock, MessageType_PlasmaReleaseReply, &fbb, message); } @@ -185,7 +186,7 @@ Status ReadDeleteRequest(uint8_t* data, size_t size, ObjectID* object_id) { Status SendDeleteReply(int sock, ObjectID object_id, int error) { flatbuffers::FlatBufferBuilder fbb; auto message = CreatePlasmaDeleteReply(fbb, fbb.CreateString(object_id.binary()), - (PlasmaError)error); + static_cast(error)); return PlasmaSend(sock, MessageType_PlasmaDeleteReply, &fbb, message); } @@ -526,8 +527,8 @@ Status ReadDataReply(uint8_t* data, size_t size, ObjectID* object_id, auto message = flatbuffers::GetRoot(data); DCHECK(verify_flatbuffer(message, data, size)); *object_id = ObjectID::from_binary(message->object_id()->str()); - *object_size = (int64_t)message->object_size(); - *metadata_size = (int64_t)message->metadata_size(); + *object_size = static_cast(message->object_size()); + *metadata_size = static_cast(message->metadata_size()); return Status::OK(); } diff --git a/cpp/src/plasma/store.cc b/cpp/src/plasma/store.cc index aaa2bad67c3..72d199ba465 100644 --- a/cpp/src/plasma/store.cc +++ b/cpp/src/plasma/store.cc @@ -458,8 +458,7 @@ void PlasmaStore::disconnect_client(int client_fd) { /// be /// buffered, and this will be called again when the send buffer has room. /// -/// @param client The client to send the notification to. -/// @return Void. +/// @param client_fd The client to send the notification to. void PlasmaStore::send_notifications(int client_fd) { auto it = pending_notifications_.find(client_fd); diff --git a/cpp/src/plasma/store.h b/cpp/src/plasma/store.h index 61a3a245610..d03d11f4ef0 100644 --- a/cpp/src/plasma/store.h +++ b/cpp/src/plasma/store.h @@ -77,7 +77,6 @@ class PlasmaStore { /// be called on objects that are returned by the eviction policy to evict. /// /// @param object_ids Object IDs of the objects to be deleted. - /// @return Void. void delete_objects(const std::vector& object_ids); /// Process a get request from a client. This method assumes that we will @@ -91,7 +90,6 @@ class PlasmaStore { /// @param client The client making this request. /// @param object_ids Object IDs of the objects to be gotten. /// @param timeout_ms The timeout for the get request in milliseconds. - /// @return Void. void process_get_request(Client* client, const std::vector& object_ids, int64_t timeout_ms); @@ -101,7 +99,6 @@ class PlasmaStore { /// @param digest The digest of the object. This is used to tell if two /// objects /// with the same object ID are the same. - /// @return Void. void seal_object(const ObjectID& object_id, unsigned char digest[]); /// Check if the plasma store contains an object: @@ -115,25 +112,21 @@ class PlasmaStore { /// /// @param object_id The object ID of the object that is being released. /// @param client The client making this request. - /// @param Void. void release_object(const ObjectID& object_id, Client* client); /// Subscribe a file descriptor to updates about new sealed objects. /// /// @param client The client making this request. - /// @return Void. void subscribe_to_updates(Client* client); /// Connect a new client to the PlasmaStore. /// /// @param listener_sock The socket that is listening to incoming connections. - /// @return Void. void connect_client(int listener_sock); /// Disconnect a client from the PlasmaStore. /// /// @param client_fd The client file descriptor that is disconnected. - /// @return Void. void disconnect_client(int client_fd); void send_notifications(int client_fd); diff --git a/cpp/src/plasma/test/serialization_tests.cc b/cpp/src/plasma/test/serialization_tests.cc index c76f5ce1092..7c9d90133a6 100644 --- a/cpp/src/plasma/test/serialization_tests.cc +++ b/cpp/src/plasma/test/serialization_tests.cc @@ -43,7 +43,7 @@ int create_temp_file(void) { * Seek to the beginning of a file and read a message from it. * * @param fd File descriptor of the file. - * @param message type Message type that we expect in the file. + * @param message_type Message type that we expect in the file. * * @return Pointer to the content of the message. Needs to be freed by the * caller. @@ -226,7 +226,7 @@ TEST(PlasmaSerialization, DeleteReply) { TEST(PlasmaSerialization, StatusRequest) { int fd = create_temp_file(); - int64_t num_objects = 2; + constexpr int64_t num_objects = 2; ObjectID object_ids[num_objects]; object_ids[0] = ObjectID::from_random(); object_ids[1] = ObjectID::from_random(); @@ -249,10 +249,11 @@ TEST(PlasmaSerialization, StatusReply) { ARROW_CHECK_OK(SendStatusReply(fd, object_ids, object_statuses, 2)); std::vector data = read_message_from_file(fd, MessageType_PlasmaStatusReply); int64_t num_objects = ReadStatusReply_num_objects(data.data(), data.size()); - ObjectID object_ids_read[num_objects]; - int object_statuses_read[num_objects]; - ARROW_CHECK_OK(ReadStatusReply(data.data(), data.size(), object_ids_read, - object_statuses_read, num_objects)); + + std::vector object_ids_read(num_objects); + std::vector object_statuses_read(num_objects); + ARROW_CHECK_OK(ReadStatusReply(data.data(), data.size(), object_ids_read.data(), + object_statuses_read.data(), num_objects)); ASSERT_EQ(object_ids[0], object_ids_read[0]); ASSERT_EQ(object_ids[1], object_ids_read[1]); ASSERT_EQ(object_statuses[0], object_statuses_read[0]); diff --git a/cpp/src/plasma/thirdparty/ae/ae_epoll.c b/cpp/src/plasma/thirdparty/ae/ae_epoll.c index 410aac70dc5..2f70550a980 100644 --- a/cpp/src/plasma/thirdparty/ae/ae_epoll.c +++ b/cpp/src/plasma/thirdparty/ae/ae_epoll.c @@ -72,7 +72,8 @@ static void aeApiFree(aeEventLoop *eventLoop) { static int aeApiAddEvent(aeEventLoop *eventLoop, int fd, int mask) { aeApiState *state = eventLoop->apidata; - struct epoll_event ee = {0}; /* avoid valgrind warning */ + struct epoll_event ee; + memset(&ee, 0, sizeof(struct epoll_event)); // avoid valgrind warning /* If the fd was already monitored for some event, we need a MOD * operation. Otherwise we need an ADD operation. */ int op = eventLoop->events[fd].mask == AE_NONE ? @@ -89,7 +90,8 @@ static int aeApiAddEvent(aeEventLoop *eventLoop, int fd, int mask) { static void aeApiDelEvent(aeEventLoop *eventLoop, int fd, int delmask) { aeApiState *state = eventLoop->apidata; - struct epoll_event ee = {0}; /* avoid valgrind warning */ + struct epoll_event ee; + memset(&ee, 0, sizeof(struct epoll_event)); // avoid valgrind warning int mask = eventLoop->events[fd].mask & (~delmask); ee.events = 0;