Skip to content

GH-53945: [C++] std::bit_width in ALP compression fails to compile with CRAN's macOS 11.3 SDK - #53946

Merged
thisisnic merged 7 commits into
apache:mainfrom
thisisnic:GH-53945-cranalp
Oct 31, 2026
Merged

thisisnic merged 7 commits into
apache:mainfrom
thisisnic:GH-53945-cranalp

Conversation

@thisisnic

@thisisnic thisisnic commented Oct 15, 2026 •

Copy link
Copy Markdown
Member

Rationale for this change

CI failures for CRAN macOS

What changes are included in this PR?

Use helper function which works on CRAN macOS in new code + refactor existing stuff

Are these changes tested?

Yup

Are there any user-facing changes?

Nope

Was AI used for this PR?

I generated the code with Claude Fable 5.1 and did adversarial reviews with Opus 5.5 and also asked some questions about why we were moving things into different files and if we had too many tests, resulting in trimming them down.

PR code and description written by:

  • Human
  • AI

Reviewed before submission by:

  • Human
  • AI
  • Not reviewed

Copilot AI balanced review requested due to automatic review settings October 15, 2026 23:10
@thisisnic

Copy link
Copy Markdown
Member Author

@github-actions crossbow submit test-r-macos-as-cran

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The focused refactor preserves existing calculations, reuses the established compatibility fallback, and adds targeted coverage with no identified blocking issues.

0 open findings

What changed in this PR

Fixes ALP compilation with CRAN’s macOS 11.3 SDK by centralizing the existing bit-width compatibility fallback.

Changes:

  • Adds bit_util::BitWidth and reuses it across Arrow and Parquet.
  • Tests unsigned boundary values and compile-time evaluation.
File Description
cpp/​src/​parquet/​encoder.cc Uses the helper for delta bit widths.
cpp/​src/​parquet/​chunker_internal.cc Uses the helper for chunk masks.
cpp/​src/​arrow/​util/​rle_encoding_test.cc Uses the helper in round-trip tests.
cpp/​src/​arrow/​util/​bit_util.h Adds the compatibility helper and updates Log2.
cpp/​src/​arrow/​util/​bit_util_test.cc Adds helper coverage.
cpp/​src/​arrow/​util/​alp/​alp_compression.cc Replaces incompatible bit-width calls.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@github-actions

Copy link
Copy Markdown

Revision: 30307d4

Submitted crossbow builds: ursacomputing/crossbow @ actions-1e99b7a881

Task Status
test-r-macos-as-cran GitHub Actions

@thisisnic
thisisnic marked this pull request as ready for review October 15, 2026 23:16
Copilot AI balanced review requested due to automatic review settings October 15, 2026 23:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The focused refactor preserves existing calculations and fallback behavior, adds targeted coverage, and has no identified blocking issues.

0 open findings

🧠 Review effort: Balanced

@thisisnic
thisisnic marked this pull request as draft October 16, 2026 00:05
@thisisnic

Copy link
Copy Markdown
Member Author

@github-actions crossbow submit test-r-macos-as-cran

@github-actions

Copy link
Copy Markdown

Revision: 896343c

Submitted crossbow builds: ursacomputing/crossbow @ actions-bffabff8ca

Task Status
test-r-macos-as-cran GitHub Actions

Comment thread cpp/src/arrow/util/bit_util_test.cc Resolved
@github-actions github-actions Bot added awaiting changes Awaiting changes awaiting change review Awaiting change review and removed awaiting committer review Awaiting committer review awaiting changes Awaiting changes labels Oct 16, 2026
@thisisnic
thisisnic marked this pull request as ready for review October 16, 2026 09:41
Copilot AI balanced review requested due to automatic review settings October 16, 2026 09:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Compilation against the target macOS 11.3 SDK remains unverified, so the compatibility fallback needs confirmation.

0 open findings

🧠 Review effort: Balanced

@github-actions
github-actions Bot marked this pull request as draft October 16, 2026 09:47
@thisisnic
thisisnic marked this pull request as ready for review October 18, 2026 17:27
@thisisnic thisisnic changed the title 53945: [C++] std::bit_width in ALP compression fails to compile with CRAN's macOS 11.3 SDK GH-53945: [C++] std::bit_width in ALP compression fails to compile with CRAN's macOS 11.3 SDK Oct 20, 2026
@thisisnic thisisnic added this to the 27.0.0 milestone Oct 20, 2026
@thisisnic thisisnic added the Priority: Blocker Marks a blocker for the release label Oct 20, 2026
@thisisnic
thisisnic requested a review from kou October 26, 2026 22:34
@thisisnic

Copy link
Copy Markdown
Member Author

@wgtmac - mind giving this a quick look over?

Comment thread cpp/src/arrow/util/bit_util.h Outdated

// Returns ceil(log2(x)).
static inline int Log2(uint64_t x) {
// DCHECK_GT(x, 0);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: remove this comment

@tadeja tadeja Oct 31, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

edit: nothing here :)

Comment thread cpp/src/arrow/util/bit_util.h Resolved
Copilot AI balanced review requested due to automatic review settings October 31, 2026 11:46
@github-actions github-actions Bot added awaiting changes Awaiting changes and removed awaiting change review Awaiting change review labels Oct 31, 2026
@thisisnic
thisisnic merged commit 340bf45 into apache:main Oct 31, 2026
58 of 60 checks passed
@thisisnic thisisnic removed the awaiting changes Awaiting changes label Oct 31, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Human assessment is needed to evaluate an unpublished critical feature-test guard mismatch that can break compilation on newer Apple SDKs.

No open issues 🎉

Review effort: Balanced

@github-actions github-actions Bot added the awaiting changes Awaiting changes label Oct 31, 2026
@tadeja tadeja mentioned this pull request Oct 31, 2026
2 tasks done
1fanwang added a commit to 1fanwang/arrow that referenced this pull request Nov 1, 2026
…w2-guard

The BitWidth and HasSingleBit helpers added in
apache#53946 replaced four of the five guard
sites, so only the two guards in bit_util.h still need the new macro.

Signed-off-by: 1fanwang <1fannnw@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants