PR libstdc++/87855 fix optional for types with non-trivial copy/move

When the contained value is not trivially copy (or move) constructible
the union's copy (or move) constructor will be deleted, and so the
_Optional_payload delegating constructors are invalid. G++ fails to
diagnose this because it incorrectly performs copy elision in the
delegating constructors. Clang does diagnose it (llvm.org/PR40245).

The solution is to avoid performing any copy (or move) when the
contained value's copy (or move) constructor isn't trivial. Instead the
contained value can be constructed by calling _M_construct. This is OK,
because the relevant constructor doesn't need to be constexpr when the
contained value isn't trivially copy (or move) constructible.

Additionally, this patch removes a lot of code duplication in the
_Optional_payload partial specializations and the _Optional_base partial
specialization, by hoisting it into common base classes.

The Python pretty printer for std::optional needs to be adjusted to
support the new layout. Retain support for the old layout, and add a
test to verify that the support still works.

	PR libstdc++/87855
	* include/std/optional (_Optional_payload_base): New class template
	for common code hoisted from _Optional_payload specializations. Use
	a template for the union, to allow a partial specialization for
	types with non-trivial destructors. Add constructors for in-place
	initialization to the union.
	(_Optional_payload(bool, const _Optional_payload&)): Use _M_construct
	to perform non-trivial copy construction, instead of relying on
	non-standard copy elision in a delegating constructor.
	(_Optional_payload(bool, _Optional_payload&&)): Likewise for
	non-trivial move construction.
	(_Optional_payload): Derive from _Optional_payload_base and use it
	for everything except the non-trivial assignment operators, which are
	defined as needed.
	(_Optional_payload<false, C, M>): Derive from the specialization
	_Optional_payload<true, false, false> and add a destructor.
	(_Optional_base_impl::_M_destruct, _Optional_base_impl::_M_reset):
	Forward to corresponding members of _Optional_payload.
	(_Optional_base_impl::_M_is_engaged, _Optional_base_impl::_M_get):
	Hoist common members from _Optional_base.
	(_Optional_base): Make all members and base class public.
	(_Optional_base::_M_get, _Optional_base::_M_is_engaged): Move to
	_Optional_base_impl.
	* python/libstdcxx/v6/printers.py (StdExpOptionalPrinter): Add
	support for new std::optional layout.
	* testsuite/libstdc++-prettyprinters/compat.cc: New test.

From-SVN: r267742

4 files changed, 323 insertions, 558 deletions

diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog
index 3adec7c..bb8f874 100644
--- a/libstdc++-v3/ChangeLog
+++ b/libstdc++-v3/ChangeLog
@@ -1,5 +1,32 @@
 2019-01-08  Jonathan Wakely  <jwakely@redhat.com>
 
+	PR libstdc++/87855
+	* include/std/optional (_Optional_payload_base): New class template
+	for common code hoisted from _Optional_payload specializations. Use
+	a template for the union, to allow a partial specialization for
+	types with non-trivial destructors. Add constructors for in-place
+	initialization to the union.
+	(_Optional_payload(bool, const _Optional_payload&)): Use _M_construct
+	to perform non-trivial copy construction, instead of relying on
+	non-standard copy elision in a delegating constructor.
+	(_Optional_payload(bool, _Optional_payload&&)): Likewise for
+	non-trivial move construction.
+	(_Optional_payload): Derive from _Optional_payload_base and use it
+	for everything except the non-trivial assignment operators, which are
+	defined as needed.
+	(_Optional_payload<false, C, M>): Derive from the specialization
+	_Optional_payload<true, false, false> and add a destructor.
+	(_Optional_base_impl::_M_destruct, _Optional_base_impl::_M_reset):
+	Forward to corresponding members of _Optional_payload.
+	(_Optional_base_impl::_M_is_engaged, _Optional_base_impl::_M_get):
+	Hoist common members from _Optional_base.
+	(_Optional_base): Make all members and base class public.
+	(_Optional_base::_M_get, _Optional_base::_M_is_engaged): Move to
+	_Optional_base_impl.
+	* python/libstdcxx/v6/printers.py (StdExpOptionalPrinter): Add
+	support for new std::optional layout.
+	* testsuite/libstdc++-prettyprinters/compat.cc: New test.
+
 	PR libstdc++/88066
 	* include/bits/locale_conv.h: Use <> for includes not "".
 	* include/ext/random: Likewise.
diff --git a/libstdc++-v3/include/std/optional b/libstdc++-v3/include/std/optional
index 327e86e..b06e30f 100644
--- a/libstdc++-v3/include/std/optional
+++ b/libstdc++-v3/include/std/optional
@@ -98,107 +98,115 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   __throw_bad_optional_access()
   { _GLIBCXX_THROW_OR_ABORT(bad_optional_access()); }
 
-
-  // Payload for optionals with non-trivial destructor.
-  template <typename _Tp,
-	    bool /*_HasTrivialDestructor*/ =
-	      is_trivially_destructible_v<_Tp>,
-	    bool /*_HasTrivialCopy */ =
-	      is_trivially_copy_assignable_v<_Tp>
-	      && is_trivially_copy_constructible_v<_Tp>,
-	    bool /*_HasTrivialMove */ =
-	      is_trivially_move_assignable_v<_Tp>
-	      && is_trivially_move_constructible_v<_Tp>>
-    struct _Optional_payload
+  // This class template manages construction/destruction of
+  // the contained value for a std::optional.
+  template <typename _Tp>
+    struct _Optional_payload_base
     {
-      constexpr _Optional_payload() noexcept : _M_empty() { }
+      using _Stored_type = remove_const_t<_Tp>;
+
+      _Optional_payload_base() = default;
+      ~_Optional_payload_base() = default;
 
-      template <typename... _Args>
+      template<typename... _Args>
 	constexpr
-	_Optional_payload(in_place_t, _Args&&... __args)
-	: _M_payload(std::forward<_Args>(__args)...), _M_engaged(true) { }
+	_Optional_payload_base(in_place_t __tag, _Args&&... __args)
+	: _M_payload(__tag, std::forward<_Args>(__args)...),
+	  _M_engaged(true)
+	{ }
 
       template<typename _Up, typename... _Args>
 	constexpr
-	_Optional_payload(std::initializer_list<_Up> __il,
-			  _Args&&... __args)
+	_Optional_payload_base(std::initializer_list<_Up> __il,
+			       _Args&&... __args)
 	: _M_payload(__il, std::forward<_Args>(__args)...),
 	  _M_engaged(true)
 	{ }
 
+      // Constructor used by _Optional_base copy constructor when the
+      // contained value is not trivially copy constructible.
       constexpr
-      _Optional_payload(bool __engaged, const _Optional_payload& __other)
-      : _Optional_payload(__other)
-      { }
-
-      constexpr
-      _Optional_payload(bool __engaged, _Optional_payload&& __other)
-      : _Optional_payload(std::move(__other))
-      { }
-
-      constexpr
-      _Optional_payload(const _Optional_payload& __other)
+      _Optional_payload_base(bool __engaged,
+			     const _Optional_payload_base& __other)
       {
 	if (__other._M_engaged)
-	  this->_M_construct(__other._M_payload);
+	  this->_M_construct(__other._M_get());
       }
 
+      // Constructor used by _Optional_base move constructor when the
+      // contained value is not trivially move constructible.
       constexpr
-      _Optional_payload(_Optional_payload&& __other)
+      _Optional_payload_base(bool __engaged,
+			     _Optional_payload_base&& __other)
       {
 	if (__other._M_engaged)
-	  this->_M_construct(std::move(__other._M_payload));
+	  this->_M_construct(std::move(__other._M_get()));
       }
 
-      constexpr
-      _Optional_payload&
-      operator=(const _Optional_payload& __other)
-      {
-        if (this->_M_engaged && __other._M_engaged)
-          this->_M_get() = __other._M_get();
-        else
-	  {
-	    if (__other._M_engaged)
-	      this->_M_construct(__other._M_get());
-	    else
-	      this->_M_reset();
-	  }
-	return *this;
-      }
+      // Copy constructor is only used to when the contained value is
+      // trivially copy constructible.
+      _Optional_payload_base(const _Optional_payload_base&) = default;
 
-      constexpr
-      _Optional_payload&
-      operator=(_Optional_payload&& __other)
-      noexcept(__and_v<is_nothrow_move_constructible<_Tp>,
-		       is_nothrow_move_assignable<_Tp>>)
-      {
-	if (this->_M_engaged && __other._M_engaged)
-	  this->_M_get() = std::move(__other._M_get());
-	else
-	  {
-	    if (__other._M_engaged)
-	      this->_M_construct(std::move(__other._M_get()));
-	    else
-	      this->_M_reset();
-	  }
-	return *this;
-      }
+      // Move constructor is only used to when the contained value is
+      // trivially copy constructible.
+      _Optional_payload_base(_Optional_payload_base&&) = default;
 
-      using _Stored_type = remove_const_t<_Tp>;
+      _Optional_payload_base&
+      operator=(const _Optional_payload_base&) = default;
+
+      _Optional_payload_base&
+      operator=(_Optional_payload_base&&) = default;
 
       struct _Empty_byte { };
 
-      union {
-          _Empty_byte _M_empty;
-          _Stored_type _M_payload;
-      };
-      bool _M_engaged = false;
+      template<typename _Up, bool = is_trivially_destructible_v<_Up>>
+	union _Storage
+	{
+	  constexpr _Storage() noexcept : _M_empty() { }
 
-      ~_Optional_payload()
-      {
-        if (_M_engaged)
-          _M_payload.~_Stored_type();
-      }
+	  template<typename... _Args>
+	    constexpr
+	    _Storage(in_place_t, _Args&&... __args)
+	    : _M_value(std::forward<_Args>(__args)...)
+	    { }
+
+	  template<typename _Vp, typename... _Args>
+	    constexpr
+	    _Storage(std::initializer_list<_Vp> __il, _Args&&... __args)
+	    : _M_value(__il, std::forward<_Args>(__args)...)
+	    { }
+
+	  _Empty_byte _M_empty;
+          _Up _M_value;
+	};
+
+      template<typename _Up>
+	union _Storage<_Up, false>
+	{
+	  constexpr _Storage() noexcept : _M_empty() { }
+
+	  template<typename... _Args>
+	    constexpr
+	    _Storage(in_place_t, _Args&&... __args)
+	    : _M_value(std::forward<_Args>(__args)...)
+	    { }
+
+	  template<typename _Vp, typename... _Args>
+	    constexpr
+	    _Storage(std::initializer_list<_Vp> __il, _Args&&... __args)
+	    : _M_value(__il, std::forward<_Args>(__args)...)
+	    { }
+
+	  // User-provided destructor is needed when _Up has non-trivial dtor.
+	  ~_Storage() { }
+
+	  _Empty_byte _M_empty;
+          _Up _M_value;
+	};
+
+      _Storage<_Stored_type> _M_payload;
+
+      bool _M_engaged = false;
 
       template<typename... _Args>
         void
@@ -210,147 +218,70 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
           this->_M_engaged = true;
         }
 
-      // The _M_get operations have _M_engaged as a precondition.
+      constexpr void
+      _M_destroy() noexcept
+      {
+	_M_engaged = false;
+	_M_payload._M_value.~_Stored_type();
+      }
+
+      // The _M_get() operations have _M_engaged as a precondition.
+      // They exist to access the contained value with the appropriate
+      // const-qualification, because _M_payload has had the const removed.
+
       constexpr _Tp&
       _M_get() noexcept
-      { return this->_M_payload; }
+      { return this->_M_payload._M_value; }
 
       constexpr const _Tp&
       _M_get() const noexcept
-      { return this->_M_payload; }
+      { return this->_M_payload._M_value; }
 
       // _M_reset is a 'safe' operation with no precondition.
-      constexpr
-      void
+      constexpr void
       _M_reset() noexcept
       {
 	if (this->_M_engaged)
-	  {
-	    this->_M_engaged = false;
-	    this->_M_payload.~_Stored_type();
-	  }
+	  _M_destroy();
       }
     };
 
-  // Payload for potentially-constexpr optionals.
+  // Class template that manages the payload for optionals.
+  template <typename _Tp,
+	    bool /*_HasTrivialDestructor*/ =
+	      is_trivially_destructible_v<_Tp>,
+	    bool /*_HasTrivialCopy */ =
+	      is_trivially_copy_assignable_v<_Tp>
+	      && is_trivially_copy_constructible_v<_Tp>,
+	    bool /*_HasTrivialMove */ =
+	      is_trivially_move_assignable_v<_Tp>
+	      && is_trivially_move_constructible_v<_Tp>>
+    struct _Optional_payload;
+
+  // Payload for potentially-constexpr optionals (trivial copy/move/destroy).
   template <typename _Tp>
     struct _Optional_payload<_Tp, true, true, true>
+    : _Optional_payload_base<_Tp>
     {
-      constexpr _Optional_payload() noexcept
-      : _M_empty(), _M_engaged(false) { }
+      using _Optional_payload_base<_Tp>::_Optional_payload_base;
 
-      template<typename... _Args>
-	constexpr
-	_Optional_payload(in_place_t, _Args&&... __args)
-	: _M_payload(std::forward<_Args>(__args)...), _M_engaged(true)
-	{ }
-
-      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 {};
-
-      constexpr
-      _Optional_payload(__ctor_tag<bool>, const _Tp& __other)
-      : _M_payload(__other), _M_engaged(true)
-      { }
-
-      constexpr _Optional_payload(__ctor_tag<void>) noexcept
-      : _M_empty(), _M_engaged(false)
-      { }
-
-      constexpr _Optional_payload(__ctor_tag<bool>, _Tp&& __other)
-      : _M_payload(std::move(__other)), _M_engaged(true)
-      { }
-
-      constexpr
-      _Optional_payload(bool __engaged, const _Optional_payload& __other)
-      : _Optional_payload(__engaged ?
-			  _Optional_payload(__ctor_tag<bool>{},
-					    __other._M_payload) :
-			  _Optional_payload(__ctor_tag<void>{}))
-      { }
-
-      constexpr
-      _Optional_payload(bool __engaged, _Optional_payload&& __other)
-      : _Optional_payload(__engaged
-			  ? _Optional_payload(__ctor_tag<bool>{},
-					      std::move(__other._M_payload))
-			  : _Optional_payload(__ctor_tag<void>{}))
-      { }
-
-      using _Stored_type = remove_const_t<_Tp>;
-
-      struct _Empty_byte { };
-
-      union {
-	  _Empty_byte _M_empty;
-          _Stored_type _M_payload;
-      };
-      bool _M_engaged;
+      _Optional_payload() = default;
     };
 
-  // Payload for optionals with non-trivial copy assignment.
+  // Payload for optionals with non-trivial copy construction/assignment.
   template <typename _Tp>
     struct _Optional_payload<_Tp, true, false, true>
+    : _Optional_payload_base<_Tp>
     {
-      constexpr _Optional_payload() noexcept
-      : _M_empty(), _M_engaged(false) { }
-
-      template<typename... _Args>
-	constexpr
-	_Optional_payload(in_place_t, _Args&&... __args)
-	: _M_payload(std::forward<_Args>(__args)...), _M_engaged(true)
-	{ }
-
-      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 {};
-
-      constexpr _Optional_payload(__ctor_tag<bool>, const _Tp& __other)
-      : _M_payload(__other),
-	_M_engaged(true)
-      { }
-
-      constexpr _Optional_payload(__ctor_tag<void>) noexcept
-      : _M_empty(), _M_engaged(false)
-      { }
-
-      constexpr _Optional_payload(__ctor_tag<bool>, _Tp&& __other)
-      : _M_payload(std::move(__other)),
-	_M_engaged(true)
-      { }
-
-      constexpr
-      _Optional_payload(bool __engaged, const _Optional_payload& __other)
-      : _Optional_payload(__engaged ?
-			  _Optional_payload(__ctor_tag<bool>{},
-					    __other._M_payload) :
-			  _Optional_payload(__ctor_tag<void>{}))
-      { }
-
-      constexpr
-      _Optional_payload(bool __engaged, _Optional_payload&& __other)
-      : _Optional_payload(__engaged
-			  ? _Optional_payload(__ctor_tag<bool>{},
-					      std::move(__other._M_payload))
-			  : _Optional_payload(__ctor_tag<void>{}))
-      { }
+      using _Optional_payload_base<_Tp>::_Optional_payload_base;
 
+      _Optional_payload() = default;
+      ~_Optional_payload() = default;
       _Optional_payload(const _Optional_payload&) = default;
       _Optional_payload(_Optional_payload&&) = default;
+      _Optional_payload& operator=(_Optional_payload&&) = default;
 
+      // Non-trivial copy assignment.
       constexpr
       _Optional_payload&
       operator=(const _Optional_payload& __other)
@@ -366,113 +297,22 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  }
 	return *this;
       }
-
-      _Optional_payload&
-      operator=(_Optional_payload&& __other) = default;
-
-      using _Stored_type = remove_const_t<_Tp>;
-
-      struct _Empty_byte { };
-
-      union {
-          _Empty_byte _M_empty;
-          _Stored_type _M_payload;
-      };
-      bool _M_engaged;
-
-      template<typename... _Args>
-        void
-        _M_construct(_Args&&... __args)
-        noexcept(is_nothrow_constructible_v<_Stored_type, _Args...>)
-        {
-          ::new ((void *) std::__addressof(this->_M_payload))
-            _Stored_type(std::forward<_Args>(__args)...);
-          this->_M_engaged = true;
-        }
-
-      // The _M_get operations have _M_engaged as a precondition.
-      constexpr _Tp&
-      _M_get() noexcept
-      { return this->_M_payload; }
-
-      constexpr const _Tp&
-      _M_get() const noexcept
-      { return this->_M_payload; }
-
-      // _M_reset is a 'safe' operation with no precondition.
-      constexpr
-      void
-      _M_reset() noexcept
-      {
-	if (this->_M_engaged)
-	  {
-	    this->_M_engaged = false;
-	    this->_M_payload.~_Stored_type();
-	  }
-      }
     };
 
-  // Payload for optionals with non-trivial move assignment.
+  // Payload for optionals with non-trivial move construction/assignment.
   template <typename _Tp>
     struct _Optional_payload<_Tp, true, true, false>
+    : _Optional_payload_base<_Tp>
     {
-      constexpr _Optional_payload() noexcept
-      : _M_empty(), _M_engaged(false) { }
-
-      template<typename... _Args>
-	constexpr
-	_Optional_payload(in_place_t, _Args&&... __args)
-	: _M_payload(std::forward<_Args>(__args)...),
-	  _M_engaged(true)
-	{ }
-
-      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 {};
-
-      constexpr
-      _Optional_payload(__ctor_tag<bool>, const _Tp& __other)
-      : _M_payload(__other),
-	_M_engaged(true)
-      { }
-
-      constexpr _Optional_payload(__ctor_tag<void>) noexcept
-      : _M_empty(), _M_engaged(false)
-      { }
-
-      constexpr _Optional_payload(__ctor_tag<bool>, _Tp&& __other)
-      : _M_payload(std::move(__other)),
-	_M_engaged(true)
-      { }
-
-      constexpr
-      _Optional_payload(bool __engaged, const _Optional_payload& __other)
-      : _Optional_payload(__engaged ?
-			  _Optional_payload(__ctor_tag<bool>{},
-					    __other._M_payload) :
-			  _Optional_payload(__ctor_tag<void>{}))
-      { }
-
-      constexpr
-      _Optional_payload(bool __engaged, _Optional_payload&& __other)
-      : _Optional_payload(__engaged
-			  ? _Optional_payload(__ctor_tag<bool>{},
-					      std::move(__other._M_payload))
-			  : _Optional_payload(__ctor_tag<void>{}))
-      { }
+      using _Optional_payload_base<_Tp>::_Optional_payload_base;
 
+      _Optional_payload() = default;
+      ~_Optional_payload() = default;
       _Optional_payload(const _Optional_payload&) = default;
       _Optional_payload(_Optional_payload&&) = default;
+      _Optional_payload& operator=(const _Optional_payload&) = default;
 
-      _Optional_payload&
-      operator=(const _Optional_payload& __other) = default;
-
+      // Non-trivial move assignment.
       constexpr
       _Optional_payload&
       operator=(_Optional_payload&& __other)
@@ -490,106 +330,21 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  }
 	return *this;
       }
-
-      using _Stored_type = remove_const_t<_Tp>;
-
-      struct _Empty_byte { };
-
-      union {
-          _Empty_byte _M_empty;
-          _Stored_type _M_payload;
-      };
-      bool _M_engaged;
-
-      template<typename... _Args>
-        void
-        _M_construct(_Args&&... __args)
-        noexcept(is_nothrow_constructible_v<_Stored_type, _Args...>)
-        {
-          ::new ((void *) std::__addressof(this->_M_payload))
-            _Stored_type(std::forward<_Args>(__args)...);
-          this->_M_engaged = true;
-        }
-
-      // The _M_get operations have _M_engaged as a precondition.
-      constexpr _Tp&
-      _M_get() noexcept
-      { return this->_M_payload; }
-
-      constexpr const _Tp&
-      _M_get() const noexcept
-      { return this->_M_payload; }
-
-      // _M_reset is a 'safe' operation with no precondition.
-      constexpr
-      void
-      _M_reset() noexcept
-      {
-	if (this->_M_engaged)
-	  {
-	    this->_M_engaged = false;
-	    this->_M_payload.~_Stored_type();
-	  }
-      }
     };
 
   // Payload for optionals with non-trivial copy and move assignment.
   template <typename _Tp>
     struct _Optional_payload<_Tp, true, false, false>
+    : _Optional_payload_base<_Tp>
     {
-      constexpr _Optional_payload() noexcept
-      : _M_empty(), _M_engaged(false) {}
-
-      template<typename... _Args>
-	constexpr
-	_Optional_payload(in_place_t, _Args&&... __args)
-	: _M_payload(std::forward<_Args>(__args)...),
-	  _M_engaged(true)
-	{ }
-
-      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 {};
-
-      constexpr _Optional_payload(__ctor_tag<bool>, const _Tp& __other)
-      : _M_payload(__other),
-	    _M_engaged(true)
-      { }
-
-      constexpr _Optional_payload(__ctor_tag<void>) noexcept
-      : _M_empty(), _M_engaged(false)
-      { }
-
-      constexpr _Optional_payload(__ctor_tag<bool>, _Tp&& __other)
-      : _M_payload(std::move(__other)),
-	_M_engaged(true)
-      { }
-
-      constexpr
-      _Optional_payload(bool __engaged, const _Optional_payload& __other)
-      : _Optional_payload(__engaged ?
-			  _Optional_payload(__ctor_tag<bool>{},
-					    __other._M_payload) :
-			  _Optional_payload(__ctor_tag<void>{}))
-      { }
-
-      constexpr
-      _Optional_payload(bool __engaged, _Optional_payload&& __other)
-      : _Optional_payload(__engaged
-			  ? _Optional_payload(__ctor_tag<bool>{},
-					      std::move(__other._M_payload))
-			  : _Optional_payload(__ctor_tag<void>{}))
-      { }
+      using _Optional_payload_base<_Tp>::_Optional_payload_base;
 
+      _Optional_payload() = default;
+      ~_Optional_payload() = default;
       _Optional_payload(const _Optional_payload&) = default;
       _Optional_payload(_Optional_payload&&) = default;
 
+      // Non-trivial copy
       constexpr
       _Optional_payload&
       operator=(const _Optional_payload& __other)
@@ -606,6 +361,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	return *this;
       }
 
+      // Non-trivial move assignment.
       constexpr
       _Optional_payload&
       operator=(_Optional_payload&& __other)
@@ -623,56 +379,33 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  }
 	return *this;
       }
+    };
 
-      using _Stored_type = remove_const_t<_Tp>;
-
-      struct _Empty_byte { };
-
-      union {
-          _Empty_byte _M_empty;
-          _Stored_type _M_payload;
-      };
-      bool _M_engaged;
-
-      template<typename... _Args>
-        constexpr
-        void
-        _M_construct(_Args&&... __args)
-        noexcept(is_nothrow_constructible_v<_Stored_type, _Args...>)
-        {
-          ::new ((void *) std::__addressof(this->_M_payload))
-            _Stored_type(std::forward<_Args>(__args)...);
-          this->_M_engaged = true;
-        }
-
-      // The _M_get operations have _M_engaged as a precondition.
-      constexpr _Tp&
-      _M_get() noexcept
-      { return this->_M_payload; }
-
-      constexpr const _Tp&
-      _M_get() const noexcept
-      { return this->_M_payload; }
+  // Payload for optionals with non-trivial destructors.
+  template <typename _Tp, bool _Copy, bool _Move>
+    struct _Optional_payload<_Tp, false, _Copy, _Move>
+    : _Optional_payload<_Tp, true, false, false>
+    {
+      // Base class implements all the constructors and assignment operators:
+      using _Optional_payload<_Tp, true, false, false>::_Optional_payload;
+      _Optional_payload() = default;
+      _Optional_payload(const _Optional_payload&) = default;
+      _Optional_payload(_Optional_payload&&) = default;
+      _Optional_payload& operator=(const _Optional_payload&) = default;
+      _Optional_payload& operator=(_Optional_payload&&) = default;
 
-      // _M_reset is a 'safe' operation with no precondition.
-      constexpr
-      void
-      _M_reset() noexcept
-      {
-	if (this->_M_engaged)
-	  {
-	    this->_M_engaged = false;
-	    this->_M_payload.~_Stored_type();
-	  }
-      }
+      // Destructor needs to destroy the contained value:
+      ~_Optional_payload() { this->_M_reset(); }
     };
 
+  // Common base class for _Optional_base<T> to avoid repeating these
+  // member functions in each specialization.
   template<typename _Tp, typename _Dp>
     class _Optional_base_impl
     {
     protected:
       using _Stored_type = remove_const_t<_Tp>;
-      
+
       // The _M_construct operation has !_M_engaged as a precondition
       // while _M_destruct has _M_engaged as a precondition.
       template<typename... _Args>
@@ -685,42 +418,59 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	    _Stored_type(std::forward<_Args>(__args)...);
 	  static_cast<_Dp*>(this)->_M_payload._M_engaged = true;
 	}
-      
+
       void
       _M_destruct() noexcept
-      {
-	static_cast<_Dp*>(this)->_M_payload._M_engaged = false;
-	static_cast<_Dp*>(this)->_M_payload._M_payload.~_Stored_type();
-      }
-      
+      { static_cast<_Dp*>(this)->_M_payload._M_destroy(); }
+
       // _M_reset is a 'safe' operation with no precondition.
-      constexpr
-      void
+      constexpr void
       _M_reset() noexcept
+      { static_cast<_Dp*>(this)->_M_payload._M_reset(); }
+
+      constexpr bool _M_is_engaged() const noexcept
+      { return static_cast<const _Dp*>(this)->_M_payload._M_engaged; }
+
+      // The _M_get operations have _M_engaged as a precondition.
+      constexpr _Tp&
+      _M_get() noexcept
       {
-	if (static_cast<_Dp*>(this)->_M_payload._M_engaged)
-	  static_cast<_Dp*>(this)->_M_destruct();
+	__glibcxx_assert(this->_M_is_engaged());
+	return static_cast<_Dp*>(this)->_M_payload._M_get();
       }
-  };
+
+      constexpr const _Tp&
+      _M_get() const noexcept
+      {
+	__glibcxx_assert(this->_M_is_engaged());
+	return static_cast<const _Dp*>(this)->_M_payload._M_get();
+      }
+    };
 
   /**
-    * @brief Class template that takes care of copy/move constructors
-    of optional
+    * @brief Class template that provides copy/move constructors of optional.
     *
     * Such a separate base class template is necessary in order to
     * conditionally make copy/move constructors trivial.
+    *
+    * When the contained value is trivally copy/move constructible,
+    * the copy/move constructors of _Optional_base will invoke the
+    * trivial copy/move constructor of _Optional_payload. Otherwise,
+    * they will invoke _Optional_payload(bool, const _Optional_payload&)
+    * or _Optional_payload(bool, _Optional_payload&&) to initialize
+    * the contained value, if copying/moving an engaged optional.
+    *
+    * Whether the other special members are trivial is determined by the
+    * _Optional_payload<_Tp> specialization used for the _M_payload member.
+    *
     * @see optional, _Enable_special_members
     */
   template<typename _Tp,
 	   bool = is_trivially_copy_constructible_v<_Tp>,
 	   bool = is_trivially_move_constructible_v<_Tp>>
-    class _Optional_base
-    // protected inheritance because optional needs to reach that base too
-      : protected _Optional_base_impl<_Tp, _Optional_base<_Tp>>
+    struct _Optional_base
+      : _Optional_base_impl<_Tp, _Optional_base<_Tp>>
     {
-      friend class _Optional_base_impl<_Tp, _Optional_base<_Tp>>;
-
-    public:
       // Constructors for disengaged optionals.
       constexpr _Optional_base() = default;
 
@@ -758,37 +508,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       _Optional_base& operator=(const _Optional_base&) = default;
       _Optional_base& operator=(_Optional_base&&) = default;
 
-    protected:
-
-      constexpr bool _M_is_engaged() const noexcept
-      { return this->_M_payload._M_engaged; }
-
-      // The _M_get operations have _M_engaged as a precondition.
-      constexpr _Tp&
-	_M_get() noexcept
-      {
-	__glibcxx_assert(this->_M_is_engaged());
-	return this->_M_payload._M_payload;
-      }
-
-      constexpr const _Tp&
-	_M_get() const noexcept
-      {
-	__glibcxx_assert(this->_M_is_engaged());
-	return this->_M_payload._M_payload;
-      }
-
-    private:
       _Optional_payload<_Tp> _M_payload;
     };
 
   template<typename _Tp>
-    class _Optional_base<_Tp, false, true>
-      : protected _Optional_base_impl<_Tp, _Optional_base<_Tp>>
+    struct _Optional_base<_Tp, false, true>
+      : _Optional_base_impl<_Tp, _Optional_base<_Tp>>
     {
-      friend class _Optional_base_impl<_Tp, _Optional_base<_Tp>>;
-    public:
-
       // Constructors for disengaged optionals.
       constexpr _Optional_base() = default;
 
@@ -822,37 +548,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       _Optional_base& operator=(const _Optional_base&) = default;
       _Optional_base& operator=(_Optional_base&&) = default;
 
-    protected:
-
-      constexpr bool _M_is_engaged() const noexcept
-      { return this->_M_payload._M_engaged; }
-
-      // The _M_get operations have _M_engaged as a precondition.
-      constexpr _Tp&
-	_M_get() noexcept
-      {
-	__glibcxx_assert(this->_M_is_engaged());
-	return this->_M_payload._M_payload;
-      }
-
-      constexpr const _Tp&
-	_M_get() const noexcept
-      {
-	__glibcxx_assert(this->_M_is_engaged());
-	return this->_M_payload._M_payload;
-      }
-
-    private:
       _Optional_payload<_Tp> _M_payload;
     };
 
   template<typename _Tp>
-    class _Optional_base<_Tp, true, false>
-      : protected _Optional_base_impl<_Tp, _Optional_base<_Tp>>
+    struct _Optional_base<_Tp, true, false>
+      : _Optional_base_impl<_Tp, _Optional_base<_Tp>>
     {
-      friend class _Optional_base_impl<_Tp, _Optional_base<_Tp>>;
-    public:
-
       // Constructors for disengaged optionals.
       constexpr _Optional_base() = default;
 
@@ -887,37 +589,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       _Optional_base& operator=(const _Optional_base&) = default;
       _Optional_base& operator=(_Optional_base&&) = default;
 
-    protected:
-
-      constexpr bool _M_is_engaged() const noexcept
-      { return this->_M_payload._M_engaged; }
-
-      // The _M_get operations have _M_engaged as a precondition.
-      constexpr _Tp&
-	_M_get() noexcept
-      {
-	__glibcxx_assert(this->_M_is_engaged());
-	return this->_M_payload._M_payload;
-      }
-
-      constexpr const _Tp&
-	_M_get() const noexcept
-      {
-	__glibcxx_assert(this->_M_is_engaged());
-	return this->_M_payload._M_payload;
-      }
-
-    private:
       _Optional_payload<_Tp> _M_payload;
     };
 
   template<typename _Tp>
-    class _Optional_base<_Tp, true, true>
-      : protected _Optional_base_impl<_Tp, _Optional_base<_Tp>>
+    struct _Optional_base<_Tp, true, true>
+      : _Optional_base_impl<_Tp, _Optional_base<_Tp>>
     {
-      friend class _Optional_base_impl<_Tp, _Optional_base<_Tp>>;
-    public:
-
       // Constructors for disengaged optionals.
       constexpr _Optional_base() = default;
 
@@ -947,27 +625,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       _Optional_base& operator=(const _Optional_base&) = default;
       _Optional_base& operator=(_Optional_base&&) = default;
 
-    protected:
-
-      constexpr bool _M_is_engaged() const noexcept
-      { return this->_M_payload._M_engaged; }
-
-      // The _M_get operations have _M_engaged as a precondition.
-      constexpr _Tp&
-	_M_get() noexcept
-      {
-	__glibcxx_assert(this->_M_is_engaged());
-	return this->_M_payload._M_payload;
-      }
-
-      constexpr const _Tp&
-	_M_get() const noexcept
-      {
-	__glibcxx_assert(this->_M_is_engaged());
-	return this->_M_payload._M_payload;
-      }
-
-    private:
       _Optional_payload<_Tp> _M_payload;
     };
 
@@ -1262,36 +919,32 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       value() const&
       {
 	return this->_M_is_engaged()
-	  ?  this->_M_get()
-	  : (__throw_bad_optional_access(),
-	     this->_M_get());
+	  ? this->_M_get()
+	  : (__throw_bad_optional_access(), this->_M_get());
       }
 
       constexpr _Tp&
       value()&
       {
 	return this->_M_is_engaged()
-	  ?  this->_M_get()
-	  : (__throw_bad_optional_access(),
-	     this->_M_get());
+	  ? this->_M_get()
+	  : (__throw_bad_optional_access(), this->_M_get());
       }
 
       constexpr _Tp&&
       value()&&
       {
 	return this->_M_is_engaged()
-	  ?  std::move(this->_M_get())
-	  : (__throw_bad_optional_access(),
-	     std::move(this->_M_get()));
+	  ? std::move(this->_M_get())
+	  : (__throw_bad_optional_access(), std::move(this->_M_get()));
       }
 
       constexpr const _Tp&&
       value() const&&
       {
 	return this->_M_is_engaged()
-	  ?  std::move(this->_M_get())
-	  : (__throw_bad_optional_access(),
-	     std::move(this->_M_get()));
+	  ? std::move(this->_M_get())
+	  : (__throw_bad_optional_access(), std::move(this->_M_get()));
       }
 
       template<typename _Up>
@@ -1302,8 +955,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  static_assert(is_convertible_v<_Up&&, _Tp>);
 
 	  return this->_M_is_engaged()
-	    ? this->_M_get()
-	    : static_cast<_Tp>(std::forward<_Up>(__u));
+	    ? this->_M_get() : static_cast<_Tp>(std::forward<_Up>(__u));
 	}
 
       template<typename _Up>
diff --git a/libstdc++-v3/python/libstdcxx/v6/printers.py b/libstdc++-v3/python/libstdcxx/v6/printers.py
index 9e80e99..967d03a 100644
--- a/libstdc++-v3/python/libstdcxx/v6/printers.py
+++ b/libstdc++-v3/python/libstdcxx/v6/printers.py
@@ -1093,13 +1093,23 @@ class StdExpOptionalPrinter(SingleObjContainerPrinter):
 
     def __init__ (self, typename, val):
         valtype = self._recognize (val.type.template_argument(0))
-        self.typename = strip_versioned_namespace(typename)
-        self.typename = re.sub('^std::(experimental::|)(fundamentals_v\d::|)(.*)', r'std::\1\3<%s>' % valtype, self.typename, 1)
-        if not self.typename.startswith('std::experimental'):
-            val = val['_M_payload']
-        self.val = val
-        contained_value = val['_M_payload'] if self.val['_M_engaged'] else None
-        visualizer = gdb.default_visualizer (val['_M_payload'])
+        typename = strip_versioned_namespace(typename)
+        self.typename = re.sub('^std::(experimental::|)(fundamentals_v\d::|)(.*)', r'std::\1\3<%s>' % valtype, typename, 1)
+        payload = val['_M_payload']
+        if self.typename.startswith('std::experimental'):
+            engaged = val['_M_engaged']
+            contained_value = payload
+        else:
+            engaged = payload['_M_engaged']
+            contained_value = payload['_M_payload']
+            try:
+                # Since GCC 9
+                contained_value = contained_value['_M_value']
+            except:
+                pass
+        visualizer = gdb.default_visualizer (contained_value)
+        if not engaged:
+            contained_value = None
         super (StdExpOptionalPrinter, self).__init__ (contained_value, visualizer)
 
     def to_string (self):
diff --git a/libstdc++-v3/testsuite/libstdc++-prettyprinters/compat.cc b/libstdc++-v3/testsuite/libstdc++-prettyprinters/compat.cc
new file mode 100644
index 0000000..39271dd
--- /dev/null
+++ b/libstdc++-v3/testsuite/libstdc++-prettyprinters/compat.cc
@@ -0,0 +1,76 @@
+// { dg-options "-g -O0" }
+// { dg-do run }
+// { dg-skip-if "" { *-*-* } { "-D_GLIBCXX_PROFILE" } }
+
+// Copyright (C) 2014-2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// Test that current printers still support old definitions of types.
+
+namespace std
+{
+  // Old representation of std::optional, before GCC 9
+  template<typename T>
+    struct _Optional_payload
+    {
+      _Optional_payload() : _M_empty(), _M_engaged(false) { }
+      struct _Empty_byte { };
+      union {
+	_Empty_byte _M_empty;
+	T _M_payload;
+      };
+      bool _M_engaged;
+    };
+
+  template<typename T>
+    struct _Optional_base
+    {
+      _Optional_payload<T> _M_payload;
+    };
+
+  template<typename T>
+    struct optional : _Optional_base<T>
+    {
+      optional() { }
+
+      optional(T t)
+      {
+	this->_M_payload._M_payload = t;
+	this->_M_payload._M_engaged = true;
+      }
+    };
+} // namespace std
+
+int
+main()
+{
+  using std::optional;
+
+  optional<int> o;
+// { dg-final { note-test o {std::optional<int> [no contained value]} } }
+  optional<bool> ob{false};
+// { dg-final { note-test ob {std::optional<bool> = {[contained value] = false}} } }
+  optional<int> oi{5};
+// { dg-final { note-test oi {std::optional<int> = {[contained value] = 5}} } }
+  optional<void*> op{nullptr};
+// { dg-final { note-test op {std::optional<void *> = {[contained value] = 0x0}} } }
+
+  __builtin_puts("");
+  return 0;			// Mark SPOT
+}
+
+// { dg-final { gdb-test SPOT } }