Skip to content

Commit

Permalink
ARROW-6052: [C++] Split up arrow/array.h/cc into multiple files under…
Browse files Browse the repository at this point in the history
… arrow/array/, move ArrayData to separate header, make ArrayData::dictionary ArrayData

One meaningful change in this patch was to change `ArrayData::dictionary` to be `shared_ptr<ArrayData>`, which was not difficult.

Otherwise, this allows some files that only need ArrayData or just Array to not include a bunch of unnecessary header code. The increase in number of lines of code is pretty much all due to license headers.

I did a modest amount of IWYU cleaning, there are plenty of still usages of "arrow/array.h" that might be better replaced by more specific headers.

As part of this patch I disabled `build/include_what_you_use` in cpplint per ARROW-8994. If we're going to spend time doing IWYU cleaning (which IMHO is overall worthwhile) we should rely on its output rather than having IWYU checks by two different tools.

Closes #7310 from wesm/cpp-split-array-h

Authored-by: Wes McKinney <wesm+git@apache.org>
Signed-off-by: Wes McKinney <wesm+git@apache.org>
  • Loading branch information
wesm committed Jun 2, 2020
1 parent 6152d00 commit 94a5026
Show file tree
Hide file tree
Showing 128 changed files with 4,248 additions and 3,593 deletions.
4 changes: 3 additions & 1 deletion cpp/build-support/iwyu/iwyu.sh
Expand Up @@ -25,11 +25,13 @@ IWYU_LOG=$(mktemp -t arrow-cpp-iwyu.XXXXXX)
trap "rm -f $IWYU_LOG" EXIT

IWYU_MAPPINGS_PATH="$ROOT/cpp/build-support/iwyu/mappings"
IWYU_ARGS="--mapping_file=$IWYU_MAPPINGS_PATH/boost-all.imp \
IWYU_ARGS="--no_fwd_decls \
--mapping_file=$IWYU_MAPPINGS_PATH/boost-all.imp \
--mapping_file=$IWYU_MAPPINGS_PATH/boost-all-private.imp \
--mapping_file=$IWYU_MAPPINGS_PATH/boost-extra.imp \
--mapping_file=$IWYU_MAPPINGS_PATH/gflags.imp \
--mapping_file=$IWYU_MAPPINGS_PATH/glog.imp \
--mapping_file=$IWYU_MAPPINGS_PATH/gmock.imp \
--mapping_file=$IWYU_MAPPINGS_PATH/gtest.imp \
--mapping_file=$IWYU_MAPPINGS_PATH/arrow-misc.imp"

Expand Down
4 changes: 3 additions & 1 deletion cpp/build-support/iwyu/mappings/arrow-misc.imp
Expand Up @@ -32,6 +32,8 @@
{ include: ["<bits/stdint-uintn.h>", private, "<cstdint>", public ] },
{ include: ["<bits/shared_ptr.h>", private, "<memory>", public ] },
{ include: ["<initializer_list>", public, "<vector>", public ] },
{ include: ["<stdint.h>", public, "<cstdint>", public ] },
{ include: ["<string.h>", public, "<cstring>", public ] },
{ symbol: ["bool", private, "<cstdint>", public ] },
{ symbol: ["false", private, "<cstdint>", public ] },
{ symbol: ["true", private, "<cstdint>", public ] },
Expand All @@ -45,7 +47,7 @@
{ symbol: ["uint64_t", private, "<cstdint>", public ] },
{ symbol: ["size_t", private, "<cstddef>", public ] },
{ symbol: ["variant", private, "\"arrow/compute/kernel.h\"", public ] },
{ symbol: ["Array", private, "\"arrow/type_fwd.h\"", public ] },
{ symbol: ["default_memory_pool", private, "\"arrow/type_fwd.h\"", public ] },
{ symbol: ["make_shared", private, "<memory>", public ] },
{ symbol: ["shared_ptr", private, "<memory>", public ] },
{ symbol: ["_Node_const_iterator", private, "<flatbuffers/flatbuffers.h>", public ] },
Expand Down
23 changes: 23 additions & 0 deletions cpp/build-support/iwyu/mappings/gmock.imp
@@ -0,0 +1,23 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
#include
#include

[
{ include: [ "<gmock/gmock-generated-matchers.h>", private, "<gmock/gmock.h>", public ] },
{ include: [ "<gmock/gmock-matchers.h>", private, "<gmock/gmock.h>", public ] }
]
1 change: 1 addition & 0 deletions cpp/build-support/run_cpplint.py
Expand Up @@ -38,6 +38,7 @@
-readability/alt_tokens
-build/header_guard
-build/c++11
-build/include_what_you_use
-runtime/references
-build/include_order
'''.split()
Expand Down
26 changes: 16 additions & 10 deletions cpp/src/arrow/CMakeLists.txt
Expand Up @@ -120,7 +120,12 @@ function(ADD_ARROW_BENCHMARK REL_TEST_NAME)
endfunction()

set(ARROW_SRCS
array.cc
array/array_base.cc
array/array_binary.cc
array/array_decimal.cc
array/array_dict.cc
array/array_nested.cc
array/array_primitive.cc
array/builder_adaptive.cc
array/builder_base.cc
array/builder_binary.cc
Expand All @@ -130,8 +135,9 @@ set(ARROW_SRCS
array/builder_primitive.cc
array/builder_union.cc
array/concatenate.cc
array/dict_internal.cc
array/data.cc
array/diff.cc
array/util.cc
array/validate.cc
builder.cc
buffer.cc
Expand Down Expand Up @@ -511,18 +517,18 @@ install(FILES ${CMAKE_CURRENT_SOURCE_DIR}/arrow-config.cmake
#
# Unit tests
#

add_arrow_test(array_test
SOURCES
array_test.cc
array_binary_test.cc
array_dict_test.cc
array_list_test.cc
array_struct_test.cc
array_union_test.cc
array_view_test.cc
array/array_test.cc
array/array_binary_test.cc
array/array_dict_test.cc
array/array_list_test.cc
array/array_struct_test.cc
array/array_union_test.cc
array/array_view_test.cc
PRECOMPILED_HEADERS
"$<$<COMPILE_LANGUAGE:CXX>:arrow/testing/pch.h>")

add_arrow_test(buffer_test)

if(ARROW_IPC)
Expand Down

0 comments on commit 94a5026

Please sign in to comment.