Skip to content

Conversation

@jonkeane
Copy link
Member

@jonkeane jonkeane commented Jan 24, 2026

Rationale for this change

Fix a building RE2 with C++20

What changes are included in this PR?

The fix, a test

Are these changes tested?

Yes

Are there any user-facing changes?

No

@github-actions github-actions bot added the awaiting committer review Awaiting committer review label Jan 24, 2026
@jonkeane
Copy link
Member Author

@github-actions crossbow submit test-r-fedora-clang

@github-actions
Copy link

Revision: 16cd49b

Submitted crossbow builds: ursacomputing/crossbow @ actions-d22016819b

Task Status
test-r-fedora-clang GitHub Actions

@jonkeane
Copy link
Member Author

@github-actions crossbow submit test-r-fedora-clang

@github-actions
Copy link

Revision: 654937b

Submitted crossbow builds: ursacomputing/crossbow @ actions-6b659d9faf

Task Status
test-r-fedora-clang GitHub Actions

@jonkeane
Copy link
Member Author

This resolves the warning in this environment (and we now have a test for Fedora+clang with libc++. AFAICT there's no current pre-built fedora clang R image these days.)

The warning seems harmless, anonymous types declared in an anonymous union are an extension but curious if anyone thinks this is something we should upstream a change to re2 for (cc @kou @pitrou)

@jonkeane jonkeane requested a review from thisisnic January 25, 2026 00:58
Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

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

+1

# in anonymous unions (a compiler extension).
# See: https://github.com/apache/arrow/issues/48973
if(CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
if(TARGET re2)
Copy link
Member

Choose a reason for hiding this comment

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

We don't need this check.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, yes of course it already targets re2. I will fix that. Thanks

@github-actions github-actions bot added awaiting merge Awaiting merge awaiting changes Awaiting changes and removed awaiting committer review Awaiting committer review awaiting merge Awaiting merge labels Jan 25, 2026
@github-actions github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Jan 25, 2026
@jonkeane
Copy link
Member Author

The failure is on main + being worked on separately: #48961

@jonkeane jonkeane merged commit 64ce4bd into apache:main Jan 25, 2026
48 of 49 checks passed
@jonkeane jonkeane removed the awaiting change review Awaiting change review label Jan 25, 2026
@conbench-apache-arrow
Copy link

After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit 64ce4bd.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 4 possible false positives for unstable benchmarks that are known to sometimes produce them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants