Skip to content

Conversation

@frederick-vs-ja
Copy link
Contributor

This patch contains several changes to deallocate_size.pass.cpp:

  1. static_cast-ing parameters to suitable types to avoid narrowing.
  2. Allowing allocated_ == 1 for MSVC STL because it dynamically allocates one container proxy object in debug modes.
  3. Changing the type of allocated_ to possibly larger and seemingly more appropriate ptrdiff_t.

This patch contains several changes to `deallocate_size.pass.cpp`:
1. `static_cast`-ing parameters to suitable types to avoid narrowing.
2. Allows `allocated_ == 1` for MSVC STL because it dynamically
allocates one container proxy object in debug modes.
3. Changes the type of `allocated_` to possibly larger and seemingly
more appropriate `ptrdiff_t`.
@frederick-vs-ja frederick-vs-ja requested a review from a team as a code owner October 26, 2025 17:18
@frederick-vs-ja frederick-vs-ja added libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. test-suite labels Oct 26, 2025
@llvmbot
Copy link
Member

llvmbot commented Oct 26, 2025

@llvm/pr-subscribers-libcxx

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

Changes

This patch contains several changes to deallocate_size.pass.cpp:

  1. static_cast-ing parameters to suitable types to avoid narrowing.
  2. Allowing allocated_ == 1 for MSVC STL because it dynamically allocates one container proxy object in debug modes.
  3. Changing the type of allocated_ to possibly larger and seemingly more appropriate ptrdiff_t.

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

1 Files Affected:

  • (modified) libcxx/test/std/strings/basic.string/string.capacity/deallocate_size.pass.cpp (+10-5)
diff --git a/libcxx/test/std/strings/basic.string/string.capacity/deallocate_size.pass.cpp b/libcxx/test/std/strings/basic.string/string.capacity/deallocate_size.pass.cpp
index 00f9e2b846783..b402bbc9bf823 100644
--- a/libcxx/test/std/strings/basic.string/string.capacity/deallocate_size.pass.cpp
+++ b/libcxx/test/std/strings/basic.string/string.capacity/deallocate_size.pass.cpp
@@ -12,12 +12,13 @@
 
 #include <string>
 #include <cassert>
+#include <cstddef>
 #include <cstdint>
 #include <type_traits>
 
 #include "test_macros.h"
 
-static int allocated_;
+static std::ptrdiff_t allocated_;
 
 template <class T, class Sz>
 struct test_alloc {
@@ -40,13 +41,13 @@ struct test_alloc {
   TEST_CONSTEXPR test_alloc(const test_alloc<U, Sz>&) TEST_NOEXCEPT {}
 
   pointer allocate(size_type n, const void* = nullptr) {
-    allocated_ += n;
-    return std::allocator<value_type>().allocate(n);
+    allocated_ += static_cast<std::ptrdiff_t>(n);
+    return std::allocator<value_type>().allocate(static_cast<std::size_t>(n));
   }
 
   void deallocate(pointer p, size_type s) {
-    allocated_ -= s;
-    std::allocator<value_type>().deallocate(p, s);
+    allocated_ -= static_cast<std::ptrdiff_t>(s);
+    std::allocator<value_type>().deallocate(p, static_cast<std::size_t>(s));
   }
 
   template <class U>
@@ -68,7 +69,11 @@ void test() {
     using Str = std::basic_string<char, std::char_traits<char>, test_alloc<char, Sz> >;
     {
       Str s(i, 't');
+#ifdef _MSVC_STL_VERSION // MSVC STL dynamically allocates one container proxy object in debug modes.
+      assert(allocated_ == 0 || allocated_ == 1 || allocated_ >= i);
+#else  // ^^^ MSVC STL / other vvv
       assert(allocated_ == 0 || allocated_ >= i);
+#endif // ^^^ other ^^^
     }
   }
   assert(allocated_ == 0);

{
Str s(i, 't');
#ifdef _MSVC_STL_VERSION // MSVC STL dynamically allocates one container proxy object in debug modes.
assert(allocated_ == 0 || allocated_ == 1 || allocated_ >= i);
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this assert provide much value as-is? I feel like it would be much better to move the assertion below into the loop and just drop this assertion. That would make this test actually portable.

#include "test_macros.h"

static int allocated_;
static std::ptrdiff_t allocated_;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why don't we make this a size_t instead?

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