[libc++] Add the __is_replaceable type trait #132408
Open
Conversation
This file contains 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: Louis Dionne (ldionne) ChangesThat type trait represents whether move-assigning an object is equivalent to destroying it and then move-constructing a new one from the same argument. This will be useful in a few places where we may want to destroy + construct instead of doing an assignment, in particular when implementing some container operations in terms of relocation. Eventually, the library "emulation" added by this patch can be replaced once the compiler implements the trivial relocation proposals. Patch is 30.52 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/132408.diff 18 Files Affected:
diff --git a/libcxx/include/CMakeLists.txt b/libcxx/include/CMakeLists.txt
index 443e58fffe0d4..c8de4481409a9 100644
--- a/libcxx/include/CMakeLists.txt
+++ b/libcxx/include/CMakeLists.txt
@@ -838,6 +838,7 @@ set(files
__type_traits/is_reference.h
__type_traits/is_reference_wrapper.h
__type_traits/is_referenceable.h
+ __type_traits/is_replaceable.h
__type_traits/is_same.h
__type_traits/is_scalar.h
__type_traits/is_signed.h
diff --git a/libcxx/include/__exception/exception_ptr.h b/libcxx/include/__exception/exception_ptr.h
index 6257e6f729bf3..e93971cf7fc48 100644
--- a/libcxx/include/__exception/exception_ptr.h
+++ b/libcxx/include/__exception/exception_ptr.h
@@ -65,8 +65,11 @@ class _LIBCPP_EXPORTED_FROM_ABI exception_ptr {
friend _LIBCPP_HIDE_FROM_ABI exception_ptr make_exception_ptr(_Ep) _NOEXCEPT;
public:
- // exception_ptr is basically a COW string.
+ // exception_ptr is basically a COW string so it is trivially relocatable.
+ // However, it's not replaceable because destroying and move-constructing could cause
+ // the underlying refcount to hit 0 if we're self-assigning.
using __trivially_relocatable _LIBCPP_NODEBUG = exception_ptr;
+ using __replaceable _LIBCPP_NODEBUG = void;
_LIBCPP_HIDE_FROM_ABI exception_ptr() _NOEXCEPT : __ptr_() {}
_LIBCPP_HIDE_FROM_ABI exception_ptr(nullptr_t) _NOEXCEPT : __ptr_() {}
diff --git a/libcxx/include/__expected/expected.h b/libcxx/include/__expected/expected.h
index 03bbd1623ed5c..73d2845efa548 100644
--- a/libcxx/include/__expected/expected.h
+++ b/libcxx/include/__expected/expected.h
@@ -29,6 +29,7 @@
#include <__type_traits/is_nothrow_assignable.h>
#include <__type_traits/is_nothrow_constructible.h>
#include <__type_traits/is_reference.h>
+#include <__type_traits/is_replaceable.h>
#include <__type_traits/is_same.h>
#include <__type_traits/is_swappable.h>
#include <__type_traits/is_trivially_constructible.h>
@@ -470,6 +471,8 @@ class expected : private __expected_base<_Tp, _Err> {
__conditional_t<__libcpp_is_trivially_relocatable<_Tp>::value && __libcpp_is_trivially_relocatable<_Err>::value,
expected,
void>;
+ using __replaceable _LIBCPP_NODEBUG =
+ __conditional_t<__is_replaceable<_Tp>::value && __is_replaceable<_Err>::value, expected, void>;
template <class _Up>
using rebind = expected<_Up, error_type>;
diff --git a/libcxx/include/__locale b/libcxx/include/__locale
index 5ae3228989749..429395c740a05 100644
--- a/libcxx/include/__locale
+++ b/libcxx/include/__locale
@@ -50,8 +50,11 @@ _LIBCPP_HIDE_FROM_ABI const _Facet& use_facet(const locale&);
class _LIBCPP_EXPORTED_FROM_ABI locale {
public:
- // locale is essentially a shared_ptr that doesn't support weak_ptrs and never got a move constructor.
+ // locale is essentially a shared_ptr that doesn't support weak_ptrs and never got a move constructor,
+ // so it is trivially relocatable. However, it is not replaceable because self-assignment must prevent
+ // the refcount from hitting 0.
using __trivially_relocatable _LIBCPP_NODEBUG = locale;
+ using __replaceable _LIBCPP_NODEBUG = void;
// types:
class _LIBCPP_EXPORTED_FROM_ABI facet;
diff --git a/libcxx/include/__memory/shared_ptr.h b/libcxx/include/__memory/shared_ptr.h
index e1d49c4594866..752773e2cf934 100644
--- a/libcxx/include/__memory/shared_ptr.h
+++ b/libcxx/include/__memory/shared_ptr.h
@@ -317,7 +317,11 @@ class _LIBCPP_SHARED_PTR_TRIVIAL_ABI _LIBCPP_TEMPLATE_VIS shared_ptr {
// A shared_ptr contains only two raw pointers which point to the heap and move constructing already doesn't require
// any bookkeeping, so it's always trivially relocatable.
+ //
+ // However, it's not replaceable because of self-assignment, which must prevent the refcount from
+ // hitting 0.
using __trivially_relocatable _LIBCPP_NODEBUG = shared_ptr;
+ using __replaceable _LIBCPP_NODEBUG = void;
private:
element_type* __ptr_;
@@ -1212,7 +1216,10 @@ class _LIBCPP_SHARED_PTR_TRIVIAL_ABI _LIBCPP_TEMPLATE_VIS weak_ptr {
// A weak_ptr contains only two raw pointers which point to the heap and move constructing already doesn't require
// any bookkeeping, so it's always trivially relocatable.
+ //
+ // However, it's not replaceable because we must preserve a non-zero refcount through self-assignment.
using __trivially_relocatable _LIBCPP_NODEBUG = weak_ptr;
+ using __replaceable _LIBCPP_NODEBUG = void;
private:
element_type* __ptr_;
diff --git a/libcxx/include/__memory/unique_ptr.h b/libcxx/include/__memory/unique_ptr.h
index 6f1dc98db5a9f..b3219b8ed214e 100644
--- a/libcxx/include/__memory/unique_ptr.h
+++ b/libcxx/include/__memory/unique_ptr.h
@@ -39,6 +39,7 @@
#include <__type_traits/is_function.h>
#include <__type_traits/is_pointer.h>
#include <__type_traits/is_reference.h>
+#include <__type_traits/is_replaceable.h>
#include <__type_traits/is_same.h>
#include <__type_traits/is_swappable.h>
#include <__type_traits/is_trivially_relocatable.h>
@@ -144,6 +145,8 @@ class _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI _LIBCPP_TEMPLATE_VIS unique_ptr {
__libcpp_is_trivially_relocatable<pointer>::value && __libcpp_is_trivially_relocatable<deleter_type>::value,
unique_ptr,
void>;
+ using __replaceable _LIBCPP_NODEBUG =
+ __conditional_t<__is_replaceable<pointer>::value && __is_replaceable<deleter_type>::value, unique_ptr, void>;
private:
_LIBCPP_COMPRESSED_PAIR(pointer, __ptr_, deleter_type, __deleter_);
@@ -410,6 +413,8 @@ class _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI _LIBCPP_TEMPLATE_VIS unique_ptr<_Tp[], _Dp>
__libcpp_is_trivially_relocatable<pointer>::value && __libcpp_is_trivially_relocatable<deleter_type>::value,
unique_ptr,
void>;
+ using __replaceable _LIBCPP_NODEBUG =
+ __conditional_t<__is_replaceable<pointer>::value && __is_replaceable<deleter_type>::value, unique_ptr, void>;
private:
template <class _Up, class _OtherDeleter>
diff --git a/libcxx/include/__split_buffer b/libcxx/include/__split_buffer
index 721d4d497f2a5..1d75657baa4f8 100644
--- a/libcxx/include/__split_buffer
+++ b/libcxx/include/__split_buffer
@@ -28,6 +28,7 @@
#include <__type_traits/integral_constant.h>
#include <__type_traits/is_nothrow_assignable.h>
#include <__type_traits/is_nothrow_constructible.h>
+#include <__type_traits/is_replaceable.h>
#include <__type_traits/is_swappable.h>
#include <__type_traits/is_trivially_destructible.h>
#include <__type_traits/is_trivially_relocatable.h>
@@ -72,6 +73,10 @@ public:
__libcpp_is_trivially_relocatable<pointer>::value && __libcpp_is_trivially_relocatable<allocator_type>::value,
__split_buffer,
void>;
+ using __replaceable _LIBCPP_NODEBUG =
+ __conditional_t<__is_replaceable<pointer>::value && __container_allocator_is_replaceable<__alloc_traits>::value,
+ __split_buffer,
+ void>;
pointer __first_;
pointer __begin_;
diff --git a/libcxx/include/__type_traits/is_replaceable.h b/libcxx/include/__type_traits/is_replaceable.h
new file mode 100644
index 0000000000000..46eb6f79ecb69
--- /dev/null
+++ b/libcxx/include/__type_traits/is_replaceable.h
@@ -0,0 +1,59 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef _LIBCPP___TYPE_TRAITS_IS_REPLACEABLE_H
+#define _LIBCPP___TYPE_TRAITS_IS_REPLACEABLE_H
+
+#include <__config>
+#include <__type_traits/enable_if.h>
+#include <__type_traits/integral_constant.h>
+#include <__type_traits/is_same.h>
+#include <__type_traits/is_trivially_copyable.h>
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+# pragma GCC system_header
+#endif
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+// A type is replaceable if `x = std::move(y)` is equivalent to:
+//
+// std::destroy_at(&x)
+// std::construct_at(&x, std::move(y))
+//
+// This allows turning a move-assignment into a sequence of destroy + move-construct, which
+// is often more efficient. This is especially relevant when the move-construct is in fact
+// part of a trivial relocation from somewhere else, in which case there is a huge win.
+//
+// Note that this requires language support in order to be really effective, but we
+// currently emulate the base template with something very conservative.
+template <class _Tp, class = void>
+struct __is_replaceable : is_trivially_copyable<_Tp> {};
+
+template <class _Tp>
+struct __is_replaceable<_Tp, __enable_if_t<is_same<_Tp, typename _Tp::__replaceable>::value> > : true_type {};
+
+// Determines whether an allocator member of a container is replaceable.
+//
+// We take into account whether the allocator is propagated on assignments. If the allocator
+// always compares equal, then it doesn't matter whether we propagate it or not on assignments,
+// the result will be the same and we can just as much move-construct it instead.
+//
+// If the allocator does not always compare equal, we check whether it propagates on assignment
+// and it is replaceable.
+template <class _AllocatorTraits>
+struct __container_allocator_is_replaceable
+ : integral_constant<bool,
+ _AllocatorTraits::is_always_equal::value ||
+ (_AllocatorTraits::propagate_on_container_move_assignment::value &&
+ _AllocatorTraits::propagate_on_container_copy_assignment::value &&
+ __is_replaceable<typename _AllocatorTraits::allocator_type>::value)> {};
+
+_LIBCPP_END_NAMESPACE_STD
+
+#endif // _LIBCPP___TYPE_TRAITS_IS_REPLACEABLE_H
diff --git a/libcxx/include/__utility/pair.h b/libcxx/include/__utility/pair.h
index b4ac977f587a5..6f2ef5bafabac 100644
--- a/libcxx/include/__utility/pair.h
+++ b/libcxx/include/__utility/pair.h
@@ -31,6 +31,8 @@
#include <__type_traits/is_implicitly_default_constructible.h>
#include <__type_traits/is_nothrow_assignable.h>
#include <__type_traits/is_nothrow_constructible.h>
+#include <__type_traits/is_replaceable.h>
+#include <__type_traits/is_same.h>
#include <__type_traits/is_swappable.h>
#include <__type_traits/is_trivially_relocatable.h>
#include <__type_traits/nat.h>
@@ -72,6 +74,8 @@ struct _LIBCPP_TEMPLATE_VIS pair
__conditional_t<__libcpp_is_trivially_relocatable<_T1>::value && __libcpp_is_trivially_relocatable<_T2>::value,
pair,
void>;
+ using __replaceable _LIBCPP_NODEBUG =
+ __conditional_t<__is_replaceable<_T1>::value && __is_replaceable<_T2>::value, pair, void>;
_LIBCPP_HIDE_FROM_ABI pair(pair const&) = default;
_LIBCPP_HIDE_FROM_ABI pair(pair&&) = default;
diff --git a/libcxx/include/__vector/vector.h b/libcxx/include/__vector/vector.h
index 9155fb52a69b1..535359ff6831f 100644
--- a/libcxx/include/__vector/vector.h
+++ b/libcxx/include/__vector/vector.h
@@ -55,6 +55,7 @@
#include <__type_traits/is_nothrow_assignable.h>
#include <__type_traits/is_nothrow_constructible.h>
#include <__type_traits/is_pointer.h>
+#include <__type_traits/is_replaceable.h>
#include <__type_traits/is_same.h>
#include <__type_traits/is_trivially_relocatable.h>
#include <__type_traits/type_identity.h>
@@ -120,6 +121,10 @@ class _LIBCPP_TEMPLATE_VIS vector {
__libcpp_is_trivially_relocatable<pointer>::value && __libcpp_is_trivially_relocatable<allocator_type>::value,
vector,
void>;
+ using __replaceable _LIBCPP_NODEBUG =
+ __conditional_t<__is_replaceable<pointer>::value && __container_allocator_is_replaceable<__alloc_traits>::value,
+ vector,
+ void>;
static_assert(__check_valid_allocator<allocator_type>::value, "");
static_assert(is_same<typename allocator_type::value_type, value_type>::value,
diff --git a/libcxx/include/array b/libcxx/include/array
index 44c6bb5c5cc93..573d41bab52e8 100644
--- a/libcxx/include/array
+++ b/libcxx/include/array
@@ -134,6 +134,7 @@ template <size_t I, class T, size_t N> const T&& get(const array<T, N>&&) noexce
# include <__type_traits/is_const.h>
# include <__type_traits/is_constructible.h>
# include <__type_traits/is_nothrow_constructible.h>
+# include <__type_traits/is_replaceable.h>
# include <__type_traits/is_same.h>
# include <__type_traits/is_swappable.h>
# include <__type_traits/is_trivially_relocatable.h>
@@ -175,6 +176,7 @@ template <class _Tp, size_t _Size>
struct _LIBCPP_TEMPLATE_VIS array {
using __trivially_relocatable _LIBCPP_NODEBUG =
__conditional_t<__libcpp_is_trivially_relocatable<_Tp>::value, array, void>;
+ using __replaceable _LIBCPP_NODEBUG = __conditional_t<__is_replaceable<_Tp>::value, array, void>;
// types:
using __self _LIBCPP_NODEBUG = array;
diff --git a/libcxx/include/deque b/libcxx/include/deque
index 04788c277e428..ef0dda802f2f1 100644
--- a/libcxx/include/deque
+++ b/libcxx/include/deque
@@ -230,6 +230,7 @@ template <class T, class Allocator, class Predicate>
# include <__type_traits/is_convertible.h>
# include <__type_traits/is_nothrow_assignable.h>
# include <__type_traits/is_nothrow_constructible.h>
+# include <__type_traits/is_replaceable.h>
# include <__type_traits/is_same.h>
# include <__type_traits/is_swappable.h>
# include <__type_traits/is_trivially_relocatable.h>
@@ -530,6 +531,10 @@ public:
__libcpp_is_trivially_relocatable<__map>::value && __libcpp_is_trivially_relocatable<allocator_type>::value,
deque,
void>;
+ using __replaceable _LIBCPP_NODEBUG =
+ __conditional_t<__is_replaceable<__map>::value && __container_allocator_is_replaceable<__alloc_traits>::value,
+ deque,
+ void>;
static_assert(is_nothrow_default_constructible<allocator_type>::value ==
is_nothrow_default_constructible<__pointer_allocator>::value,
diff --git a/libcxx/include/module.modulemap b/libcxx/include/module.modulemap
index 82c51ba73e41e..67403bc9ff8c7 100644
--- a/libcxx/include/module.modulemap
+++ b/libcxx/include/module.modulemap
@@ -275,6 +275,10 @@ module std_core [system] {
header "__type_traits/is_referenceable.h"
export std_core.type_traits.integral_constant
}
+ module is_replaceable {
+ header "__type_traits/is_replaceable.h"
+ export std_core.type_traits.integral_constant
+ }
module is_same {
header "__type_traits/is_same.h"
export std_core.type_traits.integral_constant
diff --git a/libcxx/include/optional b/libcxx/include/optional
index 294c3ddf993fa..b4d7127c1ea99 100644
--- a/libcxx/include/optional
+++ b/libcxx/include/optional
@@ -210,6 +210,7 @@ namespace std {
# include <__type_traits/is_nothrow_constructible.h>
# include <__type_traits/is_object.h>
# include <__type_traits/is_reference.h>
+# include <__type_traits/is_replaceable.h>
# include <__type_traits/is_same.h>
# include <__type_traits/is_scalar.h>
# include <__type_traits/is_swappable.h>
@@ -590,6 +591,7 @@ public:
using __trivially_relocatable _LIBCPP_NODEBUG =
conditional_t<__libcpp_is_trivially_relocatable<_Tp>::value, optional, void>;
+ using __replaceable _LIBCPP_NODEBUG = conditional_t<__is_replaceable<_Tp>::value, optional, void>;
private:
// Disable the reference extension using this static assert.
diff --git a/libcxx/include/string b/libcxx/include/string
index fa87dc2fddb59..c7e33a83fd758 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -630,6 +630,7 @@ basic_string<char32_t> operator""s( const char32_t *str, size_t len );
# include <__type_traits/is_convertible.h>
# include <__type_traits/is_nothrow_assignable.h>
# include <__type_traits/is_nothrow_constructible.h>
+# include <__type_traits/is_replaceable.h>
# include <__type_traits/is_same.h>
# include <__type_traits/is_standard_layout.h>
# include <__type_traits/is_trivial.h>
@@ -754,6 +755,9 @@ public:
// external memory. In such cases, the destructor is responsible for unpoisoning
// the memory to avoid triggering false positives.
// Therefore it's crucial to ensure the destructor is called.
+ //
+ // However, it is replaceable since implementing move-assignment as a destroy + move-construct
+ // will maintain the right ASAN state.
using __trivially_relocatable = void;
# else
using __trivially_relocatable _LIBCPP_NODEBUG = __conditional_t<
@@ -761,6 +765,10 @@ public:
basic_string,
void>;
# endif
+ using __replaceable _LIBCPP_NODEBUG =
+ __conditional_t<__is_replaceable<pointer>::value && __container_allocator_is_replaceable<__alloc_traits>::value,
+ basic_string,
+ void>;
# if _LIBCPP_HAS_ASAN && _LIBCPP_INSTRUMENTED_WITH_ASAN
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 pointer __asan_volatile_wrapper(pointer const& __ptr) const {
diff --git a/libcxx/include/tuple b/libcxx/include/tuple
index 1e0703421da01..20199aeb88a49 100644
--- a/libcxx/include/tuple
+++ b/libcxx/include/tuple
@@ -250,6 +250,7 @@ template <class... Types>
# include <__type_traits/is_nothrow_assignable.h>
# include <__type_traits/is_nothrow_constructible.h>
# include <__type_traits/is_reference.h>
+# include <__type_traits/is_replaceable.h>
# include <__type_traits/is_same.h>
# include <__type_traits/is_swappable.h>
# include <__type_traits/is_trivially_relocatable.h>
@@ -555,6 +556,7 @@ class _LIBCPP_TEMPLATE_VIS _LIBCPP_NO_SPECIALIZATIONS tuple {
public:
using __trivially_relocatable _LIBCPP_NODEBUG =
__conditional_t<_And<__libcpp_is_trivially_relocatable<_Tp>...>::value, tuple, void>;
+ using __replaceable _LIBCPP_NODEBUG = __conditional_t<_And<__is_replaceable<_Tp>...>::value, tuple, void>;
// [tuple.cnstr]
diff --git a/libcxx/include/variant b/libcxx/include/variant
index 9e78ff2cc1a4f..3d2a426391166 100644
--- a/libcxx/include/variant
+++ b/libcxx/include/variant
@@ -246,6 +246,7 @@ namespace std {
# include <__type_traits/is_nothrow_assignable.h>
# include <__type_traits/is_nothrow_constructible.h>
# include <__type_traits/is_reference.h>
+# include <__type_traits/is_replaceable.h>
# include <__type_traits/is_same.h>
# include <__type_traits/is_swappable.h>
# include <__type_traits/is_trivially_assignable.h>
@@ -1176,6 +1177,7 @@ class _LIBCPP_TEMPLATE_VIS _LIBCPP_DECLSPEC_EMPTY_BASES _LIBCPP_NO_SPECIALIZATIO
public:
using __trivially_relocatable _LIBCPP_NODEBUG =
conditional_t<_And<__libcpp_is_trivially_relocatable<_Types>...>::value, variant, void>;
+ using __replaceable = conditional_t<_And<__is_replaceable<_Types>...>::value, variant, void>;
template <bool _Dummy = true,
enable_if_t<__dependent_type<is_default_constructible<__first_type>, _Dummy>::value, int> = 0>
diff --git a/libcxx/test/libcxx/type_traits/is_replaceable.compile.pass.cpp b/libcxx/test/libcxx/type_traits/is_replaceable.compile.pass.cpp
new file mode 100644
index 0000000000000..c52f5462f8bb3
--- /dev/null
+++ b/libcxx/test/libcxx/type_traits/is_replaceable.compile.pass.cpp
@@ -0,0 +1,269 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#include <__type_traits/is_replaceable.h>
+#include <array>
+#include <deque>
+#include <exception>
+#include <expected>
+#include <memory>
+#include <optional>
+#include <string>
+#include <tuple>
+#include <type_traits>
+#include <variant>
+#include <vector>
+
+#include "constexpr_char_traits.h"
+#include "test_allocator.h"
+#include "test_macros.h"
+
+#ifndef TEST_HAS_NO_LOCALIZATION
+# include <locale>
+#endif
+
+template <class T>
+struct NonPropagatingStatefulMoveAssignAlloc : std::allocator<T> {
+ using propagate_on_container_move_assignment = std::false_type;
+ using is_always_equal = std::fals...
[truncated]
|
Yes, that's what it is! I updated the commit message. |
mordante
reviewed
Mar 21, 2025
| using __replaceable _LIBCPP_NODEBUG = | ||
| __conditional_t<__is_replaceable<pointer>::value && __container_allocator_is_replaceable<__alloc_traits>::value, | ||
| vector, | ||
| void>; |
There was a problem hiding this comment.
Not related to this patch. How does this affect std::vector<bool> when vector_bool.his included directly andvector` is not? I expect this can cause an ODR violation.
There was a problem hiding this comment.
That's a good question. We have a forward declaration of vector<bool> inside __fwd/vector.h, so the base template definition in vector.h doesn't count for vector<bool>. It should be a compiler error if you try to use vector<bool> without having included it. For example: https://godbolt.org/z/GT5zePY97
There was a problem hiding this comment.
I'm more concerned about
//--- MyHeader.h
#include <__vector/vector_bool.h>
void f(std::vector<bool>);
//--- TU1.cpp
#include "MyHeader.h"
// uses std::vector<bool>
//--- TU2.cpp
#include <vector>
#include "MyHeader.h"
// uses std::vector<bool>
Users shouldn't include <__vector/vector_bool.h> but one of our headers may just do that.
There was a problem hiding this comment.
I don't understand the concern here. This code in no way concerns vector<bool>.
There was a problem hiding this comment.
@mordante Can you please explain the issue you see with the code snippet above? I've been staring at it and I'm not sure what you have in mind. Is there something wrong with that code?
There was a problem hiding this comment.
The issue I see is that the value of __replaceable differs. The TU1 does not define it for std::vector<bool> while TU2 does. This due to the place where __replaceable is defined. This definition is not included by the header "__vector/vector_bool.h".
There was a problem hiding this comment.
The value isn't different. If you query __is_replacable on an incomplete type you will get a compilation error: https://godbolt.org/z/chGxx5jxG
That type trait represents whether move-assigning an object is equivalent to destroying it and then move-constructing a new one from the same argument. This will be useful in a few places where we may want to destroy + construct instead of doing an assignment, in particular when implementing some container operations in terms of relocation. Eventually, the library "emulation" added by this patch can be replaced once the compiler implements the trivial relocation proposals.
This patch itself does not change the performance of anything. However, knowing whether a type is replaceable will be necessary when we want to implement some |
ldionne
force-pushed
the
review/tr-1-add-is_replaceable
branch
from
79a8d46 to
be5f620
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
mordante
reviewed
Mar 24, 2025
| using __replaceable _LIBCPP_NODEBUG = | ||
| __conditional_t<__is_replaceable<pointer>::value && __container_allocator_is_replaceable<__alloc_traits>::value, | ||
| vector, | ||
| void>; |
There was a problem hiding this comment.
I'm more concerned about
//--- MyHeader.h
#include <__vector/vector_bool.h>
void f(std::vector<bool>);
//--- TU1.cpp
#include "MyHeader.h"
// uses std::vector<bool>
//--- TU2.cpp
#include <vector>
#include "MyHeader.h"
// uses std::vector<bool>
Users shouldn't include <__vector/vector_bool.h> but one of our headers may just do that.
philnik777
reviewed
Mar 25, 2025
Comment on lines
68
to
70
| // exception_ptr is basically a COW string so it is trivially relocatable. | ||
| // However, it's not replaceable because destroying and move-constructing could cause | ||
| // the underlying refcount to hit 0 if we're self-assigning. |
There was a problem hiding this comment.
I'm not sure I understand this. If we're self-assigning it's a no-op and shouldn't potentially delete the underlying resource.
There was a problem hiding this comment.
The semantics of __is_replaceable mean that move-assignment is equivalent to destroy followed by a move construction. However, you can't replace a move-assignment by a destroy + move-construct for std::exception_ptr, since if both the source and destination are the same object, the destroy + move-construction will result in the refcount hitting 0 (after destruction) whereas the move-assignment wouldn't have.
This actually makes me realize: you can never do destroy + move-construct if the source and the destination are the same object. This has nothing to do with the presence of a refcount in the object. How is __is_replaceable supposed to work in that case? Aren't all types non-replaceable, then?
There was a problem hiding this comment.
I don't think you have to consider the same memory location case. You can't relocate an object into the same place it is already in, since you can't relocate from an object that is already destroyed (which you just did by relocating into the same memory location).
There was a problem hiding this comment.
That makes me wonder how useful __is_replaceable is as a trait outside of relocation use cases. It might not be.
In order to make progress for the time being, I will add documentation clarifying that __is_replaceable doesn't apply if you're doing a self-assignment. And then I will revisit whether these types can be replaceable or not based on that new point of view.
| using __replaceable _LIBCPP_NODEBUG = | ||
| __conditional_t<__is_replaceable<pointer>::value && __container_allocator_is_replaceable<__alloc_traits>::value, | ||
| vector, | ||
| void>; |
There was a problem hiding this comment.
I don't understand the concern here. This code in no way concerns vector<bool>.
|
#127636 add a builtin for that |
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.
That type trait represents whether move-assigning an object is equivalent to destroying it and then move-constructing a new one from the same argument. This will be useful in a few places where we may want to destroy + construct instead of doing an assignment, in particular when implementing some container operations in terms of relocation.
This is effectively adding a library emulation of P2786R12's is_replaceable trait, similarly to what we do for trivial relocation. Eventually, we can replace this library emulation by the real compiler-backed trait.
This is building towards #129328