Skip to content

[libc++][test] Fix size_type issues with MinSequenceContainer and min_allocator #126267

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

Merged

Conversation

frederick-vs-ja
Copy link
Contributor

MinSequenceContainer::size can be narrowing on 64-bit platforms, and MSVC complains about such implicit conversion. This PR changes the implicit conversion to explicit static_cast.

min_allocator::allocate and min_allocator::deallocate have ptrdiff_t as the parameter type, which seems weird, because the underlying std::allocator's member functions take size_t. It seems better to use size_t consistently.

@frederick-vs-ja frederick-vs-ja added libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. test-suite labels Feb 7, 2025
@frederick-vs-ja frederick-vs-ja requested a review from a team as a code owner February 7, 2025 17:15
@llvmbot
Copy link
Member

llvmbot commented Feb 7, 2025

@llvm/pr-subscribers-libcxx

Author: A. Jiang (frederick-vs-ja)

Changes

MinSequenceContainer::size can be narrowing on 64-bit platforms, and MSVC complains about such implicit conversion. This PR changes the implicit conversion to explicit static_cast.

min_allocator::allocate and min_allocator::deallocate have ptrdiff_t as the parameter type, which seems weird, because the underlying std::allocator's member functions take size_t. It seems better to use size_t consistently.


Full diff: https://github.com/llvm/llvm-project/pull/126267.diff

2 Files Affected:

  • (modified) libcxx/test/support/MinSequenceContainer.h (+1-1)
  • (modified) libcxx/test/support/min_allocator.h (+2-2)
diff --git a/libcxx/test/support/MinSequenceContainer.h b/libcxx/test/support/MinSequenceContainer.h
index d0e29ae40c400d3..7fee4dd0fbdc193 100644
--- a/libcxx/test/support/MinSequenceContainer.h
+++ b/libcxx/test/support/MinSequenceContainer.h
@@ -31,7 +31,7 @@ struct MinSequenceContainer {
   const_iterator cbegin() const { return const_iterator(data_.data()); }
   iterator end() { return begin() + size(); }
   const_iterator end() const { return begin() + size(); }
-  size_type size() const { return data_.size(); }
+  size_type size() const { return static_cast<size_type>(data_.size()); }
   bool empty() const { return data_.empty(); }
 
   void clear() { data_.clear(); }
diff --git a/libcxx/test/support/min_allocator.h b/libcxx/test/support/min_allocator.h
index 18f51f8072640d1..beee83feb95f1c0 100644
--- a/libcxx/test/support/min_allocator.h
+++ b/libcxx/test/support/min_allocator.h
@@ -394,12 +394,12 @@ class min_allocator
     template <class U>
     TEST_CONSTEXPR_CXX20 min_allocator(min_allocator<U>) {}
 
-    TEST_CONSTEXPR_CXX20 pointer allocate(std::ptrdiff_t n)
+    TEST_CONSTEXPR_CXX20 pointer allocate(std::size_t n)
     {
         return pointer(std::allocator<T>().allocate(n));
     }
 
-    TEST_CONSTEXPR_CXX20 void deallocate(pointer p, std::ptrdiff_t n)
+    TEST_CONSTEXPR_CXX20 void deallocate(pointer p, std::size_t n)
     {
         std::allocator<T>().deallocate(p.ptr_, n);
     }

Copy link

github-actions bot commented Feb 7, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@frederick-vs-ja frederick-vs-ja force-pushed the libcxx-test-container-size-types branch from 5071d8a to 15bfb29 Compare February 7, 2025 17:23
@frederick-vs-ja frederick-vs-ja merged commit 51ba981 into llvm:main Feb 8, 2025
76 checks passed
@frederick-vs-ja frederick-vs-ja deleted the libcxx-test-container-size-types branch February 8, 2025 01:27
Icohedron pushed a commit to Icohedron/llvm-project that referenced this pull request Feb 11, 2025
… `min_allocator` (llvm#126267)

`MinSequenceContainer::size` can be narrowing on 64-bit platforms, and
MSVC complains about such implicit conversion. This PR changes the
implicit conversion to explicit `static_cast`.

`min_allocator::allocate` and `min_allocator::deallocate` have
`ptrdiff_t` as the parameter type, which seems weird, because the
underlying `std::allocator`'s member functions take `size_t`. It seems
better to use `size_t` consistently.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. test-suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants