From 5d129991b3369b0e45cb79d1efe6ba2fd8dd21d0 Mon Sep 17 00:00:00 2001 From: Micah Kornfield Date: Fri, 1 Apr 2016 21:40:20 -0700 Subject: [PATCH] ARROW-71: [C++] Add clang-tidy and clang-format to the the tool chain. I changed the ubuntu flavor for building to precise because https://github.com/travis-ci/apt-source-whitelist/issues/199 is currently blocking using trusty. I also expect there might be a couple of iterations on settings for clang-format and clang-tidy (or if we even want them as standard parts of the toolchain). @wesm I noticed the lint target explicitly turns off some checks, I don't know if these were copy and pasted or you really don't like them. If the latter I can do a first pass of turning the same ones off for clang-tidy. In terms of reviewing: It is likely useful, to look at the PR commit by commit, since the last two commits are 99% driven by the first commit. The main chunk of code that wasn't machine fixed is FatalLog in logging. The good news is clang-tidy caught one potential corner case segfault when a column happened to be null :) Author: Micah Kornfield Closes #55 from emkornfield/emk_add_clang_tidy_PR and squashes the following commits: 2fafb10 [Micah Kornfield] adjust line length from 88 to 90, turn on bin packing of parameters. increase penality for before first call parameter 169352f [Micah Kornfield] add llvm tool chain as travis source e7723d1 [Micah Kornfield] upgrade to precise to verify if build works. address self comments d3f76d8 [Micah Kornfield] clang format change 9c556ef [Micah Kornfield] cleanup from clang-tidy 26945e9 [Micah Kornfield] add more failure checks for build_thirdparty 4dd0b81 [Micah Kornfield] Add clang-format and clang-tidy targets to toolchain --- .travis.yml | 5 +- ci/travis_script_cpp.sh | 4 + cpp/CMakeLists.txt | 39 +++++- cpp/README.md | 16 +++ cpp/build-support/run-clang-format.sh | 42 ++++++ cpp/build-support/run-clang-tidy.sh | 40 ++++++ cpp/cmake_modules/FindClangTools.cmake | 60 +++++++++ cpp/src/.clang-format | 65 ++++++++++ cpp/src/.clang-tidy | 14 ++ cpp/src/arrow/api.h | 2 +- cpp/src/arrow/array-test.cc | 16 +-- cpp/src/arrow/array.cc | 17 +-- cpp/src/arrow/array.h | 28 ++-- cpp/src/arrow/builder.cc | 2 +- cpp/src/arrow/builder.h | 40 +++--- cpp/src/arrow/column-benchmark.cc | 23 ++-- cpp/src/arrow/column-test.cc | 2 +- cpp/src/arrow/column.cc | 27 ++-- cpp/src/arrow/column.h | 41 ++---- cpp/src/arrow/ipc/adapter.cc | 50 ++++---- cpp/src/arrow/ipc/adapter.h | 18 +-- cpp/src/arrow/ipc/ipc-adapter-test.cc | 20 ++- cpp/src/arrow/ipc/ipc-memory-test.cc | 15 +-- cpp/src/arrow/ipc/ipc-metadata-test.cc | 8 +- cpp/src/arrow/ipc/memory.cc | 46 +++---- cpp/src/arrow/ipc/memory.h | 22 ++-- cpp/src/arrow/ipc/metadata-internal.cc | 70 +++++----- cpp/src/arrow/ipc/metadata-internal.h | 12 +- cpp/src/arrow/ipc/metadata.cc | 72 ++++------- cpp/src/arrow/ipc/metadata.h | 20 +-- cpp/src/arrow/ipc/test-common.h | 10 +- cpp/src/arrow/parquet/parquet-schema-test.cc | 63 ++++----- cpp/src/arrow/parquet/schema.cc | 15 +-- cpp/src/arrow/parquet/schema.h | 11 +- cpp/src/arrow/schema-test.cc | 6 +- cpp/src/arrow/schema.cc | 20 +-- cpp/src/arrow/schema.h | 10 +- cpp/src/arrow/table-test.cc | 16 +-- cpp/src/arrow/table.cc | 31 ++--- cpp/src/arrow/table.h | 38 ++---- cpp/src/arrow/test-util.h | 58 ++++----- cpp/src/arrow/type.cc | 8 +- cpp/src/arrow/type.h | 94 +++++--------- cpp/src/arrow/types/binary.h | 6 +- cpp/src/arrow/types/collection.h | 12 +- cpp/src/arrow/types/construct.cc | 42 +++--- cpp/src/arrow/types/construct.h | 11 +- cpp/src/arrow/types/datetime.h | 39 ++---- cpp/src/arrow/types/decimal-test.cc | 2 +- cpp/src/arrow/types/decimal.cc | 3 +- cpp/src/arrow/types/decimal.h | 11 +- cpp/src/arrow/types/json.cc | 5 +- cpp/src/arrow/types/json.h | 8 +- cpp/src/arrow/types/list-test.cc | 11 +- cpp/src/arrow/types/list.cc | 25 ++-- cpp/src/arrow/types/list.h | 65 ++++------ cpp/src/arrow/types/primitive-test.cc | 107 +++++++--------- cpp/src/arrow/types/primitive.cc | 75 ++++------- cpp/src/arrow/types/primitive.h | 128 +++++++------------ cpp/src/arrow/types/string-test.cc | 20 +-- cpp/src/arrow/types/string.cc | 10 +- cpp/src/arrow/types/string.h | 48 +++---- cpp/src/arrow/types/struct-test.cc | 4 +- cpp/src/arrow/types/struct.cc | 4 +- cpp/src/arrow/types/struct.h | 6 +- cpp/src/arrow/types/test-common.h | 9 +- cpp/src/arrow/types/union.cc | 6 +- cpp/src/arrow/types/union.h | 17 +-- cpp/src/arrow/util/bit-util-test.cc | 2 +- cpp/src/arrow/util/bit-util.cc | 10 +- cpp/src/arrow/util/bit-util.h | 6 +- cpp/src/arrow/util/buffer-test.cc | 5 +- cpp/src/arrow/util/buffer.cc | 16 +-- cpp/src/arrow/util/buffer.h | 61 +++------ cpp/src/arrow/util/logging.h | 78 +++++++---- cpp/src/arrow/util/macros.h | 6 +- cpp/src/arrow/util/memory-pool-test.cc | 2 +- cpp/src/arrow/util/memory-pool.cc | 2 +- cpp/src/arrow/util/memory-pool.h | 4 +- cpp/src/arrow/util/random.h | 27 ++-- cpp/src/arrow/util/status.cc | 10 +- cpp/src/arrow/util/status.h | 45 ++++--- cpp/src/arrow/util/test_main.cc | 2 +- cpp/thirdparty/build_thirdparty.sh | 4 +- 84 files changed, 1015 insertions(+), 1155 deletions(-) create mode 100755 cpp/build-support/run-clang-format.sh create mode 100755 cpp/build-support/run-clang-tidy.sh create mode 100644 cpp/cmake_modules/FindClangTools.cmake create mode 100644 cpp/src/.clang-format create mode 100644 cpp/src/.clang-tidy diff --git a/.travis.yml b/.travis.yml index d89a200b892e6..a0138a79598a1 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,11 +1,14 @@ sudo: required -dist: trusty +dist: precise addons: apt: sources: - ubuntu-toolchain-r-test - kalakris-cmake + - llvm-toolchain-precise-3.7 packages: + - clang-format-3.7 + - clang-tidy-3.7 - gcc-4.9 # Needed for C++11 - g++-4.9 # Needed for C++11 - gdb diff --git a/ci/travis_script_cpp.sh b/ci/travis_script_cpp.sh index 997bdf35e83d2..c9b3b5f1442a1 100755 --- a/ci/travis_script_cpp.sh +++ b/ci/travis_script_cpp.sh @@ -7,6 +7,10 @@ set -e pushd $CPP_BUILD_DIR make lint +if [ $TRAVIS_OS_NAME == "linux" ]; then + make check-format + make check-clang-tidy +fi ctest -L unittest diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt index 26d12d2424796..f803c0fb3e428 100644 --- a/cpp/CMakeLists.txt +++ b/cpp/CMakeLists.txt @@ -30,10 +30,11 @@ set(THIRDPARTY_DIR "${CMAKE_SOURCE_DIR}/thirdparty") # Must be declared in the top-level CMakeLists.txt. set(CMAKE_SKIP_INSTALL_ALL_DEPENDENCY true) -# Generate a Clang compile_commands.json "compilation database" file for use -# with various development tools, such as Vim's YouCompleteMe plugin. -# See http://clang.llvm.org/docs/JSONCompilationDatabase.html -if ("$ENV{CMAKE_EXPORT_COMPILE_COMMANDS}" STREQUAL "1") +find_package(ClangTools) +if ("$ENV{CMAKE_EXPORT_COMPILE_COMMANDS}" STREQUAL "1" OR CLANG_TIDY_FOUND) + # Generate a Clang compile_commands.json "compilation database" file for use + # with various development tools, such as Vim's YouCompleteMe plugin. + # See http://clang.llvm.org/docs/JSONCompilationDatabase.html set(CMAKE_EXPORT_COMPILE_COMMANDS 1) endif() @@ -540,6 +541,36 @@ if (UNIX) `find ${CMAKE_CURRENT_SOURCE_DIR}/src -name \\*.cc -or -name \\*.h | sed -e '/_generated/g'`) endif (UNIX) + +############################################################ +# "make format" and "make check-format" targets +############################################################ +if (${CLANG_FORMAT_FOUND}) + # runs clang format and updates files in place. + add_custom_target(format ${BUILD_SUPPORT_DIR}/run-clang-format.sh ${CMAKE_CURRENT_SOURCE_DIR} ${CLANG_FORMAT_BIN} 1 + `find ${CMAKE_CURRENT_SOURCE_DIR}/src -name \\*.cc -or -name \\*.h | sed -e '/_generated/g'`) + + # runs clang format and exits with a non-zero exit code if any files need to be reformatted + add_custom_target(check-format ${BUILD_SUPPORT_DIR}/run-clang-format.sh ${CMAKE_CURRENT_SOURCE_DIR} ${CLANG_FORMAT_BIN} 0 + `find ${CMAKE_CURRENT_SOURCE_DIR}/src -name \\*.cc -or -name \\*.h | sed -e '/_generated/g'`) +endif() + + +############################################################ +# "make clang-tidy" and "make check-clang-tidy" targets +############################################################ +if (${CLANG_TIDY_FOUND}) + # runs clang-tidy and attempts to fix any warning automatically + add_custom_target(clang-tidy ${BUILD_SUPPORT_DIR}/run-clang-tidy.sh ${CLANG_TIDY_BIN} ${CMAKE_BINARY_DIR}/compile_commands.json 1 + `find ${CMAKE_CURRENT_SOURCE_DIR}/src -name \\*.cc | sed -e '/_generated/g'`) + # runs clang-tidy and exits with a non-zero exit code if any errors are found. + add_custom_target(check-clang-tidy ${BUILD_SUPPORT_DIR}/run-clang-tidy.sh ${CLANG_TIDY_BIN} ${CMAKE_BINARY_DIR}/compile_commands.json + 0 `find ${CMAKE_CURRENT_SOURCE_DIR}/src -name \\*.cc | sed -e '/_generated/g'`) + +endif() + + + ############################################################ # Subdirectories ############################################################ diff --git a/cpp/README.md b/cpp/README.md index 9026cf963f8ee..3f5da21b7d417 100644 --- a/cpp/README.md +++ b/cpp/README.md @@ -61,3 +61,19 @@ variables * Googletest: `GTEST_HOME` (only required to build the unit tests) * Google Benchmark: `GBENCHMARK_HOME` (only required if building benchmarks) * Flatbuffers: `FLATBUFFERS_HOME` (only required for the IPC extensions) + +## Continuous Integration + +Pull requests are run through travis-ci for continuous integration. You can avoid +build failures by running the following checks before submitting your pull request: + + make unittest + make lint + # The next two commands may change your code. It is recommended you commit + # before running them. + make clang-tidy # requires clang-tidy is installed + make format # requires clang-format is installed + +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. diff --git a/cpp/build-support/run-clang-format.sh b/cpp/build-support/run-clang-format.sh new file mode 100755 index 0000000000000..ba525dfc33c69 --- /dev/null +++ b/cpp/build-support/run-clang-format.sh @@ -0,0 +1,42 @@ +#!/bin/bash +# +# 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. +# +# Runs clang format in the given directory +# Arguments: +# $1 - Path to the source tree +# $2 - Path to the clang format binary +# $3 - Apply fixes (will raise an error if false and not there where changes) +# $ARGN - Files to run clang format on +# +SOURCE_DIR=$1 +shift +CLANG_FORMAT=$1 +shift +APPLY_FIXES=$1 +shift + +# clang format will only find its configuration if we are in +# the source tree or in a path relative to the source tree +pushd $SOURCE_DIR +if [ "$APPLY_FIXES" == "1" ]; then + $CLANG_FORMAT -i $@ +else + + NUM_CORRECTIONS=`$CLANG_FORMAT -output-replacements-xml $@ | grep offset | wc -l` + if [ "$NUM_CORRECTIONS" -gt "0" ]; then + echo "clang-format suggested changes, please run 'make format'!!!!" + exit 1 + fi +fi +popd diff --git a/cpp/build-support/run-clang-tidy.sh b/cpp/build-support/run-clang-tidy.sh new file mode 100755 index 0000000000000..4ba8ab8cd766d --- /dev/null +++ b/cpp/build-support/run-clang-tidy.sh @@ -0,0 +1,40 @@ +#!/bin/bash +# +# 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. +# +# +# Runs clang format in the given directory +# Arguments: +# $1 - Path to the clang tidy binary +# $2 - Path to the compile_commands.json to use +# $3 - Apply fixes (will raise an error if false and not there where changes) +# $ARGN - Files to run clang-tidy on +# +CLANG_TIDY=$1 +shift +COMPILE_COMMANDS=$1 +shift +APPLY_FIXES=$1 +shift + +# clang format will only find its configuration if we are in +# the source tree or in a path relative to the source tree +if [ "$APPLY_FIXES" == "1" ]; then + $CLANG_TIDY -p $COMPILE_COMMANDS -fix $@ +else + NUM_CORRECTIONS=`$CLANG_TIDY -p $COMPILE_COMMANDS $@ 2>&1 | grep -v Skipping | grep "warnings* generated" | wc -l` + if [ "$NUM_CORRECTIONS" -gt "0" ]; then + echo "clang-tidy had suggested fixes. Please fix these!!!" + exit 1 + fi +fi diff --git a/cpp/cmake_modules/FindClangTools.cmake b/cpp/cmake_modules/FindClangTools.cmake new file mode 100644 index 0000000000000..c07c7d244493e --- /dev/null +++ b/cpp/cmake_modules/FindClangTools.cmake @@ -0,0 +1,60 @@ +# +# 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. +# +# Tries to find the clang-tidy and clang-format modules +# +# Usage of this module as follows: +# +# find_package(ClangTools) +# +# Variables used by this module, they can change the default behaviour and need +# to be set before calling find_package: +# +# ClangToolsBin_HOME - +# When set, this path is inspected instead of standard library binary locations +# to find clang-tidy and clang-format +# +# This module defines +# CLANG_TIDY_BIN, The path to the clang tidy binary +# CLANG_TIDY_FOUND, Whether clang tidy was found +# CLANG_FORMAT_BIN, The path to the clang format binary +# CLANG_TIDY_FOUND, Whether clang format was found + +find_program(CLANG_TIDY_BIN + NAMES clang-tidy-3.8 clang-tidy-3.7 clang-tidy-3.6 clang-tidy + PATHS ${ClangTools_PATH} $ENV{CLANG_TOOLS_PATH} /usr/local/bin /usr/bin + NO_DEFAULT_PATH +) + +if ( "${CLANG_TIDY_BIN}" STREQUAL "CLANG_TIDY_BIN-NOTFOUND" ) + set(CLANG_TIDY_FOUND 0) + message("clang-tidy not found") +else() + set(CLANG_TIDY_FOUND 1) + message("clang-tidy found at ${CLANG_TIDY_BIN}") +endif() + +find_program(CLANG_FORMAT_BIN + NAMES clang-format-3.8 clang-format-3.7 clang-format-3.6 clang-format + PATHS ${ClangTools_PATH} $ENV{CLANG_TOOLS_PATH} /usr/local/bin /usr/bin + NO_DEFAULT_PATH +) + +if ( "${CLANG_FORMAT_BIN}" STREQUAL "CLANG_FORMAT_BIN-NOTFOUND" ) + set(CLANG_FORMAT_FOUND 0) + message("clang-format not found") +else() + set(CLANG_FORMAT_FOUND 1) + message("clang-format found at ${CLANG_FORMAT_BIN}") +endif() + diff --git a/cpp/src/.clang-format b/cpp/src/.clang-format new file mode 100644 index 0000000000000..7d5b3cf30ef51 --- /dev/null +++ b/cpp/src/.clang-format @@ -0,0 +1,65 @@ +--- +Language: Cpp +# BasedOnStyle: Google +AccessModifierOffset: -1 +AlignAfterOpenBracket: false +AlignConsecutiveAssignments: false +AlignEscapedNewlinesLeft: true +AlignOperands: true +AlignTrailingComments: true +AllowAllParametersOfDeclarationOnNextLine: true +AllowShortBlocksOnASingleLine: true +AllowShortCaseLabelsOnASingleLine: false +AllowShortFunctionsOnASingleLine: Inline +AllowShortIfStatementsOnASingleLine: true +AllowShortLoopsOnASingleLine: false +AlwaysBreakAfterDefinitionReturnType: None +AlwaysBreakBeforeMultilineStrings: true +AlwaysBreakTemplateDeclarations: true +BinPackArguments: true +BinPackParameters: true +BreakBeforeBinaryOperators: None +BreakBeforeBraces: Attach +BreakBeforeTernaryOperators: true +BreakConstructorInitializersBeforeComma: false +ColumnLimit: 90 +CommentPragmas: '^ IWYU pragma:' +ConstructorInitializerAllOnOneLineOrOnePerLine: true +ConstructorInitializerIndentWidth: 4 +ContinuationIndentWidth: 4 +Cpp11BracedListStyle: true +DerivePointerAlignment: false +DisableFormat: false +ExperimentalAutoDetectBinPacking: false +ForEachMacros: [ foreach, Q_FOREACH, BOOST_FOREACH ] +IndentCaseLabels: true +IndentWidth: 2 +IndentWrappedFunctionNames: false +KeepEmptyLinesAtTheStartOfBlocks: false +MacroBlockBegin: '' +MacroBlockEnd: '' +MaxEmptyLinesToKeep: 1 +NamespaceIndentation: None +ObjCBlockIndentWidth: 2 +ObjCSpaceAfterProperty: false +ObjCSpaceBeforeProtocolList: false +PenaltyBreakBeforeFirstCallParameter: 1000 +PenaltyBreakComment: 300 +PenaltyBreakFirstLessLess: 120 +PenaltyBreakString: 1000 +PenaltyExcessCharacter: 1000000 +PenaltyReturnTypeOnItsOwnLine: 200 +PointerAlignment: Left +SpaceAfterCStyleCast: false +SpaceBeforeAssignmentOperators: true +SpaceBeforeParens: ControlStatements +SpaceInEmptyParentheses: false +SpacesBeforeTrailingComments: 2 +SpacesInAngles: false +SpacesInContainerLiterals: true +SpacesInCStyleCastParentheses: false +SpacesInParentheses: false +SpacesInSquareBrackets: false +Standard: Cpp11 +TabWidth: 8 +UseTab: Never diff --git a/cpp/src/.clang-tidy b/cpp/src/.clang-tidy new file mode 100644 index 0000000000000..deaa9bdf97fa1 --- /dev/null +++ b/cpp/src/.clang-tidy @@ -0,0 +1,14 @@ +--- +Checks: 'clang-diagnostic-*,clang-analyzer-*,-clang-analyzer-alpha*,google-.*,modernize-.*,readablity-.*' +HeaderFilterRegex: 'arrow/.*' +AnalyzeTemporaryDtors: true +CheckOptions: + - key: google-readability-braces-around-statements.ShortStatementLines + value: '1' + - key: google-readability-function-size.StatementThreshold + value: '800' + - key: google-readability-namespace-comments.ShortNamespaceLines + value: '10' + - key: google-readability-namespace-comments.SpacesBeforeComments + value: '2' + diff --git a/cpp/src/arrow/api.h b/cpp/src/arrow/api.h index 2ae80f642f29d..2d317b49cb7b6 100644 --- a/cpp/src/arrow/api.h +++ b/cpp/src/arrow/api.h @@ -37,4 +37,4 @@ #include "arrow/util/memory-pool.h" #include "arrow/util/status.h" -#endif // ARROW_API_H +#endif // ARROW_API_H diff --git a/cpp/src/arrow/array-test.cc b/cpp/src/arrow/array-test.cc index 121b802d994fa..b4c727997ee7e 100644 --- a/cpp/src/arrow/array-test.cc +++ b/cpp/src/arrow/array-test.cc @@ -33,15 +33,12 @@ namespace arrow { class TestArray : public ::testing::Test { public: - void SetUp() { - pool_ = default_memory_pool(); - } + void SetUp() { pool_ = default_memory_pool(); } protected: MemoryPool* pool_; }; - TEST_F(TestArray, TestNullCount) { auto data = std::make_shared(pool_); auto null_bitmap = std::make_shared(pool_); @@ -53,7 +50,6 @@ TEST_F(TestArray, TestNullCount) { ASSERT_EQ(0, arr_no_nulls->null_count()); } - TEST_F(TestArray, TestLength) { auto data = std::make_shared(pool_); std::unique_ptr arr(new Int32Array(100, data)); @@ -61,14 +57,16 @@ TEST_F(TestArray, TestLength) { } TEST_F(TestArray, TestIsNull) { + // clang-format off std::vector null_bitmap = {1, 0, 1, 1, 0, 1, 0, 0, 1, 0, 1, 1, 0, 1, 0, 0, 1, 0, 1, 1, 0, 1, 0, 0, 1, 0, 1, 1, 0, 1, 0, 0, 1, 0, 0, 1}; + // clang-format on int32_t null_count = 0; for (uint8_t x : null_bitmap) { - if (x == 0) ++null_count; + if (x == 0) { ++null_count; } } std::shared_ptr null_buf = test::bytes_to_null_buffer(null_bitmap); @@ -85,8 +83,6 @@ TEST_F(TestArray, TestIsNull) { } } +TEST_F(TestArray, TestCopy) {} -TEST_F(TestArray, TestCopy) { -} - -} // namespace arrow +} // namespace arrow diff --git a/cpp/src/arrow/array.cc b/cpp/src/arrow/array.cc index 3736732740b5b..a1536861a20be 100644 --- a/cpp/src/arrow/array.cc +++ b/cpp/src/arrow/array.cc @@ -32,30 +32,25 @@ Array::Array(const TypePtr& type, int32_t length, int32_t null_count, length_ = length; null_count_ = null_count; null_bitmap_ = null_bitmap; - if (null_bitmap_) { - null_bitmap_data_ = null_bitmap_->data(); - } + if (null_bitmap_) { null_bitmap_data_ = null_bitmap_->data(); } } bool Array::EqualsExact(const Array& other) const { - if (this == &other) return true; + if (this == &other) { return true; } if (length_ != other.length_ || null_count_ != other.null_count_ || type_enum() != other.type_enum()) { return false; } if (null_count_ > 0) { return null_bitmap_->Equals(*other.null_bitmap_, util::bytes_for_bits(length_)); - } else { - return true; } + return true; } bool NullArray::Equals(const std::shared_ptr& arr) const { - if (this == arr.get()) return true; - if (Type::NA != arr->type_enum()) { - return false; - } + if (this == arr.get()) { return true; } + if (Type::NA != arr->type_enum()) { return false; } return arr->length() == length_; } -} // namespace arrow +} // namespace arrow diff --git a/cpp/src/arrow/array.h b/cpp/src/arrow/array.h index 097634d74f890..c6735f87d8f42 100644 --- a/cpp/src/arrow/array.h +++ b/cpp/src/arrow/array.h @@ -36,8 +36,7 @@ class Buffer; // count is greater than 0 class Array { public: - Array(const std::shared_ptr& type, int32_t length, - int32_t null_count = 0, + Array(const std::shared_ptr& type, int32_t length, int32_t null_count = 0, const std::shared_ptr& null_bitmap = nullptr); virtual ~Array() {} @@ -47,19 +46,15 @@ class Array { return null_count_ > 0 && util::bit_not_set(null_bitmap_data_, i); } - int32_t length() const { return length_;} - int32_t null_count() const { return null_count_;} + int32_t length() const { return length_; } + int32_t null_count() const { return null_count_; } - const std::shared_ptr& type() const { return type_;} - Type::type type_enum() const { return type_->type;} + const std::shared_ptr& type() const { return type_; } + Type::type type_enum() const { return type_->type; } - const std::shared_ptr& null_bitmap() const { - return null_bitmap_; - } + const std::shared_ptr& null_bitmap() const { return null_bitmap_; } - const uint8_t* null_bitmap_data() const { - return null_bitmap_data_; - } + const uint8_t* null_bitmap_data() const { return null_bitmap_data_; } bool EqualsExact(const Array& arr) const; virtual bool Equals(const std::shared_ptr& arr) const = 0; @@ -80,17 +75,16 @@ class Array { // Degenerate null type Array class NullArray : public Array { public: - NullArray(const std::shared_ptr& type, int32_t length) : - Array(type, length, length, nullptr) {} + NullArray(const std::shared_ptr& type, int32_t length) + : Array(type, length, length, nullptr) {} - explicit NullArray(int32_t length) : - NullArray(std::make_shared(), length) {} + explicit NullArray(int32_t length) : NullArray(std::make_shared(), length) {} bool Equals(const std::shared_ptr& arr) const override; }; typedef std::shared_ptr ArrayPtr; -} // namespace arrow +} // namespace arrow #endif diff --git a/cpp/src/arrow/builder.cc b/cpp/src/arrow/builder.cc index 4061f35fd5e53..1447078f76028 100644 --- a/cpp/src/arrow/builder.cc +++ b/cpp/src/arrow/builder.cc @@ -62,4 +62,4 @@ Status ArrayBuilder::Reserve(int32_t elements) { return Status::OK(); } -} // namespace arrow +} // namespace arrow diff --git a/cpp/src/arrow/builder.h b/cpp/src/arrow/builder.h index d1a49dce79961..21a6341ef5086 100644 --- a/cpp/src/arrow/builder.h +++ b/cpp/src/arrow/builder.h @@ -37,30 +37,26 @@ static constexpr int32_t MIN_BUILDER_CAPACITY = 1 << 5; // Base class for all data array builders class ArrayBuilder { public: - explicit ArrayBuilder(MemoryPool* pool, const TypePtr& type) : - pool_(pool), - type_(type), - null_bitmap_(nullptr), - null_count_(0), - null_bitmap_data_(nullptr), - length_(0), - capacity_(0) {} + explicit ArrayBuilder(MemoryPool* pool, const TypePtr& type) + : pool_(pool), + type_(type), + null_bitmap_(nullptr), + null_count_(0), + null_bitmap_data_(nullptr), + length_(0), + capacity_(0) {} virtual ~ArrayBuilder() {} // For nested types. Since the objects are owned by this class instance, we // skip shared pointers and just return a raw pointer - ArrayBuilder* child(int i) { - return children_[i].get(); - } + ArrayBuilder* child(int i) { return children_[i].get(); } - int num_children() const { - return children_.size(); - } + int num_children() const { return children_.size(); } - int32_t length() const { return length_;} - int32_t null_count() const { return null_count_;} - int32_t capacity() const { return capacity_;} + int32_t length() const { return length_; } + int32_t null_count() const { return null_count_; } + int32_t capacity() const { return capacity_; } // Allocates requires memory at this level, but children need to be // initialized independently @@ -76,15 +72,13 @@ class ArrayBuilder { // this function responsibly. Status Advance(int32_t elements); - const std::shared_ptr& null_bitmap() const { return null_bitmap_;} + const std::shared_ptr& null_bitmap() const { return null_bitmap_; } // Creates new array object to hold the contents of the builder and transfers // ownership of the data virtual std::shared_ptr Finish() = 0; - const std::shared_ptr& type() const { - return type_; - } + const std::shared_ptr& type() const { return type_; } protected: MemoryPool* pool_; @@ -107,6 +101,6 @@ class ArrayBuilder { DISALLOW_COPY_AND_ASSIGN(ArrayBuilder); }; -} // namespace arrow +} // namespace arrow -#endif // ARROW_BUILDER_H_ +#endif // ARROW_BUILDER_H_ diff --git a/cpp/src/arrow/column-benchmark.cc b/cpp/src/arrow/column-benchmark.cc index 335d581782ac0..edea0948860de 100644 --- a/cpp/src/arrow/column-benchmark.cc +++ b/cpp/src/arrow/column-benchmark.cc @@ -15,7 +15,6 @@ // specific language governing permissions and limitations // under the License. - #include "benchmark/benchmark.h" #include "arrow/test-util.h" @@ -24,19 +23,19 @@ namespace arrow { namespace { - template - std::shared_ptr MakePrimitive(int32_t length, int32_t null_count = 0) { - auto pool = default_memory_pool(); - auto data = std::make_shared(pool); - auto null_bitmap = std::make_shared(pool); - data->Resize(length * sizeof(typename ArrayType::value_type)); - null_bitmap->Resize(util::bytes_for_bits(length)); - return std::make_shared(length, data, 10, null_bitmap); - } +template +std::shared_ptr MakePrimitive(int32_t length, int32_t null_count = 0) { + auto pool = default_memory_pool(); + auto data = std::make_shared(pool); + auto null_bitmap = std::make_shared(pool); + data->Resize(length * sizeof(typename ArrayType::value_type)); + null_bitmap->Resize(util::bytes_for_bits(length)); + return std::make_shared(length, data, 10, null_bitmap); +} } // anonymous namespace - -static void BM_BuildInt32ColumnByChunk(benchmark::State& state) { //NOLINT non-const reference +static void BM_BuildInt32ColumnByChunk( + benchmark::State& state) { // NOLINT non-const reference ArrayVector arrays; for (int chunk_n = 0; chunk_n < state.range_x(); ++chunk_n) { arrays.push_back(MakePrimitive(100, 10)); diff --git a/cpp/src/arrow/column-test.cc b/cpp/src/arrow/column-test.cc index 0630785630e81..1edf313d49bf6 100644 --- a/cpp/src/arrow/column-test.cc +++ b/cpp/src/arrow/column-test.cc @@ -72,4 +72,4 @@ TEST_F(TestColumn, ChunksInhomogeneous) { ASSERT_RAISES(Invalid, column_->ValidateData()); } -} // namespace arrow +} // namespace arrow diff --git a/cpp/src/arrow/column.cc b/cpp/src/arrow/column.cc index 46acf8df2ff57..52e4c58e1dc3d 100644 --- a/cpp/src/arrow/column.cc +++ b/cpp/src/arrow/column.cc @@ -26,8 +26,7 @@ namespace arrow { -ChunkedArray::ChunkedArray(const ArrayVector& chunks) : - chunks_(chunks) { +ChunkedArray::ChunkedArray(const ArrayVector& chunks) : chunks_(chunks) { length_ = 0; null_count_ = 0; for (const std::shared_ptr& chunk : chunks) { @@ -36,35 +35,31 @@ ChunkedArray::ChunkedArray(const ArrayVector& chunks) : } } -Column::Column(const std::shared_ptr& field, const ArrayVector& chunks) : - field_(field) { +Column::Column(const std::shared_ptr& field, const ArrayVector& chunks) + : field_(field) { data_ = std::make_shared(chunks); } -Column::Column(const std::shared_ptr& field, - const std::shared_ptr& data) : - field_(field) { +Column::Column(const std::shared_ptr& field, const std::shared_ptr& data) + : field_(field) { data_ = std::make_shared(ArrayVector({data})); } -Column::Column(const std::shared_ptr& field, - const std::shared_ptr& data) : - field_(field), - data_(data) {} +Column::Column( + const std::shared_ptr& field, const std::shared_ptr& data) + : field_(field), data_(data) {} Status Column::ValidateData() { for (int i = 0; i < data_->num_chunks(); ++i) { const std::shared_ptr& type = data_->chunk(i)->type(); if (!this->type()->Equals(type)) { std::stringstream ss; - ss << "In chunk " << i << " expected type " - << this->type()->ToString() - << " but saw " - << type->ToString(); + ss << "In chunk " << i << " expected type " << this->type()->ToString() + << " but saw " << type->ToString(); return Status::Invalid(ss.str()); } } return Status::OK(); } -} // namespace arrow +} // namespace arrow diff --git a/cpp/src/arrow/column.h b/cpp/src/arrow/column.h index 1ad97b20863c8..22becc3454780 100644 --- a/cpp/src/arrow/column.h +++ b/cpp/src/arrow/column.h @@ -39,21 +39,13 @@ class ChunkedArray { explicit ChunkedArray(const ArrayVector& chunks); // @returns: the total length of the chunked array; computed on construction - int64_t length() const { - return length_; - } + int64_t length() const { return length_; } - int64_t null_count() const { - return null_count_; - } + int64_t null_count() const { return null_count_; } - int num_chunks() const { - return chunks_.size(); - } + int num_chunks() const { return chunks_.size(); } - const std::shared_ptr& chunk(int i) const { - return chunks_[i]; - } + const std::shared_ptr& chunk(int i) const { return chunks_[i]; } protected: ArrayVector chunks_; @@ -67,33 +59,22 @@ class ChunkedArray { class Column { public: Column(const std::shared_ptr& field, const ArrayVector& chunks); - Column(const std::shared_ptr& field, - const std::shared_ptr& data); + Column(const std::shared_ptr& field, const std::shared_ptr& data); Column(const std::shared_ptr& field, const std::shared_ptr& data); - int64_t length() const { - return data_->length(); - } + int64_t length() const { return data_->length(); } - int64_t null_count() const { - return data_->null_count(); - } + int64_t null_count() const { return data_->null_count(); } // @returns: the column's name in the passed metadata - const std::string& name() const { - return field_->name; - } + const std::string& name() const { return field_->name; } // @returns: the column's type according to the metadata - const std::shared_ptr& type() const { - return field_->type; - } + const std::shared_ptr& type() const { return field_->type; } // @returns: the column's data as a chunked logical array - const std::shared_ptr& data() const { - return data_; - } + const std::shared_ptr& data() const { return data_; } // Verify that the column's array data is consistent with the passed field's // metadata Status ValidateData(); @@ -103,6 +84,6 @@ class Column { std::shared_ptr data_; }; -} // namespace arrow +} // namespace arrow #endif // ARROW_COLUMN_H diff --git a/cpp/src/arrow/ipc/adapter.cc b/cpp/src/arrow/ipc/adapter.cc index c79e8469530f7..2f72c3aa8467a 100644 --- a/cpp/src/arrow/ipc/adapter.cc +++ b/cpp/src/arrow/ipc/adapter.cc @@ -94,8 +94,7 @@ Status VisitArray(const Array* arr, std::vector* field_nodes class RowBatchWriter { public: - explicit RowBatchWriter(const RowBatch* batch) : - batch_(batch) {} + explicit RowBatchWriter(const RowBatch* batch) : batch_(batch) {} Status AssemblePayload() { // Perform depth-first traversal of the row-batch @@ -138,12 +137,12 @@ class RowBatchWriter { // determine the data header size then request a buffer such that you can // construct the flatbuffer data accessor object (see arrow::ipc::Message) std::shared_ptr data_header; - RETURN_NOT_OK(WriteDataHeader(batch_->num_rows(), offset, - field_nodes_, buffer_meta_, &data_header)); + RETURN_NOT_OK(WriteDataHeader( + batch_->num_rows(), offset, field_nodes_, buffer_meta_, &data_header)); // Write the data header at the end - RETURN_NOT_OK(dst->Write(position + offset, data_header->data(), - data_header->size())); + RETURN_NOT_OK( + dst->Write(position + offset, data_header->data(), data_header->size())); *data_header_offset = position + offset; return Status::OK(); @@ -174,8 +173,8 @@ class RowBatchWriter { std::vector> buffers_; }; -Status WriteRowBatch(MemorySource* dst, const RowBatch* batch, int64_t position, - int64_t* header_offset) { +Status WriteRowBatch( + MemorySource* dst, const RowBatch* batch, int64_t position, int64_t* header_offset) { RowBatchWriter serializer(batch); RETURN_NOT_OK(serializer.AssemblePayload()); return serializer.Write(dst, position, header_offset); @@ -187,15 +186,14 @@ static constexpr int64_t INIT_METADATA_SIZE = 4096; class RowBatchReader::Impl { public: - Impl(MemorySource* source, const std::shared_ptr& metadata) : - source_(source), - metadata_(metadata) { + Impl(MemorySource* source, const std::shared_ptr& metadata) + : source_(source), metadata_(metadata) { num_buffers_ = metadata->num_buffers(); num_flattened_fields_ = metadata->num_fields(); } - Status AssembleBatch(const std::shared_ptr& schema, - std::shared_ptr* out) { + Status AssembleBatch( + const std::shared_ptr& schema, std::shared_ptr* out) { std::vector> arrays(schema->num_fields()); // The field_index and buffer_index are incremented in NextArray based on @@ -208,8 +206,7 @@ class RowBatchReader::Impl { RETURN_NOT_OK(NextArray(field, &arrays[i])); } - *out = std::make_shared(schema, metadata_->length(), - arrays); + *out = std::make_shared(schema, metadata_->length(), arrays); return Status::OK(); } @@ -243,11 +240,10 @@ class RowBatchReader::Impl { } else { data.reset(new Buffer(nullptr, 0)); } - return MakePrimitiveArray(type, field_meta.length, data, - field_meta.null_count, null_bitmap, out); - } else { - return Status::NotImplemented("Non-primitive types not complete yet"); + return MakePrimitiveArray( + type, field_meta.length, data, field_meta.null_count, null_bitmap, out); } + return Status::NotImplemented("Non-primitive types not complete yet"); } Status GetBuffer(int buffer_index, std::shared_ptr* out) { @@ -264,8 +260,8 @@ class RowBatchReader::Impl { int num_flattened_fields_; }; -Status RowBatchReader::Open(MemorySource* source, int64_t position, - std::shared_ptr* out) { +Status RowBatchReader::Open( + MemorySource* source, int64_t position, std::shared_ptr* out) { std::shared_ptr metadata; RETURN_NOT_OK(source->ReadAt(position, INIT_METADATA_SIZE, &metadata)); @@ -274,8 +270,7 @@ Status RowBatchReader::Open(MemorySource* source, int64_t position, // We may not need to call source->ReadAt again if (metadata_size > static_cast(INIT_METADATA_SIZE - sizeof(int32_t))) { // We don't have enough data, read the indicated metadata size. - RETURN_NOT_OK(source->ReadAt(position + sizeof(int32_t), - metadata_size, &metadata)); + RETURN_NOT_OK(source->ReadAt(position + sizeof(int32_t), metadata_size, &metadata)); } // TODO(wesm): buffer slicing here would be better in case ReadAt returns @@ -297,11 +292,10 @@ Status RowBatchReader::Open(MemorySource* source, int64_t position, return Status::OK(); } -Status RowBatchReader::GetRowBatch(const std::shared_ptr& schema, - std::shared_ptr* out) { +Status RowBatchReader::GetRowBatch( + const std::shared_ptr& schema, std::shared_ptr* out) { return impl_->AssembleBatch(schema, out); } - -} // namespace ipc -} // namespace arrow +} // namespace ipc +} // namespace arrow diff --git a/cpp/src/arrow/ipc/adapter.h b/cpp/src/arrow/ipc/adapter.h index 26dea6d04b889..d453fa05f4982 100644 --- a/cpp/src/arrow/ipc/adapter.h +++ b/cpp/src/arrow/ipc/adapter.h @@ -52,8 +52,8 @@ class RecordBatchMessage; // // Finally, the memory offset to the start of the metadata / data header is // returned in an out-variable -Status WriteRowBatch(MemorySource* dst, const RowBatch* batch, int64_t position, - int64_t* header_offset); +Status WriteRowBatch( + MemorySource* dst, const RowBatch* batch, int64_t position, int64_t* header_offset); // int64_t GetRowBatchMetadata(const RowBatch* batch); @@ -67,20 +67,20 @@ int64_t GetRowBatchSize(const RowBatch* batch); class RowBatchReader { public: - static Status Open(MemorySource* source, int64_t position, - std::shared_ptr* out); + static Status Open( + MemorySource* source, int64_t position, std::shared_ptr* out); // Reassemble the row batch. A Schema is required to be able to construct the // right array containers - Status GetRowBatch(const std::shared_ptr& schema, - std::shared_ptr* out); + Status GetRowBatch( + const std::shared_ptr& schema, std::shared_ptr* out); private: class Impl; std::unique_ptr impl_; }; -} // namespace ipc -} // namespace arrow +} // namespace ipc +} // namespace arrow -#endif // ARROW_IPC_MEMORY_H +#endif // ARROW_IPC_MEMORY_H diff --git a/cpp/src/arrow/ipc/ipc-adapter-test.cc b/cpp/src/arrow/ipc/ipc-adapter-test.cc index 79b4d710d282f..fbdda77e4919c 100644 --- a/cpp/src/arrow/ipc/ipc-adapter-test.cc +++ b/cpp/src/arrow/ipc/ipc-adapter-test.cc @@ -42,12 +42,8 @@ namespace ipc { class TestWriteRowBatch : public ::testing::Test, public MemoryMapFixture { public: - void SetUp() { - pool_ = default_memory_pool(); - } - void TearDown() { - MemoryMapFixture::TearDown(); - } + void SetUp() { pool_ = default_memory_pool(); } + void TearDown() { MemoryMapFixture::TearDown(); } void InitMemoryMap(int64_t size) { std::string path = "test-write-row-batch"; @@ -83,8 +79,8 @@ TEST_F(TestWriteRowBatch, IntegerRoundTrip) { test::random_bytes(null_bytes, 0, null_bitmap->mutable_data()); auto a0 = std::make_shared(length, data); - auto a1 = std::make_shared(length, data, - test::bitmap_popcount(null_bitmap->data(), length), null_bitmap); + auto a1 = std::make_shared( + length, data, test::bitmap_popcount(null_bitmap->data(), length), null_bitmap); RowBatch batch(schema, length, {a0, a1}); @@ -103,10 +99,10 @@ TEST_F(TestWriteRowBatch, IntegerRoundTrip) { EXPECT_EQ(batch.num_rows(), batch_result->num_rows()); for (int i = 0; i < batch.num_columns(); ++i) { - EXPECT_TRUE(batch.column(i)->Equals(batch_result->column(i))) - << i << batch.column_name(i); + EXPECT_TRUE(batch.column(i)->Equals(batch_result->column(i))) << i + << batch.column_name(i); } } -} // namespace ipc -} // namespace arrow +} // namespace ipc +} // namespace arrow diff --git a/cpp/src/arrow/ipc/ipc-memory-test.cc b/cpp/src/arrow/ipc/ipc-memory-test.cc index 332ad2a2b809b..1933921222595 100644 --- a/cpp/src/arrow/ipc/ipc-memory-test.cc +++ b/cpp/src/arrow/ipc/ipc-memory-test.cc @@ -35,13 +35,10 @@ namespace ipc { class TestMemoryMappedSource : public ::testing::Test, public MemoryMapFixture { public: - void TearDown() { - MemoryMapFixture::TearDown(); - } + void TearDown() { MemoryMapFixture::TearDown(); } }; -TEST_F(TestMemoryMappedSource, InvalidUsages) { -} +TEST_F(TestMemoryMappedSource, InvalidUsages) {} TEST_F(TestMemoryMappedSource, WriteRead) { const int64_t buffer_size = 1024; @@ -74,9 +71,9 @@ TEST_F(TestMemoryMappedSource, InvalidFile) { std::string non_existent_path = "invalid-file-name-asfd"; std::shared_ptr result; - ASSERT_RAISES(IOError, MemoryMappedSource::Open(non_existent_path, - MemorySource::READ_ONLY, &result)); + ASSERT_RAISES(IOError, + MemoryMappedSource::Open(non_existent_path, MemorySource::READ_ONLY, &result)); } -} // namespace ipc -} // namespace arrow +} // namespace ipc +} // namespace arrow diff --git a/cpp/src/arrow/ipc/ipc-metadata-test.cc b/cpp/src/arrow/ipc/ipc-metadata-test.cc index ceabec0fa7c29..51d79cfb4c4bb 100644 --- a/cpp/src/arrow/ipc/ipc-metadata-test.cc +++ b/cpp/src/arrow/ipc/ipc-metadata-test.cc @@ -86,14 +86,12 @@ TEST_F(TestSchemaMessage, NestedFields) { auto type = std::make_shared(std::make_shared()); auto f0 = std::make_shared("f0", type); - std::shared_ptr type2(new StructType({ - std::make_shared("k1", INT32), - std::make_shared("k2", INT32), - std::make_shared("k3", INT32)})); + std::shared_ptr type2(new StructType({std::make_shared("k1", INT32), + std::make_shared("k2", INT32), std::make_shared("k3", INT32)})); auto f1 = std::make_shared("f1", type2); Schema schema({f0, f1}); CheckRoundtrip(&schema); } -} // namespace arrow +} // namespace arrow diff --git a/cpp/src/arrow/ipc/memory.cc b/cpp/src/arrow/ipc/memory.cc index e630ccd109b77..2b077e9792925 100644 --- a/cpp/src/arrow/ipc/memory.cc +++ b/cpp/src/arrow/ipc/memory.cc @@ -17,7 +17,7 @@ #include "arrow/ipc/memory.h" -#include // For memory-mapping +#include // For memory-mapping #include #include #include @@ -32,8 +32,7 @@ namespace arrow { namespace ipc { -MemorySource::MemorySource(AccessMode access_mode) : - access_mode_(access_mode) {} +MemorySource::MemorySource(AccessMode access_mode) : access_mode_(access_mode) {} MemorySource::~MemorySource() {} @@ -41,10 +40,7 @@ MemorySource::~MemorySource() {} class MemoryMappedSource::Impl { public: - Impl() : - file_(nullptr), - is_open_(false), - data_(nullptr) {} + Impl() : file_(nullptr), is_open_(false), data_(nullptr) {} ~Impl() { if (is_open_) { @@ -54,9 +50,7 @@ class MemoryMappedSource::Impl { } Status Open(const std::string& path, MemorySource::AccessMode mode) { - if (is_open_) { - return Status::IOError("A file is already open"); - } + if (is_open_) { return Status::IOError("A file is already open"); } path_ = path; @@ -72,18 +66,15 @@ class MemoryMappedSource::Impl { } fseek(file_, 0L, SEEK_END); - if (ferror(file_)) { - return Status::IOError("Unable to seek to end of file"); - } + if (ferror(file_)) { return Status::IOError("Unable to seek to end of file"); } size_ = ftell(file_); fseek(file_, 0L, SEEK_SET); is_open_ = true; // TODO(wesm): Add read-only version of this - data_ = reinterpret_cast(mmap(nullptr, size_, - PROT_READ | PROT_WRITE, - MAP_SHARED, fileno(file_), 0)); + data_ = reinterpret_cast( + mmap(nullptr, size_, PROT_READ | PROT_WRITE, MAP_SHARED, fileno(file_), 0)); if (data_ == nullptr) { std::stringstream ss; ss << "Memory mapping file failed, errno: " << errno; @@ -93,13 +84,9 @@ class MemoryMappedSource::Impl { return Status::OK(); } - int64_t size() const { - return size_; - } + int64_t size() const { return size_; } - uint8_t* data() { - return data_; - } + uint8_t* data() { return data_; } private: std::string path_; @@ -111,8 +98,8 @@ class MemoryMappedSource::Impl { uint8_t* data_; }; -MemoryMappedSource::MemoryMappedSource(AccessMode access_mode) : - MemorySource(access_mode) {} +MemoryMappedSource::MemoryMappedSource(AccessMode access_mode) + : MemorySource(access_mode) {} Status MemoryMappedSource::Open(const std::string& path, AccessMode access_mode, std::shared_ptr* out) { @@ -134,8 +121,8 @@ Status MemoryMappedSource::Close() { return Status::OK(); } -Status MemoryMappedSource::ReadAt(int64_t position, int64_t nbytes, - std::shared_ptr* out) { +Status MemoryMappedSource::ReadAt( + int64_t position, int64_t nbytes, std::shared_ptr* out) { if (position < 0 || position >= impl_->size()) { return Status::Invalid("position is out of bounds"); } @@ -145,8 +132,7 @@ Status MemoryMappedSource::ReadAt(int64_t position, int64_t nbytes, return Status::OK(); } -Status MemoryMappedSource::Write(int64_t position, const uint8_t* data, - int64_t nbytes) { +Status MemoryMappedSource::Write(int64_t position, const uint8_t* data, int64_t nbytes) { if (position < 0 || position >= impl_->size()) { return Status::Invalid("position is out of bounds"); } @@ -158,5 +144,5 @@ Status MemoryMappedSource::Write(int64_t position, const uint8_t* data, return Status::OK(); } -} // namespace ipc -} // namespace arrow +} // namespace ipc +} // namespace arrow diff --git a/cpp/src/arrow/ipc/memory.h b/cpp/src/arrow/ipc/memory.h index 0b4d8347c342f..e529603dc6e2a 100644 --- a/cpp/src/arrow/ipc/memory.h +++ b/cpp/src/arrow/ipc/memory.h @@ -52,8 +52,8 @@ class OutputStream { // memory map class BufferOutputStream : public OutputStream { public: - explicit BufferOutputStream(const std::shared_ptr& buffer): - buffer_(buffer) {} + explicit BufferOutputStream(const std::shared_ptr& buffer) + : buffer_(buffer) {} // Implement the OutputStream interface Status Close() override; @@ -72,10 +72,7 @@ class BufferOutputStream : public OutputStream { class MemorySource { public: // Indicates the access permissions of the memory source - enum AccessMode { - READ_ONLY, - READ_WRITE - }; + enum AccessMode { READ_ONLY, READ_WRITE }; virtual ~MemorySource(); @@ -83,8 +80,8 @@ class MemorySource { // the indicated location // @returns: arrow::Status indicating success / failure. The buffer is set // into the *out argument - virtual Status ReadAt(int64_t position, int64_t nbytes, - std::shared_ptr* out) = 0; + virtual Status ReadAt( + int64_t position, int64_t nbytes, std::shared_ptr* out) = 0; virtual Status Close() = 0; @@ -110,8 +107,7 @@ class MemoryMappedSource : public MemorySource { Status Close() override; - Status ReadAt(int64_t position, int64_t nbytes, - std::shared_ptr* out) override; + Status ReadAt(int64_t position, int64_t nbytes, std::shared_ptr* out) override; Status Write(int64_t position, const uint8_t* data, int64_t nbytes) override; @@ -125,7 +121,7 @@ class MemoryMappedSource : public MemorySource { std::unique_ptr impl_; }; -} // namespace ipc -} // namespace arrow +} // namespace ipc +} // namespace arrow -#endif // ARROW_IPC_MEMORY_H +#endif // ARROW_IPC_MEMORY_H diff --git a/cpp/src/arrow/ipc/metadata-internal.cc b/cpp/src/arrow/ipc/metadata-internal.cc index 14b186906c3a0..ad5951d17e2c0 100644 --- a/cpp/src/arrow/ipc/metadata-internal.cc +++ b/cpp/src/arrow/ipc/metadata-internal.cc @@ -52,11 +52,12 @@ const std::shared_ptr UINT64 = std::make_shared(); const std::shared_ptr FLOAT = std::make_shared(); const std::shared_ptr DOUBLE = std::make_shared(); -static Status IntFromFlatbuffer(const flatbuf::Int* int_data, - std::shared_ptr* out) { +static Status IntFromFlatbuffer( + const flatbuf::Int* int_data, std::shared_ptr* out) { if (int_data->bitWidth() % 8 != 0) { return Status::NotImplemented("Integers not in cstdint are not implemented"); - } else if (int_data->bitWidth() > 64) { + } + if (int_data->bitWidth() > 64) { return Status::NotImplemented("Integers with more than 64 bits not implemented"); } @@ -80,8 +81,8 @@ static Status IntFromFlatbuffer(const flatbuf::Int* int_data, return Status::OK(); } -static Status FloatFromFlatuffer(const flatbuf::FloatingPoint* float_data, - std::shared_ptr* out) { +static Status FloatFromFlatuffer( + const flatbuf::FloatingPoint* float_data, std::shared_ptr* out) { if (float_data->precision() == flatbuf::Precision_SINGLE) { *out = FLOAT; } else { @@ -90,9 +91,8 @@ static Status FloatFromFlatuffer(const flatbuf::FloatingPoint* float_data, return Status::OK(); } -static Status TypeFromFlatbuffer(flatbuf::Type type, - const void* type_data, const std::vector>& children, - std::shared_ptr* out) { +static Status TypeFromFlatbuffer(flatbuf::Type type, const void* type_data, + const std::vector>& children, std::shared_ptr* out) { switch (type) { case flatbuf::Type_NONE: return Status::Invalid("Type metadata cannot be none"); @@ -101,8 +101,8 @@ static Status TypeFromFlatbuffer(flatbuf::Type type, case flatbuf::Type_Bit: return Status::NotImplemented("Type is not implemented"); case flatbuf::Type_FloatingPoint: - return FloatFromFlatuffer(static_cast(type_data), - out); + return FloatFromFlatuffer( + static_cast(type_data), out); case flatbuf::Type_Binary: case flatbuf::Type_Utf8: return Status::NotImplemented("Type is not implemented"); @@ -128,16 +128,14 @@ static Status TypeFromFlatbuffer(flatbuf::Type type, } // Forward declaration -static Status FieldToFlatbuffer(FBB& fbb, const std::shared_ptr& field, - FieldOffset* offset); +static Status FieldToFlatbuffer( + FBB& fbb, const std::shared_ptr& field, FieldOffset* offset); -static Offset IntToFlatbuffer(FBB& fbb, int bitWidth, - bool is_signed) { +static Offset IntToFlatbuffer(FBB& fbb, int bitWidth, bool is_signed) { return flatbuf::CreateInt(fbb, bitWidth, is_signed).Union(); } -static Offset FloatToFlatbuffer(FBB& fbb, - flatbuf::Precision precision) { +static Offset FloatToFlatbuffer(FBB& fbb, flatbuf::Precision precision) { return flatbuf::CreateFloatingPoint(fbb, precision).Union(); } @@ -166,10 +164,8 @@ static Status StructToFlatbuffer(FBB& fbb, const std::shared_ptr& type *offset = IntToFlatbuffer(fbb, BIT_WIDTH, IS_SIGNED); \ break; - static Status TypeToFlatbuffer(FBB& fbb, const std::shared_ptr& type, - std::vector* children, - flatbuf::Type* out_type, Offset* offset) { + std::vector* children, flatbuf::Type* out_type, Offset* offset) { switch (type->type) { case Type::BOOL: *out_type = flatbuf::Type_Bool; @@ -206,16 +202,16 @@ static Status TypeToFlatbuffer(FBB& fbb, const std::shared_ptr& type, *out_type = flatbuf::Type_Tuple; return StructToFlatbuffer(fbb, type, children, offset); default: + *out_type = flatbuf::Type_NONE; // Make clang-tidy happy std::stringstream ss; - ss << "Unable to convert type: " << type->ToString() - << std::endl; + ss << "Unable to convert type: " << type->ToString() << std::endl; return Status::NotImplemented(ss.str()); } return Status::OK(); } -static Status FieldToFlatbuffer(FBB& fbb, const std::shared_ptr& field, - FieldOffset* offset) { +static Status FieldToFlatbuffer( + FBB& fbb, const std::shared_ptr& field, FieldOffset* offset) { auto fb_name = fbb.CreateString(field->name); flatbuf::Type type_enum; @@ -225,14 +221,13 @@ static Status FieldToFlatbuffer(FBB& fbb, const std::shared_ptr& field, RETURN_NOT_OK(TypeToFlatbuffer(fbb, field->type, &children, &type_enum, &type_data)); auto fb_children = fbb.CreateVector(children); - *offset = flatbuf::CreateField(fbb, fb_name, field->nullable, type_enum, - type_data, fb_children); + *offset = flatbuf::CreateField( + fbb, fb_name, field->nullable, type_enum, type_data, fb_children); return Status::OK(); } -Status FieldFromFlatbuffer(const flatbuf::Field* field, - std::shared_ptr* out) { +Status FieldFromFlatbuffer(const flatbuf::Field* field, std::shared_ptr* out) { std::shared_ptr type; auto children = field->children(); @@ -241,8 +236,8 @@ Status FieldFromFlatbuffer(const flatbuf::Field* field, RETURN_NOT_OK(FieldFromFlatbuffer(children->Get(i), &child_fields[i])); } - RETURN_NOT_OK(TypeFromFlatbuffer(field->type_type(), - field->type(), child_fields, &type)); + RETURN_NOT_OK( + TypeFromFlatbuffer(field->type_type(), field->type(), child_fields, &type)); *out = std::make_shared(field->name()->str(), type); return Status::OK(); @@ -270,19 +265,17 @@ Status MessageBuilder::SetRecordBatch(int32_t length, int64_t body_length, const std::vector& nodes, const std::vector& buffers) { header_type_ = flatbuf::MessageHeader_RecordBatch; - header_ = flatbuf::CreateRecordBatch(fbb_, length, - fbb_.CreateVectorOfStructs(nodes), - fbb_.CreateVectorOfStructs(buffers)).Union(); + header_ = flatbuf::CreateRecordBatch(fbb_, length, fbb_.CreateVectorOfStructs(nodes), + fbb_.CreateVectorOfStructs(buffers)) + .Union(); body_length_ = body_length; return Status::OK(); } - Status WriteDataHeader(int32_t length, int64_t body_length, const std::vector& nodes, - const std::vector& buffers, - std::shared_ptr* out) { + const std::vector& buffers, std::shared_ptr* out) { MessageBuilder message; RETURN_NOT_OK(message.SetRecordBatch(length, body_length, nodes, buffers)); RETURN_NOT_OK(message.Finish()); @@ -290,8 +283,7 @@ Status WriteDataHeader(int32_t length, int64_t body_length, } Status MessageBuilder::Finish() { - auto message = flatbuf::CreateMessage(fbb_, header_type_, header_, - body_length_); + auto message = flatbuf::CreateMessage(fbb_, header_type_, header_, body_length_); fbb_.Finish(message); return Status::OK(); } @@ -313,5 +305,5 @@ Status MessageBuilder::GetBuffer(std::shared_ptr* out) { return Status::OK(); } -} // namespace ipc -} // namespace arrow +} // namespace ipc +} // namespace arrow diff --git a/cpp/src/arrow/ipc/metadata-internal.h b/cpp/src/arrow/ipc/metadata-internal.h index f7365d2a49f95..779c5a30a044a 100644 --- a/cpp/src/arrow/ipc/metadata-internal.h +++ b/cpp/src/arrow/ipc/metadata-internal.h @@ -36,8 +36,7 @@ class Status; namespace ipc { -Status FieldFromFlatbuffer(const flatbuf::Field* field, - std::shared_ptr* out); +Status FieldFromFlatbuffer(const flatbuf::Field* field, std::shared_ptr* out); class MessageBuilder { public: @@ -60,10 +59,9 @@ class MessageBuilder { Status WriteDataHeader(int32_t length, int64_t body_length, const std::vector& nodes, - const std::vector& buffers, - std::shared_ptr* out); + const std::vector& buffers, std::shared_ptr* out); -} // namespace ipc -} // namespace arrow +} // namespace ipc +} // namespace arrow -#endif // ARROW_IPC_METADATA_INTERNAL_H +#endif // ARROW_IPC_METADATA_INTERNAL_H diff --git a/cpp/src/arrow/ipc/metadata.cc b/cpp/src/arrow/ipc/metadata.cc index 642f21a41e640..bcf104f0b8ba6 100644 --- a/cpp/src/arrow/ipc/metadata.cc +++ b/cpp/src/arrow/ipc/metadata.cc @@ -48,10 +48,8 @@ Status WriteSchema(const Schema* schema, std::shared_ptr* out) { class Message::Impl { public: - explicit Impl(const std::shared_ptr& buffer, - const flatbuf::Message* message) : - buffer_(buffer), - message_(message) {} + explicit Impl(const std::shared_ptr& buffer, const flatbuf::Message* message) + : buffer_(buffer), message_(message) {} Message::Type type() const { switch (message_->header_type()) { @@ -66,13 +64,9 @@ class Message::Impl { } } - const void* header() const { - return message_->header(); - } + const void* header() const { return message_->header(); } - int64_t body_length() const { - return message_->bodyLength(); - } + int64_t body_length() const { return message_->bodyLength(); } private: // Owns the memory this message accesses @@ -83,16 +77,12 @@ class Message::Impl { class SchemaMessage::Impl { public: - explicit Impl(const void* schema) : - schema_(static_cast(schema)) {} + explicit Impl(const void* schema) + : schema_(static_cast(schema)) {} - const flatbuf::Field* field(int i) const { - return schema_->fields()->Get(i); - } + const flatbuf::Field* field(int i) const { return schema_->fields()->Get(i); } - int num_fields() const { - return schema_->fields()->size(); - } + int num_fields() const { return schema_->fields()->size(); } private: const flatbuf::Schema* schema_; @@ -100,8 +90,8 @@ class SchemaMessage::Impl { Message::Message() {} -Status Message::Open(const std::shared_ptr& buffer, - std::shared_ptr* out) { +Status Message::Open( + const std::shared_ptr& buffer, std::shared_ptr* out) { std::shared_ptr result(new Message()); // The buffer is prefixed by its size as int32_t @@ -128,12 +118,11 @@ std::shared_ptr Message::get_shared_ptr() { } std::shared_ptr Message::GetSchema() { - return std::make_shared(this->shared_from_this(), - impl_->header()); + return std::make_shared(this->shared_from_this(), impl_->header()); } -SchemaMessage::SchemaMessage(const std::shared_ptr& message, - const void* schema) { +SchemaMessage::SchemaMessage( + const std::shared_ptr& message, const void* schema) { message_ = message; impl_.reset(new Impl(schema)); } @@ -158,31 +147,21 @@ Status SchemaMessage::GetSchema(std::shared_ptr* out) const { class RecordBatchMessage::Impl { public: - explicit Impl(const void* batch) : - batch_(static_cast(batch)) { + explicit Impl(const void* batch) + : batch_(static_cast(batch)) { nodes_ = batch_->nodes(); buffers_ = batch_->buffers(); } - const flatbuf::FieldNode* field(int i) const { - return nodes_->Get(i); - } + const flatbuf::FieldNode* field(int i) const { return nodes_->Get(i); } - const flatbuf::Buffer* buffer(int i) const { - return buffers_->Get(i); - } + const flatbuf::Buffer* buffer(int i) const { return buffers_->Get(i); } - int32_t length() const { - return batch_->length(); - } + int32_t length() const { return batch_->length(); } - int num_buffers() const { - return batch_->buffers()->size(); - } + int num_buffers() const { return batch_->buffers()->size(); } - int num_fields() const { - return batch_->nodes()->size(); - } + int num_fields() const { return batch_->nodes()->size(); } private: const flatbuf::RecordBatch* batch_; @@ -191,12 +170,11 @@ class RecordBatchMessage::Impl { }; std::shared_ptr Message::GetRecordBatch() { - return std::make_shared(this->shared_from_this(), - impl_->header()); + return std::make_shared(this->shared_from_this(), impl_->header()); } -RecordBatchMessage::RecordBatchMessage(const std::shared_ptr& message, - const void* batch) { +RecordBatchMessage::RecordBatchMessage( + const std::shared_ptr& message, const void* batch) { message_ = message; impl_.reset(new Impl(batch)); } @@ -234,5 +212,5 @@ int RecordBatchMessage::num_fields() const { return impl_->num_fields(); } -} // namespace ipc -} // namespace arrow +} // namespace ipc +} // namespace arrow diff --git a/cpp/src/arrow/ipc/metadata.h b/cpp/src/arrow/ipc/metadata.h index c7288529b9fbd..838a4a676ea35 100644 --- a/cpp/src/arrow/ipc/metadata.h +++ b/cpp/src/arrow/ipc/metadata.h @@ -85,8 +85,7 @@ struct BufferMetadata { class RecordBatchMessage { public: // Accepts an opaque flatbuffer pointer - RecordBatchMessage(const std::shared_ptr& message, - const void* batch_meta); + RecordBatchMessage(const std::shared_ptr& message, const void* batch_meta); FieldMetadata field(int i) const; BufferMetadata buffer(int i) const; @@ -111,15 +110,10 @@ class DictionaryBatchMessage { class Message : public std::enable_shared_from_this { public: - enum Type { - NONE, - SCHEMA, - DICTIONARY_BATCH, - RECORD_BATCH - }; + enum Type { NONE, SCHEMA, DICTIONARY_BATCH, RECORD_BATCH }; - static Status Open(const std::shared_ptr& buffer, - std::shared_ptr* out); + static Status Open( + const std::shared_ptr& buffer, std::shared_ptr* out); std::shared_ptr get_shared_ptr(); @@ -140,7 +134,7 @@ class Message : public std::enable_shared_from_this { std::unique_ptr impl_; }; -} // namespace ipc -} // namespace arrow +} // namespace ipc +} // namespace arrow -#endif // ARROW_IPC_METADATA_H +#endif // ARROW_IPC_METADATA_H diff --git a/cpp/src/arrow/ipc/test-common.h b/cpp/src/arrow/ipc/test-common.h index 0fccce941071b..65c837dc8b141 100644 --- a/cpp/src/arrow/ipc/test-common.h +++ b/cpp/src/arrow/ipc/test-common.h @@ -36,9 +36,7 @@ class MemoryMapFixture { void CreateFile(const std::string path, int64_t size) { FILE* file = fopen(path.c_str(), "w"); - if (file != nullptr) { - tmp_files_.push_back(path); - } + if (file != nullptr) { tmp_files_.push_back(path); } ftruncate(fileno(file), size); fclose(file); } @@ -47,7 +45,7 @@ class MemoryMapFixture { std::vector tmp_files_; }; -} // namespace ipc -} // namespace arrow +} // namespace ipc +} // namespace arrow -#endif // ARROW_IPC_TEST_COMMON_H +#endif // ARROW_IPC_TEST_COMMON_H diff --git a/cpp/src/arrow/parquet/parquet-schema-test.cc b/cpp/src/arrow/parquet/parquet-schema-test.cc index a289ddbfde6eb..e2280f41189ef 100644 --- a/cpp/src/arrow/parquet/parquet-schema-test.cc +++ b/cpp/src/arrow/parquet/parquet-schema-test.cc @@ -45,8 +45,7 @@ const auto INT64 = std::make_shared(); const auto FLOAT = std::make_shared(); const auto DOUBLE = std::make_shared(); const auto UTF8 = std::make_shared(); -const auto BINARY = std::make_shared( - std::make_shared("", UINT8)); +const auto BINARY = std::make_shared(std::make_shared("", UINT8)); const auto DECIMAL_8_4 = std::make_shared(8, 4); class TestConvertParquetSchema : public ::testing::Test { @@ -58,8 +57,8 @@ class TestConvertParquetSchema : public ::testing::Test { for (int i = 0; i < expected_schema->num_fields(); ++i) { auto lhs = result_schema_->field(i); auto rhs = expected_schema->field(i); - EXPECT_TRUE(lhs->Equals(rhs)) - << i << " " << lhs->ToString() << " != " << rhs->ToString(); + EXPECT_TRUE(lhs->Equals(rhs)) << i << " " << lhs->ToString() + << " != " << rhs->ToString(); } } @@ -99,20 +98,15 @@ TEST_F(TestConvertParquetSchema, ParquetFlatPrimitives) { arrow_fields.push_back(std::make_shared("double", DOUBLE)); parquet_fields.push_back( - PrimitiveNode::Make("binary", Repetition::OPTIONAL, - ParquetType::BYTE_ARRAY)); + PrimitiveNode::Make("binary", Repetition::OPTIONAL, ParquetType::BYTE_ARRAY)); arrow_fields.push_back(std::make_shared("binary", BINARY)); - parquet_fields.push_back( - PrimitiveNode::Make("string", Repetition::OPTIONAL, - ParquetType::BYTE_ARRAY, - LogicalType::UTF8)); + parquet_fields.push_back(PrimitiveNode::Make( + "string", Repetition::OPTIONAL, ParquetType::BYTE_ARRAY, LogicalType::UTF8)); arrow_fields.push_back(std::make_shared("string", UTF8)); - parquet_fields.push_back( - PrimitiveNode::Make("flba-binary", Repetition::OPTIONAL, - ParquetType::FIXED_LEN_BYTE_ARRAY, - LogicalType::NONE, 12)); + parquet_fields.push_back(PrimitiveNode::Make("flba-binary", Repetition::OPTIONAL, + ParquetType::FIXED_LEN_BYTE_ARRAY, LogicalType::NONE, 12)); arrow_fields.push_back(std::make_shared("flba-binary", BINARY)); auto arrow_schema = std::make_shared(arrow_fields); @@ -125,28 +119,20 @@ TEST_F(TestConvertParquetSchema, ParquetFlatDecimals) { std::vector parquet_fields; std::vector> arrow_fields; - parquet_fields.push_back( - PrimitiveNode::Make("flba-decimal", Repetition::OPTIONAL, - ParquetType::FIXED_LEN_BYTE_ARRAY, - LogicalType::DECIMAL, 4, 8, 4)); + parquet_fields.push_back(PrimitiveNode::Make("flba-decimal", Repetition::OPTIONAL, + ParquetType::FIXED_LEN_BYTE_ARRAY, LogicalType::DECIMAL, 4, 8, 4)); arrow_fields.push_back(std::make_shared("flba-decimal", DECIMAL_8_4)); - parquet_fields.push_back( - PrimitiveNode::Make("binary-decimal", Repetition::OPTIONAL, - ParquetType::BYTE_ARRAY, - LogicalType::DECIMAL, -1, 8, 4)); + parquet_fields.push_back(PrimitiveNode::Make("binary-decimal", Repetition::OPTIONAL, + ParquetType::BYTE_ARRAY, LogicalType::DECIMAL, -1, 8, 4)); arrow_fields.push_back(std::make_shared("binary-decimal", DECIMAL_8_4)); - parquet_fields.push_back( - PrimitiveNode::Make("int32-decimal", Repetition::OPTIONAL, - ParquetType::INT32, - LogicalType::DECIMAL, -1, 8, 4)); + parquet_fields.push_back(PrimitiveNode::Make("int32-decimal", Repetition::OPTIONAL, + ParquetType::INT32, LogicalType::DECIMAL, -1, 8, 4)); arrow_fields.push_back(std::make_shared("int32-decimal", DECIMAL_8_4)); - parquet_fields.push_back( - PrimitiveNode::Make("int64-decimal", Repetition::OPTIONAL, - ParquetType::INT64, - LogicalType::DECIMAL, -1, 8, 4)); + parquet_fields.push_back(PrimitiveNode::Make("int64-decimal", Repetition::OPTIONAL, + ParquetType::INT64, LogicalType::DECIMAL, -1, 8, 4)); arrow_fields.push_back(std::make_shared("int64-decimal", DECIMAL_8_4)); auto arrow_schema = std::make_shared(arrow_fields); @@ -164,22 +150,19 @@ TEST_F(TestConvertParquetSchema, UnsupportedThings) { unsupported_nodes.push_back( GroupNode::Make("repeated-group", Repetition::REPEATED, {})); - unsupported_nodes.push_back( - PrimitiveNode::Make("int32", Repetition::OPTIONAL, - ParquetType::INT32, LogicalType::DATE)); + unsupported_nodes.push_back(PrimitiveNode::Make( + "int32", Repetition::OPTIONAL, ParquetType::INT32, LogicalType::DATE)); - unsupported_nodes.push_back( - PrimitiveNode::Make("int64", Repetition::OPTIONAL, - ParquetType::INT64, LogicalType::TIMESTAMP_MILLIS)); + unsupported_nodes.push_back(PrimitiveNode::Make( + "int64", Repetition::OPTIONAL, ParquetType::INT64, LogicalType::TIMESTAMP_MILLIS)); for (const NodePtr& node : unsupported_nodes) { ASSERT_RAISES(NotImplemented, ConvertSchema({node})); } } -TEST(TestNodeConversion, DateAndTime) { -} +TEST(TestNodeConversion, DateAndTime) {} -} // namespace parquet +} // namespace parquet -} // namespace arrow +} // namespace arrow diff --git a/cpp/src/arrow/parquet/schema.cc b/cpp/src/arrow/parquet/schema.cc index 14f4f5be53ce9..066388b4d0e23 100644 --- a/cpp/src/arrow/parquet/schema.cc +++ b/cpp/src/arrow/parquet/schema.cc @@ -43,8 +43,7 @@ const auto INT64 = std::make_shared(); const auto FLOAT = std::make_shared(); const auto DOUBLE = std::make_shared(); const auto UTF8 = std::make_shared(); -const auto BINARY = std::make_shared( - std::make_shared("", UINT8)); +const auto BINARY = std::make_shared(std::make_shared("", UINT8)); TypePtr MakeDecimalType(const PrimitiveNode* node) { int precision = node->decimal_metadata().precision; @@ -167,12 +166,12 @@ Status NodeToField(const NodePtr& node, std::shared_ptr* out) { return Status::OK(); } -Status FromParquetSchema(const ::parquet::SchemaDescriptor* parquet_schema, - std::shared_ptr* out) { +Status FromParquetSchema( + const ::parquet::SchemaDescriptor* parquet_schema, std::shared_ptr* out) { // TODO(wesm): Consider adding an arrow::Schema name attribute, which comes // from the root Parquet node - const GroupNode* schema_node = static_cast( - parquet_schema->schema().get()); + const GroupNode* schema_node = + static_cast(parquet_schema->schema().get()); std::vector> fields(schema_node->field_count()); for (int i = 0; i < schema_node->field_count(); i++) { @@ -183,6 +182,6 @@ Status FromParquetSchema(const ::parquet::SchemaDescriptor* parquet_schema, return Status::OK(); } -} // namespace parquet +} // namespace parquet -} // namespace arrow +} // namespace arrow diff --git a/cpp/src/arrow/parquet/schema.h b/cpp/src/arrow/parquet/schema.h index a8408970ede48..a44a9a4b6a892 100644 --- a/cpp/src/arrow/parquet/schema.h +++ b/cpp/src/arrow/parquet/schema.h @@ -31,14 +31,13 @@ class Status; namespace parquet { -Status NodeToField(const ::parquet::schema::NodePtr& node, - std::shared_ptr* out); +Status NodeToField(const ::parquet::schema::NodePtr& node, std::shared_ptr* out); -Status FromParquetSchema(const ::parquet::SchemaDescriptor* parquet_schema, - std::shared_ptr* out); +Status FromParquetSchema( + const ::parquet::SchemaDescriptor* parquet_schema, std::shared_ptr* out); -} // namespace parquet +} // namespace parquet -} // namespace arrow +} // namespace arrow #endif diff --git a/cpp/src/arrow/schema-test.cc b/cpp/src/arrow/schema-test.cc index a1de1dc5ac8a4..8cc80be120a44 100644 --- a/cpp/src/arrow/schema-test.cc +++ b/cpp/src/arrow/schema-test.cc @@ -86,8 +86,8 @@ TEST_F(TestSchema, ToString) { auto f0 = std::make_shared("f0", INT32); auto f1 = std::make_shared("f1", std::make_shared(), false); auto f2 = std::make_shared("f2", std::make_shared()); - auto f3 = std::make_shared("f3", - std::make_shared(std::make_shared())); + auto f3 = std::make_shared( + "f3", std::make_shared(std::make_shared())); vector> fields = {f0, f1, f2, f3}; auto schema = std::make_shared(fields); @@ -101,4 +101,4 @@ f3: list)"; ASSERT_EQ(expected, result); } -} // namespace arrow +} // namespace arrow diff --git a/cpp/src/arrow/schema.cc b/cpp/src/arrow/schema.cc index 18aad0e806ff2..a38acaa94ba56 100644 --- a/cpp/src/arrow/schema.cc +++ b/cpp/src/arrow/schema.cc @@ -26,18 +26,14 @@ namespace arrow { -Schema::Schema(const std::vector>& fields) : - fields_(fields) {} +Schema::Schema(const std::vector>& fields) : fields_(fields) {} bool Schema::Equals(const Schema& other) const { - if (this == &other) return true; - if (num_fields() != other.num_fields()) { - return false; - } + if (this == &other) { return true; } + + if (num_fields() != other.num_fields()) { return false; } for (int i = 0; i < num_fields(); ++i) { - if (!field(i)->Equals(*other.field(i).get())) { - return false; - } + if (!field(i)->Equals(*other.field(i).get())) { return false; } } return true; } @@ -51,13 +47,11 @@ std::string Schema::ToString() const { int i = 0; for (auto field : fields_) { - if (i > 0) { - buffer << std::endl; - } + if (i > 0) { buffer << std::endl; } buffer << field->ToString(); ++i; } return buffer.str(); } -} // namespace arrow +} // namespace arrow diff --git a/cpp/src/arrow/schema.h b/cpp/src/arrow/schema.h index 52f3c1ceae46d..a8b0d8444ac92 100644 --- a/cpp/src/arrow/schema.h +++ b/cpp/src/arrow/schema.h @@ -35,21 +35,17 @@ class Schema { bool Equals(const std::shared_ptr& other) const; // Return the ith schema element. Does not boundscheck - const std::shared_ptr& field(int i) const { - return fields_[i]; - } + const std::shared_ptr& field(int i) const { return fields_[i]; } // Render a string representation of the schema suitable for debugging std::string ToString() const; - int num_fields() const { - return fields_.size(); - } + int num_fields() const { return fields_.size(); } private: std::vector> fields_; }; -} // namespace arrow +} // namespace arrow #endif // ARROW_FIELD_H diff --git a/cpp/src/arrow/table-test.cc b/cpp/src/arrow/table-test.cc index 4c7b8f80486de..385e7d831500a 100644 --- a/cpp/src/arrow/table-test.cc +++ b/cpp/src/arrow/table-test.cc @@ -49,10 +49,9 @@ class TestTable : public TestBase { schema_ = std::make_shared(fields); columns_ = { - std::make_shared(schema_->field(0), MakePrimitive(length)), - std::make_shared(schema_->field(1), MakePrimitive(length)), - std::make_shared(schema_->field(2), MakePrimitive(length)) - }; + std::make_shared(schema_->field(0), MakePrimitive(length)), + std::make_shared(schema_->field(1), MakePrimitive(length)), + std::make_shared(schema_->field(2), MakePrimitive(length))}; } protected: @@ -116,13 +115,12 @@ TEST_F(TestTable, InvalidColumns) { ASSERT_RAISES(Invalid, table_->ValidateColumns()); columns_ = { - std::make_shared(schema_->field(0), MakePrimitive(length)), - std::make_shared(schema_->field(1), MakePrimitive(length)), - std::make_shared(schema_->field(2), MakePrimitive(length - 1)) - }; + std::make_shared(schema_->field(0), MakePrimitive(length)), + std::make_shared(schema_->field(1), MakePrimitive(length)), + std::make_shared(schema_->field(2), MakePrimitive(length - 1))}; table_.reset(new Table("data", schema_, columns_, length)); ASSERT_RAISES(Invalid, table_->ValidateColumns()); } -} // namespace arrow +} // namespace arrow diff --git a/cpp/src/arrow/table.cc b/cpp/src/arrow/table.cc index e405c1d508c22..d9573eae74ddd 100644 --- a/cpp/src/arrow/table.cc +++ b/cpp/src/arrow/table.cc @@ -28,20 +28,16 @@ namespace arrow { RowBatch::RowBatch(const std::shared_ptr& schema, int num_rows, - const std::vector>& columns) : - schema_(schema), - num_rows_(num_rows), - columns_(columns) {} + const std::vector>& columns) + : schema_(schema), num_rows_(num_rows), columns_(columns) {} const std::string& RowBatch::column_name(int i) const { return schema_->field(i)->name; } Table::Table(const std::string& name, const std::shared_ptr& schema, - const std::vector>& columns) : - name_(name), - schema_(schema), - columns_(columns) { + const std::vector>& columns) + : name_(name), schema_(schema), columns_(columns) { if (columns.size() == 0) { num_rows_ = 0; } else { @@ -50,11 +46,8 @@ Table::Table(const std::string& name, const std::shared_ptr& schema, } Table::Table(const std::string& name, const std::shared_ptr& schema, - const std::vector>& columns, int64_t num_rows) : - name_(name), - schema_(schema), - columns_(columns), - num_rows_(num_rows) {} + const std::vector>& columns, int64_t num_rows) + : name_(name), schema_(schema), columns_(columns), num_rows_(num_rows) {} Status Table::ValidateColumns() const { if (num_columns() != schema_->num_fields()) { @@ -66,21 +59,17 @@ Status Table::ValidateColumns() const { const Column* col = columns_[i].get(); if (col == nullptr) { std::stringstream ss; - ss << "Column " << i << " named " << col->name() - << " was null"; + ss << "Column " << i << " was null"; return Status::Invalid(ss.str()); } if (col->length() != num_rows_) { std::stringstream ss; - ss << "Column " << i << " named " << col->name() - << " expected length " - << num_rows_ - << " but got length " - << col->length(); + ss << "Column " << i << " named " << col->name() << " expected length " << num_rows_ + << " but got length " << col->length(); return Status::Invalid(ss.str()); } } return Status::OK(); } -} // namespace arrow +} // namespace arrow diff --git a/cpp/src/arrow/table.h b/cpp/src/arrow/table.h index e2f73a2eeddcb..756b2a19593f4 100644 --- a/cpp/src/arrow/table.h +++ b/cpp/src/arrow/table.h @@ -42,27 +42,19 @@ class RowBatch { const std::vector>& columns); // @returns: the table's schema - const std::shared_ptr& schema() const { - return schema_; - } + const std::shared_ptr& schema() const { return schema_; } // @returns: the i-th column // Note: Does not boundscheck - const std::shared_ptr& column(int i) const { - return columns_[i]; - } + const std::shared_ptr& column(int i) const { return columns_[i]; } const std::string& column_name(int i) const; // @returns: the number of columns in the table - int num_columns() const { - return columns_.size(); - } + int num_columns() const { return columns_.size(); } // @returns: the number of rows (the corresponding length of each column) - int64_t num_rows() const { - return num_rows_; - } + int64_t num_rows() const { return num_rows_; } private: std::shared_ptr schema_; @@ -85,30 +77,20 @@ class Table { const std::vector>& columns, int64_t num_rows); // @returns: the table's name, if any (may be length 0) - const std::string& name() const { - return name_; - } + const std::string& name() const { return name_; } // @returns: the table's schema - const std::shared_ptr& schema() const { - return schema_; - } + const std::shared_ptr& schema() const { return schema_; } // Note: Does not boundscheck // @returns: the i-th column - const std::shared_ptr& column(int i) const { - return columns_[i]; - } + const std::shared_ptr& column(int i) const { return columns_[i]; } // @returns: the number of columns in the table - int num_columns() const { - return columns_.size(); - } + int num_columns() const { return columns_.size(); } // @returns: the number of rows (the corresponding length of each column) - int64_t num_rows() const { - return num_rows_; - } + int64_t num_rows() const { return num_rows_; } // After construction, perform any checks to validate the input arguments Status ValidateColumns() const; @@ -123,6 +105,6 @@ class Table { int64_t num_rows_; }; -} // namespace arrow +} // namespace arrow #endif // ARROW_TABLE_H diff --git a/cpp/src/arrow/test-util.h b/cpp/src/arrow/test-util.h index b2bce269992d0..538d9b233d990 100644 --- a/cpp/src/arrow/test-util.h +++ b/cpp/src/arrow/test-util.h @@ -36,38 +36,29 @@ #include "arrow/util/random.h" #include "arrow/util/status.h" -#define ASSERT_RAISES(ENUM, expr) \ - do { \ - Status s = (expr); \ - if (!s.Is##ENUM()) { \ - FAIL() << s.ToString(); \ - } \ +#define ASSERT_RAISES(ENUM, expr) \ + do { \ + Status s = (expr); \ + if (!s.Is##ENUM()) { FAIL() << s.ToString(); } \ } while (0) - -#define ASSERT_OK(expr) \ - do { \ - Status s = (expr); \ - if (!s.ok()) { \ - FAIL() << s.ToString(); \ - } \ +#define ASSERT_OK(expr) \ + do { \ + Status s = (expr); \ + if (!s.ok()) { FAIL() << s.ToString(); } \ } while (0) - -#define EXPECT_OK(expr) \ - do { \ - Status s = (expr); \ - EXPECT_TRUE(s.ok()); \ +#define EXPECT_OK(expr) \ + do { \ + Status s = (expr); \ + EXPECT_TRUE(s.ok()); \ } while (0) - namespace arrow { class TestBase : public ::testing::Test { public: - void SetUp() { - pool_ = default_memory_pool(); - } + void SetUp() { pool_ = default_memory_pool(); } template std::shared_ptr MakePrimitive(int32_t length, int32_t null_count = 0) { @@ -97,10 +88,8 @@ void randint(int64_t N, T lower, T upper, std::vector* out) { } } - template -void random_real(int n, uint32_t seed, T min_value, T max_value, - std::vector* out) { +void random_real(int n, uint32_t seed, T min_value, T max_value, std::vector* out) { std::mt19937 gen(seed); std::uniform_real_distribution d(min_value, max_value); for (int i = 0; i < n; ++i) { @@ -108,11 +97,10 @@ void random_real(int n, uint32_t seed, T min_value, T max_value, } } - template std::shared_ptr to_buffer(const std::vector& values) { - return std::make_shared(reinterpret_cast(values.data()), - values.size() * sizeof(T)); + return std::make_shared( + reinterpret_cast(values.data()), values.size() * sizeof(T)); } void random_null_bitmap(int64_t n, double pct_null, uint8_t* null_bitmap) { @@ -143,8 +131,8 @@ void rand_uniform_int(int n, uint32_t seed, T min_value, T max_value, T* out) { static inline int bitmap_popcount(const uint8_t* data, int length) { int count = 0; for (int i = 0; i < length; ++i) { - // TODO: accelerate this - if (util::get_bit(data, i)) ++count; + // TODO(wesm): accelerate this + if (util::get_bit(data, i)) { ++count; } } return count; } @@ -152,9 +140,7 @@ static inline int bitmap_popcount(const uint8_t* data, int length) { static inline int null_count(const std::vector& valid_bytes) { int result = 0; for (size_t i = 0; i < valid_bytes.size(); ++i) { - if (valid_bytes[i] == 0) { - ++result; - } + if (valid_bytes[i] == 0) { ++result; } } return result; } @@ -167,7 +153,7 @@ std::shared_ptr bytes_to_null_buffer(const std::vector& bytes) return out; } -} // namespace test -} // namespace arrow +} // namespace test +} // namespace arrow -#endif // ARROW_TEST_UTIL_H_ +#endif // ARROW_TEST_UTIL_H_ diff --git a/cpp/src/arrow/type.cc b/cpp/src/arrow/type.cc index f7f835e96a729..4e686d9cf4a6f 100644 --- a/cpp/src/arrow/type.cc +++ b/cpp/src/arrow/type.cc @@ -25,9 +25,7 @@ namespace arrow { std::string Field::ToString() const { std::stringstream ss; ss << this->name << ": " << this->type->ToString(); - if (!this->nullable) { - ss << " not null"; - } + if (!this->nullable) { ss << " not null"; } return ss.str(); } @@ -50,7 +48,7 @@ std::string StructType::ToString() const { std::stringstream s; s << "struct<"; for (int i = 0; i < this->num_children(); ++i) { - if (i > 0) s << ", "; + if (i > 0) { s << ", "; } const std::shared_ptr& field = this->child(i); s << field->name << ": " << field->type->ToString(); } @@ -58,4 +56,4 @@ std::string StructType::ToString() const { return s.str(); } -} // namespace arrow +} // namespace arrow diff --git a/cpp/src/arrow/type.h b/cpp/src/arrow/type.h index 86e47791b7cea..051ab46b199f9 100644 --- a/cpp/src/arrow/type.h +++ b/cpp/src/arrow/type.h @@ -110,8 +110,7 @@ struct DataType { std::vector> children_; - explicit DataType(Type::type type) : - type(type) {} + explicit DataType(Type::type type) : type(type) {} virtual ~DataType(); @@ -120,21 +119,13 @@ struct DataType { return this == other || (this->type == other->type); } - bool Equals(const std::shared_ptr& other) { - return Equals(other.get()); - } + bool Equals(const std::shared_ptr& other) { return Equals(other.get()); } - const std::shared_ptr& child(int i) const { - return children_[i]; - } + const std::shared_ptr& child(int i) const { return children_[i]; } - int num_children() const { - return children_.size(); - } + int num_children() const { return children_.size(); } - virtual int value_size() const { - return -1; - } + virtual int value_size() const { return -1; } virtual std::string ToString() const = 0; }; @@ -153,28 +144,20 @@ struct Field { // Fields can be nullable bool nullable; - Field(const std::string& name, const TypePtr& type, bool nullable = true) : - name(name), - type(type), - nullable(nullable) {} + Field(const std::string& name, const TypePtr& type, bool nullable = true) + : name(name), type(type), nullable(nullable) {} - bool operator==(const Field& other) const { - return this->Equals(other); - } + bool operator==(const Field& other) const { return this->Equals(other); } - bool operator!=(const Field& other) const { - return !this->Equals(other); - } + bool operator!=(const Field& other) const { return !this->Equals(other); } bool Equals(const Field& other) const { - return (this == &other) || (this->name == other.name && - this->nullable == other.nullable && - this->type->Equals(other.type.get())); + return (this == &other) || + (this->name == other.name && this->nullable == other.nullable && + this->type->Equals(other.type.get())); } - bool Equals(const std::shared_ptr& other) const { - return Equals(*other.get()); - } + bool Equals(const std::shared_ptr& other) const { return Equals(*other.get()); } std::string ToString() const; }; @@ -192,20 +175,15 @@ inline std::string PrimitiveType::ToString() const { return result; } -#define PRIMITIVE_DECL(TYPENAME, C_TYPE, ENUM, SIZE, NAME) \ - typedef C_TYPE c_type; \ - static constexpr Type::type type_enum = Type::ENUM; \ - \ - TYPENAME() \ - : PrimitiveType() {} \ - \ - virtual int value_size() const { \ - return SIZE; \ - } \ - \ - static const char* name() { \ - return NAME; \ - } +#define PRIMITIVE_DECL(TYPENAME, C_TYPE, ENUM, SIZE, NAME) \ + typedef C_TYPE c_type; \ + static constexpr Type::type type_enum = Type::ENUM; \ + \ + TYPENAME() : PrimitiveType() {} \ + \ + virtual int value_size() const { return SIZE; } \ + \ + static const char* name() { return NAME; } struct NullType : public PrimitiveType { PRIMITIVE_DECL(NullType, void, NA, 0, "null"); @@ -257,27 +235,19 @@ struct DoubleType : public PrimitiveType { struct ListType : public DataType { // List can contain any other logical value type - explicit ListType(const std::shared_ptr& value_type) - : DataType(Type::LIST) { + explicit ListType(const std::shared_ptr& value_type) : DataType(Type::LIST) { children_ = {std::make_shared("item", value_type)}; } - explicit ListType(const std::shared_ptr& value_field) - : DataType(Type::LIST) { + explicit ListType(const std::shared_ptr& value_field) : DataType(Type::LIST) { children_ = {value_field}; } - const std::shared_ptr& value_field() const { - return children_[0]; - } + const std::shared_ptr& value_field() const { return children_[0]; } - const std::shared_ptr& value_type() const { - return children_[0]->type; - } + const std::shared_ptr& value_type() const { return children_[0]->type; } - static char const *name() { - return "list"; - } + static char const* name() { return "list"; } std::string ToString() const override; }; @@ -286,9 +256,7 @@ struct ListType : public DataType { struct StringType : public DataType { StringType(); - static char const *name() { - return "string"; - } + static char const* name() { return "string"; } std::string ToString() const override; }; @@ -304,10 +272,8 @@ struct StructType : public DataType { // These will be defined elsewhere template -struct type_traits { -}; - +struct type_traits {}; -} // namespace arrow +} // namespace arrow #endif // ARROW_TYPE_H diff --git a/cpp/src/arrow/types/binary.h b/cpp/src/arrow/types/binary.h index 1fd675e5fdebf..201fbb6e79536 100644 --- a/cpp/src/arrow/types/binary.h +++ b/cpp/src/arrow/types/binary.h @@ -23,8 +23,6 @@ #include "arrow/type.h" -namespace arrow { +namespace arrow {} // namespace arrow -} // namespace arrow - -#endif // ARROW_TYPES_BINARY_H +#endif // ARROW_TYPES_BINARY_H diff --git a/cpp/src/arrow/types/collection.h b/cpp/src/arrow/types/collection.h index 46d84f1f183c8..1712030203fa2 100644 --- a/cpp/src/arrow/types/collection.h +++ b/cpp/src/arrow/types/collection.h @@ -31,15 +31,11 @@ struct CollectionType : public DataType { CollectionType() : DataType(T) {} - const TypePtr& child(int i) const { - return child_types_[i]; - } + const TypePtr& child(int i) const { return child_types_[i]; } - int num_children() const { - return child_types_.size(); - } + int num_children() const { return child_types_.size(); } }; -} // namespace arrow +} // namespace arrow -#endif // ARROW_TYPES_COLLECTION_H +#endif // ARROW_TYPES_COLLECTION_H diff --git a/cpp/src/arrow/types/construct.cc b/cpp/src/arrow/types/construct.cc index 34647a5005b90..0a30929b97c51 100644 --- a/cpp/src/arrow/types/construct.cc +++ b/cpp/src/arrow/types/construct.cc @@ -30,10 +30,10 @@ namespace arrow { class ArrayBuilder; -#define BUILDER_CASE(ENUM, BuilderType) \ - case Type::ENUM: \ - out->reset(new BuilderType(pool, type)); \ - return Status::OK(); +#define BUILDER_CASE(ENUM, BuilderType) \ + case Type::ENUM: \ + out->reset(new BuilderType(pool, type)); \ + return Status::OK(); // Initially looked at doing this with vtables, but shared pointers makes it // difficult @@ -58,30 +58,28 @@ Status MakeBuilder(MemoryPool* pool, const std::shared_ptr& type, BUILDER_CASE(STRING, StringBuilder); - case Type::LIST: - { - std::shared_ptr value_builder; + case Type::LIST: { + std::shared_ptr value_builder; - const std::shared_ptr& value_type = static_cast( - type.get())->value_type(); - RETURN_NOT_OK(MakeBuilder(pool, value_type, &value_builder)); - out->reset(new ListBuilder(pool, type, value_builder)); - return Status::OK(); - } + const std::shared_ptr& value_type = + static_cast(type.get())->value_type(); + RETURN_NOT_OK(MakeBuilder(pool, value_type, &value_builder)); + out->reset(new ListBuilder(pool, type, value_builder)); + return Status::OK(); + } default: return Status::NotImplemented(type->ToString()); } } -#define MAKE_PRIMITIVE_ARRAY_CASE(ENUM, ArrayType) \ - case Type::ENUM: \ - out->reset(new ArrayType(type, length, data, null_count, null_bitmap)); \ - return Status::OK(); +#define MAKE_PRIMITIVE_ARRAY_CASE(ENUM, ArrayType) \ + case Type::ENUM: \ + out->reset(new ArrayType(type, length, data, null_count, null_bitmap)); \ + return Status::OK(); -Status MakePrimitiveArray(const std::shared_ptr& type, - int32_t length, const std::shared_ptr& data, - int32_t null_count, const std::shared_ptr& null_bitmap, - std::shared_ptr* out) { +Status MakePrimitiveArray(const std::shared_ptr& type, int32_t length, + const std::shared_ptr& data, int32_t null_count, + const std::shared_ptr& null_bitmap, std::shared_ptr* out) { switch (type->type) { MAKE_PRIMITIVE_ARRAY_CASE(BOOL, BooleanArray); MAKE_PRIMITIVE_ARRAY_CASE(UINT8, UInt8Array); @@ -99,4 +97,4 @@ Status MakePrimitiveArray(const std::shared_ptr& type, } } -} // namespace arrow +} // namespace arrow diff --git a/cpp/src/arrow/types/construct.h b/cpp/src/arrow/types/construct.h index 228faeccc4e4d..27fb7bd2149cf 100644 --- a/cpp/src/arrow/types/construct.h +++ b/cpp/src/arrow/types/construct.h @@ -33,11 +33,10 @@ class Status; Status MakeBuilder(MemoryPool* pool, const std::shared_ptr& type, std::shared_ptr* out); -Status MakePrimitiveArray(const std::shared_ptr& type, - int32_t length, const std::shared_ptr& data, - int32_t null_count, const std::shared_ptr& null_bitmap, - std::shared_ptr* out); +Status MakePrimitiveArray(const std::shared_ptr& type, int32_t length, + const std::shared_ptr& data, int32_t null_count, + const std::shared_ptr& null_bitmap, std::shared_ptr* out); -} // namespace arrow +} // namespace arrow -#endif // ARROW_BUILDER_H_ +#endif // ARROW_BUILDER_H_ diff --git a/cpp/src/arrow/types/datetime.h b/cpp/src/arrow/types/datetime.h index e57b66ab46adb..b782455546c33 100644 --- a/cpp/src/arrow/types/datetime.h +++ b/cpp/src/arrow/types/datetime.h @@ -23,49 +23,30 @@ namespace arrow { struct DateType : public DataType { - enum class Unit: char { - DAY = 0, - MONTH = 1, - YEAR = 2 - }; + enum class Unit : char { DAY = 0, MONTH = 1, YEAR = 2 }; Unit unit; - explicit DateType(Unit unit = Unit::DAY) - : DataType(Type::DATE), - unit(unit) {} + explicit DateType(Unit unit = Unit::DAY) : DataType(Type::DATE), unit(unit) {} - DateType(const DateType& other) - : DateType(other.unit) {} + DateType(const DateType& other) : DateType(other.unit) {} - static char const *name() { - return "date"; - } + static char const* name() { return "date"; } }; - struct TimestampType : public DataType { - enum class Unit: char { - SECOND = 0, - MILLI = 1, - MICRO = 2, - NANO = 3 - }; + enum class Unit : char { SECOND = 0, MILLI = 1, MICRO = 2, NANO = 3 }; Unit unit; explicit TimestampType(Unit unit = Unit::MILLI) - : DataType(Type::TIMESTAMP), - unit(unit) {} + : DataType(Type::TIMESTAMP), unit(unit) {} - TimestampType(const TimestampType& other) - : TimestampType(other.unit) {} + TimestampType(const TimestampType& other) : TimestampType(other.unit) {} - static char const *name() { - return "timestamp"; - } + static char const* name() { return "timestamp"; } }; -} // namespace arrow +} // namespace arrow -#endif // ARROW_TYPES_DATETIME_H +#endif // ARROW_TYPES_DATETIME_H diff --git a/cpp/src/arrow/types/decimal-test.cc b/cpp/src/arrow/types/decimal-test.cc index 89896c8b425d0..7296ff8176113 100644 --- a/cpp/src/arrow/types/decimal-test.cc +++ b/cpp/src/arrow/types/decimal-test.cc @@ -37,4 +37,4 @@ TEST(TypesTest, TestDecimalType) { ASSERT_EQ(t2.scale, 4); } -} // namespace arrow +} // namespace arrow diff --git a/cpp/src/arrow/types/decimal.cc b/cpp/src/arrow/types/decimal.cc index f120c1a9dfde6..1d9a5e50e460b 100644 --- a/cpp/src/arrow/types/decimal.cc +++ b/cpp/src/arrow/types/decimal.cc @@ -28,5 +28,4 @@ std::string DecimalType::ToString() const { return s.str(); } -} // namespace arrow - +} // namespace arrow diff --git a/cpp/src/arrow/types/decimal.h b/cpp/src/arrow/types/decimal.h index 26243b42b0e7d..1be489d4f51b6 100644 --- a/cpp/src/arrow/types/decimal.h +++ b/cpp/src/arrow/types/decimal.h @@ -26,18 +26,15 @@ namespace arrow { struct DecimalType : public DataType { explicit DecimalType(int precision_, int scale_) - : DataType(Type::DECIMAL), precision(precision_), - scale(scale_) { } + : DataType(Type::DECIMAL), precision(precision_), scale(scale_) {} int precision; int scale; - static char const *name() { - return "decimal"; - } + static char const* name() { return "decimal"; } std::string ToString() const override; }; -} // namespace arrow +} // namespace arrow -#endif // ARROW_TYPES_DECIMAL_H +#endif // ARROW_TYPES_DECIMAL_H diff --git a/cpp/src/arrow/types/json.cc b/cpp/src/arrow/types/json.cc index fb731edd6073f..a4e0d085620a0 100644 --- a/cpp/src/arrow/types/json.cc +++ b/cpp/src/arrow/types/json.cc @@ -30,9 +30,8 @@ static const TypePtr String(new StringType()); static const TypePtr Double(new DoubleType()); static const TypePtr Bool(new BooleanType()); -static const std::vector json_types = {Null, Int32, String, - Double, Bool}; +static const std::vector json_types = {Null, Int32, String, Double, Bool}; TypePtr JSONScalar::dense_type = TypePtr(new DenseUnionType(json_types)); TypePtr JSONScalar::sparse_type = TypePtr(new SparseUnionType(json_types)); -} // namespace arrow +} // namespace arrow diff --git a/cpp/src/arrow/types/json.h b/cpp/src/arrow/types/json.h index 9c850afac0af4..9de961f79a60a 100644 --- a/cpp/src/arrow/types/json.h +++ b/cpp/src/arrow/types/json.h @@ -28,11 +28,9 @@ struct JSONScalar : public DataType { static TypePtr dense_type; static TypePtr sparse_type; - explicit JSONScalar(bool dense = true) - : DataType(Type::JSON_SCALAR), - dense(dense) {} + explicit JSONScalar(bool dense = true) : DataType(Type::JSON_SCALAR), dense(dense) {} }; -} // namespace arrow +} // namespace arrow -#endif // ARROW_TYPES_JSON_H +#endif // ARROW_TYPES_JSON_H diff --git a/cpp/src/arrow/types/list-test.cc b/cpp/src/arrow/types/list-test.cc index 4eb560ea52256..aa34f23cc0230 100644 --- a/cpp/src/arrow/types/list-test.cc +++ b/cpp/src/arrow/types/list-test.cc @@ -76,9 +76,7 @@ class TestListBuilder : public TestBuilder { builder_ = std::dynamic_pointer_cast(tmp); } - void Done() { - result_ = std::dynamic_pointer_cast(builder_->Finish()); - } + void Done() { result_ = std::dynamic_pointer_cast(builder_->Finish()); } protected: TypePtr value_type_; @@ -88,9 +86,7 @@ class TestListBuilder : public TestBuilder { shared_ptr result_; }; - -TEST_F(TestListBuilder, TestResize) { -} +TEST_F(TestListBuilder, TestResize) {} TEST_F(TestListBuilder, TestAppendNull) { ASSERT_OK(builder_->AppendNull()); @@ -155,5 +151,4 @@ TEST_F(TestListBuilder, TestZeroLength) { Done(); } - -} // namespace arrow +} // namespace arrow diff --git a/cpp/src/arrow/types/list.cc b/cpp/src/arrow/types/list.cc index d64c06d90c174..23f12ddc4ecd7 100644 --- a/cpp/src/arrow/types/list.cc +++ b/cpp/src/arrow/types/list.cc @@ -20,32 +20,25 @@ namespace arrow { bool ListArray::EqualsExact(const ListArray& other) const { - if (this == &other) return true; - if (null_count_ != other.null_count_) { - return false; - } + if (this == &other) { return true; } + if (null_count_ != other.null_count_) { return false; } - bool equal_offsets = offset_buf_->Equals(*other.offset_buf_, - length_ + 1); + bool equal_offsets = offset_buf_->Equals(*other.offset_buf_, length_ + 1); bool equal_null_bitmap = true; if (null_count_ > 0) { - equal_null_bitmap = null_bitmap_->Equals(*other.null_bitmap_, - util::bytes_for_bits(length_)); + equal_null_bitmap = + null_bitmap_->Equals(*other.null_bitmap_, util::bytes_for_bits(length_)); } - if (!(equal_offsets && equal_null_bitmap)) { - return false; - } + if (!(equal_offsets && equal_null_bitmap)) { return false; } return values()->Equals(other.values()); } bool ListArray::Equals(const std::shared_ptr& arr) const { - if (this == arr.get()) return true; - if (this->type_enum() != arr->type_enum()) { - return false; - } + if (this == arr.get()) { return true; } + if (this->type_enum() != arr->type_enum()) { return false; } return EqualsExact(*static_cast(arr.get())); } -} // namespace arrow +} // namespace arrow diff --git a/cpp/src/arrow/types/list.h b/cpp/src/arrow/types/list.h index 8073b5121764d..6b815460ecb1e 100644 --- a/cpp/src/arrow/types/list.h +++ b/cpp/src/arrow/types/list.h @@ -37,13 +37,12 @@ class MemoryPool; class ListArray : public Array { public: ListArray(const TypePtr& type, int32_t length, std::shared_ptr offsets, - const ArrayPtr& values, - int32_t null_count = 0, - std::shared_ptr null_bitmap = nullptr) : - Array(type, length, null_count, null_bitmap) { + const ArrayPtr& values, int32_t null_count = 0, + std::shared_ptr null_bitmap = nullptr) + : Array(type, length, null_count, null_bitmap) { offset_buf_ = offsets; - offsets_ = offsets == nullptr? nullptr : - reinterpret_cast(offset_buf_->data()); + offsets_ = offsets == nullptr ? nullptr + : reinterpret_cast(offset_buf_->data()); values_ = values; } @@ -51,19 +50,17 @@ class ListArray : public Array { // Return a shared pointer in case the requestor desires to share ownership // with this array. - const std::shared_ptr& values() const {return values_;} + const std::shared_ptr& values() const { return values_; } - const std::shared_ptr& value_type() const { - return values_->type(); - } + const std::shared_ptr& value_type() const { return values_->type(); } - const int32_t* offsets() const { return offsets_;} + const int32_t* offsets() const { return offsets_; } - int32_t offset(int i) const { return offsets_[i];} + int32_t offset(int i) const { return offsets_[i]; } // Neither of these functions will perform boundschecking - int32_t value_offset(int i) { return offsets_[i];} - int32_t value_length(int i) { return offsets_[i + 1] - offsets_[i];} + int32_t value_offset(int i) { return offsets_[i]; } + int32_t value_length(int i) { return offsets_[i + 1] - offsets_[i]; } bool EqualsExact(const ListArray& other) const; bool Equals(const std::shared_ptr& arr) const override; @@ -77,7 +74,6 @@ class ListArray : public Array { // ---------------------------------------------------------------------- // Array builder - // Builder class for variable-length list array value types // // To use this class, you must append values to the child array builder and use @@ -85,10 +81,9 @@ class ListArray : public Array { // have been appended to the child array) class ListBuilder : public Int32Builder { public: - ListBuilder(MemoryPool* pool, const TypePtr& type, - std::shared_ptr value_builder) - : Int32Builder(pool, type), - value_builder_(value_builder) {} + ListBuilder( + MemoryPool* pool, const TypePtr& type, std::shared_ptr value_builder) + : Int32Builder(pool, type), value_builder_(value_builder) {} Status Init(int32_t elements) { // One more than requested. @@ -116,12 +111,9 @@ class ListBuilder : public Int32Builder { int32_t new_capacity = util::next_power2(length_ + length); RETURN_NOT_OK(Resize(new_capacity)); } - memcpy(raw_data_ + length_, values, - type_traits::bytes_required(length)); + memcpy(raw_data_ + length_, values, type_traits::bytes_required(length)); - if (valid_bytes != nullptr) { - AppendNulls(valid_bytes, length); - } + if (valid_bytes != nullptr) { AppendNulls(valid_bytes, length); } length_ += length; return Status::OK(); @@ -132,12 +124,10 @@ class ListBuilder : public Int32Builder { std::shared_ptr items = value_builder_->Finish(); // Add final offset if the length is non-zero - if (length_) { - raw_data_[length_] = items->length(); - } + if (length_) { raw_data_[length_] = items->length(); } - auto result = std::make_shared(type_, length_, data_, items, - null_count_, null_bitmap_); + auto result = std::make_shared( + type_, length_, data_, items, null_count_, null_bitmap_); data_ = null_bitmap_ = nullptr; capacity_ = length_ = null_count_ = 0; @@ -145,9 +135,7 @@ class ListBuilder : public Int32Builder { return result; } - std::shared_ptr Finish() override { - return Transfer(); - } + std::shared_ptr Finish() override { return Transfer(); } // Start a new variable-length list slot // @@ -167,19 +155,14 @@ class ListBuilder : public Int32Builder { return Status::OK(); } - Status AppendNull() { - return Append(true); - } + Status AppendNull() { return Append(true); } - const std::shared_ptr& value_builder() const { - return value_builder_; - } + const std::shared_ptr& value_builder() const { return value_builder_; } protected: std::shared_ptr value_builder_; }; +} // namespace arrow -} // namespace arrow - -#endif // ARROW_TYPES_LIST_H +#endif // ARROW_TYPES_LIST_H diff --git a/cpp/src/arrow/types/primitive-test.cc b/cpp/src/arrow/types/primitive-test.cc index 761845d93812a..6bd9e73eb46ac 100644 --- a/cpp/src/arrow/types/primitive-test.cc +++ b/cpp/src/arrow/types/primitive-test.cc @@ -41,15 +41,15 @@ namespace arrow { class Array; -#define PRIMITIVE_TEST(KLASS, ENUM, NAME) \ - TEST(TypesTest, TestPrimitive_##ENUM) { \ - KLASS tp; \ - \ - ASSERT_EQ(tp.type, Type::ENUM); \ - ASSERT_EQ(tp.name(), string(NAME)); \ - \ - KLASS tp_copy = tp; \ - ASSERT_EQ(tp_copy.type, Type::ENUM); \ +#define PRIMITIVE_TEST(KLASS, ENUM, NAME) \ + TEST(TypesTest, TestPrimitive_##ENUM) { \ + KLASS tp; \ + \ + ASSERT_EQ(tp.type, Type::ENUM); \ + ASSERT_EQ(tp.name(), string(NAME)); \ + \ + KLASS tp_copy = tp; \ + ASSERT_EQ(tp_copy.type, Type::ENUM); \ } PRIMITIVE_TEST(Int8Type, INT8, "int8"); @@ -108,8 +108,8 @@ class TestPrimitiveBuilder : public TestBuilder { void Check(const std::shared_ptr& builder, bool nullable) { int size = builder->length(); - auto ex_data = std::make_shared(reinterpret_cast(draws_.data()), - size * sizeof(T)); + auto ex_data = std::make_shared( + reinterpret_cast(draws_.data()), size * sizeof(T)); std::shared_ptr ex_null_bitmap; int32_t ex_null_count = 0; @@ -121,10 +121,10 @@ class TestPrimitiveBuilder : public TestBuilder { ex_null_bitmap = nullptr; } - auto expected = std::make_shared(size, ex_data, ex_null_count, - ex_null_bitmap); - std::shared_ptr result = std::dynamic_pointer_cast( - builder->Finish()); + auto expected = + std::make_shared(size, ex_data, ex_null_count, ex_null_bitmap); + std::shared_ptr result = + std::dynamic_pointer_cast(builder->Finish()); // Builder is now reset ASSERT_EQ(0, builder->length()); @@ -145,30 +145,30 @@ class TestPrimitiveBuilder : public TestBuilder { vector valid_bytes_; }; -#define PTYPE_DECL(CapType, c_type) \ - typedef CapType##Array ArrayType; \ - typedef CapType##Builder BuilderType; \ - typedef CapType##Type Type; \ - typedef c_type T; \ - \ - static std::shared_ptr type() { \ - return std::shared_ptr(new Type()); \ +#define PTYPE_DECL(CapType, c_type) \ + typedef CapType##Array ArrayType; \ + typedef CapType##Builder BuilderType; \ + typedef CapType##Type Type; \ + typedef c_type T; \ + \ + static std::shared_ptr type() { \ + return std::shared_ptr(new Type()); \ } -#define PINT_DECL(CapType, c_type, LOWER, UPPER) \ - struct P##CapType { \ - PTYPE_DECL(CapType, c_type); \ - static void draw(int N, vector* draws) { \ - test::randint(N, LOWER, UPPER, draws); \ - } \ +#define PINT_DECL(CapType, c_type, LOWER, UPPER) \ + struct P##CapType { \ + PTYPE_DECL(CapType, c_type); \ + static void draw(int N, vector* draws) { \ + test::randint(N, LOWER, UPPER, draws); \ + } \ } -#define PFLOAT_DECL(CapType, c_type, LOWER, UPPER) \ - struct P##CapType { \ - PTYPE_DECL(CapType, c_type); \ - static void draw(int N, vector* draws) { \ - test::random_real(N, 0, LOWER, UPPER, draws); \ - } \ +#define PFLOAT_DECL(CapType, c_type, LOWER, UPPER) \ + struct P##CapType { \ + PTYPE_DECL(CapType, c_type); \ + static void draw(int N, vector* draws) { \ + test::random_real(N, 0, LOWER, UPPER, draws); \ + } \ } PINT_DECL(UInt8, uint8_t, 0, UINT8_MAX); @@ -214,10 +214,10 @@ void TestPrimitiveBuilder::Check( ex_null_bitmap = nullptr; } - auto expected = std::make_shared(size, ex_data, ex_null_count, - ex_null_bitmap); - std::shared_ptr result = std::dynamic_pointer_cast( - builder->Finish()); + auto expected = + std::make_shared(size, ex_data, ex_null_count, ex_null_bitmap); + std::shared_ptr result = + std::dynamic_pointer_cast(builder->Finish()); // Builder is now reset ASSERT_EQ(0, builder->length()); @@ -230,31 +230,23 @@ void TestPrimitiveBuilder::Check( ASSERT_EQ(expected->length(), result->length()); for (int i = 0; i < result->length(); ++i) { - if (nullable) { - ASSERT_EQ(valid_bytes_[i] == 0, result->IsNull(i)) << i; - } + if (nullable) { ASSERT_EQ(valid_bytes_[i] == 0, result->IsNull(i)) << i; } bool actual = util::get_bit(result->raw_data(), i); ASSERT_EQ(static_cast(draws_[i]), actual) << i; } ASSERT_TRUE(result->EqualsExact(*expected.get())); } -typedef ::testing::Types Primitives; +typedef ::testing::Types Primitives; TYPED_TEST_CASE(TestPrimitiveBuilder, Primitives); -#define DECL_T() \ - typedef typename TestFixture::T T; +#define DECL_T() typedef typename TestFixture::T T; -#define DECL_TYPE() \ - typedef typename TestFixture::Type Type; - -#define DECL_ARRAYTYPE() \ - typedef typename TestFixture::ArrayType ArrayType; +#define DECL_TYPE() typedef typename TestFixture::Type Type; +#define DECL_ARRAYTYPE() typedef typename TestFixture::ArrayType ArrayType; TYPED_TEST(TestPrimitiveBuilder, TestInit) { DECL_TYPE(); @@ -369,7 +361,6 @@ TYPED_TEST(TestPrimitiveBuilder, TestAppendScalar) { this->Check(this->builder_nn_, false); } - TYPED_TEST(TestPrimitiveBuilder, TestAppendVector) { DECL_T(); @@ -424,8 +415,7 @@ TYPED_TEST(TestPrimitiveBuilder, TestResize) { ASSERT_EQ(cap, this->builder_->capacity()); ASSERT_EQ(type_traits::bytes_required(cap), this->builder_->data()->size()); - ASSERT_EQ(util::bytes_for_bits(cap), - this->builder_->null_bitmap()->size()); + ASSERT_EQ(util::bytes_for_bits(cap), this->builder_->null_bitmap()->size()); } TYPED_TEST(TestPrimitiveBuilder, TestReserve) { @@ -437,8 +427,7 @@ TYPED_TEST(TestPrimitiveBuilder, TestReserve) { ASSERT_OK(this->builder_->Advance(100)); ASSERT_OK(this->builder_->Reserve(MIN_BUILDER_CAPACITY)); - ASSERT_EQ(util::next_power2(MIN_BUILDER_CAPACITY + 100), - this->builder_->capacity()); + ASSERT_EQ(util::next_power2(MIN_BUILDER_CAPACITY + 100), this->builder_->capacity()); } -} // namespace arrow +} // namespace arrow diff --git a/cpp/src/arrow/types/primitive.cc b/cpp/src/arrow/types/primitive.cc index c54d0757c4789..9549c47b41157 100644 --- a/cpp/src/arrow/types/primitive.cc +++ b/cpp/src/arrow/types/primitive.cc @@ -28,26 +28,21 @@ namespace arrow { // Primitive array base PrimitiveArray::PrimitiveArray(const TypePtr& type, int32_t length, - const std::shared_ptr& data, - int32_t null_count, - const std::shared_ptr& null_bitmap) : - Array(type, length, null_count, null_bitmap) { + const std::shared_ptr& data, int32_t null_count, + const std::shared_ptr& null_bitmap) + : Array(type, length, null_count, null_bitmap) { data_ = data; - raw_data_ = data == nullptr? nullptr : data_->data(); + raw_data_ = data == nullptr ? nullptr : data_->data(); } bool PrimitiveArray::EqualsExact(const PrimitiveArray& other) const { - if (this == &other) return true; - if (null_count_ != other.null_count_) { - return false; - } + if (this == &other) { return true; } + if (null_count_ != other.null_count_) { return false; } if (null_count_ > 0) { - bool equal_bitmap = null_bitmap_->Equals(*other.null_bitmap_, - util::ceil_byte(length_) / 8); - if (!equal_bitmap) { - return false; - } + bool equal_bitmap = + null_bitmap_->Equals(*other.null_bitmap_, util::ceil_byte(length_) / 8); + if (!equal_bitmap) { return false; } const uint8_t* this_data = raw_data_; const uint8_t* other_data = other.raw_data_; @@ -56,9 +51,7 @@ bool PrimitiveArray::EqualsExact(const PrimitiveArray& other) const { DCHECK_GT(value_size, 0); for (int i = 0; i < length_; ++i) { - if (!IsNull(i) && memcmp(this_data, other_data, value_size)) { - return false; - } + if (!IsNull(i) && memcmp(this_data, other_data, value_size)) { return false; } this_data += value_size; other_data += value_size; } @@ -69,10 +62,8 @@ bool PrimitiveArray::EqualsExact(const PrimitiveArray& other) const { } bool PrimitiveArray::Equals(const std::shared_ptr& arr) const { - if (this == arr.get()) return true; - if (this->type_enum() != arr->type_enum()) { - return false; - } + if (this == arr.get()) { return true; } + if (this->type_enum() != arr->type_enum()) { return false; } return EqualsExact(*static_cast(arr.get())); } @@ -92,9 +83,7 @@ Status PrimitiveBuilder::Init(int32_t capacity) { template Status PrimitiveBuilder::Resize(int32_t capacity) { // XXX: Set floor size for now - if (capacity < MIN_BUILDER_CAPACITY) { - capacity = MIN_BUILDER_CAPACITY; - } + if (capacity < MIN_BUILDER_CAPACITY) { capacity = MIN_BUILDER_CAPACITY; } if (capacity_ == 0) { RETURN_NOT_OK(Init(capacity)); @@ -122,8 +111,8 @@ Status PrimitiveBuilder::Reserve(int32_t elements) { } template -Status PrimitiveBuilder::Append(const value_type* values, int32_t length, - const uint8_t* valid_bytes) { +Status PrimitiveBuilder::Append( + const value_type* values, int32_t length, const uint8_t* valid_bytes) { RETURN_NOT_OK(PrimitiveBuilder::Reserve(length)); if (length > 0) { @@ -156,9 +145,8 @@ void PrimitiveBuilder::AppendNulls(const uint8_t* valid_bytes, int32_t length template std::shared_ptr PrimitiveBuilder::Finish() { - std::shared_ptr result = std::make_shared< - typename type_traits::ArrayType>( - type_, length_, data_, null_count_, null_bitmap_); + std::shared_ptr result = std::make_shared::ArrayType>( + type_, length_, data_, null_count_, null_bitmap_); data_ = null_bitmap_ = nullptr; capacity_ = length_ = null_count_ = 0; @@ -166,8 +154,8 @@ std::shared_ptr PrimitiveBuilder::Finish() { } template <> -Status PrimitiveBuilder::Append(const uint8_t* values, int32_t length, - const uint8_t* valid_bytes) { +Status PrimitiveBuilder::Append( + const uint8_t* values, int32_t length, const uint8_t* valid_bytes) { RETURN_NOT_OK(Reserve(length)); for (int i = 0; i < length; ++i) { @@ -202,23 +190,18 @@ template class PrimitiveBuilder; template class PrimitiveBuilder; BooleanArray::BooleanArray(int32_t length, const std::shared_ptr& data, - int32_t null_count, - const std::shared_ptr& null_bitmap) : - PrimitiveArray(std::make_shared(), length, - data, null_count, null_bitmap) {} + int32_t null_count, const std::shared_ptr& null_bitmap) + : PrimitiveArray( + std::make_shared(), length, data, null_count, null_bitmap) {} bool BooleanArray::EqualsExact(const BooleanArray& other) const { if (this == &other) return true; - if (null_count_ != other.null_count_) { - return false; - } + if (null_count_ != other.null_count_) { return false; } if (null_count_ > 0) { - bool equal_bitmap = null_bitmap_->Equals(*other.null_bitmap_, - util::bytes_for_bits(length_)); - if (!equal_bitmap) { - return false; - } + bool equal_bitmap = + null_bitmap_->Equals(*other.null_bitmap_, util::bytes_for_bits(length_)); + if (!equal_bitmap) { return false; } const uint8_t* this_data = raw_data_; const uint8_t* other_data = other.raw_data_; @@ -236,10 +219,8 @@ bool BooleanArray::EqualsExact(const BooleanArray& other) const { bool BooleanArray::Equals(const std::shared_ptr& arr) const { if (this == arr.get()) return true; - if (Type::BOOL != arr->type_enum()) { - return false; - } + if (Type::BOOL != arr->type_enum()) { return false; } return EqualsExact(*static_cast(arr.get())); } -} // namespace arrow +} // namespace arrow diff --git a/cpp/src/arrow/types/primitive.h b/cpp/src/arrow/types/primitive.h index ec6fee35513ce..fcd3db4e96e53 100644 --- a/cpp/src/arrow/types/primitive.h +++ b/cpp/src/arrow/types/primitive.h @@ -34,17 +34,14 @@ namespace arrow { class MemoryPool; - // Base class for fixed-size logical types class PrimitiveArray : public Array { public: - PrimitiveArray(const TypePtr& type, int32_t length, - const std::shared_ptr& data, - int32_t null_count = 0, - const std::shared_ptr& null_bitmap = nullptr); + PrimitiveArray(const TypePtr& type, int32_t length, const std::shared_ptr& data, + int32_t null_count = 0, const std::shared_ptr& null_bitmap = nullptr); virtual ~PrimitiveArray() {} - const std::shared_ptr& data() const { return data_;} + const std::shared_ptr& data() const { return data_; } bool EqualsExact(const PrimitiveArray& other) const; bool Equals(const std::shared_ptr& arr) const override; @@ -54,31 +51,25 @@ class PrimitiveArray : public Array { const uint8_t* raw_data_; }; -#define NUMERIC_ARRAY_DECL(NAME, TypeClass, T) \ -class NAME : public PrimitiveArray { \ - public: \ - using value_type = T; \ - using PrimitiveArray::PrimitiveArray; \ - \ - NAME(int32_t length, const std::shared_ptr& data, \ - int32_t null_count = 0, \ - const std::shared_ptr& null_bitmap = nullptr) : \ - PrimitiveArray(std::make_shared(), length, \ - data, null_count, null_bitmap) {} \ - \ - bool EqualsExact(const NAME& other) const { \ - return PrimitiveArray::EqualsExact( \ - *static_cast(&other)); \ - } \ - \ - const T* raw_data() const { \ - return reinterpret_cast(raw_data_); \ - } \ - \ - T Value(int i) const { \ - return raw_data()[i]; \ - } \ -}; +#define NUMERIC_ARRAY_DECL(NAME, TypeClass, T) \ + class NAME : public PrimitiveArray { \ + public: \ + using value_type = T; \ + using PrimitiveArray::PrimitiveArray; \ + \ + NAME(int32_t length, const std::shared_ptr& data, int32_t null_count = 0, \ + const std::shared_ptr& null_bitmap = nullptr) \ + : PrimitiveArray( \ + std::make_shared(), length, data, null_count, null_bitmap) {} \ + \ + bool EqualsExact(const NAME& other) const { \ + return PrimitiveArray::EqualsExact(*static_cast(&other)); \ + } \ + \ + const T* raw_data() const { return reinterpret_cast(raw_data_); } \ + \ + T Value(int i) const { return raw_data()[i]; } \ + }; NUMERIC_ARRAY_DECL(UInt8Array, UInt8Type, uint8_t); NUMERIC_ARRAY_DECL(Int8Array, Int8Type, int8_t); @@ -96,9 +87,8 @@ class PrimitiveBuilder : public ArrayBuilder { public: typedef typename Type::c_type value_type; - explicit PrimitiveBuilder(MemoryPool* pool, const TypePtr& type) : - ArrayBuilder(pool, type), - data_(nullptr) {} + explicit PrimitiveBuilder(MemoryPool* pool, const TypePtr& type) + : ArrayBuilder(pool, type), data_(nullptr) {} virtual ~PrimitiveBuilder() {} @@ -117,16 +107,14 @@ class PrimitiveBuilder : public ArrayBuilder { return Status::OK(); } - std::shared_ptr data() const { - return data_; - } + std::shared_ptr data() const { return data_; } // Vector append // // If passed, valid_bytes is of equal length to values, and any zero byte // will be considered as a null for that slot - Status Append(const value_type* values, int32_t length, - const uint8_t* valid_bytes = nullptr); + Status Append( + const value_type* values, int32_t length, const uint8_t* valid_bytes = nullptr); // Ensure that builder can accommodate an additional number of // elements. Resizes if the current capacity is not sufficient @@ -172,89 +160,69 @@ template <> struct type_traits { typedef UInt8Array ArrayType; - static inline int bytes_required(int elements) { - return elements; - } + static inline int bytes_required(int elements) { return elements; } }; template <> struct type_traits { typedef Int8Array ArrayType; - static inline int bytes_required(int elements) { - return elements; - } + static inline int bytes_required(int elements) { return elements; } }; template <> struct type_traits { typedef UInt16Array ArrayType; - static inline int bytes_required(int elements) { - return elements * sizeof(uint16_t); - } + static inline int bytes_required(int elements) { return elements * sizeof(uint16_t); } }; template <> struct type_traits { typedef Int16Array ArrayType; - static inline int bytes_required(int elements) { - return elements * sizeof(int16_t); - } + static inline int bytes_required(int elements) { return elements * sizeof(int16_t); } }; template <> struct type_traits { typedef UInt32Array ArrayType; - static inline int bytes_required(int elements) { - return elements * sizeof(uint32_t); - } + static inline int bytes_required(int elements) { return elements * sizeof(uint32_t); } }; template <> struct type_traits { typedef Int32Array ArrayType; - static inline int bytes_required(int elements) { - return elements * sizeof(int32_t); - } + static inline int bytes_required(int elements) { return elements * sizeof(int32_t); } }; template <> struct type_traits { typedef UInt64Array ArrayType; - static inline int bytes_required(int elements) { - return elements * sizeof(uint64_t); - } + static inline int bytes_required(int elements) { return elements * sizeof(uint64_t); } }; template <> struct type_traits { typedef Int64Array ArrayType; - static inline int bytes_required(int elements) { - return elements * sizeof(int64_t); - } + static inline int bytes_required(int elements) { return elements * sizeof(int64_t); } }; template <> struct type_traits { typedef FloatArray ArrayType; - static inline int bytes_required(int elements) { - return elements * sizeof(float); - } + static inline int bytes_required(int elements) { return elements * sizeof(float); } }; template <> struct type_traits { typedef DoubleArray ArrayType; - static inline int bytes_required(int elements) { - return elements * sizeof(double); - } + static inline int bytes_required(int elements) { return elements * sizeof(double); } }; // Builders @@ -272,25 +240,19 @@ typedef NumericBuilder Int64Builder; typedef NumericBuilder FloatBuilder; typedef NumericBuilder DoubleBuilder; - class BooleanArray : public PrimitiveArray { public: using PrimitiveArray::PrimitiveArray; BooleanArray(int32_t length, const std::shared_ptr& data, - int32_t null_count = 0, - const std::shared_ptr& null_bitmap = nullptr); + int32_t null_count = 0, const std::shared_ptr& null_bitmap = nullptr); bool EqualsExact(const BooleanArray& other) const; bool Equals(const std::shared_ptr& arr) const override; - const uint8_t* raw_data() const { - return reinterpret_cast(raw_data_); - } + const uint8_t* raw_data() const { return reinterpret_cast(raw_data_); } - bool Value(int i) const { - return util::get_bit(raw_data(), i); - } + bool Value(int i) const { return util::get_bit(raw_data(), i); } }; template <> @@ -304,8 +266,8 @@ struct type_traits { class BooleanBuilder : public PrimitiveBuilder { public: - explicit BooleanBuilder(MemoryPool* pool, const TypePtr& type) : - PrimitiveBuilder(pool, type) {} + explicit BooleanBuilder(MemoryPool* pool, const TypePtr& type) + : PrimitiveBuilder(pool, type) {} virtual ~BooleanBuilder() {} @@ -322,11 +284,9 @@ class BooleanBuilder : public PrimitiveBuilder { ++length_; } - void Append(uint8_t val) { - Append(static_cast(val)); - } + void Append(uint8_t val) { Append(static_cast(val)); } }; -} // namespace arrow +} // namespace arrow #endif // ARROW_TYPES_PRIMITIVE_H diff --git a/cpp/src/arrow/types/string-test.cc b/cpp/src/arrow/types/string-test.cc index d3a4cc37f9c4c..ee4307c4d168a 100644 --- a/cpp/src/arrow/types/string-test.cc +++ b/cpp/src/arrow/types/string-test.cc @@ -48,7 +48,6 @@ TEST(TypesTest, TestCharType) { ASSERT_EQ(t2.size, 5); } - TEST(TypesTest, TestVarcharType) { VarcharType t1(5); @@ -72,7 +71,7 @@ TEST(TypesTest, TestStringType) { // ---------------------------------------------------------------------- // String container -class TestStringContainer : public ::testing::Test { +class TestStringContainer : public ::testing::Test { public: void SetUp() { chars_ = {'a', 'b', 'b', 'c', 'c', 'c'}; @@ -95,8 +94,8 @@ class TestStringContainer : public ::testing::Test { null_bitmap_ = test::bytes_to_null_buffer(valid_bytes_); null_count_ = test::null_count(valid_bytes_); - strings_ = std::make_shared(length_, offsets_buf_, values_, - null_count_, null_bitmap_); + strings_ = std::make_shared( + length_, offsets_buf_, values_, null_count_, null_bitmap_); } protected: @@ -117,7 +116,6 @@ class TestStringContainer : public ::testing::Test { std::shared_ptr strings_; }; - TEST_F(TestStringContainer, TestArrayBasics) { ASSERT_EQ(length_, strings_->length()); ASSERT_EQ(1, strings_->null_count()); @@ -130,7 +128,6 @@ TEST_F(TestStringContainer, TestType) { ASSERT_EQ(Type::STRING, strings_->type_enum()); } - TEST_F(TestStringContainer, TestListFunctions) { int pos = 0; for (size_t i = 0; i < expected_.size(); ++i) { @@ -140,10 +137,9 @@ TEST_F(TestStringContainer, TestListFunctions) { } } - TEST_F(TestStringContainer, TestDestructor) { - auto arr = std::make_shared(length_, offsets_buf_, values_, - null_count_, null_bitmap_); + auto arr = std::make_shared( + length_, offsets_buf_, values_, null_count_, null_bitmap_); } TEST_F(TestStringContainer, TestGetString) { @@ -167,9 +163,7 @@ class TestStringBuilder : public TestBuilder { builder_.reset(new StringBuilder(pool_, type_)); } - void Done() { - result_ = std::dynamic_pointer_cast(builder_->Finish()); - } + void Done() { result_ = std::dynamic_pointer_cast(builder_->Finish()); } protected: TypePtr type_; @@ -222,4 +216,4 @@ TEST_F(TestStringBuilder, TestZeroLength) { Done(); } -} // namespace arrow +} // namespace arrow diff --git a/cpp/src/arrow/types/string.cc b/cpp/src/arrow/types/string.cc index 80b075cdfbb23..29d97d039477c 100644 --- a/cpp/src/arrow/types/string.cc +++ b/cpp/src/arrow/types/string.cc @@ -26,11 +26,10 @@ namespace arrow { const std::shared_ptr STRING(new StringType()); -StringArray::StringArray(int32_t length, - const std::shared_ptr& offsets, +StringArray::StringArray(int32_t length, const std::shared_ptr& offsets, const ArrayPtr& values, int32_t null_count, - const std::shared_ptr& null_bitmap) : - StringArray(STRING, length, offsets, values, null_count, null_bitmap) {} + const std::shared_ptr& null_bitmap) + : StringArray(STRING, length, offsets, values, null_count, null_bitmap) {} std::string CharType::ToString() const { std::stringstream s; @@ -38,7 +37,6 @@ std::string CharType::ToString() const { return s.str(); } - std::string VarcharType::ToString() const { std::stringstream s; s << "varchar(" << size << ")"; @@ -47,4 +45,4 @@ std::string VarcharType::ToString() const { TypePtr StringBuilder::value_type_ = TypePtr(new UInt8Type()); -} // namespace arrow +} // namespace arrow diff --git a/cpp/src/arrow/types/string.h b/cpp/src/arrow/types/string.h index 84cd0326ec850..c5cbe1058c7cf 100644 --- a/cpp/src/arrow/types/string.h +++ b/cpp/src/arrow/types/string.h @@ -37,48 +37,37 @@ class MemoryPool; struct CharType : public DataType { int size; - explicit CharType(int size) - : DataType(Type::CHAR), - size(size) {} + explicit CharType(int size) : DataType(Type::CHAR), size(size) {} - CharType(const CharType& other) - : CharType(other.size) {} + CharType(const CharType& other) : CharType(other.size) {} virtual std::string ToString() const; }; - // Variable-length, null-terminated strings, up to a certain length struct VarcharType : public DataType { int size; - explicit VarcharType(int size) - : DataType(Type::VARCHAR), - size(size) {} - VarcharType(const VarcharType& other) - : VarcharType(other.size) {} + explicit VarcharType(int size) : DataType(Type::VARCHAR), size(size) {} + VarcharType(const VarcharType& other) : VarcharType(other.size) {} virtual std::string ToString() const; }; -// TODO: add a BinaryArray layer in between +// TODO(wesm): add a BinaryArray layer in between class StringArray : public ListArray { public: - StringArray(const TypePtr& type, int32_t length, - const std::shared_ptr& offsets, - const ArrayPtr& values, - int32_t null_count = 0, - const std::shared_ptr& null_bitmap = nullptr) : - ListArray(type, length, offsets, values, null_count, null_bitmap) { + StringArray(const TypePtr& type, int32_t length, const std::shared_ptr& offsets, + const ArrayPtr& values, int32_t null_count = 0, + const std::shared_ptr& null_bitmap = nullptr) + : ListArray(type, length, offsets, values, null_count, null_bitmap) { // For convenience bytes_ = static_cast(values.get()); raw_bytes_ = bytes_->raw_data(); } - StringArray(int32_t length, - const std::shared_ptr& offsets, - const ArrayPtr& values, - int32_t null_count = 0, + StringArray(int32_t length, const std::shared_ptr& offsets, + const ArrayPtr& values, int32_t null_count = 0, const std::shared_ptr& null_bitmap = nullptr); // Compute the pointer t @@ -103,21 +92,18 @@ class StringArray : public ListArray { // Array builder class StringBuilder : public ListBuilder { public: - explicit StringBuilder(MemoryPool* pool, const TypePtr& type) : - ListBuilder(pool, type, std::make_shared(pool, value_type_)) { + explicit StringBuilder(MemoryPool* pool, const TypePtr& type) + : ListBuilder(pool, type, std::make_shared(pool, value_type_)) { byte_builder_ = static_cast(value_builder_.get()); } - Status Append(const std::string& value) { - return Append(value.c_str(), value.size()); - } + Status Append(const std::string& value) { return Append(value.c_str(), value.size()); } Status Append(const char* value, int32_t length) { RETURN_NOT_OK(ListBuilder::Append()); return byte_builder_->Append(reinterpret_cast(value), length); } - Status Append(const std::vector& values, - uint8_t* null_bytes); + Status Append(const std::vector& values, uint8_t* null_bytes); std::shared_ptr Finish() override { return ListBuilder::Transfer(); @@ -130,6 +116,6 @@ class StringBuilder : public ListBuilder { static TypePtr value_type_; }; -} // namespace arrow +} // namespace arrow -#endif // ARROW_TYPES_STRING_H +#endif // ARROW_TYPES_STRING_H diff --git a/cpp/src/arrow/types/struct-test.cc b/cpp/src/arrow/types/struct-test.cc index d94396f42c52a..79d560e19bcc0 100644 --- a/cpp/src/arrow/types/struct-test.cc +++ b/cpp/src/arrow/types/struct-test.cc @@ -49,7 +49,7 @@ TEST(TestStructType, Basics) { ASSERT_EQ(struct_type.ToString(), "struct"); - // TODO: out of bounds for field(...) + // TODO(wesm): out of bounds for field(...) } -} // namespace arrow +} // namespace arrow diff --git a/cpp/src/arrow/types/struct.cc b/cpp/src/arrow/types/struct.cc index 02af600b017d0..04a277a86fa58 100644 --- a/cpp/src/arrow/types/struct.cc +++ b/cpp/src/arrow/types/struct.cc @@ -17,6 +17,4 @@ #include "arrow/types/struct.h" -namespace arrow { - -} // namespace arrow +namespace arrow {} // namespace arrow diff --git a/cpp/src/arrow/types/struct.h b/cpp/src/arrow/types/struct.h index 5842534d35be1..17e32993bf975 100644 --- a/cpp/src/arrow/types/struct.h +++ b/cpp/src/arrow/types/struct.h @@ -24,8 +24,6 @@ #include "arrow/type.h" -namespace arrow { +namespace arrow {} // namespace arrow -} // namespace arrow - -#endif // ARROW_TYPES_STRUCT_H +#endif // ARROW_TYPES_STRUCT_H diff --git a/cpp/src/arrow/types/test-common.h b/cpp/src/arrow/types/test-common.h index 227aca632ef3c..1957636b141fd 100644 --- a/cpp/src/arrow/types/test-common.h +++ b/cpp/src/arrow/types/test-common.h @@ -28,10 +28,10 @@ #include "arrow/type.h" #include "arrow/util/memory-pool.h" -using std::unique_ptr; - namespace arrow { +using std::unique_ptr; + class TestBuilder : public ::testing::Test { public: void SetUp() { @@ -40,6 +40,7 @@ class TestBuilder : public ::testing::Test { builder_.reset(new UInt8Builder(pool_, type_)); builder_nn_.reset(new UInt8Builder(pool_, type_)); } + protected: MemoryPool* pool_; @@ -48,6 +49,6 @@ class TestBuilder : public ::testing::Test { unique_ptr builder_nn_; }; -} // namespace arrow +} // namespace arrow -#endif // ARROW_TYPES_TEST_COMMON_H +#endif // ARROW_TYPES_TEST_COMMON_H diff --git a/cpp/src/arrow/types/union.cc b/cpp/src/arrow/types/union.cc index db3f81795eae2..c891b4a5357ef 100644 --- a/cpp/src/arrow/types/union.cc +++ b/cpp/src/arrow/types/union.cc @@ -30,7 +30,7 @@ static inline std::string format_union(const std::vector& child_types) std::stringstream s; s << "union<"; for (size_t i = 0; i < child_types.size(); ++i) { - if (i) s << ", "; + if (i) { s << ", "; } s << child_types[i]->ToString(); } s << ">"; @@ -41,10 +41,8 @@ std::string DenseUnionType::ToString() const { return format_union(child_types_); } - std::string SparseUnionType::ToString() const { return format_union(child_types_); } - -} // namespace arrow +} // namespace arrow diff --git a/cpp/src/arrow/types/union.h b/cpp/src/arrow/types/union.h index 29cda90b972dd..d2ee9bde04d0d 100644 --- a/cpp/src/arrow/types/union.h +++ b/cpp/src/arrow/types/union.h @@ -33,27 +33,23 @@ class Buffer; struct DenseUnionType : public CollectionType { typedef CollectionType Base; - explicit DenseUnionType(const std::vector& child_types) : - Base() { + explicit DenseUnionType(const std::vector& child_types) : Base() { child_types_ = child_types; } virtual std::string ToString() const; }; - struct SparseUnionType : public CollectionType { typedef CollectionType Base; - explicit SparseUnionType(const std::vector& child_types) : - Base() { + explicit SparseUnionType(const std::vector& child_types) : Base() { child_types_ = child_types; } virtual std::string ToString() const; }; - class UnionArray : public Array { protected: // The data are types encoded as int16 @@ -61,16 +57,13 @@ class UnionArray : public Array { std::vector> children_; }; - class DenseUnionArray : public UnionArray { protected: Buffer* offset_buf_; }; +class SparseUnionArray : public UnionArray {}; -class SparseUnionArray : public UnionArray { -}; - -} // namespace arrow +} // namespace arrow -#endif // ARROW_TYPES_UNION_H +#endif // ARROW_TYPES_UNION_H diff --git a/cpp/src/arrow/util/bit-util-test.cc b/cpp/src/arrow/util/bit-util-test.cc index 220bff084fd6e..26554d2c9069c 100644 --- a/cpp/src/arrow/util/bit-util-test.cc +++ b/cpp/src/arrow/util/bit-util-test.cc @@ -41,4 +41,4 @@ TEST(UtilTests, TestNextPower2) { ASSERT_EQ(1LL << 62, next_power2((1LL << 62) - 1)); } -} // namespace arrow +} // namespace arrow diff --git a/cpp/src/arrow/util/bit-util.cc b/cpp/src/arrow/util/bit-util.cc index 6c6d5330eab0d..475576e87cadd 100644 --- a/cpp/src/arrow/util/bit-util.cc +++ b/cpp/src/arrow/util/bit-util.cc @@ -26,14 +26,12 @@ namespace arrow { void util::bytes_to_bits(const std::vector& bytes, uint8_t* bits) { for (size_t i = 0; i < bytes.size(); ++i) { - if (bytes[i] > 0) { - set_bit(bits, i); - } + if (bytes[i] > 0) { set_bit(bits, i); } } } -Status util::bytes_to_bits(const std::vector& bytes, - std::shared_ptr* out) { +Status util::bytes_to_bits( + const std::vector& bytes, std::shared_ptr* out) { int bit_length = util::bytes_for_bits(bytes.size()); auto buffer = std::make_shared(); @@ -45,4 +43,4 @@ Status util::bytes_to_bits(const std::vector& bytes, return Status::OK(); } -} // namespace arrow +} // namespace arrow diff --git a/cpp/src/arrow/util/bit-util.h b/cpp/src/arrow/util/bit-util.h index 8d6287130dd2b..1f0f08c4d88ef 100644 --- a/cpp/src/arrow/util/bit-util.h +++ b/cpp/src/arrow/util/bit-util.h @@ -74,8 +74,8 @@ static inline int64_t next_power2(int64_t n) { void bytes_to_bits(const std::vector& bytes, uint8_t* bits); Status bytes_to_bits(const std::vector&, std::shared_ptr*); -} // namespace util +} // namespace util -} // namespace arrow +} // namespace arrow -#endif // ARROW_UTIL_BIT_UTIL_H +#endif // ARROW_UTIL_BIT_UTIL_H diff --git a/cpp/src/arrow/util/buffer-test.cc b/cpp/src/arrow/util/buffer-test.cc index 1d58226d84a46..dad0f7461d914 100644 --- a/cpp/src/arrow/util/buffer-test.cc +++ b/cpp/src/arrow/util/buffer-test.cc @@ -29,8 +29,7 @@ using std::string; namespace arrow { -class TestBuffer : public ::testing::Test { -}; +class TestBuffer : public ::testing::Test {}; TEST_F(TestBuffer, Resize) { PoolBuffer buf; @@ -54,4 +53,4 @@ TEST_F(TestBuffer, ResizeOOM) { ASSERT_RAISES(OutOfMemory, buf.Resize(to_alloc)); } -} // namespace arrow +} // namespace arrow diff --git a/cpp/src/arrow/util/buffer.cc b/cpp/src/arrow/util/buffer.cc index 04cdcd75cd41a..bc9c22c10de44 100644 --- a/cpp/src/arrow/util/buffer.cc +++ b/cpp/src/arrow/util/buffer.cc @@ -24,8 +24,7 @@ namespace arrow { -Buffer::Buffer(const std::shared_ptr& parent, int64_t offset, - int64_t size) { +Buffer::Buffer(const std::shared_ptr& parent, int64_t offset, int64_t size) { data_ = parent->data() + offset; size_ = size; parent_ = parent; @@ -37,18 +36,13 @@ std::shared_ptr MutableBuffer::GetImmutableView() { return std::make_shared(this->get_shared_ptr(), 0, size()); } -PoolBuffer::PoolBuffer(MemoryPool* pool) : - ResizableBuffer(nullptr, 0) { - if (pool == nullptr) { - pool = default_memory_pool(); - } +PoolBuffer::PoolBuffer(MemoryPool* pool) : ResizableBuffer(nullptr, 0) { + if (pool == nullptr) { pool = default_memory_pool(); } pool_ = pool; } PoolBuffer::~PoolBuffer() { - if (mutable_data_ != nullptr) { - pool_->Free(mutable_data_, capacity_); - } + if (mutable_data_ != nullptr) { pool_->Free(mutable_data_, capacity_); } } Status PoolBuffer::Reserve(int64_t new_capacity) { @@ -74,4 +68,4 @@ Status PoolBuffer::Resize(int64_t new_size) { return Status::OK(); } -} // namespace arrow +} // namespace arrow diff --git a/cpp/src/arrow/util/buffer.h b/cpp/src/arrow/util/buffer.h index c15f9b630cd97..94e53b61f2e83 100644 --- a/cpp/src/arrow/util/buffer.h +++ b/cpp/src/arrow/util/buffer.h @@ -38,9 +38,7 @@ class Status; // class instance class Buffer : public std::enable_shared_from_this { public: - Buffer(const uint8_t* data, int64_t size) : - data_(data), - size_(size) {} + Buffer(const uint8_t* data, int64_t size) : data_(data), size_(size) {} virtual ~Buffer(); // An offset into data that is owned by another buffer, but we want to be @@ -48,40 +46,28 @@ class Buffer : public std::enable_shared_from_this { // parent buffer have been destroyed Buffer(const std::shared_ptr& parent, int64_t offset, int64_t size); - std::shared_ptr get_shared_ptr() { - return shared_from_this(); - } + std::shared_ptr get_shared_ptr() { return shared_from_this(); } // Return true if both buffers are the same size and contain the same bytes // up to the number of compared bytes bool Equals(const Buffer& other, int64_t nbytes) const { - return this == &other || - (size_ >= nbytes && other.size_ >= nbytes && - !memcmp(data_, other.data_, nbytes)); + return this == &other || (size_ >= nbytes && other.size_ >= nbytes && + !memcmp(data_, other.data_, nbytes)); } bool Equals(const Buffer& other) const { - return this == &other || - (size_ == other.size_ && !memcmp(data_, other.data_, size_)); + return this == &other || (size_ == other.size_ && !memcmp(data_, other.data_, size_)); } - const uint8_t* data() const { - return data_; - } + const uint8_t* data() const { return data_; } - int64_t size() const { - return size_; - } + int64_t size() const { return size_; } // Returns true if this Buffer is referencing memory (possibly) owned by some // other buffer - bool is_shared() const { - return static_cast(parent_); - } + bool is_shared() const { return static_cast(parent_); } - const std::shared_ptr parent() const { - return parent_; - } + const std::shared_ptr parent() const { return parent_; } protected: const uint8_t* data_; @@ -97,22 +83,17 @@ class Buffer : public std::enable_shared_from_this { // A Buffer whose contents can be mutated. May or may not own its data. class MutableBuffer : public Buffer { public: - MutableBuffer(uint8_t* data, int64_t size) : - Buffer(data, size) { + MutableBuffer(uint8_t* data, int64_t size) : Buffer(data, size) { mutable_data_ = data; } - uint8_t* mutable_data() { - return mutable_data_; - } + uint8_t* mutable_data() { return mutable_data_; } // Get a read-only view of this buffer std::shared_ptr GetImmutableView(); protected: - MutableBuffer() : - Buffer(nullptr, 0), - mutable_data_(nullptr) {} + MutableBuffer() : Buffer(nullptr, 0), mutable_data_(nullptr) {} uint8_t* mutable_data_; }; @@ -128,9 +109,8 @@ class ResizableBuffer : public MutableBuffer { virtual Status Reserve(int64_t new_capacity) = 0; protected: - ResizableBuffer(uint8_t* data, int64_t size) : - MutableBuffer(data, size), - capacity_(size) {} + ResizableBuffer(uint8_t* data, int64_t size) + : MutableBuffer(data, size), capacity_(size) {} int64_t capacity_; }; @@ -152,16 +132,11 @@ static constexpr int64_t MIN_BUFFER_CAPACITY = 1024; class BufferBuilder { public: - explicit BufferBuilder(MemoryPool* pool) : - pool_(pool), - capacity_(0), - size_(0) {} + explicit BufferBuilder(MemoryPool* pool) : pool_(pool), capacity_(0), size_(0) {} Status Append(const uint8_t* data, int length) { if (capacity_ < length + size_) { - if (capacity_ == 0) { - buffer_ = std::make_shared(pool_); - } + if (capacity_ == 0) { buffer_ = std::make_shared(pool_); } capacity_ = std::max(MIN_BUFFER_CAPACITY, capacity_); while (capacity_ < length + size_) { capacity_ *= 2; @@ -188,6 +163,6 @@ class BufferBuilder { int64_t size_; }; -} // namespace arrow +} // namespace arrow -#endif // ARROW_UTIL_BUFFER_H +#endif // ARROW_UTIL_BUFFER_H diff --git a/cpp/src/arrow/util/logging.h b/cpp/src/arrow/util/logging.h index 3ce4ccc1e9c26..527ce423e7751 100644 --- a/cpp/src/arrow/util/logging.h +++ b/cpp/src/arrow/util/logging.h @@ -19,6 +19,7 @@ #define ARROW_UTIL_LOGGING_H #include +#include namespace arrow { @@ -37,19 +38,34 @@ namespace arrow { #define ARROW_LOG_INTERNAL(level) arrow::internal::CerrLog(level) #define ARROW_LOG(level) ARROW_LOG_INTERNAL(ARROW_##level) -#define ARROW_CHECK(condition) \ - (condition) ? 0 : ARROW_LOG(FATAL) << "Check failed: " #condition " " +#define ARROW_CHECK(condition) \ + (condition) ? 0 : ::arrow::internal::FatalLog(ARROW_FATAL) \ + << __FILE__ << __LINE__ << "Check failed: " #condition " " #ifdef NDEBUG #define ARROW_DFATAL ARROW_WARNING -#define DCHECK(condition) while (false) arrow::internal::NullLog() -#define DCHECK_EQ(val1, val2) while (false) arrow::internal::NullLog() -#define DCHECK_NE(val1, val2) while (false) arrow::internal::NullLog() -#define DCHECK_LE(val1, val2) while (false) arrow::internal::NullLog() -#define DCHECK_LT(val1, val2) while (false) arrow::internal::NullLog() -#define DCHECK_GE(val1, val2) while (false) arrow::internal::NullLog() -#define DCHECK_GT(val1, val2) while (false) arrow::internal::NullLog() +#define DCHECK(condition) \ + while (false) \ + arrow::internal::NullLog() +#define DCHECK_EQ(val1, val2) \ + while (false) \ + arrow::internal::NullLog() +#define DCHECK_NE(val1, val2) \ + while (false) \ + arrow::internal::NullLog() +#define DCHECK_LE(val1, val2) \ + while (false) \ + arrow::internal::NullLog() +#define DCHECK_LT(val1, val2) \ + while (false) \ + arrow::internal::NullLog() +#define DCHECK_GE(val1, val2) \ + while (false) \ + arrow::internal::NullLog() +#define DCHECK_GT(val1, val2) \ + while (false) \ + arrow::internal::NullLog() #else #define ARROW_DFATAL ARROW_FATAL @@ -62,13 +78,13 @@ namespace arrow { #define DCHECK_GE(val1, val2) ARROW_CHECK((val1) >= (val2)) #define DCHECK_GT(val1, val2) ARROW_CHECK((val1) > (val2)) -#endif // NDEBUG +#endif // NDEBUG namespace internal { class NullLog { public: - template + template NullLog& operator<<(const T& t) { return *this; } @@ -76,34 +92,42 @@ class NullLog { class CerrLog { public: - CerrLog(int severity) // NOLINT(runtime/explicit) - : severity_(severity), - has_logged_(false) { - } + CerrLog(int severity) // NOLINT(runtime/explicit) + : severity_(severity), + has_logged_(false) {} - ~CerrLog() { - if (has_logged_) { - std::cerr << std::endl; - } - if (severity_ == ARROW_FATAL) { - exit(1); - } + virtual ~CerrLog() { + if (has_logged_) { std::cerr << std::endl; } + if (severity_ == ARROW_FATAL) { std::exit(1); } } - template + template CerrLog& operator<<(const T& t) { has_logged_ = true; std::cerr << t; return *this; } - private: + protected: const int severity_; bool has_logged_; }; -} // namespace internal +// Clang-tidy isn't smart enough to determine that DCHECK using CerrLog doesn't +// return so we create a new class to give it a hint. +class FatalLog : public CerrLog { + public: + FatalLog(int /* severity */) // NOLINT + : CerrLog(ARROW_FATAL) {} + + [[noreturn]] ~FatalLog() { + if (has_logged_) { std::cerr << std::endl; } + std::exit(1); + } +}; + +} // namespace internal -} // namespace arrow +} // namespace arrow -#endif // ARROW_UTIL_LOGGING_H +#endif // ARROW_UTIL_LOGGING_H diff --git a/cpp/src/arrow/util/macros.h b/cpp/src/arrow/util/macros.h index 069e627c90eaa..51e605ee50ac4 100644 --- a/cpp/src/arrow/util/macros.h +++ b/cpp/src/arrow/util/macros.h @@ -19,8 +19,8 @@ #define ARROW_UTIL_MACROS_H // From Google gutil -#define DISALLOW_COPY_AND_ASSIGN(TypeName) \ - TypeName(const TypeName&) = delete; \ +#define DISALLOW_COPY_AND_ASSIGN(TypeName) \ + TypeName(const TypeName&) = delete; \ void operator=(const TypeName&) = delete -#endif // ARROW_UTIL_MACROS_H +#endif // ARROW_UTIL_MACROS_H diff --git a/cpp/src/arrow/util/memory-pool-test.cc b/cpp/src/arrow/util/memory-pool-test.cc index 6ef07a07ada3f..e4600a9bd9b27 100644 --- a/cpp/src/arrow/util/memory-pool-test.cc +++ b/cpp/src/arrow/util/memory-pool-test.cc @@ -45,4 +45,4 @@ TEST(DefaultMemoryPool, OOM) { ASSERT_RAISES(OutOfMemory, pool->Allocate(to_alloc, &data)); } -} // namespace arrow +} // namespace arrow diff --git a/cpp/src/arrow/util/memory-pool.cc b/cpp/src/arrow/util/memory-pool.cc index 0b885e9376a62..fb417e74daf53 100644 --- a/cpp/src/arrow/util/memory-pool.cc +++ b/cpp/src/arrow/util/memory-pool.cc @@ -75,4 +75,4 @@ MemoryPool* default_memory_pool() { return &default_memory_pool_; } -} // namespace arrow +} // namespace arrow diff --git a/cpp/src/arrow/util/memory-pool.h b/cpp/src/arrow/util/memory-pool.h index 0d2478686f5a4..824c7248e2e86 100644 --- a/cpp/src/arrow/util/memory-pool.h +++ b/cpp/src/arrow/util/memory-pool.h @@ -36,6 +36,6 @@ class MemoryPool { MemoryPool* default_memory_pool(); -} // namespace arrow +} // namespace arrow -#endif // ARROW_UTIL_MEMORY_POOL_H +#endif // ARROW_UTIL_MEMORY_POOL_H diff --git a/cpp/src/arrow/util/random.h b/cpp/src/arrow/util/random.h index 64c197ef080fd..31f2b0680fe0a 100644 --- a/cpp/src/arrow/util/random.h +++ b/cpp/src/arrow/util/random.h @@ -15,10 +15,10 @@ namespace arrow { namespace random_internal { -static const uint32_t M = 2147483647L; // 2^31-1 +static const uint32_t M = 2147483647L; // 2^31-1 const double kTwoPi = 6.283185307179586476925286; -} // namespace random_internal +} // namespace random_internal // A very simple random number generator. Not especially good at // generating truly random bits, but good enough for our needs in this @@ -27,9 +27,7 @@ class Random { public: explicit Random(uint32_t s) : seed_(s & 0x7fffffffu) { // Avoid bad seeds. - if (seed_ == 0 || seed_ == random_internal::M) { - seed_ = 1; - } + if (seed_ == 0 || seed_ == random_internal::M) { seed_ = 1; } } // Next pseudo-random 32-bit unsigned integer. @@ -50,9 +48,7 @@ class Random { // The first reduction may overflow by 1 bit, so we may need to // repeat. mod == M is not possible; using > allows the faster // sign-bit-based test. - if (seed_ > random_internal::M) { - seed_ -= random_internal::M; - } + if (seed_ > random_internal::M) { seed_ -= random_internal::M; } return seed_; } @@ -91,9 +87,7 @@ class Random { // Skewed: pick "base" uniformly from range [0,max_log] and then // return "base" random bits. The effect is to pick a number in the // range [0,2^max_log-1] with exponential bias towards smaller numbers. - uint32_t Skewed(int max_log) { - return Uniform(1 << Uniform(max_log + 1)); - } + uint32_t Skewed(int max_log) { return Uniform(1 << Uniform(max_log + 1)); } // Creates a normal distribution variable using the // Box-Muller transform. See: @@ -103,8 +97,9 @@ class Random { double Normal(double mean, double std_dev) { double uniform1 = (Next() + 1.0) / (random_internal::M + 1.0); double uniform2 = (Next() + 1.0) / (random_internal::M + 1.0); - return (mean + std_dev * sqrt(-2 * ::log(uniform1)) * - cos(random_internal::kTwoPi * uniform2)); + return ( + mean + + std_dev * sqrt(-2 * ::log(uniform1)) * cos(random_internal::kTwoPi * uniform2)); } // Return a random number between 0.0 and 1.0 inclusive. @@ -116,13 +111,11 @@ class Random { uint32_t seed_; }; - uint32_t random_seed() { - // TODO: use system time to get a reasonably random seed + // TODO(wesm): use system time to get a reasonably random seed return 0; } - -} // namespace arrow +} // namespace arrow #endif // ARROW_UTIL_RANDOM_H_ diff --git a/cpp/src/arrow/util/status.cc b/cpp/src/arrow/util/status.cc index 43cb87e1a8c56..d194ed5572f52 100644 --- a/cpp/src/arrow/util/status.cc +++ b/cpp/src/arrow/util/status.cc @@ -36,9 +36,7 @@ const char* Status::CopyState(const char* state) { } std::string Status::CodeAsString() const { - if (state_ == NULL) { - return "OK"; - } + if (state_ == NULL) { return "OK"; } const char* type; switch (code()) { @@ -66,9 +64,7 @@ std::string Status::CodeAsString() const { std::string Status::ToString() const { std::string result(CodeAsString()); - if (state_ == NULL) { - return result; - } + if (state_ == NULL) { return result; } result.append(": "); @@ -78,4 +74,4 @@ std::string Status::ToString() const { return result; } -} // namespace arrow +} // namespace arrow diff --git a/cpp/src/arrow/util/status.h b/cpp/src/arrow/util/status.h index 4e273edcb8f1f..6ddc177a9a50d 100644 --- a/cpp/src/arrow/util/status.h +++ b/cpp/src/arrow/util/status.h @@ -20,32 +20,36 @@ #include // Return the given status if it is not OK. -#define ARROW_RETURN_NOT_OK(s) do { \ - ::arrow::Status _s = (s); \ - if (!_s.ok()) return _s; \ +#define ARROW_RETURN_NOT_OK(s) \ + do { \ + ::arrow::Status _s = (s); \ + if (!_s.ok()) { return _s; } \ } while (0); // Return the given status if it is not OK, but first clone it and // prepend the given message. -#define ARROW_RETURN_NOT_OK_PREPEND(s, msg) do { \ - ::arrow::Status _s = (s); \ +#define ARROW_RETURN_NOT_OK_PREPEND(s, msg) \ + do { \ + ::arrow::Status _s = (s); \ if (::gutil::PREDICT_FALSE(!_s.ok())) return _s.CloneAndPrepend(msg); \ } while (0); // Return 'to_return' if 'to_call' returns a bad status. // The substitution for 'to_return' may reference the variable // 's' for the bad status. -#define ARROW_RETURN_NOT_OK_RET(to_call, to_return) do { \ - ::arrow::Status s = (to_call); \ - if (::gutil::PREDICT_FALSE(!s.ok())) return (to_return); \ +#define ARROW_RETURN_NOT_OK_RET(to_call, to_return) \ + do { \ + ::arrow::Status s = (to_call); \ + if (::gutil::PREDICT_FALSE(!s.ok())) return (to_return); \ } while (0); // If 'to_call' returns a bad status, CHECK immediately with a logged message // of 'msg' followed by the status. -#define ARROW_CHECK_OK_PREPEND(to_call, msg) do { \ -::arrow::Status _s = (to_call); \ -ARROW_CHECK(_s.ok()) << (msg) << ": " << _s.ToString(); \ -} while (0); +#define ARROW_CHECK_OK_PREPEND(to_call, msg) \ + do { \ + ::arrow::Status _s = (to_call); \ + ARROW_CHECK(_s.ok()) << (msg) << ": " << _s.ToString(); \ + } while (0); // If the status is bad, CHECK immediately, appending the status to the // logged message. @@ -53,12 +57,13 @@ ARROW_CHECK(_s.ok()) << (msg) << ": " << _s.ToString(); \ namespace arrow { -#define RETURN_NOT_OK(s) do { \ - Status _s = (s); \ - if (!_s.ok()) return _s; \ +#define RETURN_NOT_OK(s) \ + do { \ + Status _s = (s); \ + if (!_s.ok()) { return _s; } \ } while (0); -enum class StatusCode: char { +enum class StatusCode : char { OK = 0, OutOfMemory = 1, KeyError = 2, @@ -71,7 +76,7 @@ enum class StatusCode: char { class Status { public: // Create a success status. - Status() : state_(NULL) { } + Status() : state_(NULL) {} ~Status() { delete[] state_; } // Copy the specified status. @@ -132,8 +137,7 @@ class Status { const char* state_; StatusCode code() const { - return ((state_ == NULL) ? - StatusCode::OK : static_cast(state_[4])); + return ((state_ == NULL) ? StatusCode::OK : static_cast(state_[4])); } Status(StatusCode code, const std::string& msg, int16_t posix_code); @@ -155,5 +159,4 @@ inline void Status::operator=(const Status& s) { } // namespace arrow - -#endif // ARROW_STATUS_H_ +#endif // ARROW_STATUS_H_ diff --git a/cpp/src/arrow/util/test_main.cc b/cpp/src/arrow/util/test_main.cc index adc8466fb0be9..f928047023966 100644 --- a/cpp/src/arrow/util/test_main.cc +++ b/cpp/src/arrow/util/test_main.cc @@ -17,7 +17,7 @@ #include "gtest/gtest.h" -int main(int argc, char **argv) { +int main(int argc, char** argv) { ::testing::InitGoogleTest(&argc, argv); int ret = RUN_ALL_TESTS(); diff --git a/cpp/thirdparty/build_thirdparty.sh b/cpp/thirdparty/build_thirdparty.sh index 3d5f532b16309..f1738ff748299 100755 --- a/cpp/thirdparty/build_thirdparty.sh +++ b/cpp/thirdparty/build_thirdparty.sh @@ -84,8 +84,8 @@ if [ -n "$F_ALL" -o -n "$F_FLATBUFFERS" ]; then cd $TP_DIR/$FLATBUFFERS_BASEDIR CXXFLAGS=-fPIC cmake -DCMAKE_INSTALL_PREFIX:PATH=$PREFIX -DFLATBUFFERS_BUILD_TESTS=OFF . || { echo "cmake $FLATBUFFERS_ERROR" ; exit 1; } - make -j$PARALLEL - make install + make VERBOSE=1 -j$PARALLEL || { echo "make $FLATBUFFERS_ERROR" ; exit 1; } + make install || { echo "install $FLATBUFFERS_ERROR" ; exit 1; } fi echo "---------------------"