Skip to content

Commit 823dd43

Browse files
wesmwesm
committed
ARROW-4399: [C++] Do not use extern template class with NumericArray<T> and NumericTensor<T>
Using this has little benefit with this class, and can cause tricky symbol linkage issues with some environments Author: Wes McKinney <wesm+git@apache.org> Closes #3509 from wesm/numeric-array-extern-template and squashes the following commits: 948ff7d <Wes McKinney> Do not use instantiate NumericTensor<T> in tensor.cc a688a3e <Wes McKinney> Remove comment f3ecba5 <Wes McKinney> Also remove visibility define 7ccac89 <Wes McKinney> Do not use extern template class with NumericArray<T>
1 parent 250dc8b commit 823dd43

File tree

6 files changed

+33
-125
lines changed

6 files changed

+33
-125
lines changed

‎cpp/src/arrow/array.cc‎

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -165,12 +165,6 @@ PrimitiveArray::PrimitiveArray(const std::shared_ptr<DataType>& type, int64_t le
165165
SetData(ArrayData::Make(type, length, {null_bitmap, data}, null_count, offset));
166166
}
167167

168-
template <typename T>
169-
NumericArray<T>::NumericArray(const std::shared_ptr<ArrayData>& data)
170-
: PrimitiveArray(data) {
171-
DCHECK_EQ(data->type->id(), T::type_id);
172-
}
173-
174168
// ----------------------------------------------------------------------
175169
// BooleanArray
176170

@@ -987,24 +981,4 @@ std::vector<ArrayVector> RechunkArraysConsistently(
987981

988982
} // namespace internal
989983

990-
// ----------------------------------------------------------------------
991-
// Instantiate templates
992-
993-
template class ARROW_TEMPLATE_EXPORT NumericArray<UInt8Type>;
994-
template class ARROW_TEMPLATE_EXPORT NumericArray<UInt16Type>;
995-
template class ARROW_TEMPLATE_EXPORT NumericArray<UInt32Type>;
996-
template class ARROW_TEMPLATE_EXPORT NumericArray<UInt64Type>;
997-
template class ARROW_TEMPLATE_EXPORT NumericArray<Int8Type>;
998-
template class ARROW_TEMPLATE_EXPORT NumericArray<Int16Type>;
999-
template class ARROW_TEMPLATE_EXPORT NumericArray<Int32Type>;
1000-
template class ARROW_TEMPLATE_EXPORT NumericArray<Int64Type>;
1001-
template class ARROW_TEMPLATE_EXPORT NumericArray<TimestampType>;
1002-
template class ARROW_TEMPLATE_EXPORT NumericArray<Date32Type>;
1003-
template class ARROW_TEMPLATE_EXPORT NumericArray<Date64Type>;
1004-
template class ARROW_TEMPLATE_EXPORT NumericArray<Time32Type>;
1005-
template class ARROW_TEMPLATE_EXPORT NumericArray<Time64Type>;
1006-
template class ARROW_TEMPLATE_EXPORT NumericArray<HalfFloatType>;
1007-
template class ARROW_TEMPLATE_EXPORT NumericArray<FloatType>;
1008-
template class ARROW_TEMPLATE_EXPORT NumericArray<DoubleType>;
1009-
1010984
} // namespace arrow

‎cpp/src/arrow/array.h‎

