This is an automated email from the ASF dual-hosted git repository.
apitrou pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new 51d380cfe6 GH-41536: [C++] Replace std::aligned_storage that is
deprecated in C++23 (#45019)
51d380cfe6 is described below
commit 51d380cfe6546bc656935047b6b17b4f5d190448
Author: Sylvain Joubert <[email protected]>
AuthorDate: Wed Dec 18 10:21:34 2024 +0100
GH-41536: [C++] Replace std::aligned_storage that is deprecated in C++23
(#45019)
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](https://github.com/apache/arrow/blob/main/cpp/src/arrow/util/aligned_storage.h#L122-L139)
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.
* GitHub Issue: #41536
Authored-by: Sylvain Joubert <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
---
cpp/cmake_modules/SetupCxxFlags.cmake | 15 ---------------
cpp/src/arrow/util/aligned_storage.h | 21 +--------------------
2 files changed, 1 insertion(+), 35 deletions(-)
diff --git a/cpp/cmake_modules/SetupCxxFlags.cmake
b/cpp/cmake_modules/SetupCxxFlags.cmake
index fd26dc7dd9..fdb28b540e 100644
--- a/cpp/cmake_modules/SetupCxxFlags.cmake
+++ b/cpp/cmake_modules/SetupCxxFlags.cmake
@@ -163,21 +163,6 @@ if(WIN32)
# insecure, like std::getenv
add_definitions(-D_CRT_SECURE_NO_WARNINGS)
- # Disable static assertion in Microsoft C++ standard library.
- #
- # """[...]\include\type_traits(1271): error C2338:
- # You've instantiated std::aligned_storage<Len, Align> with an extended
- # alignment (in other words, Align > alignof(max_align_t)).
- # Before VS 2017 15.8, the member type would non-conformingly have an
- # alignment of only alignof(max_align_t). VS 2017 15.8 was fixed to handle
- # this correctly, but the fix inherently changes layout and breaks binary
- # compatibility (*only* for uses of aligned_storage with extended
alignments).
- # Please define either (1) _ENABLE_EXTENDED_ALIGNED_STORAGE to acknowledge
- # that you understand this message and that you actually want a type with
- # an extended alignment, or (2) _DISABLE_EXTENDED_ALIGNED_STORAGE to silence
- # this message and get the old non-conformant behavior."""
- add_definitions(-D_ENABLE_EXTENDED_ALIGNED_STORAGE)
-
if(MSVC)
# ARROW-1931 See https://github.com/google/googletest/issues/1318
#
diff --git a/cpp/src/arrow/util/aligned_storage.h
b/cpp/src/arrow/util/aligned_storage.h
index 01e3ced2d1..5888065070 100644
--- a/cpp/src/arrow/util/aligned_storage.h
+++ b/cpp/src/arrow/util/aligned_storage.h
@@ -119,26 +119,7 @@ class AlignedStorage {
}
private:
-#if !defined(__clang__) && defined(__GNUC__) && defined(__i386__)
- // Workaround for GCC bug on i386:
- // alignof(int64 | float64) can give different results depending on the
- // compilation context, leading to internal ABI mismatch manifesting
- // in incorrect propagation of Result<int64 | float64> between
- // compilation units.
- // (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88115)
- static constexpr size_t alignment() {
- if (std::is_integral_v<T> && sizeof(T) == 8) {
- return 4;
- } else if (std::is_floating_point_v<T> && sizeof(T) == 8) {
- return 4;
- }
- return alignof(T);
- }
-
- typename std::aligned_storage<sizeof(T), alignment()>::type data_;
-#else
- typename std::aligned_storage<sizeof(T), alignof(T)>::type data_;
-#endif
+ alignas(T) std::byte data_[sizeof(T)];
};
} // namespace internal
