Remove undefined behaviour (#117)

* Add assertions for cases where the API documents undefined behaviour.
This affects operator->, operator*, and error() on a tl::expected
instance. The performance impact is likely negligible for typical uses.
refs #113

Signed-off-by: Daira Hopwood <daira@jacaranda.org>

* Add tests for assertions.

Signed-off-by: Daira Hopwood <daira@jacaranda.org>

* Add documentation for assertion behaviour.

Signed-off-by: Daira Hopwood <daira@jacaranda.org>

---------

Signed-off-by: Daira Hopwood <daira@jacaranda.org>

README.md

@@ -57,6 +57,8 @@ The interface is the same as `std::expected` as proposed in [p0323r3](http://www
 - `or_else`: calls some function if there is no value stored.
   * `exp.or_else([] { throw std::runtime_error{"oh no"}; });`
 
+p0323r3 specifies calling `.error()` on an expected value, or using the `*` or `->` operators on an unexpected value, to be undefined behaviour. In this implementation it causes an assertion failure. The implementation of assertions can be overridden by defining the macro `TL_ASSERT(boolean_condition)` before #including <tl/expected.hpp>; by default, `assert(boolean_condition)` from the `<cassert>` header is used. Note that correct code would not rely on these assertions.
+
 ### Compiler support
 
 Tested on:

include/tl/expected.hpp

@@ -25,6 +25,11 @@
 #include <type_traits>
 #include <utility>
 
+#if !defined(TL_ASSERT)
+#include <cassert>
+#define TL_ASSERT assert
+#endif
+
 #if defined(__EXCEPTIONS) || defined(_CPPUNWIND)
 #define TL_EXPECTED_EXCEPTIONS_ENABLED
 #endif
@@ -1890,27 +1895,37 @@ public:
     }
   }
 
-  constexpr const T *operator->() const { return valptr(); }
-  TL_EXPECTED_11_CONSTEXPR T *operator->() { return valptr(); }
+  constexpr const T *operator->() const {
+    TL_ASSERT(has_value());
+    return valptr();
+  }
+  TL_EXPECTED_11_CONSTEXPR T *operator->() {
+    TL_ASSERT(has_value());
+    return valptr();
+  }
 
   template <class U = T,
             detail::enable_if_t<!std::is_void<U>::value> * = nullptr>
   constexpr const U &operator*() const & {
+    TL_ASSERT(has_value());
     return val();
   }
   template <class U = T,
             detail::enable_if_t<!std::is_void<U>::value> * = nullptr>
   TL_EXPECTED_11_CONSTEXPR U &operator*() & {
+    TL_ASSERT(has_value());
     return val();
   }
   template <class U = T,
             detail::enable_if_t<!std::is_void<U>::value> * = nullptr>
   constexpr const U &&operator*() const && {
+    TL_ASSERT(has_value());
     return std::move(val());
   }
   template <class U = T,
             detail::enable_if_t<!std::is_void<U>::value> * = nullptr>
   TL_EXPECTED_11_CONSTEXPR U &&operator*() && {
+    TL_ASSERT(has_value());
     return std::move(val());
   }
 
@@ -1946,10 +1961,22 @@ public:
     return std::move(val());
   }
 
-  constexpr const E &error() const & { return err().value(); }
-  TL_EXPECTED_11_CONSTEXPR E &error() & { return err().value(); }
-  constexpr const E &&error() const && { return std::move(err().value()); }
-  TL_EXPECTED_11_CONSTEXPR E &&error() && { return std::move(err().value()); }
+  constexpr const E &error() const & {
+    TL_ASSERT(!has_value());
+    return err().value();
+  }
+  TL_EXPECTED_11_CONSTEXPR E &error() & {
+    TL_ASSERT(!has_value());
+    return err().value();
+  }
+  constexpr const E &&error() const && {
+    TL_ASSERT(!has_value());
+    return std::move(err().value());
+  }
+  TL_EXPECTED_11_CONSTEXPR E &&error() && {
+    TL_ASSERT(!has_value());
+    return std::move(err().value());
+  }
 
   template <class U> constexpr T value_or(U &&v) const & {
     static_assert(std::is_copy_constructible<T>::value &&

tests/assertions.cpp

@@ -0,0 +1,18 @@
+#include <catch2/catch.hpp>
+#include <stdexcept>
+
+#define TL_ASSERT(cond) if (!(cond)) { throw std::runtime_error(std::string("assertion failure")); }
+
+#include <tl/expected.hpp>
+
+TEST_CASE("Assertions", "[assertions]") {
+  tl::expected<int,int> o1 = 42;
+  REQUIRE_THROWS_WITH(o1.error(), "assertion failure");
+
+  tl::expected<int,int> o2 {tl::unexpect, 0};
+  REQUIRE_THROWS_WITH(*o2, "assertion failure");
+
+  struct foo { int bar; };
+  tl::expected<struct foo,int> o3 {tl::unexpect, 0};
+  REQUIRE_THROWS_WITH(o3->bar, "assertion failure");
+}