Copilot commented on code in PR #50998:
URL: https://github.com/apache/arrow/pull/50998#discussion_r4434670359
##########
cpp/src/arrow/util/bit_util.h:
##########
@@ -142,12 +142,14 @@ static inline uint64_t TrailingBits(uint64_t v, int
num_bits) {
//
// The libc++ in the macOS 11.3 SDK used by CRAN only has the pre-standard
// name std::log2p1. __apple_build_version__ should be defined only on Apple
-// clang.
+// clang. The macOS 12 SDKs have std::bit_width but leave __cpp_lib_bitops
+// undefined, so the check uses __cpp_lib_int_pow2, the feature-test macro for
+// std::bit_width and std::has_single_bit.
// TODO: We can remove this condition once CRAN upgrades its macOS SDK from
11.3.
template <typename T>
constexpr int BitWidth(T x) noexcept {
static_assert(std::is_unsigned_v<T>, "BitWidth requires an unsigned integer
type");
-#if defined(__apple_build_version__) && !defined(__cpp_lib_bitops)
+#if defined(__apple_build_version__) && !defined(__cpp_lib_int_pow2)
Review Comment:
The existing `BitWidth`/`HasSingleBit` tests only exercise the feature
branch selected by the host SDK, so they do not cover the compatibility
combination changed here (`__cpp_lib_bitops` undefined while
`__cpp_lib_int_pow2` is defined). A regression to the old guard would therefore
pass the current suite while reintroducing the macOS 12 build failure. Please
add a compile/routing regression test (or a CI probe) that covers the macOS 12
and 11.3 macro combinations for both helpers.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
