Hello,

this patch adds 2 simple __glibcxx_assert in optional that match the precondition in the comment above. I am not sure if there was a reason the author wrote that comment instead of the assertion, but constexpr use still seems to work.

I hesitated about having the assertion in operator*, etc, so that the error message would be clearer, but we would still be missing the key information of where this function was called from (in user code), so the real solution would be for __glibcxx_assert to print (a few lines of) a stack trace, an unrelated issue.

Bootstrap+testsuite on powerpc64le-unknown-linux-gnu.

2017-05-15  Marc Glisse  <marc.gli...@inria.fr>

        * include/std/optional (_Optional_base::_M_get): Check precondition.
        * testsuite/20_util/optional/cons/value_neg.cc: Update line numbers.

--
Marc Glisse
Index: include/std/optional
===================================================================
--- include/std/optional	(revision 248008)
+++ include/std/optional	(working copy)
@@ -379,25 +379,31 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       }
       // The following functionality is also needed by optional, hence the
       // protected accessibility.
     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
-      { return this->_M_payload._M_payload; }
+      {
+	__glibcxx_assert(_M_is_engaged());
+	return this->_M_payload._M_payload;
+      }
 
       constexpr const _Tp&
       _M_get() const noexcept
-      { return this->_M_payload._M_payload; }
+      {
+	__glibcxx_assert(_M_is_engaged());
+	return this->_M_payload._M_payload;
+      }
 
       // 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._M_payload))
             _Stored_type(std::forward<_Args>(__args)...);
Index: testsuite/20_util/optional/cons/value_neg.cc
===================================================================
--- testsuite/20_util/optional/cons/value_neg.cc	(revision 248008)
+++ testsuite/20_util/optional/cons/value_neg.cc	(working copy)
@@ -30,15 +30,15 @@ int main()
     struct X
     {
       explicit X(int) {}
     };
     std::optional<X> ox{42};
     std::optional<X> ox2 = 42; // { dg-error "conversion" }
     std::optional<std::unique_ptr<int>> oup{new int};
     std::optional<std::unique_ptr<int>> oup2 = new int;  // { dg-error "conversion" }
     struct U { explicit U(std::in_place_t); };
     std::optional<U> ou(std::in_place); // { dg-error "no matching" }
-    // { dg-error "no type" "" { target { *-*-* } } 487 }
-    // { dg-error "no type" "" { target { *-*-* } } 497 }
-    // { dg-error "no type" "" { target { *-*-* } } 554 }
+    // { dg-error "no type" "" { target { *-*-* } } 493 }
+    // { dg-error "no type" "" { target { *-*-* } } 503 }
+    // { dg-error "no type" "" { target { *-*-* } } 560 }
   }
 }

Reply via email to