You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2020/06/02 11:18:00 UTC

[GitHub] [arrow] pitrou commented on a change in pull request #7310: ARROW-6052: [C++] Split up arrow/array.h/cc into multiple files under arrow/array/, move ArrayData to separate header, make ArrayData::dictionary ArrayData

pitrou commented on a change in pull request #7310:
URL: https://github.com/apache/arrow/pull/7310#discussion_r433799924



##########
File path: cpp/src/arrow/table.h
##########
@@ -24,13 +24,18 @@
 #include <vector>
 
 #include "arrow/record_batch.h"
+#include "arrow/result.h"
+#include "arrow/status.h"
 #include "arrow/type.h"
-#include "arrow/type_fwd.h"
 #include "arrow/util/macros.h"
 #include "arrow/util/visibility.h"
 
 namespace arrow {
 
+class Array;
+class KeyValueMetadata;
+class MemoryPool;

Review comment:
       Include "type_fwd.h" instead?

##########
File path: cpp/src/arrow/compute/exec.h
##########
@@ -36,6 +36,7 @@
 namespace arrow {
 
 class Array;
+class BooleanArray;

Review comment:
       Shouldn't this be in `type_fwd.h`? You could remove all those explicit forward declarations...

##########
File path: cpp/src/arrow/table.cc
##########
@@ -37,6 +39,10 @@
 
 namespace arrow {
 
+class KeyValueMetadata;
+class MemoryPool;
+struct ArrayData;

Review comment:
       Include "type_fwd.h" instead?

##########
File path: cpp/src/arrow/util/hashing.h
##########
@@ -31,25 +31,29 @@
 #include <utility>
 #include <vector>
 
-#include "arrow/array.h"
-#include "arrow/buffer.h"
-#include "arrow/builder.h"
+#include "arrow/array/builder_binary.h"
+#include "arrow/buffer_builder.h"
+#include "arrow/result.h"
 #include "arrow/status.h"
-#include "arrow/type.h"
 #include "arrow/type_traits.h"
 #include "arrow/util/bit_util.h"
-#include "arrow/util/checked_cast.h"
 #include "arrow/util/logging.h"
 #include "arrow/util/macros.h"
-#include "arrow/util/string_view.h"
+#include "arrow/util/ubsan.h"
 
 #define XXH_INLINE_ALL
 #define XXH_PRIVATE_API
 #define XXH_NAMESPACE arrow_hashing_
 
-#include "arrow/vendored/xxhash.h"
+#include "arrow/vendored/xxhash.h"  // IWYU pragma: keep
 
 namespace arrow {
+
+class BooleanType;
+class Buffer;
+class LargeBinaryType;
+class MemoryPool;

Review comment:
       Include "type_fwd.h" instead?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org