From 5bf9935a7fe097c8cdb850147618ef88080c7b8b Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Sat, 13 Jul 2024 10:32:11 +0900 Subject: [PATCH] Don't use virtual functions in constructor directly --- cpp/src/arrow/array/array_base.h | 28 +++++++------ cpp/src/arrow/array/array_binary.h | 53 +++++++++++++---------- cpp/src/arrow/array/array_dict.cc | 5 ++- cpp/src/arrow/array/array_nested.cc | 8 ++-- cpp/src/arrow/array/array_nested.h | 60 ++++++++++++++------------- cpp/src/arrow/array/array_primitive.h | 22 +++++----- cpp/src/arrow/array/array_run_end.h | 5 ++- 7 files changed, 100 insertions(+), 81 deletions(-) diff --git a/cpp/src/arrow/array/array_base.h b/cpp/src/arrow/array/array_base.h index 6e47584b03298..76f29ae2e49bd 100644 --- a/cpp/src/arrow/array/array_base.h +++ b/cpp/src/arrow/array/array_base.h @@ -240,24 +240,26 @@ class ARROW_EXPORT Array { protected: Array() = default; - explicit Array(const std::shared_ptr& data, - const std::shared_ptr& statistics = NULLPTR) { + ARROW_DEFAULT_MOVE_AND_ASSIGN(Array); + + std::shared_ptr data_; + const uint8_t* null_bitmap_data_ = NULLPTR; + + /// Protected method for constructors + void Init(const std::shared_ptr& data, + const std::shared_ptr& statistics) { + ValidateData(data); SetData(data); if (statistics) { SetStatistics(statistics); } } - ARROW_DEFAULT_MOVE_AND_ASSIGN(Array); - - std::shared_ptr data_; - const uint8_t* null_bitmap_data_ = NULLPTR; /// Protected method for constructors virtual void ValidateData(const std::shared_ptr& data) {} /// Protected method for constructors virtual void SetData(const std::shared_ptr& data) { - ValidateData(data); if (data->buffers.size() > 0) { null_bitmap_data_ = data->GetValuesSafe(0, /*offset=*/0); } else { @@ -306,13 +308,14 @@ class ARROW_EXPORT PrimitiveArray : public FlatArray { PrimitiveArray() : raw_values_(NULLPTR) {} void SetData(const std::shared_ptr& data) override { - this->Array::SetData(data); + Array::SetData(data); raw_values_ = data->GetValuesSafe(1, /*offset=*/0); } explicit PrimitiveArray(const std::shared_ptr& data, - const std::shared_ptr& statistics = NULLPTR) - : FlatArray(data, statistics) {} + const std::shared_ptr& statistics = NULLPTR) { + Init(data, statistics); + } const uint8_t* raw_values_; }; @@ -323,8 +326,9 @@ class ARROW_EXPORT NullArray : public FlatArray { using TypeClass = NullType; explicit NullArray(const std::shared_ptr& data, - const std::shared_ptr& statistics = NULLPTR) - : FlatArray(data, statistics) {} + const std::shared_ptr& statistics = NULLPTR) { + Init(data, statistics); + } explicit NullArray(int64_t length); private: diff --git a/cpp/src/arrow/array/array_binary.h b/cpp/src/arrow/array/array_binary.h index ff16e4b6c57a2..76bf3d6e23ceb 100644 --- a/cpp/src/arrow/array/array_binary.h +++ b/cpp/src/arrow/array/array_binary.h @@ -140,13 +140,10 @@ class BaseBinaryArray : public FlatArray { protected: // For subclasses BaseBinaryArray() = default; - explicit BaseBinaryArray(const std::shared_ptr& data, - const std::shared_ptr& statistics = NULLPTR) - : FlatArray(data, statistics) {} // Protected method for constructors void SetData(const std::shared_ptr& data) override { - this->Array::SetData(data); + Array::SetData(data); raw_value_offsets_ = data->GetValuesSafe(1, /*offset=*/0); raw_data_ = data->GetValuesSafe(2, /*offset=*/0); } @@ -159,8 +156,9 @@ class BaseBinaryArray : public FlatArray { class ARROW_EXPORT BinaryArray : public BaseBinaryArray { public: explicit BinaryArray(const std::shared_ptr& data, - const std::shared_ptr& statistics = NULLPTR) - : BaseBinaryArray(data, statistics) {} + const std::shared_ptr& statistics = NULLPTR) { + Init(data, statistics); + } BinaryArray(int64_t length, const std::shared_ptr& value_offsets, const std::shared_ptr& data, @@ -180,8 +178,9 @@ class ARROW_EXPORT StringArray : public BinaryArray { using TypeClass = StringType; explicit StringArray(const std::shared_ptr& data, - const std::shared_ptr& statistics = NULLPTR) - : BinaryArray(data, statistics) {} + const std::shared_ptr& statistics = NULLPTR) { + Init(data, statistics); + } StringArray(int64_t length, const std::shared_ptr& value_offsets, const std::shared_ptr& data, @@ -200,9 +199,11 @@ class ARROW_EXPORT StringArray : public BinaryArray { /// Concrete Array class for large variable-size binary data class ARROW_EXPORT LargeBinaryArray : public BaseBinaryArray { public: - explicit LargeBinaryArray(const std::shared_ptr& data, - const std::shared_ptr& statistics = NULLPTR) - : BaseBinaryArray(data, statistics) {} + explicit LargeBinaryArray( + const std::shared_ptr& data, + const std::shared_ptr& statistics = NULLPTR) { + Init(data, statistics); + } LargeBinaryArray(int64_t length, const std::shared_ptr& value_offsets, const std::shared_ptr& data, @@ -220,9 +221,11 @@ class ARROW_EXPORT LargeStringArray : public LargeBinaryArray { public: using TypeClass = LargeStringType; - explicit LargeStringArray(const std::shared_ptr& data, - const std::shared_ptr& statistics = NULLPTR) - : LargeBinaryArray(data, statistics) {} + explicit LargeStringArray( + const std::shared_ptr& data, + const std::shared_ptr& statistics = NULLPTR) { + Init(data, statistics); + } LargeStringArray(int64_t length, const std::shared_ptr& value_offsets, const std::shared_ptr& data, @@ -250,8 +253,9 @@ class ARROW_EXPORT BinaryViewArray : public FlatArray { using c_type = BinaryViewType::c_type; explicit BinaryViewArray(const std::shared_ptr& data, - const std::shared_ptr& statistics = NULLPTR) - : FlatArray(data, statistics) {} + const std::shared_ptr& statistics = NULLPTR) { + Init(data, statistics); + } BinaryViewArray(std::shared_ptr type, int64_t length, std::shared_ptr views, BufferVector data_buffers, @@ -273,7 +277,8 @@ class ARROW_EXPORT BinaryViewArray : public FlatArray { IteratorType end() const { return IteratorType(*this, length()); } protected: - using FlatArray::FlatArray; + // This constructor defers Init() to a derived array class + BinaryViewArray() = default; void ValidateData(const std::shared_ptr& data) override; @@ -292,8 +297,9 @@ class ARROW_EXPORT StringViewArray : public BinaryViewArray { using TypeClass = StringViewType; explicit StringViewArray(const std::shared_ptr& data, - const std::shared_ptr& statistics = NULLPTR) - : BinaryViewArray(data, statistics) {} + const std::shared_ptr& statistics = NULLPTR) { + Init(data, statistics); + } using BinaryViewArray::BinaryViewArray; @@ -317,8 +323,9 @@ class ARROW_EXPORT FixedSizeBinaryArray : public PrimitiveArray { explicit FixedSizeBinaryArray( const std::shared_ptr& data, - const std::shared_ptr& statistics = NULLPTR) - : PrimitiveArray(data, statistics) {} + const std::shared_ptr& statistics = NULLPTR) { + Init(data, statistics); + } FixedSizeBinaryArray(const std::shared_ptr& type, int64_t length, const std::shared_ptr& data, @@ -347,8 +354,8 @@ class ARROW_EXPORT FixedSizeBinaryArray : public PrimitiveArray { IteratorType end() const { return IteratorType(*this, length()); } protected: - void SetData(const std::shared_ptr& data) { - this->PrimitiveArray::SetData(data); + void SetData(const std::shared_ptr& data) override { + PrimitiveArray::SetData(data); byte_width_ = internal::checked_cast(*type()).byte_width(); } diff --git a/cpp/src/arrow/array/array_dict.cc b/cpp/src/arrow/array/array_dict.cc index 9e37fb42d99ab..6241104ce2acd 100644 --- a/cpp/src/arrow/array/array_dict.cc +++ b/cpp/src/arrow/array/array_dict.cc @@ -80,8 +80,9 @@ int64_t DictionaryArray::GetValueIndex(int64_t i) const { DictionaryArray::DictionaryArray(const std::shared_ptr& data, const std::shared_ptr& statistics) - : Array(data, statistics), - dict_type_(checked_cast(data->type.get())) {} + : dict_type_(checked_cast(data->type.get())) { + Init(data, statistics); +} void DictionaryArray::ValidateData(const std::shared_ptr& data) { ARROW_CHECK_EQ(data->type->id(), Type::DICTIONARY); diff --git a/cpp/src/arrow/array/array_nested.cc b/cpp/src/arrow/array/array_nested.cc index 639454a096cc9..20faa4e9499e2 100644 --- a/cpp/src/arrow/array/array_nested.cc +++ b/cpp/src/arrow/array/array_nested.cc @@ -917,7 +917,7 @@ void FixedSizeListArray::ValidateData(const std::shared_ptr& data) { } void FixedSizeListArray::SetData(const std::shared_ptr& data) { - this->Array::SetData(data); + Array::SetData(data); ARROW_CHECK_EQ(list_type()->value_type()->id(), data->child_data[0]->type->id()); DCHECK(list_type()->value_type()->Equals(data->child_data[0]->type)); @@ -1160,7 +1160,7 @@ Result> StructArray::GetFlattenedField(int index, // UnionArray void UnionArray::SetData(const std::shared_ptr& data) { - this->Array::SetData(data); + Array::SetData(data); union_type_ = checked_cast(data_->type.get()); @@ -1170,7 +1170,7 @@ void UnionArray::SetData(const std::shared_ptr& data) { } void SparseUnionArray::SetData(const std::shared_ptr& data) { - this->UnionArray::SetData(data); + UnionArray::SetData(data); ARROW_CHECK_EQ(data_->type->id(), Type::SPARSE_UNION); ARROW_CHECK_EQ(data_->buffers.size(), 2); @@ -1179,7 +1179,7 @@ void SparseUnionArray::SetData(const std::shared_ptr& data) { } void DenseUnionArray::SetData(const std::shared_ptr& data) { - this->UnionArray::SetData(data); + UnionArray::SetData(data); ARROW_CHECK_EQ(data_->type->id(), Type::DENSE_UNION); ARROW_CHECK_EQ(data_->buffers.size(), 3); diff --git a/cpp/src/arrow/array/array_nested.h b/cpp/src/arrow/array/array_nested.h index 750fcb6a309dc..95b76f61fd541 100644 --- a/cpp/src/arrow/array/array_nested.h +++ b/cpp/src/arrow/array/array_nested.h @@ -127,7 +127,6 @@ class VarLengthListLikeArray : public Array { } protected: - using Array::Array; friend void internal::SetListData(VarLengthListLikeArray* self, const std::shared_ptr& data, Type::type expected_type_id); @@ -158,17 +157,15 @@ class BaseListArray : public VarLengthListLikeArray { i += this->data_->offset; return this->raw_value_offsets_[i + 1] - this->raw_value_offsets_[i]; } - - protected: - using VarLengthListLikeArray::VarLengthListLikeArray; }; /// Concrete Array class for list data class ARROW_EXPORT ListArray : public BaseListArray { public: explicit ListArray(const std::shared_ptr& data, - const std::shared_ptr& statistics = NULLPTR) - : BaseListArray(data, statistics) {} + const std::shared_ptr& statistics = NULLPTR) { + Init(data, statistics); + } ListArray(std::shared_ptr type, int64_t length, std::shared_ptr value_offsets, std::shared_ptr values, @@ -226,7 +223,7 @@ class ARROW_EXPORT ListArray : public BaseListArray { std::shared_ptr offsets() const; protected: - // This constructor defers SetData to a derived array class + // This constructor defers Init() to a derived array class ListArray() = default; void SetData(const std::shared_ptr& data) override; @@ -236,8 +233,9 @@ class ARROW_EXPORT ListArray : public BaseListArray { class ARROW_EXPORT LargeListArray : public BaseListArray { public: explicit LargeListArray(const std::shared_ptr& data, - const std::shared_ptr& statistics = NULLPTR) - : BaseListArray(data, statistics) {} + const std::shared_ptr& statistics = NULLPTR) { + Init(data, statistics); + } LargeListArray(const std::shared_ptr& type, int64_t length, const std::shared_ptr& value_offsets, @@ -326,7 +324,6 @@ class BaseListViewArray : public VarLengthListLikeArray { } protected: - using VarLengthListLikeArray::VarLengthListLikeArray; const offset_type* raw_value_sizes_ = NULLPTR; }; @@ -334,8 +331,9 @@ class BaseListViewArray : public VarLengthListLikeArray { class ARROW_EXPORT ListViewArray : public BaseListViewArray { public: explicit ListViewArray(const std::shared_ptr& data, - const std::shared_ptr& statistics = NULLPTR) - : BaseListViewArray(data, statistics) {} + const std::shared_ptr& statistics = NULLPTR) { + Init(data, statistics); + } ListViewArray(std::shared_ptr type, int64_t length, std::shared_ptr value_offsets, @@ -413,7 +411,7 @@ class ARROW_EXPORT ListViewArray : public BaseListViewArray { std::shared_ptr sizes() const; protected: - // This constructor defers SetData to a derived array class + // This constructor defers Init() to a derived array class ListViewArray() = default; void SetData(const std::shared_ptr& data) override; }; @@ -424,8 +422,9 @@ class ARROW_EXPORT LargeListViewArray : public BaseListViewArray& data, - const std::shared_ptr& statistics = NULLPTR) - : BaseListViewArray(data, statistics) {} + const std::shared_ptr& statistics = NULLPTR) { + Init(data, statistics); + } LargeListViewArray(std::shared_ptr type, int64_t length, std::shared_ptr value_offsets, @@ -499,7 +498,7 @@ class ARROW_EXPORT LargeListViewArray : public BaseListViewArray sizes() const; protected: - // This constructor defers SetData to a derived array class + // This constructor defers Init() to a derived array class LargeListViewArray() = default; void SetData(const std::shared_ptr& data) override; @@ -516,8 +515,9 @@ class ARROW_EXPORT MapArray : public ListArray { using TypeClass = MapType; explicit MapArray(const std::shared_ptr& data, - const std::shared_ptr& statistics = NULLPTR) - : ListArray(data, statistics) {} + const std::shared_ptr& statistics = NULLPTR) { + Init(data, statistics); + } MapArray(const std::shared_ptr& type, int64_t length, const std::shared_ptr& value_offsets, @@ -597,8 +597,9 @@ class ARROW_EXPORT FixedSizeListArray : public Array { explicit FixedSizeListArray( const std::shared_ptr& data, - const std::shared_ptr& statistics = NULLPTR) - : Array(data, statistics) {} + const std::shared_ptr& statistics = NULLPTR) { + Init(data, statistics); + } FixedSizeListArray(const std::shared_ptr& type, int64_t length, const std::shared_ptr& values, @@ -691,8 +692,9 @@ class ARROW_EXPORT StructArray : public Array { using TypeClass = StructType; explicit StructArray(const std::shared_ptr& data, - const std::shared_ptr& statistics = NULLPTR) - : Array(data, statistics) {} + const std::shared_ptr& statistics = NULLPTR) { + Init(data, statistics); + } StructArray(const std::shared_ptr& type, int64_t length, const std::vector>& children, @@ -790,7 +792,6 @@ class ARROW_EXPORT UnionArray : public Array { std::shared_ptr field(int pos) const; protected: - using Array::Array; void SetData(const std::shared_ptr& data) override; const type_code_t* raw_type_codes_; @@ -805,9 +806,11 @@ class ARROW_EXPORT SparseUnionArray : public UnionArray { public: using TypeClass = SparseUnionType; - explicit SparseUnionArray(const std::shared_ptr& data, - const std::shared_ptr& statistics = NULLPTR) - : UnionArray(data, statistics) {} + explicit SparseUnionArray( + const std::shared_ptr& data, + const std::shared_ptr& statistics = NULLPTR) { + Init(data, statistics); + } SparseUnionArray(std::shared_ptr type, int64_t length, ArrayVector children, std::shared_ptr type_ids, int64_t offset = 0); @@ -861,8 +864,9 @@ class ARROW_EXPORT DenseUnionArray : public UnionArray { using TypeClass = DenseUnionType; explicit DenseUnionArray(const std::shared_ptr& data, - const std::shared_ptr& statistics = NULLPTR) - : UnionArray(data, statistics) {} + const std::shared_ptr& statistics = NULLPTR) { + Init(data, statistics); + } DenseUnionArray(std::shared_ptr type, int64_t length, ArrayVector children, std::shared_ptr type_ids, diff --git a/cpp/src/arrow/array/array_primitive.h b/cpp/src/arrow/array/array_primitive.h index ab82fde8777ea..1de5362d09877 100644 --- a/cpp/src/arrow/array/array_primitive.h +++ b/cpp/src/arrow/array/array_primitive.h @@ -42,8 +42,9 @@ class ARROW_EXPORT BooleanArray : public PrimitiveArray { using IteratorType = stl::ArrayIterator; explicit BooleanArray(const std::shared_ptr& data, - const std::shared_ptr& statistics = NULLPTR) - : PrimitiveArray(data, statistics) {} + const std::shared_ptr& statistics = NULLPTR) { + Init(data, statistics); + } BooleanArray(int64_t length, const std::shared_ptr& data, const std::shared_ptr& null_bitmap = NULLPTR, @@ -71,8 +72,6 @@ class ARROW_EXPORT BooleanArray : public PrimitiveArray { IteratorType end() const { return IteratorType(*this, length()); } protected: - using PrimitiveArray::PrimitiveArray; - void ValidateData(const std::shared_ptr& data) override; }; @@ -95,8 +94,9 @@ class NumericArray : public PrimitiveArray { using IteratorType = stl::ArrayIterator>; explicit NumericArray(const std::shared_ptr& data, - const std::shared_ptr& statistics = NULLPTR) - : PrimitiveArray(data, statistics) {} + const std::shared_ptr& statistics = NULLPTR) { + Init(data, statistics); + } // Only enable this constructor without a type argument for types without additional // metadata @@ -139,8 +139,9 @@ class ARROW_EXPORT DayTimeIntervalArray : public PrimitiveArray { explicit DayTimeIntervalArray( const std::shared_ptr& data, - const std::shared_ptr& statistics = NULLPTR) - : PrimitiveArray(data, statistics) {} + const std::shared_ptr& statistics = NULLPTR) { + Init(data, statistics); + } DayTimeIntervalArray(const std::shared_ptr& type, int64_t length, const std::shared_ptr& data, @@ -178,8 +179,9 @@ class ARROW_EXPORT MonthDayNanoIntervalArray : public PrimitiveArray { explicit MonthDayNanoIntervalArray( const std::shared_ptr& data, - const std::shared_ptr& statistics = NULLPTR) - : PrimitiveArray(data, statistics) {} + const std::shared_ptr& statistics = NULLPTR) { + Init(data, statistics); + } MonthDayNanoIntervalArray(const std::shared_ptr& type, int64_t length, const std::shared_ptr& data, diff --git a/cpp/src/arrow/array/array_run_end.h b/cpp/src/arrow/array/array_run_end.h index 2823077b185fb..66a5d8bdab8ca 100644 --- a/cpp/src/arrow/array/array_run_end.h +++ b/cpp/src/arrow/array/array_run_end.h @@ -55,8 +55,9 @@ class ARROW_EXPORT RunEndEncodedArray : public Array { explicit RunEndEncodedArray( const std::shared_ptr& data, - const std::shared_ptr& statistics = NULLPTR) - : Array(data, statistics) {} + const std::shared_ptr& statistics = NULLPTR) { + Init(data, statistics); + } /// \brief Construct a RunEndEncodedArray from all parameters ///