On Wed, Nov 30, 2016 at 8:27 AM, Jonathan Wakely wrote:
On 26/11/16 21:38 -0800, Tim Shen wrote:
This 4-patch series contains the following in order:
a.diff: Remove uses-allocator ctors. They are going away, and removing
it reduces the maintenance burden from now on.
Yay! less code.
Yay! Also removed the unused #include.
- template<typename _Type, bool __is_literal =
std::is_literal_type_v<_Type>>
+ // _Uninitialized nullify the destructor calls.
This wording confused me slightly. How about:
"_Uninitialized makes destructors trivial"
Change this section of comment to the discussed content.
+ // This is necessary, since we define _Variadic_union as a recursive
union,
+ // and we don't want to inspect the union members one by one in its
dtor,
+ // it's slow.
Please change "it's slow" to "that's slow".
N/A.
+ template<typename _Type, bool = std::is_literal_type_v<_Type>>
struct _Uninitialized;
I'm still unsure that is_literal_type is the right trait here. If it's
definitely right then we should probably *not* deprecate it in C++17!
Already discussed.
template<typename _Type>
struct _Uninitialized<_Type, false>
{
- constexpr _Uninitialized() = default;
-
template<typename... _Args>
constexpr _Uninitialized(in_place_index_t<0>, _Args&&... __args)
{ ::new (&_M_storage) _Type(std::forward<_Args>(__args)...); }
+ const _Type& _M_get() const &
+ {
+ return *static_cast<const _Type*>(
+ static_cast<const void*>(&_M_storage));
+ }
+
+ _Type& _M_get() &
+ { return *static_cast<_Type*>(static_cast<void*>(&_M_storage)); }
+
+ const _Type&& _M_get() const &&
+ {
+ return std::move(*static_cast<const _Type*>(
+ static_cast<const void*>(&_M_storage)));
+ }
+
+ _Type&& _M_get() &&
+ {
+ return
std::move(*static_cast<_Type*>(static_cast<void*>(&_M_storage)));
+ }
+
typename std::aligned_storage<sizeof(_Type), alignof(_Type)>::type
_M_storage;
I think this could use __aligned_membuf, which would reduce the
alignment requirements for some types (e.g. long long on x86-32).
Done.
That would also mean you get the _M_ptr() member so don't need all the
casts.
+ ~_Variant_storage()
+ { _M_destroy_impl(std::make_index_sequence<sizeof...(_Types)>{}); }
You can use index_sequence_for<_Types...> here.
Done
@@ -598,9 +645,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
_S_apply_all_alts(_Array_type& __vtable,
index_sequence<__indices...>)
{ (_S_apply_single_alt<__indices>(__vtable._M_arr[__indices]),
...); }
- template<size_t __index>
+ template<size_t __index, typename T>
This needs to be _Tp not T
Done.
+ return __lhs._M_equal_to(__rhs,
+
std::make_index_sequence<sizeof...(_Types)>{});
Another one that could use index_sequence_for<_Types...>
Done.
+ return __lhs._M_less_than(__rhs,
+
std::make_index_sequence<sizeof...(_Types)>{});
Same again.
Same again. ;)
* include/bits/enable_special_members.h: Make
_Enable_default_constructor constexpr.
* include/std/variant (variant::emplace, variant::swap,
std::swap,
std::hash): Sfinae on emplace and std::swap; handle
__poison_hash bases
of duplicated types.
diff --git a/libstdc++-v3/include/bits/enable_special_members.h
b/libstdc++-v3/include/bits/enable_special_members.h
index 07c6c99..4f4477b 100644
--- a/libstdc++-v3/include/bits/enable_special_members.h
+++ b/libstdc++-v3/include/bits/enable_special_members.h
@@ -118,7 +118,8 @@ template<typename _Tag>
operator=(_Enable_default_constructor&&) noexcept = default;
// Can be used in other ctors.
- explicit _Enable_default_constructor(_Enable_default_constructor_tag)
{ }
+ constexpr explicit
+ _Enable_default_constructor(_Enable_default_constructor_tag) { }
};
+ void _M_reset()
+ {
+ _M_reset_impl(std::make_index_sequence<sizeof...(_Types)>{});
+ _M_index = variant_npos;
+ }
+
~_Variant_storage()
- { _M_destroy_impl(std::make_index_sequence<sizeof...(_Types)>{}); }
+ { _M_reset(); }
These can also use index_sequence_for<_Types...>
Done.
@@ -1253,14 +1285,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
template<typename... _Types>
struct hash<variant<_Types...>>
- : private __poison_hash<remove_const_t<_Types>>...
+ : private __detail::__variant::_Variant_hash_base<
+ variant<_Types...>, std::make_index_sequence<sizeof...(_Types)>>
And again.
And again.
{
using result_type = size_t;
using argument_type = variant<_Types...>;
size_t
operator()(const variant<_Types...>& __t) const
- noexcept((... &&
noexcept(hash<decay_t<_Types>>{}(std::declval<_Types>()))))
+ noexcept((noexcept(hash<decay_t<_Types>>{}(std::declval<_Types>()))
+ && ...))
This could be
__and_<is_nothrow_callable<hash<decay_t<_Types>>(_Types)>...>
but I'm not sure it would be an improvement. The is_callable check is
expensive, but maybe we need it anyway to correctly disable this
function if the hash specialization should be posisoned?
Done. I just realized that is_nothrow_callable also handles crazy
member pointer cases.
Used fold expression instead of __and_ for consistency.
@@ -1270,17 +1239,20 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
}
template<typename _Visitor, typename... _Variants>
- decltype(auto)
+ constexpr decltype(auto)
visit(_Visitor&& __visitor, _Variants&&... __variants)
{
+ if ((__variants.valueless_by_exception() || ...))
+ __throw_bad_variant_access("Unexpected index");
+
using _Result_type =
decltype(std::forward<_Visitor>(__visitor)(get<0>(__variants)...));
- static constexpr auto _S_vtable =
+ constexpr auto _S_vtable =
If this isn't static now it could be called simply __vtable, the _S_
prefix is misleading. How many of these _S_vtable variables actually
need to be static? If they're all trivial types and constexpr then it
probably doesn't matter either way, there shouldn't be any difference.
Ah that's an oversight. Moved the static variable out of visit().
_S_vtable needs to be static, otherwise runtime O(n) assignment will
happen, where n is the size of _S_vtable.