[libc++] LWG3870: Remove voidify
#110355
Merged
[libc++] LWG3870: Remove voidify
#110355
Conversation
This file contains hidden or 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: A. Jiang (frederick-vs-ja) ChangesInstead of changing the cast sequence to implicit conversion in Test coverage for const iterators are removed. Now most affected algorithms are underconstrained, for which I submitted LWG3888. I'm not sure whether we should speculatively implement it at this moment, and thus haven't added any In some control block types and Fixes #105119. Patch is 27.86 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/110355.diff 20 Files Affected:
diff --git a/libcxx/include/CMakeLists.txt b/libcxx/include/CMakeLists.txt
index 8a63280053340f..9bd1b41b8bfac4 100644
--- a/libcxx/include/CMakeLists.txt
+++ b/libcxx/include/CMakeLists.txt
@@ -560,7 +560,6 @@ set(files
__memory/unique_temporary_buffer.h
__memory/uses_allocator.h
__memory/uses_allocator_construction.h
- __memory/voidify.h
__memory_resource/memory_resource.h
__memory_resource/monotonic_buffer_resource.h
__memory_resource/polymorphic_allocator.h
diff --git a/libcxx/include/__memory/construct_at.h b/libcxx/include/__memory/construct_at.h
index eb021324800644..d8c97467f54b9f 100644
--- a/libcxx/include/__memory/construct_at.h
+++ b/libcxx/include/__memory/construct_at.h
@@ -14,7 +14,6 @@
#include <__config>
#include <__iterator/access.h>
#include <__memory/addressof.h>
-#include <__memory/voidify.h>
#include <__type_traits/enable_if.h>
#include <__type_traits/is_array.h>
#include <__utility/declval.h>
@@ -38,7 +37,7 @@ _LIBCPP_BEGIN_NAMESPACE_STD
template <class _Tp, class... _Args, class = decltype(::new(std::declval<void*>()) _Tp(std::declval<_Args>()...))>
_LIBCPP_HIDE_FROM_ABI constexpr _Tp* construct_at(_Tp* __location, _Args&&... __args) {
_LIBCPP_ASSERT_NON_NULL(__location != nullptr, "null pointer given to construct_at");
- return ::new (std::__voidify(*__location)) _Tp(std::forward<_Args>(__args)...);
+ return ::new (static_cast<void*>(__location)) _Tp(std::forward<_Args>(__args)...);
}
#endif
@@ -49,7 +48,7 @@ _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _Tp* __construct_at(_Tp* __l
return std::construct_at(__location, std::forward<_Args>(__args)...);
#else
return _LIBCPP_ASSERT_NON_NULL(__location != nullptr, "null pointer given to construct_at"),
- ::new (std::__voidify(*__location)) _Tp(std::forward<_Args>(__args)...);
+ ::new (static_cast<void*>(__location)) _Tp(std::forward<_Args>(__args)...);
#endif
}
diff --git a/libcxx/include/__memory/shared_ptr.h b/libcxx/include/__memory/shared_ptr.h
index 70964e6122d5a6..20c1b69f45ae66 100644
--- a/libcxx/include/__memory/shared_ptr.h
+++ b/libcxx/include/__memory/shared_ptr.h
@@ -248,33 +248,35 @@ struct __for_overwrite_tag {};
template <class _Tp, class _Alloc>
struct __shared_ptr_emplace : __shared_weak_count {
+ using __value_type = __remove_cv_t<_Tp>;
+
template <class... _Args,
class _Allocator = _Alloc,
__enable_if_t<is_same<typename _Allocator::value_type, __for_overwrite_tag>::value, int> = 0>
_LIBCPP_HIDE_FROM_ABI explicit __shared_ptr_emplace(_Alloc __a, _Args&&...) : __storage_(std::move(__a)) {
static_assert(
sizeof...(_Args) == 0, "No argument should be provided to the control block when using _for_overwrite");
- ::new ((void*)__get_elem()) _Tp;
+ ::new (static_cast<void*>(__get_elem())) __value_type;
}
template <class... _Args,
class _Allocator = _Alloc,
__enable_if_t<!is_same<typename _Allocator::value_type, __for_overwrite_tag>::value, int> = 0>
_LIBCPP_HIDE_FROM_ABI explicit __shared_ptr_emplace(_Alloc __a, _Args&&... __args) : __storage_(std::move(__a)) {
- using _TpAlloc = typename __allocator_traits_rebind<_Alloc, __remove_cv_t<_Tp> >::type;
+ using _TpAlloc = typename __allocator_traits_rebind<_Alloc, __value_type>::type;
_TpAlloc __tmp(*__get_alloc());
allocator_traits<_TpAlloc>::construct(__tmp, __get_elem(), std::forward<_Args>(__args)...);
}
_LIBCPP_HIDE_FROM_ABI _Alloc* __get_alloc() _NOEXCEPT { return __storage_.__get_alloc(); }
- _LIBCPP_HIDE_FROM_ABI _Tp* __get_elem() _NOEXCEPT { return __storage_.__get_elem(); }
+ _LIBCPP_HIDE_FROM_ABI __value_type* __get_elem() _NOEXCEPT { return __storage_.__get_elem(); }
private:
template <class _Allocator = _Alloc,
__enable_if_t<is_same<typename _Allocator::value_type, __for_overwrite_tag>::value, int> = 0>
_LIBCPP_HIDE_FROM_ABI void __on_zero_shared_impl() _NOEXCEPT {
- __get_elem()->~_Tp();
+ __get_elem()->~__value_type();
}
template <class _Allocator = _Alloc,
@@ -300,7 +302,7 @@ struct __shared_ptr_emplace : __shared_weak_count {
// through `std::allocate_shared` and `std::make_shared`.
struct _Storage {
struct _Data {
- _LIBCPP_COMPRESSED_PAIR(_Alloc, __alloc_, _Tp, __elem_);
+ _LIBCPP_COMPRESSED_PAIR(_Alloc, __alloc_, __value_type, __elem_);
};
_ALIGNAS_TYPE(_Data) char __buffer_[sizeof(_Data)];
@@ -312,7 +314,7 @@ struct __shared_ptr_emplace : __shared_weak_count {
return std::addressof(reinterpret_cast<_Data*>(__buffer_)->__alloc_);
}
- _LIBCPP_HIDE_FROM_ABI _LIBCPP_NO_CFI _Tp* __get_elem() _NOEXCEPT {
+ _LIBCPP_HIDE_FROM_ABI _LIBCPP_NO_CFI __value_type* __get_elem() _NOEXCEPT {
return std::addressof(reinterpret_cast<_Data*>(__buffer_)->__elem_);
}
};
diff --git a/libcxx/include/__memory/uninitialized_algorithms.h b/libcxx/include/__memory/uninitialized_algorithms.h
index 8ff87e28b3bb51..dd72f3c10cf15a 100644
--- a/libcxx/include/__memory/uninitialized_algorithms.h
+++ b/libcxx/include/__memory/uninitialized_algorithms.h
@@ -21,7 +21,6 @@
#include <__memory/allocator_traits.h>
#include <__memory/construct_at.h>
#include <__memory/pointer_traits.h>
-#include <__memory/voidify.h>
#include <__type_traits/enable_if.h>
#include <__type_traits/extent.h>
#include <__type_traits/is_array.h>
@@ -64,7 +63,7 @@ inline _LIBCPP_HIDE_FROM_ABI pair<_InputIterator, _ForwardIterator> __uninitiali
try {
#endif
for (; __ifirst != __ilast && !__stop_copying(__idx); ++__ifirst, (void)++__idx)
- ::new (std::__voidify(*__idx)) _ValueType(*__ifirst);
+ ::new (static_cast<void*>(std::addressof(*__idx))) _ValueType(*__ifirst);
#ifndef _LIBCPP_HAS_NO_EXCEPTIONS
} catch (...) {
std::__destroy(__ofirst, __idx);
@@ -94,7 +93,7 @@ __uninitialized_copy_n(_InputIterator __ifirst, _Size __n, _ForwardIterator __of
try {
#endif
for (; __n > 0 && !__stop_copying(__idx); ++__ifirst, (void)++__idx, (void)--__n)
- ::new (std::__voidify(*__idx)) _ValueType(*__ifirst);
+ ::new (static_cast<void*>(std::addressof(*__idx))) _ValueType(*__ifirst);
#ifndef _LIBCPP_HAS_NO_EXCEPTIONS
} catch (...) {
std::__destroy(__ofirst, __idx);
@@ -124,7 +123,7 @@ __uninitialized_fill(_ForwardIterator __first, _Sentinel __last, const _Tp& __x)
try {
#endif
for (; __idx != __last; ++__idx)
- ::new (std::__voidify(*__idx)) _ValueType(__x);
+ ::new (static_cast<void*>(std::addressof(*__idx))) _ValueType(__x);
#ifndef _LIBCPP_HAS_NO_EXCEPTIONS
} catch (...) {
std::__destroy(__first, __idx);
@@ -152,7 +151,7 @@ __uninitialized_fill_n(_ForwardIterator __first, _Size __n, const _Tp& __x) {
try {
#endif
for (; __n > 0; ++__idx, (void)--__n)
- ::new (std::__voidify(*__idx)) _ValueType(__x);
+ ::new (static_cast<void*>(std::addressof(*__idx))) _ValueType(__x);
#ifndef _LIBCPP_HAS_NO_EXCEPTIONS
} catch (...) {
std::__destroy(__first, __idx);
@@ -182,7 +181,7 @@ __uninitialized_default_construct(_ForwardIterator __first, _Sentinel __last) {
try {
# endif
for (; __idx != __last; ++__idx)
- ::new (std::__voidify(*__idx)) _ValueType;
+ ::new (static_cast<void*>(std::addressof(*__idx))) _ValueType;
# ifndef _LIBCPP_HAS_NO_EXCEPTIONS
} catch (...) {
std::__destroy(__first, __idx);
@@ -208,7 +207,7 @@ inline _LIBCPP_HIDE_FROM_ABI _ForwardIterator __uninitialized_default_construct_
try {
# endif
for (; __n > 0; ++__idx, (void)--__n)
- ::new (std::__voidify(*__idx)) _ValueType;
+ ::new (static_cast<void*>(std::addressof(*__idx))) _ValueType;
# ifndef _LIBCPP_HAS_NO_EXCEPTIONS
} catch (...) {
std::__destroy(__first, __idx);
@@ -235,7 +234,7 @@ __uninitialized_value_construct(_ForwardIterator __first, _Sentinel __last) {
try {
# endif
for (; __idx != __last; ++__idx)
- ::new (std::__voidify(*__idx)) _ValueType();
+ ::new (static_cast<void*>(std::addressof(*__idx))) _ValueType();
# ifndef _LIBCPP_HAS_NO_EXCEPTIONS
} catch (...) {
std::__destroy(__first, __idx);
@@ -261,7 +260,7 @@ inline _LIBCPP_HIDE_FROM_ABI _ForwardIterator __uninitialized_value_construct_n(
try {
# endif
for (; __n > 0; ++__idx, (void)--__n)
- ::new (std::__voidify(*__idx)) _ValueType();
+ ::new (static_cast<void*>(std::addressof(*__idx))) _ValueType();
# ifndef _LIBCPP_HAS_NO_EXCEPTIONS
} catch (...) {
std::__destroy(__first, __idx);
@@ -297,7 +296,7 @@ inline _LIBCPP_HIDE_FROM_ABI pair<_InputIterator, _ForwardIterator> __uninitiali
try {
# endif
for (; __ifirst != __ilast && !__stop_moving(__idx); ++__idx, (void)++__ifirst) {
- ::new (std::__voidify(*__idx)) _ValueType(__iter_move(__ifirst));
+ ::new (static_cast<void*>(std::addressof(*__idx))) _ValueType(__iter_move(__ifirst));
}
# ifndef _LIBCPP_HAS_NO_EXCEPTIONS
} catch (...) {
@@ -335,7 +334,7 @@ inline _LIBCPP_HIDE_FROM_ABI pair<_InputIterator, _ForwardIterator> __uninitiali
try {
# endif
for (; __n > 0 && !__stop_moving(__idx); ++__idx, (void)++__ifirst, --__n)
- ::new (std::__voidify(*__idx)) _ValueType(__iter_move(__ifirst));
+ ::new (static_cast<void*>(std::addressof(*__idx))) _ValueType(__iter_move(__ifirst));
# ifndef _LIBCPP_HAS_NO_EXCEPTIONS
} catch (...) {
std::__destroy(__ofirst, __idx);
diff --git a/libcxx/include/__memory/voidify.h b/libcxx/include/__memory/voidify.h
deleted file mode 100644
index dbd083bd8c1e9a..00000000000000
--- a/libcxx/include/__memory/voidify.h
+++ /dev/null
@@ -1,30 +0,0 @@
-// -*- C++ -*-
-//===----------------------------------------------------------------------===//
-//
-// 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___MEMORY_VOIDIFY_H
-#define _LIBCPP___MEMORY_VOIDIFY_H
-
-#include <__config>
-#include <__memory/addressof.h>
-
-#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
-# pragma GCC system_header
-#endif
-
-_LIBCPP_BEGIN_NAMESPACE_STD
-
-template <typename _Tp>
-_LIBCPP_ALWAYS_INLINE _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void* __voidify(_Tp& __from) {
- // Cast away cv-qualifiers to allow modifying elements of a range through const iterators.
- return const_cast<void*>(static_cast<const volatile void*>(std::addressof(__from)));
-}
-
-_LIBCPP_END_NAMESPACE_STD
-
-#endif // _LIBCPP___MEMORY_VOIDIFY_H
diff --git a/libcxx/include/module.modulemap b/libcxx/include/module.modulemap
index aa05bde939f6c2..b2c4570fd1eafd 100644
--- a/libcxx/include/module.modulemap
+++ b/libcxx/include/module.modulemap
@@ -1523,7 +1523,6 @@ module std [system] {
}
module uses_allocator { header "__memory/uses_allocator.h" }
module uses_allocator_construction { header "__memory/uses_allocator_construction.h" }
- module voidify { header "__memory/voidify.h" }
header "memory"
export *
diff --git a/libcxx/include/optional b/libcxx/include/optional
index 7578833685ec1f..4e44ef990f5d29 100644
--- a/libcxx/include/optional
+++ b/libcxx/include/optional
@@ -287,7 +287,7 @@ struct __optional_destruct_base<_Tp, false> {
static_assert(is_object_v<value_type>, "instantiation of optional with a non-object type is undefined behavior");
union {
char __null_state_;
- value_type __val_;
+ remove_cv_t<value_type> __val_;
};
bool __engaged_;
@@ -323,7 +323,7 @@ struct __optional_destruct_base<_Tp, true> {
static_assert(is_object_v<value_type>, "instantiation of optional with a non-object type is undefined behavior");
union {
char __null_state_;
- value_type __val_;
+ remove_cv_t<value_type> __val_;
};
bool __engaged_;
@@ -377,7 +377,7 @@ struct __optional_storage_base : __optional_destruct_base<_Tp> {
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void __assign_from(_That&& __opt) {
if (this->__engaged_ == __opt.has_value()) {
if (this->__engaged_)
- this->__val_ = std::forward<_That>(__opt).__get();
+ static_cast<_Tp&>(this->__val_) = std::forward<_That>(__opt).__get();
} else {
if (this->__engaged_)
this->reset();
diff --git a/libcxx/test/std/utilities/memory/specialized.algorithms/specialized.construct/construct_at.pass.cpp b/libcxx/test/std/utilities/memory/specialized.algorithms/specialized.construct/construct_at.pass.cpp
index 13442df9db3ae5..272441ebedc2f2 100644
--- a/libcxx/test/std/utilities/memory/specialized.algorithms/specialized.construct/construct_at.pass.cpp
+++ b/libcxx/test/std/utilities/memory/specialized.algorithms/specialized.construct/construct_at.pass.cpp
@@ -80,21 +80,6 @@ constexpr bool test()
a.deallocate(p, 2);
}
- {
- std::allocator<Counted> a;
- Counted const* p = a.allocate(2);
- int count = 0;
- std::construct_at(p, count);
- assert(count == 1);
- std::construct_at(p+1, count);
- assert(count == 2);
- (p+1)->~Counted();
- assert(count == 1);
- p->~Counted();
- assert(count == 0);
- a.deallocate(const_cast<Counted*>(p), 2);
- }
-
return true;
}
diff --git a/libcxx/test/std/utilities/memory/specialized.algorithms/specialized.construct/ranges_construct_at.pass.cpp b/libcxx/test/std/utilities/memory/specialized.algorithms/specialized.construct/ranges_construct_at.pass.cpp
index 396fed7cc3e49d..f66bf0fd647778 100644
--- a/libcxx/test/std/utilities/memory/specialized.algorithms/specialized.construct/ranges_construct_at.pass.cpp
+++ b/libcxx/test/std/utilities/memory/specialized.algorithms/specialized.construct/ranges_construct_at.pass.cpp
@@ -99,16 +99,6 @@ constexpr bool test() {
alloc.deallocate(out, 2);
}
- // Works with const pointers.
- {
- int x = 1;
- const int* ptr = &x;
-
- const int* result = std::ranges::construct_at(ptr, 42);
- assert(result == ptr);
- assert(x == 42);
- }
-
return true;
}
diff --git a/libcxx/test/std/utilities/memory/specialized.algorithms/uninitialized.construct.default/ranges_uninitialized_default_construct.pass.cpp b/libcxx/test/std/utilities/memory/specialized.algorithms/uninitialized.construct.default/ranges_uninitialized_default_construct.pass.cpp
index 4581f1c909e381..ef969190c63148 100644
--- a/libcxx/test/std/utilities/memory/specialized.algorithms/uninitialized.construct.default/ranges_uninitialized_default_construct.pass.cpp
+++ b/libcxx/test/std/utilities/memory/specialized.algorithms/uninitialized.construct.default/ranges_uninitialized_default_construct.pass.cpp
@@ -163,30 +163,5 @@ int main(int, char**) {
}
#endif // TEST_HAS_NO_EXCEPTIONS
- // Works with const iterators, (iter, sentinel) overload.
- {
- constexpr int N = 5;
- Buffer<Counted, N> buf;
-
- std::ranges::uninitialized_default_construct(buf.cbegin(), buf.cend());
- assert(Counted::current_objects == N);
- assert(Counted::total_objects == N);
- std::destroy(buf.begin(), buf.end());
- Counted::reset();
- }
-
- // Works with const iterators, (range) overload.
- {
- constexpr int N = 5;
- Buffer<Counted, N> buf;
- auto range = std::ranges::subrange(buf.cbegin(), buf.cend());
-
- std::ranges::uninitialized_default_construct(range);
- assert(Counted::current_objects == N);
- assert(Counted::total_objects == N);
- std::destroy(buf.begin(), buf.end());
- Counted::reset();
- }
-
return 0;
}
diff --git a/libcxx/test/std/utilities/memory/specialized.algorithms/uninitialized.construct.default/ranges_uninitialized_default_construct_n.pass.cpp b/libcxx/test/std/utilities/memory/specialized.algorithms/uninitialized.construct.default/ranges_uninitialized_default_construct_n.pass.cpp
index 9bebe4b52a8cc9..40fbf226959098 100644
--- a/libcxx/test/std/utilities/memory/specialized.algorithms/uninitialized.construct.default/ranges_uninitialized_default_construct_n.pass.cpp
+++ b/libcxx/test/std/utilities/memory/specialized.algorithms/uninitialized.construct.default/ranges_uninitialized_default_construct_n.pass.cpp
@@ -75,17 +75,5 @@ int main(int, char**) {
}
#endif // TEST_HAS_NO_EXCEPTIONS
- // Works with const iterators.
- {
- constexpr int N = 5;
- Buffer<Counted, N> buf;
-
- std::ranges::uninitialized_default_construct_n(buf.cbegin(), N);
- assert(Counted::current_objects == N);
- assert(Counted::total_objects == N);
- std::destroy(buf.begin(), buf.end());
- Counted::reset();
- }
-
return 0;
}
diff --git a/libcxx/test/std/utilities/memory/specialized.algorithms/uninitialized.construct.value/ranges_uninitialized_value_construct.pass.cpp b/libcxx/test/std/utilities/memory/specialized.algorithms/uninitialized.construct.value/ranges_uninitialized_value_construct.pass.cpp
index ad74b82dce1f2b..6bab25ca38475b 100644
--- a/libcxx/test/std/utilities/memory/specialized.algorithms/uninitialized.construct.value/ranges_uninitialized_value_construct.pass.cpp
+++ b/libcxx/test/std/utilities/memory/specialized.algorithms/uninitialized.construct.value/ranges_uninitialized_value_construct.pass.cpp
@@ -183,30 +183,5 @@ int main(int, char**) {
}
#endif // TEST_HAS_NO_EXCEPTIONS
- // Works with const iterators, (iter, sentinel) overload.
- {
- constexpr int N = 5;
- Buffer<Counted, N> buf;
-
- std::ranges::uninitialized_value_construct(buf.cbegin(), buf.cend());
- assert(Counted::current_objects == N);
- assert(Counted::total_objects == N);
- std::destroy(buf.begin(), buf.end());
- Counted::reset();
- }
-
- // Works with const iterators, (range) overload.
- {
- constexpr int N = 5;
- Buffer<Counted, N> buf;
-
- auto range = std::ranges::subrange(buf.cbegin(), buf.cend());
- std::ranges::uninitialized_value_construct(range);
- assert(Counted::current_objects == N);
- assert(Counted::total_objects == N);
- std::destroy(buf.begin(), buf.end());
- Counted::reset();
- }
-
return 0;
}
diff --git a/libcxx/test/std/utilities/memory/specialized.algorithms/uninitialized.construct.value/ranges_uninitialized_value_construct_n.pass.cpp b/libcxx/test/std/utilities/memory/specialized.algorithms/uninitialized.construct.value/ranges_uninitialized_value_construct_n.pass.cpp
index 8f315ce0076d41..4742aefcdb5ada 100644
--- a/libcxx/test/std/utilities/memory/specialized.algorithms/uninitialized.construct.value/ranges_uninitialized_value_construct_n.pass.cpp
+++ b/libcxx/test/std/utilities/memory/specialized.algorithms/uninitialized.construct.value/ranges_uninitialized_value_construct_n.pass.cpp
@@ -94,17 +94,5 @@ int main(int, char**) {
}
#endif // TEST_HAS_NO_EXCEPTIONS
- // Works with const iterators.
- {
- constexpr int N = 5;
- Buffer<Counted, N> buf;
-
- std::ranges::uninitialized_value_construct_n(buf.cbegin(), N);
- assert(Counted::current_objects == N);
- assert(Counted::total_objects == N);
- std::destroy(buf.begin(), buf.end());
- Counted::reset();
- }
-
return 0;
}
diff --git a/libcxx/test/std/utilities/memory/specialized.algorithms/uninitialized.copy/ranges_uninitialized_copy.pass.cpp b/libcxx/test/std/utilities/memory/specialized.algorithms/uninitialized.copy/ranges_uninitialized_copy.pass.cpp
index 92dc380728e242..52ba70b009baba 100644
--- a/libcxx/test/std/utilities/memory/specialized.algorithms/uninitialized.copy/ranges_uninitialized_copy.pass.cpp
+++ b/libcxx/test/std/utilities/memory/specialized.algorithms/uninitialized.copy/ranges_uninitialized_copy.pass.cpp
@@ -278,39 +278,6 @@ int main(int, char**) {
Counted::reset();
#endif // TEST_HAS_NO_EXCEPTIONS
- // Works with const iterators, (iter, sentinel) overload.
- {
- constexpr int N = 5;
- Counted in[N] = {Counted(1), Counted(2), Counted(3), Counted(4), Counted(5)};
- Buffer<Counted, N> out;
- Counted::reset();
-
- std::ranges::uninitialized_copy(in, in + N, out.cbegin(),...
[truncated]
|
frederick-vs-ja
force-pushed
the
lwg-3870
branch
from
974e91d to
3a2a641
Compare
Instead of changing the cast sequence to implicit conversion in _`voidify`_, I think it is better to totally remove `__voidify` and use `static_cast` to `void*`, which has equivalent effects. Test coverage for const iterators are removed. Now most affected algorithms are underconstrained, for which I submitted https://cplusplus.github.io/LWG/issue3888. I'm not sure whether we should speculatively implement it at this moment, and thus haven't added any `*.verify.cpp`. In some control block types and `optional`, the stored objects are changed to have cv-unqualified type.
frederick-vs-ja
force-pushed
the
lwg-3870
branch
from
3a2a641 to
bc83638
Compare
ldionne
approved these changes
Sep 30, 2024
|
Hi @frederick-vs-ja, looks like this broke an LLDB test (TestDataFormatterGenericOptional.py) https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/as-lldb-cmake/12593/ |
|
Looks like it wasnt caught by pre-merge CI because it isnt part of the @ldionne could we add a couple more targets to the list?
It's unfortunate that the last couple are spread across targets like this.. |
|
@Michael137 That's not a problem. Do you need a revert in the meantime to fix LLDB? |
From a quick glance at the formatter implementation, I think the fix should be pretty simple. Let me try locally and get back to you if I can't resolve this today |
|
Ack. Feel free to revert if you need more time. If you do that, you will run into a merge conflict with my modulemap patch (since it touched everything), but please don't revert that one. The conflict should be easy to get through by re-adding the |
|
Hmm yea this is a bit stranger than expected. For some reason the order of how we print the optionals in the failing test affects the output. I'll have to defer to tomorrow. So reverting for now |
Michael137
added a commit
to Michael137/llvm-project
that referenced
this pull request
Sep 30, 2024
This reverts commit 78f9a8b.
Michael137
added a commit
to Michael137/llvm-project
that referenced
this pull request
Oct 1, 2024
This reverts commit 78f9a8b.
|
Looks like it's a shortcoming of the DWARF representation for template aliases. The |
|
This template alias workaround is partly to blame: |
|
If it turns out we can't do this in the formatters, I think the best course of action to unblock this is as follows:
|
|
@Michael137 I'm fine with any way of resolving this from the LLDB side, I'd just like to avoid blocking this PR for too long on something not actionable from libc++. Just let us know when we can try landing this again! |
Michael137
added a commit
to Michael137/llvm-project
that referenced
this pull request
Oct 1, 2024
…e aliases This was originally added in https://reviews.llvm.org/D142268 have LLDB display variable typenames that benefit from suppressing defaulted template arguments. We currently represent template aliases as `DW_AT_typedef`s instead of `DW_TAG_template_alias`. This means for types like: ``` template <class _Tp> using __remove_cv_t = __remove_cv(_Tp); template <class _Tp> using remove_cv_t = __remove_cv_t<_Tp>; template<typename T> class optional { using value_type = T; remove_cv_t<value_type> __val_; } ``` we would generate DWARF like: ``` 0x0000274f: DW_TAG_typedef DW_AT_type (0x0000000000002758 "__remove_cv_t<value_type>") DW_AT_name ("remove_cv_t<value_type>") ``` This is an actual libc++ type layout introduced in llvm#110355, which uncovered a shortcoming of LLDB's data-formatter infrastructure, which caches formatters on the contents of `DW_AT_name`. To unblock the libc++ change, we can revert this without much fallout. Then we have two options: 1. reland this but adjust the LLDB formatter cache so it doesn't cache formatters for template specializations 2. implement support for `DW_TAG_template_alias` in LLDB (and make Clang generate them by default).
Michael137
added a commit
that referenced
this pull request
Oct 2, 2024
…e aliases (#110767) This was originally added in https://reviews.llvm.org/D142268 to have LLDB display variable typenames that benefit from suppressing defaulted template arguments. We currently represent template aliases as `DW_AT_typedef`s instead of `DW_TAG_template_alias`. This means for types like: ``` template <class _Tp> using __remove_cv_t = __remove_cv(_Tp); template <class _Tp> using remove_cv_t = __remove_cv_t<_Tp>; template<typename T> class optional { using value_type = T; remove_cv_t<value_type> __val_; } ``` we would generate DWARF like: ``` 0x0000274f: DW_TAG_typedef DW_AT_type (0x0000000000002758 "__remove_cv_t<value_type>") DW_AT_name ("remove_cv_t<value_type>") ``` This is an actual libc++ type layout introduced in #110355, and uncovered a shortcoming of LLDB's data-formatter infrastructure, where we cache formatters on the contents of `DW_AT_name` (which currently wouldn't be a fully resolved typename for template specializations). To unblock the libc++ change, I think we can revert this without much fallout. Then we have two options for follow-up (or do both): 1. reland this but adjust the LLDB formatter cache so it doesn't cache formatters for template specializations 2. implement support for `DW_TAG_template_alias` in LLDB (and make Clang generate them by default).
|
#110767 was merged. Feel free to reland this now. Thanks for the wait |
|
@frederick-vs-ja Can you re-open a PR? The CI will re-run just so we can validate that everything works. |
frederick-vs-ja
added a commit
to frederick-vs-ja/llvm-project
that referenced
this pull request
Oct 3, 2024
…vm#110587)" This reverts commit f3d58f4.
Yeah. New PR opened now. |
Sterling-Augustine
pushed a commit
to Sterling-Augustine/llvm-project
that referenced
this pull request
Oct 3, 2024
This reverts commit 78f9a8b. This caused the LLDB test `TestDataFormatterGenericOptional.py` to fail, and we need a bit more time to look into it.
Sterling-Augustine
pushed a commit
to Sterling-Augustine/llvm-project
that referenced
this pull request
Oct 3, 2024
…e aliases (llvm#110767) This was originally added in https://reviews.llvm.org/D142268 to have LLDB display variable typenames that benefit from suppressing defaulted template arguments. We currently represent template aliases as `DW_AT_typedef`s instead of `DW_TAG_template_alias`. This means for types like: ``` template <class _Tp> using __remove_cv_t = __remove_cv(_Tp); template <class _Tp> using remove_cv_t = __remove_cv_t<_Tp>; template<typename T> class optional { using value_type = T; remove_cv_t<value_type> __val_; } ``` we would generate DWARF like: ``` 0x0000274f: DW_TAG_typedef DW_AT_type (0x0000000000002758 "__remove_cv_t<value_type>") DW_AT_name ("remove_cv_t<value_type>") ``` This is an actual libc++ type layout introduced in llvm#110355, and uncovered a shortcoming of LLDB's data-formatter infrastructure, where we cache formatters on the contents of `DW_AT_name` (which currently wouldn't be a fully resolved typename for template specializations). To unblock the libc++ change, I think we can revert this without much fallout. Then we have two options for follow-up (or do both): 1. reland this but adjust the LLDB formatter cache so it doesn't cache formatters for template specializations 2. implement support for `DW_TAG_template_alias` in LLDB (and make Clang generate them by default).
ldionne
pushed a commit
that referenced
this pull request
Oct 3, 2024
Michael137
added a commit
to Michael137/llvm-project
that referenced
this pull request
Oct 4, 2024
Follow-up to the LLDB std::optional data-formatter test failure caused by llvm#110355. Two formats are supported: 1. `__val_` has type `value_type` 2. `__val_`'s type is wrapped in `std::remove_cv_t`
Michael137
added a commit
that referenced
this pull request
Oct 7, 2024
Follow-up to the LLDB std::optional data-formatter test failure caused by #110355. Two formats are supported: 1. `__val_` has type `value_type` 2. `__val_`'s type is wrapped in `std::remove_cv_t`
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.
Instead of changing the cast sequence to implicit conversion in
voidify, I think it is better to totally remove__voidifyand usestatic_casttovoid*, which has equivalent effects.Test coverage for const iterators are removed.
Now most affected algorithms are underconstrained, for which I submitted LWG3888. I'm not sure whether we should speculatively implement it at this moment, and thus haven't added any
*.verify.cpp.In some control block types and
optional, the stored objects are changed to have cv-unqualified type.Fixes #105119.