From 9423013aca3b198315cdfcfa02fbe5d5684842c9 Mon Sep 17 00:00:00 2001 From: Simon Brand Date: Sun, 3 Dec 2017 08:25:19 +0000 Subject: [PATCH 1/8] Support void for map_error --- expected.hpp | 63 ++++++++++++++++++++++++++++++++++---------- tests/extensions.cpp | 48 +++++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 14 deletions(-) diff --git a/expected.hpp b/expected.hpp index c9dda07..58452db 100644 --- a/expected.hpp +++ b/expected.hpp @@ -921,9 +921,9 @@ public: /// \group and_then /// Carries out some operation which returns an expected on the stored object /// if there is one. \requires `std::invoke(std::forward(f), value())` - /// returns a `std::expected` for some `U`. \returns Let `U` be the result - /// of `std::invoke(std::forward(f), value())`. Returns a - /// `std::expected`. The return value is empty if `*this` is empty, + /// returns an `expected` for some `U`. \returns Let `U` be the result + /// of `std::invoke(std::forward(f), value())`. Returns an + /// `expected`. The return value is empty if `*this` is empty, /// otherwise the return value of `std::invoke(std::forward(f), value())` /// is returned. /// \synopsis template \nconstexpr auto and_then(F &&f) &; @@ -975,9 +975,9 @@ public: /// \group and_then /// Carries out some operation which returns an expected on the stored object /// if there is one. \requires `std::invoke(std::forward(f), value())` - /// returns a `std::expected` for some `U`. \returns Let `U` be the result - /// of `std::invoke(std::forward(f), value())`. Returns a - /// `std::expected`. The return value is empty if `*this` is empty, + /// returns an `expected` for some `U`. \returns Let `U` be the result + /// of `std::invoke(std::forward(f), value())`. Returns an + /// `expected`. The return value is empty if `*this` is empty, /// otherwise the return value of `std::invoke(std::forward(f), value())` /// is returned. /// \synopsis template \nconstexpr auto and_then(F &&f) &; @@ -1034,7 +1034,8 @@ public: !defined(TL_EXPECTED_GCC54) /// \brief Carries out some operation on the stored object if there is one. /// \returns Let `U` be the result of `std::invoke(std::forward(f), - /// value())`. Returns a `std::expected`. If `*this` is unexpected, the + /// value())`. If `U` is `void`, returns an `expected, otherwise + // returns an `expected`. If `*this` is unexpected, the /// result is `*this`, otherwise an `expected` is constructed from the /// return value of `std::invoke(std::forward(f), value())` and is /// returned. @@ -1065,7 +1066,8 @@ public: #else /// \brief Carries out some operation on the stored object if there is one. /// \returns Let `U` be the result of `std::invoke(std::forward(f), - /// value())`. Returns a `std::expected`. If `*this` is unexpected, the + /// value())`. If `U` is `void`, returns an `expected, otherwise + // returns an `expected`. If `*this` is unexpected, the /// result is `*this`, otherwise an `expected` is constructed from the /// return value of `std::invoke(std::forward(f), value())` and is /// returned. @@ -1114,7 +1116,8 @@ public: /// \brief Carries out some operation on the stored unexpected object if there /// is one. /// \returns Let `U` be the result of `std::invoke(std::forward(f), - /// value())`. Returns a `std::expected`. If `*this` has an expected + /// value())`. If `U` is `void`, returns an `expected`, otherwise + /// returns an `expected`. If `*this` has an expected /// value, the result is `*this`, otherwise an `expected` is constructed /// from `make_unexpected(std::invoke(std::forward(f), value()))` and is /// returned. @@ -1146,7 +1149,7 @@ public: /// \brief Carries out some operation on the stored unexpected object if there /// is one. /// \returns Let `U` be the result of `std::invoke(std::forward(f), - /// value())`. Returns a `std::expected`. If `*this` has an expected + /// value())`. Returns an `expected`. If `*this` has an expected /// value, the result is `*this`, otherwise an `expected` is constructed /// from `make_unexpected(std::invoke(std::forward(f), value()))` and is /// returned. @@ -1622,6 +1625,7 @@ public: /// \exclude namespace detail { +template using exp_t = typename detail::decay_t::error_type; template using err_t = typename detail::decay_t::error_type; template using ret_t = expected>; @@ -1683,19 +1687,35 @@ auto map_impl(Exp &&exp, F &&f) -> expected> { !defined(TL_EXPECTED_GCC54) template (), - *std::declval()))> + *std::declval())), + detail::enable_if_t::value> * = nullptr> constexpr auto map_error_impl(Exp &&exp, F &&f) { - using result = ret_t; + using result = expected, Ret>; return exp.has_value() ? result(*std::forward(exp)) : result(unexpect, detail::invoke(std::forward(f), std::forward(exp).error())); } +template (), + *std::declval())), + detail::enable_if_t::value> * = nullptr> +constexpr auto map_error_impl(Exp &&exp, F &&f) { + using result = expected, monostate>; + if (exp.has_value()) { + return result(*std::forward(exp)); + } + + detail::invoke(std::forward(f), + std::forward(exp).error()); + return result(unexpect, monostate{}); +} #else template (), - *std::declval()))> -constexpr auto map_error_impl(Exp &&exp, F &&f) -> ret_t { + *std::declval())), + detail::enable_if_t::value> * = nullptr> +constexpr auto map_error_impl(Exp &&exp, F &&f) -> expected, Ret> { using result = ret_t; return exp.has_value() @@ -1703,6 +1723,21 @@ constexpr auto map_error_impl(Exp &&exp, F &&f) -> ret_t { : result(unexpect, detail::invoke(std::forward(f), std::forward(exp).error())); } + +template (), + *std::declval())), + detail::enable_if_t::value> * = nullptr> +constexpr auto map_error_impl(Exp &&exp, F &&f) -> expected, monostate> { + using result = expected, monostate>; + if (exp.has_value()) { + return result(*std::forward(exp)); + } + + detail::invoke(std::forward(f), + std::forward(exp).error()); + return result(unexpect, monostate{}); +} #endif template e = 21; + auto ret = e.map_error(ret_void); + REQUIRE(ret); + } + + { + const tl::expected e = 21; + auto ret = e.map_error(ret_void); + REQUIRE(ret); + } + + { + tl::expected e = 21; + auto ret = std::move(e).map_error(ret_void); + REQUIRE(ret); + } + + { + const tl::expected e = 21; + auto ret = std::move(e).map_error(ret_void); + REQUIRE(ret); + } + + { + tl::expected e(tl::unexpect, 21); + auto ret = e.map_error(ret_void); + REQUIRE(!ret); + } + + { + const tl::expected e(tl::unexpect, 21); + auto ret = e.map_error(ret_void); + REQUIRE(!ret); + } + + { + tl::expected e(tl::unexpect, 21); + auto ret = std::move(e).map_error(ret_void); + REQUIRE(!ret); + } + + { + const tl::expected e(tl::unexpect, 21); + auto ret = std::move(e).map_error(ret_void); + REQUIRE(!ret); + } } TEST_CASE("And then extensions", "[extensions.and_then]") { From 759b9ae37a55777abaa63d62707e885b6b0198f0 Mon Sep 17 00:00:00 2001 From: Simon Brand Date: Sun, 3 Dec 2017 08:34:18 +0000 Subject: [PATCH 2/8] Constexpr fixes --- expected.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/expected.hpp b/expected.hpp index 5f930c3..c9fe9a6 100644 --- a/expected.hpp +++ b/expected.hpp @@ -1700,7 +1700,7 @@ template (), *std::declval())), detail::enable_if_t::value> * = nullptr> -constexpr auto map_error_impl(Exp &&exp, F &&f) { +auto map_error_impl(Exp &&exp, F &&f) { using result = expected, monostate>; if (exp.has_value()) { return result(*std::forward(exp)); @@ -1728,7 +1728,7 @@ template (), *std::declval())), detail::enable_if_t::value> * = nullptr> -constexpr auto map_error_impl(Exp &&exp, F &&f) -> expected, monostate> { +auto map_error_impl(Exp &&exp, F &&f) -> expected, monostate> { using result = expected, monostate>; if (exp.has_value()) { return result(*std::forward(exp)); From da1f12b81b39ea3d5b552ef1dbd16c2188f6e175 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Alexandre=20Boissonneault?= Date: Sun, 3 Dec 2017 22:11:12 -0500 Subject: [PATCH 3/8] Fixed missing decay in make_unexpected (ex: const lvalue expressions would create unexpected, which would not compile) --- expected.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/expected.hpp b/expected.hpp index c9fe9a6..2a3442d 100644 --- a/expected.hpp +++ b/expected.hpp @@ -158,8 +158,8 @@ constexpr bool operator>=(const unexpected &lhs, const unexpected &rhs) { /// *Example:* /// auto e1 = tl::make_unexpected(42); /// unexpected e2 (42); //same semantics -template unexpected make_unexpected(E &&e) { - return unexpected(std::forward(e)); +template unexpected::type> make_unexpected(E &&e) { + return unexpected::type>(std::forward(e)); } /// \brief A tag type to tell expected to construct the unexpected value From 7cf765e3564c7dcace92dc88489efea025fc08f7 Mon Sep 17 00:00:00 2001 From: ericLemanissier Date: Mon, 4 Dec 2017 12:32:06 +0100 Subject: [PATCH 4/8] fix typo in README --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 529a3e3..8ac5867 100644 --- a/README.md +++ b/README.md @@ -49,7 +49,7 @@ The interface is the same as `std::expected` as proposed in [p0323r3](http://www * `tl::expected s = exp_string.map(&std::string::size);` - `map_error`: carries out some operation on the unexpected object if there is one. * `my_error_code translate_error (std::error_code);` - * `tl::expected s = exp_int.map(translate_error);` + * `tl::expected s = exp_int.map_error(translate_error);` - `and_then`: like `map`, but for operations which return a `tl::expected`. * `tl::expected parse (const std::string& s);` * `tl::expected exp_ast = exp_string.and_then(parse);` From 768edb5ae76e37bb504386e55f5e7bbca6e84a51 Mon Sep 17 00:00:00 2001 From: kaboissonneault Date: Tue, 5 Dec 2017 12:38:28 -0500 Subject: [PATCH 5/8] Fixed mapping on expected not decaying the resulting expression --- expected.hpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/expected.hpp b/expected.hpp index 2a3442d..23936f0 100644 --- a/expected.hpp +++ b/expected.hpp @@ -1635,7 +1635,7 @@ template ())), detail::enable_if_t::value> * = nullptr> constexpr auto map_impl(Exp &&exp, F &&f) { - using result = ret_t; + using result = ret_t>; return exp.has_value() ? result(detail::invoke(std::forward(f), *std::forward(exp))) : result(unexpect, std::forward(exp).error()); @@ -1660,8 +1660,8 @@ template ())), detail::enable_if_t::value> * = nullptr> -constexpr auto map_impl(Exp &&exp, F &&f) -> ret_t { - using result = ret_t; +constexpr auto map_impl(Exp &&exp, F &&f) -> ret_t> { + using result = ret_t>; return exp.has_value() ? result(detail::invoke(std::forward(f), *std::forward(exp))) @@ -1690,7 +1690,7 @@ template ())), detail::enable_if_t::value> * = nullptr> constexpr auto map_error_impl(Exp &&exp, F &&f) { - using result = expected, Ret>; + using result = expected, detail::decay_t>; return exp.has_value() ? result(*std::forward(exp)) : result(unexpect, detail::invoke(std::forward(f), @@ -1715,8 +1715,8 @@ template (), *std::declval())), detail::enable_if_t::value> * = nullptr> -constexpr auto map_error_impl(Exp &&exp, F &&f) -> expected, Ret> { - using result = ret_t; +constexpr auto map_error_impl(Exp &&exp, F &&f) -> expected, detail::decay_t> { + using result = ret_t>; return exp.has_value() ? result(*std::forward(exp)) From d6cc086c8ee1e3ffc54015b82b42f9ffa76a492e Mon Sep 17 00:00:00 2001 From: Simon Brand Date: Tue, 5 Dec 2017 19:56:51 +0000 Subject: [PATCH 6/8] Add acknowledgement --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index 8ac5867..306647e 100644 --- a/README.md +++ b/README.md @@ -82,6 +82,10 @@ Requires [Standardese](https://github.com/foonathan/standardese) for generating Requires [Catch](https://github.com/philsquared/Catch) for testing. This is bundled in the test directory. +### Acknowledgements + +Thanks to [Kévin Alexandre Boissonneault](https://github.com/KABoissonneault) for various bug fixes. + ---------- [![CC0](http://i.creativecommons.org/p/zero/1.0/88x31.png)]("http://creativecommons.org/publicdomain/zero/1.0/") From 0980210d1cd26252959cccc4e24765b7f3a5125e Mon Sep 17 00:00:00 2001 From: Simon Brand Date: Tue, 5 Dec 2017 19:59:49 +0000 Subject: [PATCH 7/8] More tests --- CMakeLists.txt | 3 +++ tests/extensions.cpp | 10 ++++++++++ 2 files changed, 13 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 06ca9de..685cc9b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -11,6 +11,9 @@ target_include_directories(Catch INTERFACE ${CATCH_INCLUDE_DIR}) set(TEST_SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/tests/main.cpp ${CMAKE_CURRENT_SOURCE_DIR}/tests/extensions.cpp ${CMAKE_CURRENT_SOURCE_DIR}/tests/assignment.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/tests/emplace.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/tests/bases.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/tests/observers.cpp ${CMAKE_CURRENT_SOURCE_DIR}/tests/constructors.cpp) add_executable(tests ${TEST_SOURCES}) diff --git a/tests/extensions.cpp b/tests/extensions.cpp index c7e1934..ecd1c92 100644 --- a/tests/extensions.cpp +++ b/tests/extensions.cpp @@ -130,6 +130,15 @@ TEST_CASE("Map extensions", "[extensions.map]") { STATIC_REQUIRE( (std::is_same>::value)); } + + + // mapping functions which return references + { + tl::expected e(42); + auto ret = e.map([](int& i) -> int& { return i; }); + REQUIRE(ret); + REQUIRE(ret == 42); + } } TEST_CASE("Map error extensions", "[extensions.map_error]") { @@ -239,6 +248,7 @@ TEST_CASE("Map error extensions", "[extensions.map_error]") { auto ret = std::move(e).map_error(ret_void); REQUIRE(!ret); } + } TEST_CASE("And then extensions", "[extensions.and_then]") { From 9f913e3b71a4644a0787e30885ff263865c01eb7 Mon Sep 17 00:00:00 2001 From: Simon Brand Date: Tue, 5 Dec 2017 20:18:38 +0000 Subject: [PATCH 8/8] Integrate PR 2 fixes --- expected.hpp | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/expected.hpp b/expected.hpp index 23936f0..5d817a5 100644 --- a/expected.hpp +++ b/expected.hpp @@ -590,13 +590,13 @@ struct expected_operations_base : expected_storage_base { }; // This class manages conditionally having a trivial copy constructor -// This specialization is for when T is trivially copy constructible -template +// This specialization is for when T and E are trivially copy constructible +template struct expected_copy_base : expected_operations_base { using expected_operations_base::expected_operations_base; }; -// This specialization is for when T is not trivially copy constructible +// This specialization is for when T or E are not trivially copy constructible template struct expected_copy_base : expected_operations_base { using expected_operations_base::expected_operations_base; @@ -623,7 +623,8 @@ struct expected_copy_base : expected_operations_base { // move constructible #ifndef TL_EXPECTED_GCC49 template ::value> + bool = std::is_trivially_move_constructible::value + && std::is_trivially_move_constructible::value> struct expected_move_base : expected_copy_base { using expected_copy_base::expected_copy_base; }; @@ -654,7 +655,10 @@ struct expected_move_base : expected_copy_base { template + IS_TRIVIALLY_DESTRUCTIBLE(T) && + IS_TRIVIALLY_COPY_ASSIGNABLE(E) && + IS_TRIVIALLY_COPY_CONSTRUCTIBLE(E) && + IS_TRIVIALLY_DESTRUCTIBLE(E)> struct expected_copy_assign_base : expected_move_base { using expected_move_base::expected_move_base; }; @@ -683,8 +687,11 @@ struct expected_copy_assign_base : expected_move_base { #ifndef TL_EXPECTED_GCC49 template ::value - &&std::is_trivially_move_constructible::value - &&std::is_trivially_move_assignable::value> + &&std::is_trivially_move_constructible::value + &&std::is_trivially_move_assignable::value + &&std::is_trivially_destructible::value + &&std::is_trivially_move_constructible::value + &&std::is_trivially_move_assignable::value> struct expected_move_assign_base : expected_copy_assign_base { using expected_copy_assign_base::expected_copy_assign_base; }; @@ -701,15 +708,17 @@ struct expected_move_assign_base expected_move_assign_base(const expected_move_assign_base &rhs) = default; expected_move_assign_base(expected_move_assign_base &&rhs) = default; + expected_move_assign_base & - operator=(const expected_move_assign_base &rhs) noexcept( + operator=(const expected_move_assign_base &rhs) = default; + + expected_move_assign_base & + operator=(expected_move_assign_base &&rhs) noexcept( std::is_nothrow_move_constructible::value &&std::is_nothrow_move_assignable::value) { - this->assign(rhs); + this->assign(std::move(rhs)); return *this; } - expected_move_assign_base & - operator=(expected_move_assign_base &&rhs) = default; }; // expected_delete_ctor_base will conditionally delete copy and move @@ -718,7 +727,7 @@ template ::value && std::is_copy_constructible::value), bool EnableMove = (std::is_move_constructible::value && - std::is_move_constructible::value)> + std::is_move_constructible::value)> struct expected_delete_ctor_base { expected_delete_ctor_base() = default; expected_delete_ctor_base(const expected_delete_ctor_base &) = default;