Skip to content

Commit ba273ca

Browse files
authored
GH-49569: [CI][Python][C++] Add check targetting Apple clang on deciding whether to use std::bit_width or std::log2p1 (#49570)
### Rationale for this change Clang 15.0.7 (/opt/homebrew/bin/clang++) - Homebrew LLVM fails compiling with: ``` FAILED: [code=1] CMakeFiles/arrow_python.dir/pyarrow/src/arrow/python/arrow_to_pandas.cc.o /opt/homebrew/bin/ccache /opt/homebrew/bin/clang++ -DARROW_HAVE_NEON -DARROW_PYTHON_EXPORTING -Darrow_python_EXPORTS -I/Users/runner/work/arrow/arrow/build/python/pyarrow/src -I/var/folders/gj/d1t24fg93wbdl854js_qwvb00000gn/T/tmpj20nqu3c/build/pyarrow/src -I/Library/Frameworks/Python.framework/Versions/3.11/include/python3.11 -I/tmp/local/include -I/Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages/numpy/_core/include -fno-aligned-new -Wall -Wno-unknown-warning-option -Wno-pass-failed -march=armv8-a -Qunused-arguments -fcolor-diagnostics -fno-omit-frame-pointer -Wno-unused-variable -Wno-maybe-uninitialized -Wno-parentheses-equality -Wno-constant-logical-operand -Wno-missing-declarations -Wno-sometimes-uninitialized -Wno-return-type-c-linkage -g -O0 -std=c++20 -arch arm64 -mmacosx-version-min=12 -fPIC -Wno-parentheses-equality -MD -MT CMakeFiles/arrow_python.dir/pyarrow/src/arrow/python/arrow_to_pandas.cc.o -MF CMakeFiles/arrow_python.dir/pyarrow/src/arrow/python/arrow_to_pandas.cc.o.d -o CMakeFiles/arrow_python.dir/pyarrow/src/arrow/python/arrow_to_pandas.cc.o -c /Users/runner/work/arrow/arrow/build/python/pyarrow/src/arrow/python/arrow_to_pandas.cc In file included from /Users/runner/work/arrow/arrow/build/python/pyarrow/src/arrow/python/arrow_to_pandas.cc:34: In file included from /tmp/local/include/arrow/array.h:41: In file included from /tmp/local/include/arrow/array/array_base.h:26: In file included from /tmp/local/include/arrow/array/data.h:32: /tmp/local/include/arrow/util/bit_util.h:145:15: error: no member named 'log2p1' in namespace 'std' return std::log2p1(x - 1); ~~~~~^ 1 error generated. ``` This seems to be the case of clang not having `__cpp_lib_bitops` defined but `std::log2p1` having been removed. ### What changes are included in this PR? Check for `__apple_build_version__` instead of `__clang__` so non Apple Clang just uses `std::bit_width` ### Are these changes tested? Via CI ### Are there any user-facing changes? No * GitHub Issue: #49569 Authored-by: Raúl Cumplido <raulcumplido@gmail.com> Signed-off-by: Raúl Cumplido <raulcumplido@gmail.com>
1 parent 1ffcb4e commit ba273ca

5 files changed

Lines changed: 12 additions & 7 deletions

File tree

cpp/src/arrow/util/align_util.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@ inline BitmapWordAlignParams BitmapWordAlign(const uint8_t* data, int64_t bit_of
4646
int64_t length) {
4747
// TODO: We can remove this condition once CRAN upgrades its macOS
4848
// SDK from 11.3.
49-
#if defined(__clang__) && !defined(__cpp_lib_bitops) && !defined(__EMSCRIPTEN__)
49+
// __apple_build_version__ should be defined only on Apple clang
50+
#if defined(__apple_build_version__) && !defined(__cpp_lib_bitops)
5051
static_assert((ALIGN_IN_BYTES != 0) && ((ALIGN_IN_BYTES & (ALIGN_IN_BYTES - 1)) == 0),
5152
"ALIGN_IN_BYTES should be a positive power of two");
5253
#else

cpp/src/arrow/util/bit_util.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -139,9 +139,10 @@ static inline uint64_t TrailingBits(uint64_t v, int num_bits) {
139139
static inline int Log2(uint64_t x) {
140140
// DCHECK_GT(x, 0);
141141

142-
// TODO: We can remove this condition once CRAN upgrades its macOS
143-
// SDK from 11.3.
144-
#if defined(__clang__) && !defined(__cpp_lib_bitops) && !defined(__EMSCRIPTEN__)
142+
// TODO: We can remove this condition once CRAN upgrades its macOS
143+
// SDK from 11.3.
144+
// __apple_build_version__ should be defined only on Apple clang
145+
#if defined(__apple_build_version__) && !defined(__cpp_lib_bitops)
145146
return std::log2p1(x - 1);
146147
#else
147148
return std::bit_width(x - 1);

cpp/src/arrow/util/rle_encoding_test.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -918,7 +918,8 @@ TEST(BitRle, Random) {
918918
}
919919
// TODO: We can remove this condition once CRAN upgrades its macOS
920920
// SDK from 11.3.
921-
#if defined(__clang__) && !defined(__cpp_lib_bitops) && !defined(__EMSCRIPTEN__)
921+
// __apple_build_version__ should be defined only on Apple clang
922+
#if defined(__apple_build_version__) && !defined(__cpp_lib_bitops)
922923
if (!CheckRoundTrip(values, std::log2p1(values.size()))) {
923924
#else
924925
if (!CheckRoundTrip(values, std::bit_width(values.size()))) {

cpp/src/parquet/chunker_internal.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,8 @@ uint64_t CalculateMask(int64_t min_chunk_size, int64_t max_chunk_size, int norm_
8888
// by taking the floor(log2(target_size))
8989
// TODO: We can remove this condition once CRAN upgrades its macOS
9090
// SDK from 11.3.
91-
#if defined(__clang__) && !defined(__cpp_lib_bitops) && !defined(__EMSCRIPTEN__)
91+
// __apple_build_version__ should be defined only on Apple clang
92+
#if defined(__apple_build_version__) && !defined(__cpp_lib_bitops)
9293
auto target_bits = std::log2p1(static_cast<uint64_t>(target_size));
9394
#else
9495
auto target_bits = std::bit_width(static_cast<uint64_t>(target_size));

cpp/src/parquet/encoder.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1169,7 +1169,8 @@ void DeltaBitPackEncoder<DType>::FlushBlock() {
11691169
// See overflow comment above.
11701170
// TODO: We can remove this condition once CRAN upgrades its macOS
11711171
// SDK from 11.3.
1172-
#if defined(__clang__) && !defined(__cpp_lib_bitops) && !defined(__EMSCRIPTEN__)
1172+
// __apple_build_version__ should be defined only on Apple clang
1173+
#if defined(__apple_build_version__) && !defined(__cpp_lib_bitops)
11731174
const auto bit_width = bit_width_data[i] =
11741175
std::log2p1(static_cast<UT>(max_delta) - static_cast<UT>(min_delta));
11751176
#else

0 commit comments

Comments
 (0)