Lines changed: 2 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -398,12 +398,12 @@ class ARROW_EXPORT PrimitiveArray : public FlatArray {
398398

399399
/// Concrete Array class for numeric data.
400400
template <typename TYPE>
401-
class ARROW_EXPORT NumericArray : public PrimitiveArray {
401+
class NumericArray : public PrimitiveArray {
402402
public:
403403
using TypeClass = TYPE;
404404
using value_type = typename TypeClass::c_type;
405405

406-
explicit NumericArray(const std::shared_ptr<ArrayData>& data);
406+
explicit NumericArray(const std::shared_ptr<ArrayData>& data) : PrimitiveArray(data) {}
407407

408408
// Only enable this constructor without a type argument for types without additional
409409
// metadata
@@ -844,27 +844,6 @@ class ARROW_EXPORT DictionaryArray : public Array {
844844
std::shared_ptr<Array> indices_;
845845
};
846846

847-
// ----------------------------------------------------------------------
848-
// extern templates and other details
849-
850-
// Only instantiate these templates once
851-
ARROW_EXTERN_TEMPLATE NumericArray<Int8Type>;
852-
ARROW_EXTERN_TEMPLATE NumericArray<UInt8Type>;
853-
ARROW_EXTERN_TEMPLATE NumericArray<Int16Type>;
854-
ARROW_EXTERN_TEMPLATE NumericArray<UInt16Type>;
855-
ARROW_EXTERN_TEMPLATE NumericArray<Int32Type>;
856-
ARROW_EXTERN_TEMPLATE NumericArray<UInt32Type>;
857-
ARROW_EXTERN_TEMPLATE NumericArray<Int64Type>;
858-
ARROW_EXTERN_TEMPLATE NumericArray<UInt64Type>;
859-
ARROW_EXTERN_TEMPLATE NumericArray<HalfFloatType>;
860-
ARROW_EXTERN_TEMPLATE NumericArray<FloatType>;
861-
ARROW_EXTERN_TEMPLATE NumericArray<DoubleType>;
862-
ARROW_EXTERN_TEMPLATE NumericArray<Date32Type>;
863-
ARROW_EXTERN_TEMPLATE NumericArray<Date64Type>;
864-
ARROW_EXTERN_TEMPLATE NumericArray<Time32Type>;
865-
ARROW_EXTERN_TEMPLATE NumericArray<Time64Type>;
866-
ARROW_EXTERN_TEMPLATE NumericArray<TimestampType>;
867-
868847
/// \brief Perform any validation checks to determine obvious inconsistencies
869848
/// with the array's internal data
870849
///

‎cpp/src/arrow/sparse_tensor.cc‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -264,18 +264,18 @@ class SparseTensorConverter<TYPE, SparseCSRIndex>
264264
// ----------------------------------------------------------------------
265265
// Instantiate templates
266266

267-
#define INSTANTIATE_SPARSE_TENSOR_CONVERTER(IndexType) \
268-
template class ARROW_TEMPLATE_EXPORT SparseTensorConverter<UInt8Type, IndexType>; \
269-
template class ARROW_TEMPLATE_EXPORT SparseTensorConverter<UInt16Type, IndexType>; \
270-
template class ARROW_TEMPLATE_EXPORT SparseTensorConverter<UInt32Type, IndexType>; \
271-
template class ARROW_TEMPLATE_EXPORT SparseTensorConverter<UInt64Type, IndexType>; \
272-
template class ARROW_TEMPLATE_EXPORT SparseTensorConverter<Int8Type, IndexType>; \
273-
template class ARROW_TEMPLATE_EXPORT SparseTensorConverter<Int16Type, IndexType>; \
274-
template class ARROW_TEMPLATE_EXPORT SparseTensorConverter<Int32Type, IndexType>; \
275-
template class ARROW_TEMPLATE_EXPORT SparseTensorConverter<Int64Type, IndexType>; \
276-
template class ARROW_TEMPLATE_EXPORT SparseTensorConverter<HalfFloatType, IndexType>; \
277-
template class ARROW_TEMPLATE_EXPORT SparseTensorConverter<FloatType, IndexType>; \
278-
template class ARROW_TEMPLATE_EXPORT SparseTensorConverter<DoubleType, IndexType>
267+
#define INSTANTIATE_SPARSE_TENSOR_CONVERTER(IndexType) \
268+
template class SparseTensorConverter<UInt8Type, IndexType>; \
269+
template class SparseTensorConverter<UInt16Type, IndexType>; \
270+
template class SparseTensorConverter<UInt32Type, IndexType>; \
271+
template class SparseTensorConverter<UInt64Type, IndexType>; \
272+
template class SparseTensorConverter<Int8Type, IndexType>; \
273+
template class SparseTensorConverter<Int16Type, IndexType>; \
274+
template class SparseTensorConverter<Int32Type, IndexType>; \
275+
template class SparseTensorConverter<Int64Type, IndexType>; \
276+
template class SparseTensorConverter<HalfFloatType, IndexType>; \
277+
template class SparseTensorConverter<FloatType, IndexType>; \
278+
template class SparseTensorConverter<DoubleType, IndexType>
279279

280280
INSTANTIATE_SPARSE_TENSOR_CONVERTER(SparseCOOIndex);
281281
INSTANTIATE_SPARSE_TENSOR_CONVERTER(SparseCSRIndex);

‎cpp/src/arrow/tensor.cc‎

Lines changed: 0 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -123,50 +123,4 @@ Type::type Tensor::type_id() const { return type_->id(); }
123123

124124
bool Tensor::Equals(const Tensor& other) const { return TensorEquals(*this, other); }
125125

126-
// ----------------------------------------------------------------------
127-
// NumericTensor
128-
129-
template <typename TYPE>
130-
NumericTensor<TYPE>::NumericTensor(const std::shared_ptr<Buffer>& data,
131-
const std::vector<int64_t>& shape)
132-
: NumericTensor(data, shape, {}, {}) {}
133-
134-
template <typename TYPE>
135-
NumericTensor<TYPE>::NumericTensor(const std::shared_ptr<Buffer>& data,
136-
const std::vector<int64_t>& shape,
137-
const std::vector<int64_t>& strides)
138-
: NumericTensor(data, shape, strides, {}) {}
139-
140-
template <typename TYPE>
141-
NumericTensor<TYPE>::NumericTensor(const std::shared_ptr<Buffer>& data,
142-
const std::vector<int64_t>& shape,
143-
const std::vector<int64_t>& strides,
144-
const std::vector<std::string>& dim_names)
145-
: Tensor(TypeTraits<TYPE>::type_singleton(), data, shape, strides, dim_names) {}
146-
147-
template <typename TYPE>
148-
int64_t NumericTensor<TYPE>::CalculateValueOffset(
149-
const std::vector<int64_t>& index) const {
150-
int64_t offset = 0;
151-
for (size_t i = 0; i < index.size(); ++i) {
152-
offset += index[i] * strides_[i];
153-
}
154-
return offset;
155-
}
156-
157-
// ----------------------------------------------------------------------
158-
// Instantiate templates
159-
160-
template class ARROW_TEMPLATE_EXPORT NumericTensor<UInt8Type>;
161-
template class ARROW_TEMPLATE_EXPORT NumericTensor<UInt16Type>;
162-
template class ARROW_TEMPLATE_EXPORT NumericTensor<UInt32Type>;
163-
template class ARROW_TEMPLATE_EXPORT NumericTensor<UInt64Type>;
164-
template class ARROW_TEMPLATE_EXPORT NumericTensor<Int8Type>;
165-
template class ARROW_TEMPLATE_EXPORT NumericTensor<Int16Type>;
166-
template class ARROW_TEMPLATE_EXPORT NumericTensor<Int32Type>;
167-
template class ARROW_TEMPLATE_EXPORT NumericTensor<Int64Type>;
168-
template class ARROW_TEMPLATE_EXPORT NumericTensor<HalfFloatType>;
169-
template class ARROW_TEMPLATE_EXPORT NumericTensor<FloatType>;
170-
template class ARROW_TEMPLATE_EXPORT NumericTensor<DoubleType>;
171-
172126
} // namespace arrow

‎cpp/src/arrow/tensor.h‎

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525

2626
#include "arrow/buffer.h"
2727
#include "arrow/type.h"
28+
#include "arrow/type_traits.h"
2829
#include "arrow/util/macros.h"
2930
#include "arrow/util/visibility.h"
3031

@@ -121,22 +122,25 @@ class ARROW_EXPORT Tensor {
121122
};
122123

123124
template <typename TYPE>
124-
class ARROW_EXPORT NumericTensor : public Tensor {
125+
class NumericTensor : public Tensor {
125126
public:
126127
using TypeClass = TYPE;
127128
using value_type = typename TypeClass::c_type;
128129

130+
/// Constructor with non-negative strides and dimension names
131+
NumericTensor(const std::shared_ptr<Buffer>& data, const std::vector<int64_t>& shape,
132+
const std::vector<int64_t>& strides,
133+
const std::vector<std::string>& dim_names)
134+
: Tensor(TypeTraits<TYPE>::type_singleton(), data, shape, strides, dim_names) {}
135+
129136
/// Constructor with no dimension names or strides, data assumed to be row-major
130-
NumericTensor(const std::shared_ptr<Buffer>& data, const std::vector<int64_t>& shape);
137+
NumericTensor(const std::shared_ptr<Buffer>& data, const std::vector<int64_t>& shape)
138+
: NumericTensor(data, shape, {}, {}) {}
131139

132140
/// Constructor with non-negative strides
133141
NumericTensor(const std::shared_ptr<Buffer>& data, const std::vector<int64_t>& shape,
134-
const std::vector<int64_t>& strides);
135-
136-
/// Constructor with non-negative strides and dimension names
137-
NumericTensor(const std::shared_ptr<Buffer>& data, const std::vector<int64_t>& shape,
138-
const std::vector<int64_t>& strides,
139-
const std::vector<std::string>& dim_names);
142+
const std::vector<int64_t>& strides)
143+
: NumericTensor(data, shape, strides, {}) {}
140144

141145
const value_type& Value(const std::vector<int64_t>& index) const {
142146
int64_t offset = CalculateValueOffset(index);
@@ -145,7 +149,13 @@ class ARROW_EXPORT NumericTensor : public Tensor {
145149
}
146150

147151
protected:
148-
int64_t CalculateValueOffset(const std::vector<int64_t>& index) const;
152+
int64_t CalculateValueOffset(const std::vector<int64_t>& index) const {
153+
int64_t offset = 0;
154+
for (size_t i = 0; i < index.size(); ++i) {
155+
offset += index[i] * strides_[i];
156+
}
157+
return offset;
158+
}
149159
};
150160

151161
} // namespace arrow

‎cpp/src/arrow/util/visibility.h‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -43,15 +43,6 @@
4343
#endif
4444
#endif // Non-Windows
4545

46-
// gcc and clang disagree about how to handle template visibility when you have
47-
// explicit specializations https://llvm.org/bugs/show_bug.cgi?id=24815
48-
49-
#if defined(__clang__)
50-
#define ARROW_EXTERN_TEMPLATE extern template class ARROW_EXPORT
51-
#else
52-
#define ARROW_EXTERN_TEMPLATE extern template class
53-
#endif
54-
5546
// This is a complicated topic, some reading on it:
5647
// http://www.codesynthesis.com/~boris/blog/2010/01/18/dll-export-cxx-templates/
5748
#if defined(_MSC_VER) || defined(__clang__)

0 commit comments

Comments
 (0)