[z/OS][libc++] Remove align_val_t dependency in small_buffer.h
#114396
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: Zibi Sarbinowski (zibi2) ChangesWe need to guard 2 functions to avoid errors when Full diff: https://github.com/llvm/llvm-project/pull/114396.diff 1 Files Affected:
diff --git a/libcxx/include/__utility/small_buffer.h b/libcxx/include/__utility/small_buffer.h
index 70e068f89f62ed..6071bda4513657 100644
--- a/libcxx/include/__utility/small_buffer.h
+++ b/libcxx/include/__utility/small_buffer.h
@@ -61,6 +61,7 @@ class __small_buffer {
return *std::launder(reinterpret_cast<_Stored**>(__buffer_));
}
+# ifdef _LIBCPP_HAS_LIBRARY_ALIGNED_ALLOCATION
template <class _Stored>
_LIBCPP_HIDE_FROM_ABI _Stored* __alloc() {
if constexpr (__fits_in_buffer<_Stored>) {
@@ -72,11 +73,14 @@ class __small_buffer {
}
}
+# endif // _LIBCPP_HAS_LIBRARY_ALIGNED_ALLOCATION
+# ifdef _LIBCPP_HAS_LIBRARY_SIZED_DEALLOCATION
template <class _Stored>
_LIBCPP_HIDE_FROM_ABI void __dealloc() noexcept {
if constexpr (!__fits_in_buffer<_Stored>)
::operator delete[](*reinterpret_cast<void**>(__buffer_), sizeof(_Stored), align_val_t{alignof(_Stored)});
}
+# endif // _LIBCPP_HAS_LIBRARY_SIZED_DEALLOCATION
template <class _Stored, class... _Args>
_LIBCPP_HIDE_FROM_ABI void __construct(_Args&&... __args) {
|
zibi2
changed the title
align_val_t is available
ldionne
requested changes
Oct 31, 2024
|
Thank you for the comments, Louis please check if I addressed your suggestions correctly. |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
… allocation and sized deallocation
ldionne
reviewed
Nov 4, 2024
philnik777
requested changes
Nov 4, 2024
ldionne
reviewed
Nov 6, 2024
| @@ -75,7 +75,7 @@ class __small_buffer { | |||
| template <class _Stored> | |||
| _LIBCPP_HIDE_FROM_ABI void __dealloc() noexcept { | |||
| if constexpr (!__fits_in_buffer<_Stored>) | |||
| ::operator delete[](*reinterpret_cast<void**>(__buffer_), sizeof(_Stored), align_val_t{alignof(_Stored)}); | |||
| std::__libcpp_deallocate((void*)__buffer_, _BufferSize * sizeof(byte), alignof(_Stored)); | |||
There was a problem hiding this comment.
This is incorrect, since you're deallocating the buffer itself (which is a member variable), not the pointer contained in the buffer. The previous reinterpret_cast must be kept.
There was a problem hiding this comment.
Thanks for caching it.
philnik777
reviewed
Nov 7, 2024
| @@ -75,7 +75,7 @@ class __small_buffer { | |||
| template <class _Stored> | |||
| _LIBCPP_HIDE_FROM_ABI void __dealloc() noexcept { | |||
| if constexpr (!__fits_in_buffer<_Stored>) | |||
| ::operator delete[](*reinterpret_cast<void**>(__buffer_), sizeof(_Stored), align_val_t{alignof(_Stored)}); | |||
| std::__libcpp_deallocate(*reinterpret_cast<void**>(__buffer_), _BufferSize * sizeof(byte), alignof(_Stored)); | |||
There was a problem hiding this comment.
Why did you change the size calculation here?
There was a problem hiding this comment.
Thank you for pointing out this. The good part is that this issue was also caught by stage3 sanitizer failure.
philnik777
approved these changes
Nov 7, 2024
ldionne
approved these changes
Nov 7, 2024
|
Let's merge once CI is green. |
zibi2
changed the title
align_val_t is availablealign_val_t dependency in small_buffer.h
Groverkss
pushed a commit
to iree-org/llvm-project
that referenced
this pull request
Nov 15, 2024
…m#114396) Rewriting `__alloc()` and `__dealloc()` template functions to avoid errors when `small_buffer.h` is included in the modules LIT tests. For example: ``` test-suite-install/include/c++/v1/__utility/small_buffer.h:69:81: error: use of undeclared identifier 'align_val_t' # | 69 | byte* __allocation = static_cast<byte*>(::operator new[](sizeof(_Stored), align_val_t{alignof(_Stored)})); # | | ^ ```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We need to guard 2 functions to avoid errors when
small_buffer.his included in the modules LIT tests. For example: