[libcxx-commits] [libcxx] [libc++] Fix ambiguous call to std::max in vector<bool> (PR #119801)
Peng Liu via libcxx-commits
libcxx-commits at lists.llvm.org
Wed Mar 26 10:20:10 PDT 2025
https://github.com/winner245 updated https://github.com/llvm/llvm-project/pull/119801
>From 9d3463d6b974a5af0a40d432c654f679936fb572 Mon Sep 17 00:00:00 2001
From: Peng Liu <winner245 at hotmail.com>
Date: Thu, 12 Dec 2024 21:33:15 -0500
Subject: [PATCH 1/5] Fix ambiguous call to std::max in vector<bool>
---
libcxx/include/__cxx03/string | 8 ++++++--
libcxx/include/__cxx03/vector | 2 +-
libcxx/include/__vector/vector_bool.h | 2 +-
libcxx/include/string | 2 +-
4 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/libcxx/include/__cxx03/string b/libcxx/include/__cxx03/string
index c4431dcb04d41..c29f74290bd41 100644
--- a/libcxx/include/__cxx03/string
+++ b/libcxx/include/__cxx03/string
@@ -2483,7 +2483,9 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 void basic_string<_CharT, _Traits, _Allocator>::__
__throw_length_error();
pointer __old_p = __get_pointer();
size_type __cap =
- __old_cap < __ms / 2 - __alignment ? __recommend(std::max(__old_cap + __delta_cap, 2 * __old_cap)) : __ms - 1;
+ __old_cap < __ms / 2 - __alignment
+ ? __recommend(std::max<size_type>(__old_cap + __delta_cap, 2 * __old_cap))
+ : __ms - 1;
__annotate_delete();
auto __allocation = std::__allocate_at_least(__alloc(), __cap + 1);
pointer __p = __allocation.ptr;
@@ -2526,7 +2528,9 @@ _LIBCPP_DEPRECATED_("use __grow_by_without_replace") basic_string<_CharT, _Trait
__throw_length_error();
pointer __old_p = __get_pointer();
size_type __cap =
- __old_cap < __ms / 2 - __alignment ? __recommend(std::max(__old_cap + __delta_cap, 2 * __old_cap)) : __ms - 1;
+ __old_cap < __ms / 2 - __alignment
+ ? __recommend(std::max<size_type>(__old_cap + __delta_cap, 2 * __old_cap))
+ : __ms - 1;
__annotate_delete();
auto __allocation = std::__allocate_at_least(__alloc(), __cap + 1);
pointer __p = __allocation.ptr;
diff --git a/libcxx/include/__cxx03/vector b/libcxx/include/__cxx03/vector
index 6ee35b4e36258..80da74ded67b7 100644
--- a/libcxx/include/__cxx03/vector
+++ b/libcxx/include/__cxx03/vector
@@ -2328,7 +2328,7 @@ vector<bool, _Allocator>::__recommend(size_type __new_size) const {
const size_type __cap = capacity();
if (__cap >= __ms / 2)
return __ms;
- return std::max(2 * __cap, __align_it(__new_size));
+ return std::max<size_type>(2 * __cap, __align_it(__new_size));
}
// Default constructs __n objects starting at __end_
diff --git a/libcxx/include/__vector/vector_bool.h b/libcxx/include/__vector/vector_bool.h
index 569cc5ea898bc..bc99470f126f6 100644
--- a/libcxx/include/__vector/vector_bool.h
+++ b/libcxx/include/__vector/vector_bool.h
@@ -549,7 +549,7 @@ vector<bool, _Allocator>::__recommend(size_type __new_size) const {
const size_type __cap = capacity();
if (__cap >= __ms / 2)
return __ms;
- return std::max(2 * __cap, __align_it(__new_size));
+ return std::max<size_type>(2 * __cap, __align_it(__new_size));
}
// Default constructs __n objects starting at __end_
diff --git a/libcxx/include/string b/libcxx/include/string
index fa87dc2fddb59..e36e7f307559d 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -2629,7 +2629,7 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 void basic_string<_CharT, _Traits, _Allocator>::__
__throw_length_error();
pointer __old_p = __get_pointer();
size_type __cap =
- __old_cap < __ms / 2 - __alignment ? __recommend(std::max(__old_cap + __delta_cap, 2 * __old_cap)) : __ms;
+ __old_cap < __ms / 2 - __alignment ? __recommend(std::max<size_type>(__old_cap + __delta_cap, 2 * __old_cap)) : __ms;
__annotate_delete();
auto __guard = std::__make_scope_guard(__annotate_new_size(*this));
auto __allocation = std::__allocate_at_least(__alloc_, __cap + 1);
>From a5bac730daa45274f1a4400475303106c550882d Mon Sep 17 00:00:00 2001
From: Peng Liu <winner245 at hotmail.com>
Date: Thu, 9 Jan 2025 08:42:10 -0500
Subject: [PATCH 2/5] Add new tests
---
.../vector.bool/sized_allocator.pass.cpp | 121 ++++++++++++++++++
1 file changed, 121 insertions(+)
create mode 100644 libcxx/test/std/containers/sequences/vector.bool/sized_allocator.pass.cpp
diff --git a/libcxx/test/std/containers/sequences/vector.bool/sized_allocator.pass.cpp b/libcxx/test/std/containers/sequences/vector.bool/sized_allocator.pass.cpp
new file mode 100644
index 0000000000000..0964f3a57ed70
--- /dev/null
+++ b/libcxx/test/std/containers/sequences/vector.bool/sized_allocator.pass.cpp
@@ -0,0 +1,121 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// <vector>
+// vector<bool>
+
+// This test examines ambiguous calls to std::max in vector<bool>
+// Fix https://github.com/llvm/llvm-project/issues/121713
+
+#include <cassert>
+#include <cstddef>
+#include <cstdint>
+#include <limits>
+#include <memory>
+#include <new>
+#include <vector>
+
+#include "test_macros.h"
+
+template <typename T, typename SIZE_TYPE = std::size_t, typename DIFF_TYPE = std::ptrdiff_t>
+class sized_allocator {
+ template <typename U, typename Sz, typename Diff>
+ friend class sized_allocator;
+
+public:
+ using value_type = T;
+ using size_type = SIZE_TYPE;
+ using difference_type = DIFF_TYPE;
+ using propagate_on_container_swap = std::true_type;
+
+ TEST_CONSTEXPR_CXX20 explicit sized_allocator(int d = 0) : data_(d) {}
+
+ template <typename U, typename Sz, typename Diff>
+ TEST_CONSTEXPR_CXX20 sized_allocator(const sized_allocator<U, Sz, Diff>& a) TEST_NOEXCEPT : data_(a.data_) {}
+
+ TEST_CONSTEXPR_CXX20 T* allocate(size_type n) {
+ if (n > max_size())
+ TEST_THROW(std::bad_array_new_length());
+ return std::allocator<T>().allocate(n);
+ }
+
+ TEST_CONSTEXPR_CXX20 void deallocate(T* p, size_type n) TEST_NOEXCEPT { std::allocator<T>().deallocate(p, n); }
+
+ TEST_CONSTEXPR size_type max_size() const TEST_NOEXCEPT {
+ return std::numeric_limits<size_type>::max() / sizeof(value_type);
+ }
+
+private:
+ int data_;
+
+ TEST_CONSTEXPR friend bool operator==(const sized_allocator& a, const sized_allocator& b) {
+ return a.data_ == b.data_;
+ }
+ TEST_CONSTEXPR friend bool operator!=(const sized_allocator& a, const sized_allocator& b) {
+ return a.data_ != b.data_;
+ }
+};
+
+TEST_CONSTEXPR_CXX20 bool tests() {
+ // The following tests are typical ways to trigger reallocations where `std::max` is used to calculate the capacity.
+ {
+ using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
+ std::vector<bool, Alloc> c(Alloc(1));
+ c.resize(10);
+ }
+ {
+ using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
+ std::vector<bool, Alloc> c(Alloc(1));
+ c.assign(10, true);
+ }
+ {
+ using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
+ std::vector<bool, Alloc> c(Alloc(1));
+ c.insert(c.end(), true);
+ }
+ {
+ using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
+ std::vector<bool, Alloc> c(Alloc(1));
+ c.insert(c.end(), 10, true);
+ }
+ {
+ using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
+ std::vector<bool, Alloc> c(Alloc(1));
+ c.push_back(true);
+ }
+ {
+ using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
+ std::vector<bool, Alloc> c(Alloc(1));
+ c.resize(10, true);
+ }
+ {
+ using Alloc = sized_allocator<bool, std::uint32_t, std::int32_t>;
+ std::vector<bool, Alloc> c(Alloc(1));
+ c.resize(10);
+ }
+ {
+ using Alloc = sized_allocator<bool, std::uint64_t, std::int64_t>;
+ std::vector<bool, Alloc> c(Alloc(1));
+ c.resize(10);
+ }
+ {
+ using Alloc = sized_allocator<bool, std::size_t, std::ptrdiff_t>;
+ std::vector<bool, Alloc> c(Alloc(1));
+ c.resize(10);
+ }
+
+ return true;
+}
+
+int main(int, char**) {
+ tests();
+#if TEST_STD_VER >= 20
+ static_assert(tests());
+#endif
+ return 0;
+}
>From 8039cf1f1c75e2405a1afed603b79445482c9496 Mon Sep 17 00:00:00 2001
From: Peng Liu <winner245 at hotmail.com>
Date: Wed, 15 Jan 2025 13:04:11 -0500
Subject: [PATCH 3/5] Address ldionne's comments
---
libcxx/include/__cxx03/string | 8 +--
libcxx/include/__cxx03/vector | 2 +-
libcxx/include/string | 2 +-
.../vector.bool/sized_allocator.pass.cpp | 63 +++++++------------
4 files changed, 27 insertions(+), 48 deletions(-)
diff --git a/libcxx/include/__cxx03/string b/libcxx/include/__cxx03/string
index c29f74290bd41..c4431dcb04d41 100644
--- a/libcxx/include/__cxx03/string
+++ b/libcxx/include/__cxx03/string
@@ -2483,9 +2483,7 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 void basic_string<_CharT, _Traits, _Allocator>::__
__throw_length_error();
pointer __old_p = __get_pointer();
size_type __cap =
- __old_cap < __ms / 2 - __alignment
- ? __recommend(std::max<size_type>(__old_cap + __delta_cap, 2 * __old_cap))
- : __ms - 1;
+ __old_cap < __ms / 2 - __alignment ? __recommend(std::max(__old_cap + __delta_cap, 2 * __old_cap)) : __ms - 1;
__annotate_delete();
auto __allocation = std::__allocate_at_least(__alloc(), __cap + 1);
pointer __p = __allocation.ptr;
@@ -2528,9 +2526,7 @@ _LIBCPP_DEPRECATED_("use __grow_by_without_replace") basic_string<_CharT, _Trait
__throw_length_error();
pointer __old_p = __get_pointer();
size_type __cap =
- __old_cap < __ms / 2 - __alignment
- ? __recommend(std::max<size_type>(__old_cap + __delta_cap, 2 * __old_cap))
- : __ms - 1;
+ __old_cap < __ms / 2 - __alignment ? __recommend(std::max(__old_cap + __delta_cap, 2 * __old_cap)) : __ms - 1;
__annotate_delete();
auto __allocation = std::__allocate_at_least(__alloc(), __cap + 1);
pointer __p = __allocation.ptr;
diff --git a/libcxx/include/__cxx03/vector b/libcxx/include/__cxx03/vector
index 80da74ded67b7..6ee35b4e36258 100644
--- a/libcxx/include/__cxx03/vector
+++ b/libcxx/include/__cxx03/vector
@@ -2328,7 +2328,7 @@ vector<bool, _Allocator>::__recommend(size_type __new_size) const {
const size_type __cap = capacity();
if (__cap >= __ms / 2)
return __ms;
- return std::max<size_type>(2 * __cap, __align_it(__new_size));
+ return std::max(2 * __cap, __align_it(__new_size));
}
// Default constructs __n objects starting at __end_
diff --git a/libcxx/include/string b/libcxx/include/string
index e36e7f307559d..fa87dc2fddb59 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -2629,7 +2629,7 @@ _LIBCPP_CONSTEXPR_SINCE_CXX20 void basic_string<_CharT, _Traits, _Allocator>::__
__throw_length_error();
pointer __old_p = __get_pointer();
size_type __cap =
- __old_cap < __ms / 2 - __alignment ? __recommend(std::max<size_type>(__old_cap + __delta_cap, 2 * __old_cap)) : __ms;
+ __old_cap < __ms / 2 - __alignment ? __recommend(std::max(__old_cap + __delta_cap, 2 * __old_cap)) : __ms;
__annotate_delete();
auto __guard = std::__make_scope_guard(__annotate_new_size(*this));
auto __allocation = std::__allocate_at_least(__alloc_, __cap + 1);
diff --git a/libcxx/test/std/containers/sequences/vector.bool/sized_allocator.pass.cpp b/libcxx/test/std/containers/sequences/vector.bool/sized_allocator.pass.cpp
index 0964f3a57ed70..fc7bbbe57faa5 100644
--- a/libcxx/test/std/containers/sequences/vector.bool/sized_allocator.pass.cpp
+++ b/libcxx/test/std/containers/sequences/vector.bool/sized_allocator.pass.cpp
@@ -9,7 +9,9 @@
// <vector>
// vector<bool>
-// This test examines ambiguous calls to std::max in vector<bool>
+// XFAIL: FROZEN-CXX03-HEADERS-FIXME
+
+// This test ensures that ambiguous calls to std::max in vector<bool> is fixed.
// Fix https://github.com/llvm/llvm-project/issues/121713
#include <cassert>
@@ -20,93 +22,74 @@
#include <new>
#include <vector>
+#include "sized_allocator.h"
#include "test_macros.h"
-template <typename T, typename SIZE_TYPE = std::size_t, typename DIFF_TYPE = std::ptrdiff_t>
-class sized_allocator {
- template <typename U, typename Sz, typename Diff>
- friend class sized_allocator;
-
-public:
- using value_type = T;
- using size_type = SIZE_TYPE;
- using difference_type = DIFF_TYPE;
- using propagate_on_container_swap = std::true_type;
-
- TEST_CONSTEXPR_CXX20 explicit sized_allocator(int d = 0) : data_(d) {}
-
- template <typename U, typename Sz, typename Diff>
- TEST_CONSTEXPR_CXX20 sized_allocator(const sized_allocator<U, Sz, Diff>& a) TEST_NOEXCEPT : data_(a.data_) {}
-
- TEST_CONSTEXPR_CXX20 T* allocate(size_type n) {
- if (n > max_size())
- TEST_THROW(std::bad_array_new_length());
- return std::allocator<T>().allocate(n);
- }
-
- TEST_CONSTEXPR_CXX20 void deallocate(T* p, size_type n) TEST_NOEXCEPT { std::allocator<T>().deallocate(p, n); }
-
- TEST_CONSTEXPR size_type max_size() const TEST_NOEXCEPT {
- return std::numeric_limits<size_type>::max() / sizeof(value_type);
- }
-
-private:
- int data_;
-
- TEST_CONSTEXPR friend bool operator==(const sized_allocator& a, const sized_allocator& b) {
- return a.data_ == b.data_;
- }
- TEST_CONSTEXPR friend bool operator!=(const sized_allocator& a, const sized_allocator& b) {
- return a.data_ != b.data_;
- }
-};
-
TEST_CONSTEXPR_CXX20 bool tests() {
// The following tests are typical ways to trigger reallocations where `std::max` is used to calculate the capacity.
+ // The purpose of these tests is to ensure that the ambiguous internal calls to `std::max` have been fixed.
{
using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
std::vector<bool, Alloc> c(Alloc(1));
c.resize(10);
+ assert(c.size() == 10);
+ assert(c.capacity() >= 10);
}
{
using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
std::vector<bool, Alloc> c(Alloc(1));
c.assign(10, true);
+ assert(c.size() == 10);
+ assert(c.capacity() >= 10);
}
{
using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
std::vector<bool, Alloc> c(Alloc(1));
c.insert(c.end(), true);
+ assert(c.size() == 1);
+ assert(c.capacity() >= 1);
}
{
using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
std::vector<bool, Alloc> c(Alloc(1));
c.insert(c.end(), 10, true);
+ assert(c.size() == 10);
+ assert(c.capacity() >= 10);
}
{
using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
std::vector<bool, Alloc> c(Alloc(1));
c.push_back(true);
+ assert(c.size() == 1);
+ assert(c.capacity() >= 1);
}
{
using Alloc = sized_allocator<bool, std::uint16_t, std::int16_t>;
std::vector<bool, Alloc> c(Alloc(1));
c.resize(10, true);
+ assert(c.size() == 10);
+ assert(c.capacity() >= 10);
}
{
using Alloc = sized_allocator<bool, std::uint32_t, std::int32_t>;
std::vector<bool, Alloc> c(Alloc(1));
c.resize(10);
+ assert(c.size() == 10);
+ assert(c.capacity() >= 10);
}
{
using Alloc = sized_allocator<bool, std::uint64_t, std::int64_t>;
std::vector<bool, Alloc> c(Alloc(1));
c.resize(10);
+ assert(c.size() == 10);
+ assert(c.capacity() >= 10);
}
{
using Alloc = sized_allocator<bool, std::size_t, std::ptrdiff_t>;
std::vector<bool, Alloc> c(Alloc(1));
c.resize(10);
+ assert(c.size() == 10);
+ assert(c.capacity() >= 10);
}
return true;
>From 49467baa89577242fa5ddda4fd6651223b1effe7 Mon Sep 17 00:00:00 2001
From: Peng Liu <winner245 at hotmail.com>
Date: Fri, 21 Mar 2025 08:42:40 -0400
Subject: [PATCH 4/5] Rename test file and slighly improve comments
---
...ocator.pass.cpp => max_call_varying_size_types.pass.cpp} | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
rename libcxx/test/std/containers/sequences/vector.bool/{sized_allocator.pass.cpp => max_call_varying_size_types.pass.cpp} (87%)
diff --git a/libcxx/test/std/containers/sequences/vector.bool/sized_allocator.pass.cpp b/libcxx/test/std/containers/sequences/vector.bool/max_call_varying_size_types.pass.cpp
similarity index 87%
rename from libcxx/test/std/containers/sequences/vector.bool/sized_allocator.pass.cpp
rename to libcxx/test/std/containers/sequences/vector.bool/max_call_varying_size_types.pass.cpp
index fc7bbbe57faa5..bb849e5b8ef4e 100644
--- a/libcxx/test/std/containers/sequences/vector.bool/sized_allocator.pass.cpp
+++ b/libcxx/test/std/containers/sequences/vector.bool/max_call_varying_size_types.pass.cpp
@@ -11,8 +11,8 @@
// XFAIL: FROZEN-CXX03-HEADERS-FIXME
-// This test ensures that ambiguous calls to std::max in vector<bool> is fixed.
-// Fix https://github.com/llvm/llvm-project/issues/121713
+// This test ensures that the internal call to std::max within vector<bool> is not ambiguous under various size types.
+// Related issue: https://github.com/llvm/llvm-project/issues/121713.
#include <cassert>
#include <cstddef>
@@ -26,8 +26,6 @@
#include "test_macros.h"
TEST_CONSTEXPR_CXX20 bool tests() {
- // The following tests are typical ways to trigger reallocations where `std::max` is used to calculate the capacity.
- // The purpose of these tests is to ensure that the ambiguous internal calls to `std::max` have been fixed.
{
using Alloc = sized_allocator<bool, std::uint8_t, std::int8_t>;
std::vector<bool, Alloc> c(Alloc(1));
>From 3271827de5b1d430a333333bef3ffa2b25403421 Mon Sep 17 00:00:00 2001
From: Peng Liu <winner245 at hotmail.com>
Date: Wed, 26 Mar 2025 13:19:17 -0400
Subject: [PATCH 5/5] Apply ldionne's changes
---
...ying_size_types.pass.cpp => small_allocator_size.pass.cpp} | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
rename libcxx/test/std/containers/sequences/vector.bool/{max_call_varying_size_types.pass.cpp => small_allocator_size.pass.cpp} (93%)
diff --git a/libcxx/test/std/containers/sequences/vector.bool/max_call_varying_size_types.pass.cpp b/libcxx/test/std/containers/sequences/vector.bool/small_allocator_size.pass.cpp
similarity index 93%
rename from libcxx/test/std/containers/sequences/vector.bool/max_call_varying_size_types.pass.cpp
rename to libcxx/test/std/containers/sequences/vector.bool/small_allocator_size.pass.cpp
index bb849e5b8ef4e..95e4c18cc7988 100644
--- a/libcxx/test/std/containers/sequences/vector.bool/max_call_varying_size_types.pass.cpp
+++ b/libcxx/test/std/containers/sequences/vector.bool/small_allocator_size.pass.cpp
@@ -11,8 +11,8 @@
// XFAIL: FROZEN-CXX03-HEADERS-FIXME
-// This test ensures that the internal call to std::max within vector<bool> is not ambiguous under various size types.
-// Related issue: https://github.com/llvm/llvm-project/issues/121713.
+// This test ensures that std::vector<bool> handles allocator types with small size types
+// properly. Related issue: https://github.com/llvm/llvm-project/issues/121713.
#include <cassert>
#include <cstddef>
More information about the libcxx-commits
mailing list