From b6e242b36c927df319c7123211b31e49eaec16f2 Mon Sep 17 00:00:00 2001 From: Nerijus Arlauskas Date: Fri, 8 Jul 2016 13:32:08 +0300 Subject: [PATCH] Move accessors, additional tests and docs. --- .clang-format | 7 +- src/maybe/platforms.hpp | 68 ++++++++ src/maybe/result.hpp | 288 ++++++++++++++++++++++++-------- src/maybe/result.inline.hpp | 21 +++ tests/example_test.cpp | 4 +- tests/result_and_then_tests.cpp | 2 +- tests/result_map_err_tests.cpp | 26 ++- tests/result_map_tests.cpp | 18 +- tests/result_tests.cpp | 52 ++++-- 9 files changed, 402 insertions(+), 84 deletions(-) create mode 100644 src/maybe/platforms.hpp diff --git a/.clang-format b/.clang-format index 9ed4f1d..cd6add7 100644 --- a/.clang-format +++ b/.clang-format @@ -13,6 +13,11 @@ AlignAfterOpenBracket: true AlwaysBreakAfterDefinitionReturnType: None AlwaysBreakTemplateDeclarations: true IndentCaseLabels: true -PenaltyReturnTypeOnItsOwnLine: 300 +PenaltyBreakBeforeFirstCallParameter: 1 +PenaltyBreakComment: 300 +PenaltyBreakFirstLessLess: 120 +PenaltyBreakString: 1000 +PenaltyExcessCharacter: 1000000 +PenaltyReturnTypeOnItsOwnLine: 200 NamespaceIndentation: All Standard: Cpp11 \ No newline at end of file diff --git a/src/maybe/platforms.hpp b/src/maybe/platforms.hpp new file mode 100644 index 0000000..7f1fa01 --- /dev/null +++ b/src/maybe/platforms.hpp @@ -0,0 +1,68 @@ +#pragma once + +/** + * Figure out waht features to enable based on compiler versions. + * + * This logic is re-used from std::experimental::optional. + */ + +# if defined __GNUC__ // NOTE: GNUC is also defined for Clang +# if (__GNUC__ == 4) && (__GNUC_MINOR__ >= 8) +# define MAYBE_RESULT_GCC_4_8_AND_HIGHER___ +# elif (__GNUC__ > 4) +# define MAYBE_RESULT_GCC_4_8_AND_HIGHER___ +# endif +# +# if (__GNUC__ == 4) && (__GNUC_MINOR__ >= 7) +# define MAYBE_RESULT_GCC_4_7_AND_HIGHER___ +# elif (__GNUC__ > 4) +# define MAYBE_RESULT_GCC_4_7_AND_HIGHER___ +# endif +# +# if (__GNUC__ == 4) && (__GNUC_MINOR__ == 8) && (__GNUC_PATCHLEVEL__ >= 1) +# define MAYBE_RESULT_GCC_4_8_1_AND_HIGHER___ +# elif (__GNUC__ == 4) && (__GNUC_MINOR__ >= 9) +# define MAYBE_RESULT_GCC_4_8_1_AND_HIGHER___ +# elif (__GNUC__ > 4) +# define MAYBE_RESULT_GCC_4_8_1_AND_HIGHER___ +# endif +# endif +# +# if defined __clang_major__ +# if (__clang_major__ == 3 && __clang_minor__ >= 5) +# define MAYBE_RESULT_CLANG_3_5_AND_HIGHTER_ +# elif (__clang_major__ > 3) +# define MAYBE_RESULT_CLANG_3_5_AND_HIGHTER_ +# endif +# if defined MAYBE_RESULT_CLANG_3_5_AND_HIGHTER_ +# define MAYBE_RESULT_CLANG_3_4_2_AND_HIGHER_ +# elif (__clang_major__ == 3 && __clang_minor__ == 4 && __clang_patchlevel__ >= 2) +# define MAYBE_RESULT_CLANG_3_4_2_AND_HIGHER_ +# endif +# endif + +# if defined __clang__ +# if (__clang_major__ > 2) || (__clang_major__ == 2) && (__clang_minor__ >= 9) +# define MAYBE_RESULT_HAS_THIS_RVALUE_REFS 1 +# else +# define MAYBE_RESULT_HAS_THIS_RVALUE_REFS 0 +# endif +# elif defined MAYBE_RESULT_GCC_4_8_1_AND_HIGHER___ +# define MAYBE_RESULT_HAS_THIS_RVALUE_REFS 1 +# else +# define MAYBE_RESULT_HAS_THIS_RVALUE_REFS 0 +# endif + + +# if defined MAYBE_RESULT_CLANG_3_5_AND_HIGHTER_ && (defined __cplusplus) && (__cplusplus != 201103L) +# define MAYBE_RESULT_HAS_MOVE_ACCESSORS 1 +# else +# define MAYBE_RESULT_HAS_MOVE_ACCESSORS 0 +# endif + +# // In C++11 constexpr implies const, so we need to make non-const members also non-constexpr +# if (defined __cplusplus) && (__cplusplus == 201103L) +# define MAYBE_RESULT_MUTABLE_CONSTEXPR +# else +# define MAYBE_RESULT_MUTABLE_CONSTEXPR constexpr +# endif \ No newline at end of file diff --git a/src/maybe/result.hpp b/src/maybe/result.hpp index b5f4eef..9603b70 100644 --- a/src/maybe/result.hpp +++ b/src/maybe/result.hpp @@ -10,9 +10,11 @@ #pragma once +#include #include #include +#include "platforms.hpp" #include "result.fwd.hpp" namespace maybe { @@ -35,51 +37,77 @@ namespace maybe { // workaround: std utility functions aren't constexpr yet template - inline constexpr T&& constexpr_forward(typename std::remove_reference::type& t) noexcept + inline constexpr T&& mr_constexpr_forward(typename std::remove_reference::type& t) noexcept { return static_cast(t); } template - inline constexpr T&& constexpr_forward(typename std::remove_reference::type&& t) noexcept + inline constexpr T&& mr_constexpr_forward(typename std::remove_reference::type&& t) noexcept { static_assert(!std::is_lvalue_reference::value, "!!"); return static_cast(t); } + template + inline constexpr typename std::remove_reference::type&& mr_constexpr_move(T&& t) noexcept + { + return static_cast::type&&>(t); + } + template class result final { + private: + constexpr void copy_from(const result& other) noexcept; + constexpr void set_from(result& other) noexcept; + constexpr void init_ok(T&& value) noexcept; + constexpr void init_ok(const T& value) noexcept; + constexpr void init_err(E&& value) noexcept; + constexpr void init_err(const E& value) noexcept; + constexpr void clear() noexcept; + + internal::Value tag; + union { + T ok_val; + E err_val; + }; + public: typedef T ok_type; + typedef T err_type; result() : tag(internal::Value::NONE) { } - /** ok initializer */ + // Internal initialization. + + /** Ok initializer */ result(T&& value, internal::Placeholder) : tag(internal::Value::OK) { init_ok(std::forward(value)); } - /** ok initializer */ + /** Ok initializer */ result(const T& value, internal::Placeholder) : tag(internal::Value::OK) { init_ok(value); } - /** err initializer */ + /** Err initializer */ result(internal::Placeholder, E&& value) : tag(internal::Value::ERR) { init_err(std::forward(value)); } - /** err initializer */ + /** Err initializer */ result(internal::Placeholder, const E& value) : tag(internal::Value::ERR) { init_err(value); } + // Set up and tear down. + /** copy constructor */ result(const result& other) noexcept { @@ -114,42 +142,196 @@ namespace maybe { clear(); }; + // Static construction helpers. + /** helper constructor for default ok value */ constexpr static result default_ok() noexcept { - return result(T(), internal::Placeholder{}); + return mr_constexpr_move(result(T(), internal::Placeholder{})); } /** helper constructor for ok value */ constexpr static result ok(T&& value) noexcept { - return result(std::forward(value), internal::Placeholder{}); + return mr_constexpr_move(result(std::forward(value), internal::Placeholder{})); } /** helper constructor for ok value */ constexpr static result ok(const T& value) noexcept { - return result(value, internal::Placeholder{}); + return mr_constexpr_move(result(value, internal::Placeholder{})); } /** helper constructor for default err value */ constexpr static result default_err() noexcept { - return result(internal::Placeholder{}, E()); + return mr_constexpr_move(result(internal::Placeholder{}, E())); } /** helper constructor for err value */ constexpr static result err(E&& err) noexcept { - return result(internal::Placeholder{}, std::forward(err)); + return mr_constexpr_move(result(internal::Placeholder{}, std::forward(err))); } /** helper constructor for err value */ constexpr static result err(const E& err) noexcept { - return result(internal::Placeholder{}, err); + return mr_constexpr_move(result(internal::Placeholder{}, err)); } +#if MAYBE_RESULT_HAS_MOVE_ACCESSORS == 1 + + MAYBE_RESULT_MUTABLE_CONSTEXPR T* operator->() + { + return ok_dataptr(); + } + + MAYBE_RESULT_MUTABLE_CONSTEXPR T& operator*() & + { + return *ok_dataptr(); + } + + MAYBE_RESULT_MUTABLE_CONSTEXPR T&& operator*() && + { + return mr_constexpr_move(*ok_dataptr()); + } + + constexpr T const& ok_value() const& + { + return is_ok() ? ok_val : (throw bad_result_access("bad ok result access"), ok_val); + } + + MAYBE_RESULT_MUTABLE_CONSTEXPR T& ok_value() & + { + return is_ok() ? ok_val : (throw bad_result_access("bad ok result access"), ok_val); + } + + MAYBE_RESULT_MUTABLE_CONSTEXPR T&& ok_value() && + { + if (!is_ok()) + throw bad_result_access("bad ok result access"); + return std::move(ok_val); + } + + constexpr E const& err_value() const& + { + return is_err() ? err_val : (throw bad_result_access("bad err result access"), err_val); + } + + MAYBE_RESULT_MUTABLE_CONSTEXPR E& err_value() & + { + return is_err() ? err_val : (throw bad_result_access("bad err result access"), err_val); + } + + MAYBE_RESULT_MUTABLE_CONSTEXPR E&& err_value() && + { + if (!is_err()) + throw bad_result_access("bad err result access"); + return std::move(err_val); + } + +#else + + T* operator->() + { + return ok_dataptr(); + } + + T& operator*() + { + return *ok_dataptr(); + } + + constexpr T const& ok_value() const + { + return is_ok() ? ok_val : (throw bad_result_access("bad ok result access"), ok_val); + } + + T& ok_value() + { + return is_ok() ? ok_val : (throw bad_optional_access("bad ok result access"), ok_val); + } + + constexpr E const& err_value() const + { + return is_err() ? err_val : (throw bad_result_access("bad err result access"), err_val); + } + + E& err_value() + { + return is_err() ? err_val + : (throw bad_optional_access("bad err result access"), err_val); + } + +#endif + +#if MAYBE_RESULT_HAS_THIS_RVALUE_REFS == 1 + + template + constexpr T ok_value_or(V&& v) const& + { + return is_ok() ? ok_val : static_cast(mr_constexpr_forward(v)); + } + + template + constexpr E err_value_or(V&& v) const& + { + return is_err() ? err_val : static_cast(mr_constexpr_forward(v)); + } + +#if MAYBE_RESULT_HAS_MOVE_ACCESSORS == 1 + + template + MAYBE_RESULT_MUTABLE_CONSTEXPR T ok_value_or(V&& v) && + { + return is_ok() ? mr_constexpr_move(const_cast&>(*this).ok_val) + : static_cast(mr_constexpr_forward(v)); + } + + template + MAYBE_RESULT_MUTABLE_CONSTEXPR E err_value_or(V&& v) && + { + return is_err() ? mr_constexpr_move(const_cast&>(*this).err_val) + : static_cast(mr_constexpr_forward(v)); + } + +#else + + template + T ok_value_or(V&& v) && + { + return is_ok() ? mr_constexpr_move(const_cast&>(*this).ok_val) + : static_cast(mr_constexpr_forward(v)); + } + + template + E err_value_or(V&& v) && + { + return is_err() ? mr_constexpr_move(const_cast&>(*this).err_val) + : static_cast(mr_constexpr_forward(v)); + } + +#endif + +#else + + template + constexpr T ok_value_or(V&& v) const + { + return is_ok() ? ok_val : static_cast(mr_constexpr_forward(v)); + } + + template + constexpr E err_value_or(V&& v) const + { + return is_err() ? err_val : static_cast(mr_constexpr_forward(v)); + } + +#endif + + // Inspection. + constexpr bool is_ok() const noexcept { return tag == internal::Value::OK; @@ -160,53 +342,27 @@ namespace maybe { return tag == internal::Value::ERR; } - constexpr T const& ok_value() const& - { - return is_ok() ? ok_val : (throw bad_result_access("bad ok result access"), ok_val); - } - - constexpr T& ok_value() & - { - return is_ok() ? ok_val : (throw bad_result_access("bad ok result access"), ok_val); - } - - constexpr E const& err_value() const& - { - return is_err() ? err_val : (throw bad_result_access("bad err result access"), err_val); - } - - constexpr E& err_value() & - { - return is_err() ? err_val : (throw bad_result_access("bad err result access"), err_val); - } - - template - constexpr T ok_value_or(V&& v) const& - { - return is_ok() ? ok_val : static_cast(constexpr_forward(v)); - } - - template - constexpr E err_value_or(V&& v) const& - { - return is_err() ? err_val : static_cast(constexpr_forward(v)); - } - explicit constexpr operator bool() const noexcept { return is_ok(); } + // Unsafe access. + constexpr T* ok_dataptr() const noexcept { + assert(is_ok()); return std::addressof(ok_val); } constexpr E* err_dataptr() const noexcept { + assert(is_err()); return std::addressof(err_val); } + // Functional helpers. + /** * Maps a result to result (where U is return value of F(T)) by applying a * function F to a @@ -220,6 +376,15 @@ namespace maybe { template constexpr auto map(F f) noexcept -> maybe::result::type, E>; + /** + * Maps a result to result by always returning provided U value on success, + * leaving an err value untouched. + * + * This function can be used to compose the results of two functions. + * + * @param value U + * @return maybe::result + */ template constexpr auto map_value(U value) noexcept -> maybe::result; @@ -237,6 +402,18 @@ namespace maybe { constexpr auto map_err(F f) noexcept -> maybe::result::type>; + /** + * Maps a result to result by always returning provided U value on error, + * leaving an ok value untouched. + * + * This function can be used to compose the results of two functions. + * + * @param value U + * @return maybe::result + */ + template + constexpr auto map_err_value(U value) noexcept -> maybe::result; + /** * Calls op if the result is ok, otherwise returns the err value of self. * @@ -249,28 +426,7 @@ namespace maybe { constexpr auto and_then(F op) noexcept -> typename std::result_of::type; template - constexpr auto into_err() noexcept -> R - { - if (is_err()) { - return R::err(std::forward(err_value())); - } - return R::default_ok(); - }; - - private: - constexpr void copy_from(const result& other) noexcept; - constexpr void set_from(result& other) noexcept; - constexpr void init_ok(T&& value) noexcept; - constexpr void init_ok(const T& value) noexcept; - constexpr void init_err(E&& value) noexcept; - constexpr void init_err(const E& value) noexcept; - constexpr void clear() noexcept; - - internal::Value tag; - union { - T ok_val; - E err_val; - }; + constexpr auto into_err() noexcept -> R; }; template diff --git a/src/maybe/result.inline.hpp b/src/maybe/result.inline.hpp index 7631260..45ccc88 100644 --- a/src/maybe/result.inline.hpp +++ b/src/maybe/result.inline.hpp @@ -133,6 +133,17 @@ constexpr auto maybe::result::map_err(F f) noexcept return return_result_t::err(f(std::forward(err_value()))); }; +template +template +constexpr auto maybe::result::map_err_value(U value) noexcept -> maybe::result +{ + if (is_ok()) { + return maybe::result::ok(std::forward(ok_value())); + } + + return maybe::result::err(std::forward(value)); +}; + template template constexpr auto maybe::result::and_then(F f) noexcept -> typename std::result_of::type @@ -143,4 +154,14 @@ constexpr auto maybe::result::and_then(F f) noexcept -> typename std::resu return maybe::result::err(std::forward(err_value())); } return f(std::forward(ok_value())); +}; + +template +template +constexpr auto maybe::result::into_err() noexcept -> R +{ + if (is_err()) { + return R::err(std::forward(err_value())); + } + return R::default_ok(); }; \ No newline at end of file diff --git a/tests/example_test.cpp b/tests/example_test.cpp index e6d70ff..f9fcc6e 100644 --- a/tests/example_test.cpp +++ b/tests/example_test.cpp @@ -61,11 +61,11 @@ TEST_CASE("example") auto success_result = run(true); REQUIRE(success_result); - REQUIRE(success_result.ok_value() == 4); + REQUIRE(4 == success_result.ok_value()); auto error_result = run(false); REQUIRE(!error_result); - REQUIRE(error_result.err_value() == "file not found"); + REQUIRE("file not found" == error_result.err_value()); } } \ No newline at end of file diff --git a/tests/result_and_then_tests.cpp b/tests/result_and_then_tests.cpp index f0ec392..b3a7aa6 100644 --- a/tests/result_and_then_tests.cpp +++ b/tests/result_and_then_tests.cpp @@ -36,6 +36,6 @@ TEST_CASE("result_and_then") auto a = result::err(43); auto b = a.and_then([](A v) { return result::ok(B(v.value + " world")); }); REQUIRE(!b); - REQUIRE(b.err_value() == 43); + REQUIRE(43 == b.err_value()); } } \ No newline at end of file diff --git a/tests/result_map_err_tests.cpp b/tests/result_map_err_tests.cpp index 8e3279b..79530d3 100644 --- a/tests/result_map_err_tests.cpp +++ b/tests/result_map_err_tests.cpp @@ -10,6 +10,14 @@ public: std::string value; }; +class B final { +public: + B(std::string value) : value(value) + { + } + std::string value; +}; + using maybe::result; TEST_CASE("result_map_err") @@ -19,7 +27,7 @@ TEST_CASE("result_map_err") auto a = result::err(true); auto b = a.map_err([](bool v) { return v ? 42 : 43; }); REQUIRE(!b); - REQUIRE(b.err_value() == 42); + REQUIRE(42 == b.err_value()); } SECTION("does not convert anything and returns ok if not err") @@ -29,4 +37,20 @@ TEST_CASE("result_map_err") REQUIRE(b); REQUIRE(b.ok_value().value == "hi"); } + + SECTION("replaces err A with err B") + { + auto a = result::err(A("hello")); + auto b = a.map_err_value(B("bye")); + REQUIRE(!b); + REQUIRE(b.err_value().value == "bye"); + } + + SECTION("does not replace err A with err B if A is ok") + { + auto a = result::ok(43); + auto b = a.map_err_value(B("bye")); + REQUIRE(b); + REQUIRE(43 == b.ok_value()); + } } \ No newline at end of file diff --git a/tests/result_map_tests.cpp b/tests/result_map_tests.cpp index 1d97f62..6a736fa 100644 --- a/tests/result_map_tests.cpp +++ b/tests/result_map_tests.cpp @@ -35,6 +35,22 @@ TEST_CASE("result_map") auto a = result::err(43); auto b = a.map([](A v) { return B(v.value = " world"); }); REQUIRE(!b); - REQUIRE(b.err_value() == 43); + REQUIRE(43 == b.err_value()); + } + + SECTION("replaces result A with result B") + { + auto a = result::ok(A("hello")); + auto b = a.map_value(B("bye")); + REQUIRE(b); + REQUIRE(b.ok_value().value == "bye"); + } + + SECTION("does not replace result A with result B if A is err") + { + auto a = result::err(43); + auto b = a.map_value(B("bye")); + REQUIRE(!b); + REQUIRE(43 == b.err_value()); } } \ No newline at end of file diff --git a/tests/result_tests.cpp b/tests/result_tests.cpp index 47e3be0..e5af06c 100644 --- a/tests/result_tests.cpp +++ b/tests/result_tests.cpp @@ -1,9 +1,6 @@ #include "catch.hpp" -#include #include -#include -#include using maybe::result; @@ -46,7 +43,7 @@ TEST_CASE("result") REQUIRE(res.is_ok()); REQUIRE(!res.is_err()); REQUIRE(res); - REQUIRE(res.ok_value() == 12); + REQUIRE(12 == res.ok_value()); auto other_ok = result::ok(12); REQUIRE(res == other_ok); @@ -61,7 +58,7 @@ TEST_CASE("result") REQUIRE(res.is_err()); REQUIRE(!res.is_ok()); REQUIRE(!res); - REQUIRE(res.err_value() == 12); + REQUIRE(12 == res.err_value()); auto other_err = result::err(12); REQUIRE(res == other_err); @@ -77,13 +74,13 @@ TEST_CASE("result") REQUIRE(ok != err); } - SECTION("throws exception if invalid value accessed") - { - auto ok = result::ok(12); - auto err = result::err(12); - REQUIRE_THROWS(ok.err_value()); - REQUIRE_THROWS(err.ok_value()); - } + SECTION("throws exception if invalid value accessed") + { + auto ok = result::ok(12); + auto err = result::err(12); + REQUIRE_THROWS(ok.err_value()); + REQUIRE_THROWS(err.ok_value()); + } SECTION("returns default values") { @@ -194,4 +191,35 @@ TEST_CASE("result") REQUIRE(ss.str() == "[err]"); } + +#if MAYBE_RESULT_HAS_MOVE_ACCESSORS == 1 + + SECTION("ok move accessor works") + { + std::ostringstream ss; + + { + auto val = result::ok(NoCopy(42, [&] { ss << "[ok]"; })); + auto&& other = val.ok_value(); + REQUIRE(42 == other.get_flag()); + } + + REQUIRE(ss.str() == "[ok]"); + } + + SECTION("err move accessor works") + { + std::ostringstream ss; + + { + auto val = result::err(NoCopy(42, [&] { ss << "[err]"; })); + auto&& other = val.err_value(); + REQUIRE(42 == other.get_flag()); + } + + REQUIRE(ss.str() == "[err]"); + } + +#endif + } \ No newline at end of file