Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GH-41536: [C++] Replace std::aligned_storage that is deprecated in C++23 #45019

Merged
merged 1 commit into from
Dec 18, 2024

Conversation

sjoubert
Copy link
Contributor

@sjoubert sjoubert commented Dec 13, 2024

This fixes #41536

Rationale for this change

C++23 deprecates std::aligned_storage. The recommended alternative is an array of std::byte properly marked with alignas(). This change is compatible with C++17 and should also allow users to compile their code in C++23 mode without deprecation warnings.

What changes are included in this PR?

This replaces std::aligned_storage as recommended and also removes a preprocessor define that was needed for MSVC.

However there is still one usage of std::aligned_storage in a conditional to work around a GCC bug on i386:

  • I don't know if alignas as a replacement would have the same bug as the original alignof
  • The original bug seems to be fixed but I'm not sure in which GCC version and I also don't know which minimal version of GCC needs to be supported by the arrow project

If someone can confirm that it is ok to clean the code and use the same code unconditionally I'll update my changes.

Copy link

⚠️ GitHub issue #41536 has been automatically assigned in GitHub to PR creator.

@kou
Copy link
Member

kou commented Dec 16, 2024

It seems that the i386 path is used by test-debian-10-cpp-i386: #13991 (comment)

Debian 10 reached EOL and we're using Debian 12. So we can remove the workaround for i386.

@sjoubert sjoubert force-pushed the cpp23_aligned_storage branch from c86bfe0 to 62c6887 Compare December 16, 2024 07:37
@pitrou
Copy link
Member

pitrou commented Dec 18, 2024

@github-actions crossbow submit -g cpp

Copy link

Revision: 62c6887

Submitted crossbow builds: ursacomputing/crossbow @ actions-3dd272f526

Task Status
example-cpp-minimal-build-static GitHub Actions
example-cpp-minimal-build-static-system-dependency GitHub Actions
example-cpp-tutorial GitHub Actions
test-alpine-linux-cpp GitHub Actions
test-build-cpp-fuzz GitHub Actions
test-conda-cpp GitHub Actions
test-conda-cpp-valgrind GitHub Actions
test-cuda-cpp-ubuntu-20.04-cuda-11.2.2 GitHub Actions
test-cuda-cpp-ubuntu-22.04-cuda-11.7.1 GitHub Actions
test-debian-12-cpp-amd64 GitHub Actions
test-debian-12-cpp-i386 GitHub Actions
test-fedora-39-cpp GitHub Actions
test-ubuntu-20.04-cpp GitHub Actions
test-ubuntu-20.04-cpp-bundled GitHub Actions
test-ubuntu-22.04-cpp GitHub Actions
test-ubuntu-22.04-cpp-20 GitHub Actions
test-ubuntu-22.04-cpp-emscripten GitHub Actions
test-ubuntu-22.04-cpp-no-threading GitHub Actions
test-ubuntu-24.04-cpp GitHub Actions
test-ubuntu-24.04-cpp-bundled-offline GitHub Actions
test-ubuntu-24.04-cpp-gcc-13-bundled GitHub Actions
test-ubuntu-24.04-cpp-gcc-14 GitHub Actions
test-ubuntu-24.04-cpp-minimal-with-formats GitHub Actions
test-ubuntu-24.04-cpp-thread-sanitizer GitHub Actions

Copy link
Member

@pitrou pitrou left a comment

Choose a reason for hiding this comment

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

Looks good to me, thank you @sjoubert . CI failures are unrelated, I'll merge.

@pitrou pitrou merged commit 51d380c into apache:main Dec 18, 2024
36 of 37 checks passed
@pitrou pitrou removed the awaiting review Awaiting review label Dec 18, 2024
@sjoubert sjoubert deleted the cpp23_aligned_storage branch December 18, 2024 09:24
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.

[C++] std::aligned_storage is deprecated since C++23
3 participants