GH-49841: [C++] Fix macOS 12.3 SDK build by using __cpp_lib_int_pow2 - #50998
GH-49841: [C++] Fix macOS 12.3 SDK build by using __cpp_lib_int_pow2#509981fanwang wants to merge 2 commits into
Conversation
…_pow2 The macOS 12.3 SDK build failed with "no member named 'log2p1' in namespace 'std'". The Apple clang workaround selects std::log2p1 when __cpp_lib_bitops is undefined, but that macro covers popcount and friends, not std::bit_width. Use __cpp_lib_int_pow2, which covers std::bit_width and std::has_single_bit and is undefined on 11.3, defined on 12.3. Signed-off-by: 1fanwang <1fannnw@gmail.com>
|
|
There was a problem hiding this comment.
Pull request overview
This PR fixes Arrow C++ builds against the macOS 12.3 SDK by switching the Apple-clang-specific workaround selector from __cpp_lib_bitops to the correct feature-test macro __cpp_lib_int_pow2, which matches the availability of std::bit_width / std::has_single_bit (and avoids incorrectly selecting the std::log2p1 fallback where it no longer exists).
Changes:
- Update preprocessor guards at five call sites to use
__cpp_lib_int_pow2instead of__cpp_lib_bitops. - Add brief explanatory comments indicating the correct feature-test macro for
std::bit_width/std::has_single_bit. - Keep all fallback branches and Apple-clang gating (
__apple_build_version__) unchanged.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| cpp/src/parquet/encoder.cc | Uses __cpp_lib_int_pow2 to correctly choose between std::log2p1 and std::bit_width on Apple clang + varying SDKs. |
| cpp/src/parquet/chunker_internal.cc | Fixes the SDK-dependent selection for computing target_bits to avoid selecting missing std::log2p1. |
| cpp/src/arrow/util/rle_encoding_test.cc | Aligns the test’s feature selection with the correct macro so it matches the production behavior across SDKs. |
| cpp/src/arrow/util/bit_util.h | Fixes the Log2 helper’s Apple-clang workaround guard to correctly detect std::bit_width availability. |
| cpp/src/arrow/util/align_util.h | Corrects the guard for std::has_single_bit selection to use __cpp_lib_int_pow2. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Generated-by: GitHub Copilot CLI (Claude Opus 5) Signed-off-by: Stefan Wang <1fannnw@gmail.com>
There was a problem hiding this comment.
🟢 Approval recommended
The changes are narrowly scoped to correcting the feature-test macro used for <bit> APIs on Apple SDKs and match the stated build failure and resolution without introducing behavioral changes beyond the intended conditional selection.
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 0 new
- Review effort level: Lite
Rationale for this change
Building Arrow C++ against the macOS 12.3 SDK fails:
Apple clang shipped
std::log2p1, the pre-standard name forstd::bit_width, inolder SDKs. Arrow keeps a workaround that calls it when
__cpp_lib_bitopsisundefined, but that is the wrong feature-test macro. It covers popcount, rotl and
friends, while bit_width and has_single_bit are covered by
__cpp_lib_int_pow2.Probing both SDKs shows why that matters:
__cpp_lib_bitopsis undefined on both, so on 12.3 the guard selects the one namethat is missing. Before this change Arrow cannot be built on that SDK; after it, it
builds.
Closes #49841.
What changes are included in this PR?
Switch the discriminator to
__cpp_lib_int_pow2at the five sites using thisworkaround, keeping the
__apple_build_version__condition and every fallbackbranch unchanged. One of the five guards has_single_bit rather than bit_width. It
still compiled on 12.3 because its fallback is a manual power-of-two check, but the
macro is wrong there for the same reason, so it is corrected too.
Are these changes tested?
Each affected translation unit was compiled against both SDKs, before and after.
All five compile on both SDKs after the change. Compiling on 11.3 is itself proof
the log2p1 branch is still selected there, since bit_width does not exist in that
SDK.
The covering test target builds and passes on this machine's current SDK:
Are there any user-facing changes?
No API change. Arrow C++ now builds against macOS SDK 12.3 and later.