On Wed, 6 Mar 2019 at 11:33, Jonathan Wakely <jwak...@redhat.com> wrote:
> >+ else if constexpr (is_rvalue_reference_v<_Tp&&>)
>
> I know what this is doing, but it still looks a little odd to ask if
> T&& is an rvalue-reference.
>
> Would it be clearer to structure this as:
>
> if constexpr (is_lvalue_reference_v<_Tp>)
> {
> if constexpr (is_const_v<remove_reference_t<_Tp>>)
> return static_cast<const variant<_Types...>&>(__rhs);
> else
> return static_cast<variant<_Types...>&>(__rhs);
> }
> else
> return static_cast<variant<_Types...>&&>(__rhs);
>
> ?
> >+ ::new (std::addressof(__this_mem))
>
> Is there any way that this can find the wrong operator new?
>
> Even if it can't, saying ::new ((void*)std::addressof(__this_mem))
> would avoid having to think about that question again in future.
>
> Therre are a few other new expressions where that applies too (several
> of them already there before your patch).
> >+ ::new (&__storage) remove_reference_t<decltype(__storage)>
>
> This one definitely needs to be cast to void* and needs to use
> addressof (or __addressof), otherwise ...
Sure thing; an incremental diff attached.
diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index 5138599..4d81ceb 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -145,12 +145,15 @@ namespace __variant
template <typename... _Types, typename _Tp>
decltype(auto) __variant_cast(_Tp&& __rhs)
{
- if constexpr (is_const_v<remove_reference_t<_Tp>>)
- return static_cast<const variant<_Types...>&>(__rhs);
- else if constexpr (is_rvalue_reference_v<_Tp&&>)
- return static_cast<variant<_Types...>&&>(__rhs);
+ if constexpr (is_lvalue_reference_v<_Tp>)
+ {
+ if constexpr (is_const_v<remove_reference_t<_Tp>>)
+ return static_cast<const variant<_Types...>&>(__rhs);
+ else
+ return static_cast<variant<_Types...>&>(__rhs);
+ }
else
- return static_cast<variant<_Types...>&>(__rhs);
+ return static_cast<variant<_Types...>&&>(__rhs);
}
namespace __detail
@@ -212,7 +215,8 @@ namespace __variant
{
template<typename... _Args>
constexpr _Uninitialized(in_place_index_t<0>, _Args&&... __args)
- { ::new (&_M_storage) _Type(std::forward<_Args>(__args)...); }
+ { ::new ((void*)std::addressof(_M_storage))
+ _Type(std::forward<_Args>(__args)...); }
const _Type& _M_get() const &
{ return *_M_storage._M_ptr(); }
@@ -427,7 +431,7 @@ namespace __variant
if constexpr (is_same_v<__remove_cvref_t<decltype(__rhs_mem)>,
remove_cv_t<_Type>>
&& !is_same_v<_Type, __variant_cookie>)
- ::new (std::addressof(__this_mem))
+ ::new ((void*)std::addressof(__this_mem))
_Type(std::forward<decltype(__rhs_mem)>(__rhs_mem));
return {};
}, __variant_cast<_Types...>(__lhs),
@@ -931,8 +935,9 @@ namespace __variant
{
__v._M_index = _Np;
auto&& __storage = __detail::__variant::__get<_Np>(__v);
- ::new (&__storage) remove_reference_t<decltype(__storage)>
- (std::forward<_Args>(__args)...);
+ ::new ((void*)std::addressof(__storage))
+ remove_reference_t<decltype(__storage)>
+ (std::forward<_Args>(__args)...);
}
template<typename _Tp, typename... _Types>
