Skip to content

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);
     }

@github-actions
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