On 17/03/17 16:51 +0200, Ville Voutilainen wrote:
diff --git a/libstdc++-v3/include/std/optional b/libstdc++-v3/include/std/optional index 3f540ec..e67ba89 100644 --- a/libstdc++-v3/include/std/optional +++ b/libstdc++-v3/include/std/optional @@ -95,125 +95,127 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION - constexpr _Optional_base(nullopt_t) noexcept - : _Optional_base{} { } + template<typename _Up, typename... _Args> + constexpr _Optional_payload(std::initializer_list<_Up> __il, + _Args&&... __args) + : _M_payload(__il, std::forward<_Args>(__args)...), + _M_engaged(true) {} + + template <class _Up> struct __ctor_tag {};
Newline here please, so it doesn't look like this template<...> is on the constructor.
+ constexpr _Optional_payload(__ctor_tag<bool>, + const _Tp& __other) + : _M_payload(__other), + _M_engaged(true) + {} + + constexpr _Optional_payload(__ctor_tag<void>) + : _M_empty() + {} + + constexpr _Optional_payload(__ctor_tag<bool>, _Tp&& __other) + : _M_payload(std::move(__other)), + _M_engaged(true) + {} + + constexpr _Optional_payload(bool __engaged, + const _Optional_payload&
Looks like we don't need a newline before the parameter name here:
+ __other) + : _Optional_payload(__engaged ? + _Optional_payload(__ctor_tag<bool>{}, + __other._M_payload) : + _Optional_payload(__ctor_tag<void>{})) + {} + + constexpr _Optional_payload(bool __engaged, + _Optional_payload&&
Nor here:
+ __other) + : _Optional_payload(__engaged ? + _Optional_payload(__ctor_tag<bool>{}, + std::move(__other._M_payload)) : + _Optional_payload(__ctor_tag<void>{})) + {}
Hmm, I see we don't actually show a conditional expression in the Coding Style docs, but I'd do that as: : _Optional_payload(__engaged ? _Optional_payload(__ctor_tag<bool>{}, std::move(__other._M_payload)) : _Optional_payload(__ctor_tag<void>{}))
- // Constructors for engaged optionals. - template<typename... _Args, - enable_if_t<is_constructible_v<_Tp, _Args&&...>, bool> = false> - constexpr explicit _Optional_base(in_place_t, _Args&&... __args) - : _M_payload(std::forward<_Args>(__args)...), _M_engaged(true) { } + using _Stored_type = remove_const_t<_Tp>; + struct _Empty_byte { };
I was going to ask whether std::byte would be better here, but I think a valueless struct is better.
+ union { + _Empty_byte _M_empty; + _Stored_type _M_payload; + }; + bool _M_engaged = false; + };
+ template<typename... _Args> + void + _M_construct(_Args&&... __args) + noexcept(is_nothrow_constructible<_Stored_type, _Args...>()) + { + ::new (std::__addressof(this->_M_payload))
I think we need (void*) here to ensure we use the reserved placement form of operator new and not some other overload: struct X { }; void* operator new(decltype(sizeof(0)) n, X*); (We don't get this right everywhere that uses placement new with arbitrary types).
+ _Stored_type(std::forward<_Args>(__args)...); + this->_M_engaged = true; + } }; + // The _M_construct operation has !_M_engaged as a precondition + // while _M_destruct has _M_engaged as a precondition. template<typename... _Args> void _M_construct(_Args&&... __args) noexcept(is_nothrow_constructible<_Stored_type, _Args...>()) { - ::new (std::__addressof(this->_M_payload)) + ::new (std::__addressof(this->_M_payload._M_payload))
Here too.
_Stored_type(std::forward<_Args>(__args)...); - this->_M_engaged = true; + this->_M_payload._M_engaged = true; }
OK for trunk with those adjustements, thanks.