Skip to content

Commit df613bc

Browse files
fsaintjacquesfsaintjacques
committed
ARROW-6515: [C++] Clean type_traits.h definitions
The goal of this PR is to uniformize the usage of type traits and pattern matching on static type information. - Introduce `arrow::enable_if_t` since we're still on C++11. This removes a small amount of boilerplate, e.g. `typename (expr)::type`. Refactor most code to use this, except where type_traits was not included. - Major overhaul of `type_traits.h` by exporting a uniformized (as much as I could) aliases, i.e. `is_X_type` and the accompanying `enable_if_X`. - Removed old struct type traits, e.g. `IsSigned...`. - Removed catch-all-visitor method and replaced them with explicits missing visitor. This will help the implementation of new types to error missing implementation at compile time instead of runtime. - Uniformize usage of `enable_if` with methods, by using the return-place form instead of parameter-place (except when in the constructor). - Fixed some missing implementation when trivial. - Added c_type to BooleanType Closes apache#5885 from fsaintjacques/ARROW-6515-clean-type-traits and squashes the following commits: 41733de <François Saint-Jacques> Remove NoExtraMeta a3d1aea <François Saint-Jacques> Revert ARROW_MEMORY_POOL_DEFAULT changes 1a1bf2f <François Saint-Jacques> Review comments beea0cc <François Saint-Jacques> ARROW-6515: Clean type_traits.h definitions Authored-by: François Saint-Jacques <fsaintjacques@gmail.com> Signed-off-by: François Saint-Jacques <fsaintjacques@gmail.com>
1 parent 4204428 commit df613bc

35 files changed

Lines changed: 540 additions & 605 deletions

‎cpp/src/arrow/array.h‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -495,11 +495,10 @@ class NumericArray : public PrimitiveArray {
495495
// Only enable this constructor without a type argument for types without additional
496496
// metadata
497497
template <typename T1 = TYPE>
498-
NumericArray(
499-
typename std::enable_if<TypeTraits<T1>::is_parameter_free, int64_t>::type length,
500-
const std::shared_ptr<Buffer>& data,
501-
const std::shared_ptr<Buffer>& null_bitmap = NULLPTR,
502-
int64_t null_count = kUnknownNullCount, int64_t offset = 0)
498+
NumericArray(enable_if_parameter_free<T1, int64_t> length,
499+
const std::shared_ptr<Buffer>& data,
500+
const std::shared_ptr<Buffer>& null_bitmap = NULLPTR,
501+
int64_t null_count = kUnknownNullCount, int64_t offset = 0)
503502
: PrimitiveArray(TypeTraits<T1>::type_singleton(), length, data, null_bitmap,
504503
null_count, offset) {}
505504

‎cpp/src/arrow/array/builder_dict.h‎

Lines changed: 11 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -98,10 +98,10 @@ class DictionaryBuilderBase : public ArrayBuilder {
9898
// WARNING: the type given below is the value type, not the DictionaryType.
9999
// The DictionaryType is instantiated on the Finish() call.
100100
template <typename T1 = T>
101-
DictionaryBuilderBase(
102-
typename std::enable_if<!std::is_base_of<FixedSizeBinaryType, T1>::value,
103-
const std::shared_ptr<DataType>&>::type value_type,
104-
MemoryPool* pool = default_memory_pool())
101+
DictionaryBuilderBase(enable_if_t<!std::is_base_of<FixedSizeBinaryType, T1>::value,
102+
const std::shared_ptr<DataType>&>
103+
value_type,
104+
MemoryPool* pool = default_memory_pool())
105105
: ArrayBuilder(pool),
106106
memo_table_(new internal::DictionaryMemoTable(value_type)),
107107
delta_offset_(0),
@@ -111,8 +111,7 @@ class DictionaryBuilderBase : public ArrayBuilder {
111111

112112
template <typename T1 = T>
113113
explicit DictionaryBuilderBase(
114-
typename std::enable_if<std::is_base_of<FixedSizeBinaryType, T1>::value,
115-
const std::shared_ptr<DataType>&>::type value_type,
114+
enable_if_fixed_size_binary<T1, const std::shared_ptr<DataType>&> value_type,
116115
MemoryPool* pool = default_memory_pool())
117116
: ArrayBuilder(pool),
118117
memo_table_(new internal::DictionaryMemoTable(value_type)),
@@ -123,8 +122,7 @@ class DictionaryBuilderBase : public ArrayBuilder {
123122

124123
template <typename T1 = T>
125124
explicit DictionaryBuilderBase(
126-
typename std::enable_if<TypeTraits<T1>::is_parameter_free, MemoryPool*>::type pool =
127-
default_memory_pool())
125+
enable_if_parameter_free<T1, MemoryPool*> pool = default_memory_pool())
128126
: DictionaryBuilderBase<BuilderType, T1>(TypeTraits<T1>::type_singleton(), pool) {}
129127

130128
DictionaryBuilderBase(const std::shared_ptr<Array>& dictionary,
@@ -154,15 +152,13 @@ class DictionaryBuilderBase : public ArrayBuilder {
154152

155153
/// \brief Append a fixed-width string (only for FixedSizeBinaryType)
156154
template <typename T1 = T>
157-
Status Append(typename std::enable_if<std::is_base_of<FixedSizeBinaryType, T1>::value,
158-
const uint8_t*>::type value) {
155+
enable_if_fixed_size_binary<T1, Status> Append(const uint8_t* value) {
159156
return Append(util::string_view(reinterpret_cast<const char*>(value), byte_width_));
160157
}
161158

162159
/// \brief Append a fixed-width string (only for FixedSizeBinaryType)
163160
template <typename T1 = T>
164-
Status Append(typename std::enable_if<std::is_base_of<FixedSizeBinaryType, T1>::value,
165-
const char*>::type value) {
161+
enable_if_fixed_size_binary<T1, Status> Append(const char* value) {
166162
return Append(util::string_view(value, byte_width_));
167163
}
168164

@@ -192,9 +188,8 @@ class DictionaryBuilderBase : public ArrayBuilder {
192188

193189
/// \brief Append a whole dense array to the builder
194190
template <typename T1 = T>
195-
Status AppendArray(
196-
typename std::enable_if<!std::is_base_of<FixedSizeBinaryType, T1>::value,
197-
const Array&>::type array) {
191+
enable_if_t<!is_fixed_size_binary_type<T1>::value, Status> AppendArray(
192+
const Array& array) {
198193
using ArrayType = typename TypeTraits<T>::ArrayType;
199194

200195
const auto& concrete_array = static_cast<const ArrayType&>(array);
@@ -209,9 +204,7 @@ class DictionaryBuilderBase : public ArrayBuilder {
209204
}
210205

211206
template <typename T1 = T>
212-
Status AppendArray(
213-
typename std::enable_if<std::is_base_of<FixedSizeBinaryType, T1>::value,
214-
const Array&>::type array) {
207+
enable_if_fixed_size_binary<T1, Status> AppendArray(const Array& array) {
215208
if (!value_type_->Equals(*array.type())) {
216209
return Status::Invalid(
217210
"Cannot append FixedSizeBinary array with non-matching type");

‎cpp/src/arrow/array/builder_primitive.h‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ namespace arrow {
2929

3030
class ARROW_EXPORT NullBuilder : public ArrayBuilder {
3131
public:
32-
explicit NullBuilder(MemoryPool* pool ARROW_MEMORY_POOL_DEFAULT) : ArrayBuilder(pool) {}
32+
explicit NullBuilder(MemoryPool* pool = default_memory_pool()) : ArrayBuilder(pool) {}
3333

3434
/// \brief Append the specified number of null elements
3535
Status AppendNulls(int64_t length) final {
@@ -65,8 +65,7 @@ class NumericBuilder : public ArrayBuilder {
6565

6666
template <typename T1 = T>
6767
explicit NumericBuilder(
68-
typename std::enable_if<TypeTraits<T1>::is_parameter_free, MemoryPool*>::type pool
69-
ARROW_MEMORY_POOL_DEFAULT)
68+
enable_if_parameter_free<T1, MemoryPool*> pool = default_memory_pool())
7069
: ArrayBuilder(pool), type_(TypeTraits<T>::type_singleton()) {}
7170

7271
NumericBuilder(const std::shared_ptr<DataType>& type, MemoryPool* pool)
@@ -197,7 +196,7 @@ class NumericBuilder : public ArrayBuilder {
197196
/// or null(0) values.
198197
/// \return Status
199198
template <typename ValuesIter, typename ValidIter>
200-
typename std::enable_if<!std::is_pointer<ValidIter>::value, Status>::type AppendValues(
199+
enable_if_t<!std::is_pointer<ValidIter>::value, Status> AppendValues(
201200
ValuesIter values_begin, ValuesIter values_end, ValidIter valid_begin) {
202201
static_assert(!internal::is_null_pointer<ValidIter>::value,
203202
"Don't pass a NULLPTR directly as valid_begin, use the 2-argument "
@@ -214,7 +213,7 @@ class NumericBuilder : public ArrayBuilder {
214213

215214
// Same as above, with a pointer type ValidIter
216215
template <typename ValuesIter, typename ValidIter>
217-
typename std::enable_if<std::is_pointer<ValidIter>::value, Status>::type AppendValues(
216+
enable_if_t<std::is_pointer<ValidIter>::value, Status> AppendValues(
218217
ValuesIter values_begin, ValuesIter values_end, ValidIter valid_begin) {
219218
int64_t length = static_cast<int64_t>(std::distance(values_begin, values_end));
220219
ARROW_RETURN_NOT_OK(Reserve(length));
@@ -275,10 +274,10 @@ class ARROW_EXPORT BooleanBuilder : public ArrayBuilder {
275274
using TypeClass = BooleanType;
276275
using value_type = bool;
277276

278-
explicit BooleanBuilder(MemoryPool* pool ARROW_MEMORY_POOL_DEFAULT);
277+
explicit BooleanBuilder(MemoryPool* pool = default_memory_pool());
279278

280279
BooleanBuilder(const std::shared_ptr<DataType>& type,
281-
MemoryPool* pool ARROW_MEMORY_POOL_DEFAULT);
280+
MemoryPool* pool = default_memory_pool());
282281

283282
/// Write nulls as uint8_t* (0 value indicates null) into pre-allocated memory
284283
Status AppendNulls(int64_t length) final {
@@ -382,7 +381,7 @@ class ARROW_EXPORT BooleanBuilder : public ArrayBuilder {
382381
/// or null(0) values
383382
/// \return Status
384383
template <typename ValuesIter, typename ValidIter>
385-
typename std::enable_if<!std::is_pointer<ValidIter>::value, Status>::type AppendValues(
384+
enable_if_t<!std::is_pointer<ValidIter>::value, Status> AppendValues(
386385
ValuesIter values_begin, ValuesIter values_end, ValidIter valid_begin) {
387386
static_assert(!internal::is_null_pointer<ValidIter>::value,
388387
"Don't pass a NULLPTR directly as valid_begin, use the 2-argument "
@@ -401,7 +400,7 @@ class ARROW_EXPORT BooleanBuilder : public ArrayBuilder {
401400

402401
// Same as above, for a pointer type ValidIter
403402
template <typename ValuesIter, typename ValidIter>
404-
typename std::enable_if<std::is_pointer<ValidIter>::value, Status>::type AppendValues(
403+
enable_if_t<std::is_pointer<ValidIter>::value, Status> AppendValues(
405404
ValuesIter values_begin, ValuesIter values_end, ValidIter valid_begin) {
406405
int64_t length = static_cast<int64_t>(std::distance(values_begin, values_end));
407406
ARROW_RETURN_NOT_OK(Reserve(length));

‎cpp/src/arrow/array/dict_internal.h‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,14 +45,14 @@ struct DictionaryTraits {
4545
} // namespace internal
4646

4747
template <typename T, typename Out = void>
48-
using enable_if_memoize = typename std::enable_if<
48+
using enable_if_memoize = enable_if_t<
4949
!std::is_same<typename internal::DictionaryTraits<T>::MemoTableType, void>::value,
50-
Out>::type;
50+
Out>;
5151

5252
template <typename T, typename Out = void>
53-
using enable_if_no_memoize = typename std::enable_if<
53+
using enable_if_no_memoize = enable_if_t<
5454
std::is_same<typename internal::DictionaryTraits<T>::MemoTableType, void>::value,
55-
Out>::type;
55+
Out>;
5656

5757
namespace internal {
5858

@@ -116,7 +116,7 @@ struct DictionaryTraits<T, enable_if_has_c_type<T>> {
116116
};
117117

118118
template <typename T>
119-
struct DictionaryTraits<T, enable_if_binary<T>> {
119+
struct DictionaryTraits<T, enable_if_base_binary<T>> {
120120
using MemoTableType = typename HashTraits<T>::MemoTableType;
121121

122122
static Status GetDictionaryArrayData(MemoryPool* pool,

‎cpp/src/arrow/array/diff.cc‎

Lines changed: 30 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,7 @@ struct Slice {
6363
};
6464

6565
template <typename ArrayType, typename T = typename ArrayType::TypeClass,
66-
typename =
67-
typename std::enable_if<std::is_base_of<BaseListType, T>::value ||
68-
std::is_base_of<FixedSizeListType, T>::value>::type>
66+
typename = enable_if_list_like<T>>
6967
static Slice GetView(const ArrayType& array, int64_t index) {
7068
return Slice{array.values().get(), array.value_offset(index),
7169
array.value_length(index)};
@@ -470,9 +468,7 @@ class MakeFormatterImpl {
470468

471469
// format Numerics with std::ostream defaults
472470
template <typename T>
473-
typename std::enable_if<
474-
is_number_type<T>::value && !is_date<T>::value && !is_time<T>::value, Status>::type
475-
Visit(const T&) {
471+
enable_if_number<T, Status> Visit(const T&) {
476472
impl_ = [](const Array& array, int64_t index, std::ostream* os) {
477473
const auto& numeric = checked_cast<const NumericArray<T>&>(array);
478474
if (sizeof(decltype(numeric.Value(index))) == sizeof(char)) {
@@ -520,21 +516,22 @@ class MakeFormatterImpl {
520516
return Status::OK();
521517
}
522518

523-
// format Binary and FixedSizeBinary in hexadecimal
519+
// format Binary, LargeBinary and FixedSizeBinary in hexadecimal
524520
template <typename T>
525521
enable_if_binary_like<T, Status> Visit(const T&) {
522+
using ArrayType = typename TypeTraits<T>::ArrayType;
526523
impl_ = [](const Array& array, int64_t index, std::ostream* os) {
527-
*os << HexEncode(
528-
checked_cast<const typename TypeTraits<T>::ArrayType&>(array).GetView(index));
524+
*os << HexEncode(checked_cast<const ArrayType&>(array).GetView(index));
529525
};
530526
return Status::OK();
531527
}
532528

533529
// format Strings with \"\n\r\t\\ escaped
534-
Status Visit(const StringType&) {
530+
template <typename T>
531+
enable_if_string_like<T, Status> Visit(const T&) {
532+
using ArrayType = typename TypeTraits<T>::ArrayType;
535533
impl_ = [](const Array& array, int64_t index, std::ostream* os) {
536-
*os << "\"" << Escape(checked_cast<const StringArray&>(array).GetView(index))
537-
<< "\"";
534+
*os << "\"" << Escape(checked_cast<const ArrayType&>(array).GetView(index)) << "\"";
538535
};
539536
return Status::OK();
540537
}
@@ -547,12 +544,14 @@ class MakeFormatterImpl {
547544
return Status::OK();
548545
}
549546

550-
Status Visit(const ListType& t) {
547+
template <typename T>
548+
enable_if_list_like<T, Status> Visit(const T& t) {
551549
struct ListImpl {
552550
explicit ListImpl(Formatter f) : values_formatter_(std::move(f)) {}
553551

554552
void operator()(const Array& array, int64_t index, std::ostream* os) {
555-
const auto& list_array = checked_cast<const ListArray&>(array);
553+
const auto& list_array =
554+
checked_cast<const typename TypeTraits<T>::ArrayType&>(array);
556555
*os << "[";
557556
for (int32_t i = 0; i < list_array.value_length(index); ++i) {
558557
if (i != 0) {
@@ -660,7 +659,23 @@ class MakeFormatterImpl {
660659
return Status::OK();
661660
}
662661

663-
Status Visit(const DataType& t) {
662+
Status Visit(const NullType& t) {
663+
return Status::NotImplemented("formatting diffs between arrays of type ", t);
664+
}
665+
666+
Status Visit(const DictionaryType& t) {
667+
return Status::NotImplemented("formatting diffs between arrays of type ", t);
668+
}
669+
670+
Status Visit(const ExtensionType& t) {
671+
return Status::NotImplemented("formatting diffs between arrays of type ", t);
672+
}
673+
674+
Status Visit(const DurationType& t) {
675+
return Status::NotImplemented("formatting diffs between arrays of type ", t);
676+
}
677+
678+
Status Visit(const MonthIntervalType& t) {
664679
return Status::NotImplemented("formatting diffs between arrays of type ", t);
665680
}
666681

‎cpp/src/arrow/array_test.cc‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -427,8 +427,7 @@ class TestPrimitiveBuilder : public TestBuilder {
427427

428428
/// \brief uint8_t isn't a valid template parameter to uniform_int_distribution, so
429429
/// we use SampleType to determine which kind of integer to use to sample.
430-
template <typename T,
431-
typename = typename std::enable_if<std::is_integral<T>::value, T>::type>
430+
template <typename T, typename = enable_if_t<std::is_integral<T>::value, T>>
432431
struct UniformIntSampleType {
433432
using type = T;
434433
};

‎cpp/src/arrow/compare.cc‎

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -726,27 +726,25 @@ class TypeEqualsVisitor {
726726
}
727727

728728
template <typename T>
729-
typename std::enable_if<std::is_base_of<NoExtraMeta, T>::value ||
730-
std::is_base_of<PrimitiveCType, T>::value,
731-
Status>::type
729+
enable_if_t<is_null_type<T>::value || is_primitive_ctype<T>::value ||
730+
is_base_binary_type<T>::value,
731+
Status>
732732
Visit(const T&) {
733733
result_ = true;
734734
return Status::OK();
735735
}
736736

737737
template <typename T>
738-
typename std::enable_if<std::is_base_of<IntervalType, T>::value, Status>::type Visit(
739-
const T& left) {
738+
enable_if_interval<T, Status> Visit(const T& left) {
740739
const auto& right = checked_cast<const IntervalType&>(right_);
741740
result_ = right.interval_type() == left.interval_type();
742741
return Status::OK();
743742
}
744743

745744
template <typename T>
746-
typename std::enable_if<std::is_base_of<TimeType, T>::value ||
747-
std::is_base_of<DateType, T>::value ||
748-
std::is_base_of<DurationType, T>::value,
749-
Status>::type
745+
enable_if_t<is_time_type<T>::value || is_date_type<T>::value ||
746+
is_duration_type<T>::value,
747+
Status>
750748
Visit(const T& left) {
751749
const auto& right = checked_cast<const T&>(right_);
752750
result_ = left.unit() == right.unit();
@@ -771,9 +769,11 @@ class TypeEqualsVisitor {
771769
return Status::OK();
772770
}
773771

774-
Status Visit(const ListType& left) { return VisitChildren(left); }
775-
776-
Status Visit(const LargeListType& left) { return VisitChildren(left); }
772+
template <typename T>
773+
enable_if_t<is_list_like_type<T>::value || is_struct_type<T>::value, Status> Visit(
774+
const T& left) {
775+
return VisitChildren(left);
776+
}
777777

778778
Status Visit(const MapType& left) {
779779
const auto& right = checked_cast<const MapType&>(right_);
@@ -784,10 +784,6 @@ class TypeEqualsVisitor {
784784
return VisitChildren(left);
785785
}
786786

787-
Status Visit(const FixedSizeListType& left) { return VisitChildren(left); }
788-
789-
Status Visit(const StructType& left) { return VisitChildren(left); }
790-
791787
Status Visit(const UnionType& left) {
792788
const auto& right = checked_cast<const UnionType&>(right_);
793789

‎cpp/src/arrow/compute/kernel.h‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,7 @@ struct ARROW_EXPORT Datum {
9999
: value(value) {}
100100

101101
// Cast from subtypes of Array to Datum
102-
template <typename T,
103-
typename = typename std::enable_if<std::is_base_of<Array, T>::value>::type>
102+
template <typename T, typename = enable_if_t<std::is_base_of<Array, T>::value>>
104103
Datum(const std::shared_ptr<T>& value) // NOLINT implicit conversion
105104
: Datum(std::shared_ptr<Array>(value)) {}
106105

0 commit comments

Comments
 (0)