From d3ffface3e06a61e8754c821914828e75b29f942 Mon Sep 17 00:00:00 2001 From: Dean Berris Date: Fri, 25 Jan 2019 23:49:53 +1100 Subject: [PATCH 1/6] Work In Progress: A minimal std::expected This change includes a minimal implementation of the std::expected proposal in P0323R3 (http://wg21.link/P0323R3) which is hosted in the `cppnetlib` repository. In the process we also document some of our guidelines on the structure of the repository, on naming files, directory structure, and dependency requirements. Still TODO: * Fill out the test suite. * Update documentation on using vcpkg on dependencies. * Work our way up to a minimal interface for a connecton type, using our std::expected utility type. This is related to the work in netlib/#5 for implementing a minimal connection type suitable as a building block for higher level protocol implementations. --- .clang-format | 6 ++ CMakeLists.txt | 30 ++++++ netlib/README.md | 29 ++++++ netlib/expected.h | 216 ++++++++++++++++++++++++++++++++++++++++ netlib/expected_test.cc | 40 ++++++++ 5 files changed, 321 insertions(+) create mode 100644 .clang-format create mode 100644 CMakeLists.txt create mode 100644 netlib/README.md create mode 100644 netlib/expected.h create mode 100644 netlib/expected_test.cc diff --git a/.clang-format b/.clang-format new file mode 100644 index 0000000..ff18a80 --- /dev/null +++ b/.clang-format @@ -0,0 +1,6 @@ +--- +BasedOnStyle: Google +--- +Language: Cpp +DerivePointerAlignment: false +PointerAlignment: Left diff --git a/CMakeLists.txt b/CMakeLists.txt new file mode 100644 index 0000000..94ee531 --- /dev/null +++ b/CMakeLists.txt @@ -0,0 +1,30 @@ +# Copyright 2019 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +cmake_minimum_required(VERSION 3.13) + +enable_testing() +find_package(GTest MODULE REQUIRED) + +# Include directories accessible from here. +include_directories(.) + +# Require C++17 with no extensions. +set(CMAKE_CXX_STANDARD 17) +set(CMAKE_CXX_STANDARD_REQUIRED ON) +set(CMAKE_CXX_EXTENSIONS OFF) + +# Add the test to the std::expected implementation. +add_executable(expected_test netlib/expected_test.cc) +target_link_libraries(expected_test PRIVATE GTest::GTest GTest::Main) +add_test(expected_test expected_test) diff --git a/netlib/README.md b/netlib/README.md new file mode 100644 index 0000000..46f9243 --- /dev/null +++ b/netlib/README.md @@ -0,0 +1,29 @@ +# netlib Directory + +This is the main source directory for the project. The intent is to keep this +directory flat with subdirectories for logical groupings. This means all the +headers, implementation, and test code should be co-hosted in this directory. + +## Structure + +All implementation files must end with the `.cc` filename extension, and all +headers must end in `.h`. If we have a file named `connection.cc` the header +must be `connection.h` and the test(s) should be in `connection_test.cc`. + +We shall control the installed headers through our CMake configuration +instead of assuming that all headers are publicly accessible. When including +files, we should always include headers in the netlib repository by relative +inclusion with the `netlib/` directory (based off the root of the +repository). As an example: + +```c++ +// In connection.cc and connection_test.cc. +#include "netlib/connection.h" +``` + +## Subdirectories + +We can introduce subdirectories for logical grouping, each one following the +same structure rules as described here. For instance, if we have a +subdirectory of encoding/decoding, we can introduce a `coding/` subdirectory +with all the encoders and decoders implemented. \ No newline at end of file diff --git a/netlib/expected.h b/netlib/expected.h new file mode 100644 index 0000000..5634a89 --- /dev/null +++ b/netlib/expected.h @@ -0,0 +1,216 @@ +// Copyright 2019 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +#ifndef CPP_NETLIB_EXPECTED_H_ +#define CPP_NETLIB_EXPECTED_H_ + +#include +#include + +namespace cppnetlib { + +/// This is a minimal implementation of the proposed P0323R3 +/// std::unexpected<...> API. +template +// requires EqualityComparable && (CopyConstructible || +// MoveConstructible) +class unexpected { + public: + static_assert(!std::is_same_v, "unexpected is not supported."); + + unexpected() = delete; + constexpr explicit unexpected(const E& e) : value_(e) {} + constexpr explicit unexpected(E&& e) : value_(std::move(e)) {} + constexpr const E& value() const& { return value_; } + constexpr E& value() & { return value_; } + constexpr E&& value() && { return std::move(value_); } + constexpr const E&& value() const&& { return std::move(value_); } + + // This is a deviation from the proposal which suggests that these functions + // are namespace-level functions, instead of ADL-only found comparison + // operators (defined friend free functions). + template + friend constexpr bool operator==(const unexpected& lhs, + const unexpected& rhs) { + return lhs.value_ == rhs.value_; + } + + template + friend constexpr bool operator!=(const unexpected& lhs, + const unexpected& rhs) { + return lhs.value_ == rhs.value_; + } + + private: + E value_; +}; + +struct unexpect_t { + unexpect_t() = default; +}; +inline constexpr unexpect_t unexpect{}; + +template +class bad_expected_access {}; + +/// This is a minimal implementation of the proposed P0323R3 +/// std::expected<...> API. +template +class expected { + public: + using value_type = T; + using error_type = E; + using unexpected_type = unexpected; + + template + struct rebind { + using type = expected; + }; + + constexpr expected() : has_value_(false) {} + constexpr expected(const expected& other) + : union_storage_(other.union_storage_), has_value_(other.has_value_) {} + + constexpr expected( + expected&& other, + std::enable_if_t && + std::is_move_constructible_v>* = + 0) noexcept(std::is_nothrow_move_constructible_v&& + std::is_nothrow_move_constructible_v) + : has_value_(other.has_value_) { + if (other.has_value_) + new (union_storage_) + T(std::move(*reinterpret_cast(other.union_storage_))); + else + new (union_storage_) + unexpected(std::move(*reinterpret_cast(other.union_storage_))); + } + + private: + template + using constructor_helper_t = + std::enable_if_t and + std::is_constructible_v and + !std::is_constructible_v&> and + !std::is_constructible_v&&> and + !std::is_constructible_v&> and + !std::is_constructible_v&&> and + !std::is_convertible_v&, T> and + !std::is_convertible_v&&, T> and + !std::is_convertible_v&, T> and + !std::is_convertible_v&&, T>>; + + public: + // TODO: c++20 has support for conditional explicit, use that instead of the + // duplication. + template and + std::is_convertible_v), + bool> = false> + explicit constexpr expected(const expected& other, + constructor_helper_t* = 0) + : has_value_(other.has_value_) { + if (other.has_value_) + new (&union_storage_) T(*other); + else + new (&union_storage_) unexpected(other.error()); + } + + template and + std::is_convertible_v), + bool> = false> + constexpr expected(const expected& other, + constructor_helper_t* = 0) + : has_value_(other.has_value_) { + if (other.has_value_) + new (&union_storage_) T(*other); + else + new (&union_storage_) unexpected(other.error()); + } + + template and + std::is_convertible_v), + bool> = false> + explicit constexpr expected(const expected&& other, + constructor_helper_t* = 0) + : has_value_(other.has_value_) { + if (other.has_value_) + new (&union_storage_) T(std::move(*other)); + else + new (&union_storage_) unexpected(std::move(unexpected(other.error()))); + } + + template , bool> = false> + explicit constexpr expected( + U&& value, + std::enable_if_t and + !std::is_same_v, std::in_place_t> and + !std::is_same_v, std::decay_t> and + !std::is_same_v, std::decay_t>>* = 0) + : has_value_(true) { + new (&union_storage_) U(std::forward(std::move(value))); + } + + template , bool> = false> + constexpr expected( + U&& value, + std::enable_if_t and + !std::is_same_v, std::in_place_t> and + !std::is_same_v, std::decay_t> and + !std::is_same_v, std::decay_t>>* = 0) + : has_value_(true) { + new (&union_storage_) U(std::forward(value)); + } + + template , bool> = false> + explicit constexpr expected(unexpected const& e) : has_value_(false) { + new (&union_storage_) unexpected(e); + } + + template , bool> = false> + constexpr expected(unexpected const& e) : has_value_(false) { + new (&union_storage_) unexpected(e); + } + + template , bool> = false> + explicit constexpr expected(unexpected&& e) noexcept( + std::is_nothrow_move_constructible_v) + : has_value_(false) { + new (&union_storage_) unexpected(std::move(e.value())); + } + + template , bool> = false> + constexpr expected(unexpected&& e) noexcept( + std::is_nothrow_constructible_v) + : has_value_(false) { + new (&union_storage_) unexpected(std::move(e.value())); + } + + constexpr explicit operator bool() const noexcept { return has_value_; } + + private: + std::aligned_union_t<8, value_type, unexpected_type> union_storage_; + bool has_value_; +}; + +} // namespace cppnetlib + +#endif // CPP_NETLIB_EXPECTED_H_ diff --git a/netlib/expected_test.cc b/netlib/expected_test.cc new file mode 100644 index 0000000..9c45b92 --- /dev/null +++ b/netlib/expected_test.cc @@ -0,0 +1,40 @@ +// Copyright 2019 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +#include "netlib/expected.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +namespace cppnetlib { +namespace { + +using error_code = int; + +enum errors : error_code { undefined }; + +expected f(bool b) { + if (b) return true; + return unexpected(errors::undefined); +} + +TEST(ExpectedTest, Construction) { expected e; } + +TEST(ExpectedTest, Unexpected) { + auto e = f(true); + auto g = f(false); + ASSERT_FALSE(g); + ASSERT_TRUE(e); +} + +} // namespace +} // namespace cppnetlib From 4b7c4d97790ce1b1c9dd3fb6cf3b3d03e9fe3e61 Mon Sep 17 00:00:00 2001 From: Dean Berris Date: Fri, 1 Feb 2019 19:39:15 +1100 Subject: [PATCH 2/6] fixup: Add error() observer --- netlib/expected.h | 28 ++++++++++++++++++++++++++++ netlib/expected_test.cc | 26 ++++++++++++++++++++++---- 2 files changed, 50 insertions(+), 4 deletions(-) diff --git a/netlib/expected.h b/netlib/expected.h index 5634a89..7c9b68c 100644 --- a/netlib/expected.h +++ b/netlib/expected.h @@ -14,6 +14,7 @@ #ifndef CPP_NETLIB_EXPECTED_H_ #define CPP_NETLIB_EXPECTED_H_ +#include #include #include @@ -206,6 +207,33 @@ class expected { constexpr explicit operator bool() const noexcept { return has_value_; } + // Observer functions. + constexpr const E& error() const& { + assert(!has_value_ && + "expected must not have a value when taking an error!"); + return reinterpret_cast*>(&union_storage_)->value(); + } + + constexpr E& error() & { + assert(!has_value_ && + "expected must not have a value when taking an error!"); + return reinterpret_cast*>(&union_storage_)->value(); + } + + constexpr const E&& error() const&& { + assert(!has_value_ && + "expected must not have a value when taking an error!"); + return std::move(*reinterpret_cast*>(&union_storage_)) + ->value(); + } + + constexpr E&& error() && { + assert(!has_value_ && + "expected must not have a value when taking an error!"); + return std::move(*reinterpret_cast*>(&union_storage_)) + ->value(); + }; + private: std::aligned_union_t<8, value_type, unexpected_type> union_storage_; bool has_value_; diff --git a/netlib/expected_test.cc b/netlib/expected_test.cc index 9c45b92..632c1b1 100644 --- a/netlib/expected_test.cc +++ b/netlib/expected_test.cc @@ -18,6 +18,8 @@ namespace cppnetlib { namespace { +using ::testing::Eq; + using error_code = int; enum errors : error_code { undefined }; @@ -27,13 +29,29 @@ expected f(bool b) { return unexpected(errors::undefined); } +// For testing support for larger types. +struct test_data { + int64_t first; + int64_t second; +}; + +expected g(bool b, int64_t first, int64_t second) { + if (b) return unexpected{errors::undefined}; + return test_data{first, second}; +} + TEST(ExpectedTest, Construction) { expected e; } TEST(ExpectedTest, Unexpected) { - auto e = f(true); - auto g = f(false); - ASSERT_FALSE(g); - ASSERT_TRUE(e); + auto e1 = f(true); + ASSERT_TRUE(e1); + + auto e2 = f(false); + ASSERT_FALSE(e2); + + auto e3 = g(true, 1, 2); + ASSERT_FALSE(e2); + EXPECT_THAT(e3.error(), Eq(errors::undefined)); } } // namespace From bf4bd0b439e50adddcf370567c1b994453dcdda2 Mon Sep 17 00:00:00 2001 From: Dean Berris Date: Fri, 1 Feb 2019 20:02:45 +1100 Subject: [PATCH 3/6] fixup: Add operator* observers --- netlib/expected.h | 46 ++++++++++++++++++++++++++++++++++++++++- netlib/expected_test.cc | 1 + 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/netlib/expected.h b/netlib/expected.h index 7c9b68c..4b7fe07 100644 --- a/netlib/expected.h +++ b/netlib/expected.h @@ -62,7 +62,31 @@ struct unexpect_t { inline constexpr unexpect_t unexpect{}; template -class bad_expected_access {}; +class bad_expected_access; + +template <> +class bad_expected_access { + public: + virtual const char* what() const noexcept { + return "cppnetlib::bad_expected_access"; + } + virtual ~bad_expected_access() {} +}; + +template +class bad_expected_access : public bad_expected_access { + public: + explicit bad_expected_access(E e) : val_(e) {} + const char* what() const noexcept override { + return "cppnetlib::bad_expected_access"; + } + const E& error() const&; + E& error() &; + E&& error() &&; + + private: + E val_; +}; /// This is a minimal implementation of the proposed P0323R3 /// std::expected<...> API. @@ -208,6 +232,26 @@ class expected { constexpr explicit operator bool() const noexcept { return has_value_; } // Observer functions. + constexpr const T& operator*() const& { + if (!has_value_) throw bad_expected_access(error()); + return *reinterpret_cast(&union_storage_); + } + + constexpr T& operator*() & { + if (!has_value_) throw bad_expected_access(error()); + return *reinterpret_cast(&union_storage_); + } + + constexpr const T&& operator*() const&& { + if (!has_value_) throw bad_expected_access(error()); + return std::move(*reinterpret_cast(&union_storage_)); + } + + constexpr T&& operator*() && { + if (!has_value_) throw bad_expected_access(error()); + return std::move(*reinterpret_cast(&union_storage_)); + } + constexpr const E& error() const& { assert(!has_value_ && "expected must not have a value when taking an error!"); diff --git a/netlib/expected_test.cc b/netlib/expected_test.cc index 632c1b1..94728ac 100644 --- a/netlib/expected_test.cc +++ b/netlib/expected_test.cc @@ -48,6 +48,7 @@ TEST(ExpectedTest, Unexpected) { auto e2 = f(false); ASSERT_FALSE(e2); + ASSERT_THROW(*e2, bad_expected_access); auto e3 = g(true, 1, 2); ASSERT_FALSE(e2); From 411094e6f1c4e0f65be8b212bd411dab1f925b6a Mon Sep 17 00:00:00 2001 From: Dean Berris Date: Fri, 1 Feb 2019 20:48:34 +1100 Subject: [PATCH 4/6] fixup: Add operator-> and additional observers --- netlib/expected.h | 73 ++++++++++++++++++++++++++++++++++++++++- netlib/expected_test.cc | 8 ++++- 2 files changed, 79 insertions(+), 2 deletions(-) diff --git a/netlib/expected.h b/netlib/expected.h index 4b7fe07..a6bc662 100644 --- a/netlib/expected.h +++ b/netlib/expected.h @@ -229,9 +229,72 @@ class expected { new (&union_storage_) unexpected(std::move(e.value())); } - constexpr explicit operator bool() const noexcept { return has_value_; } + // Destructor. + ~expected() { + if constexpr (!std::is_trivially_destructible_v) { + if (has_value_) + reinterpret_cast(&union_storage_)->~T(); + else + reinterpret_cast(&union_storage_)->~unexpected_type(); + } + } + + // Assignment. + expected& operator=(const expected& other) { + if (has_value_ and other.has_value_) { + **this = *other; + return *this; + } + + if (!has_value_ and !other.has_value_) { + (*reinterpret_cast(&union_storage_)) = + unexpected(other.error()); + return *this; + } + + if (has_value_ and !other.has_value_) { + reinterpret_cast(&union_storage_)->~T(); + new (&union_storage_) unexpected_type(unexpected(other.error())); + has_value_ = false; + return *this; + } + + if (!has_value_ and other.has_value_) { + if constexpr (std::is_nothrow_copy_constructible_v) { + reinterpret_cast(&union_storage_)->~unexpected_type(); + new (&union_storage_) T(other.value()); + has_value_ = true; + } else if constexpr (std::is_nothrow_move_constructible_v) { + T tmp = *other; + reinterpret_cast(&union_storage_)->~unexpected_type(); + new (&union_storage_) T(tmp); + has_value_ = true; + } else if constexpr (std::is_nothrow_move_constructible_v) { + unexpected_type tmp = unexpected(std::move(error())); + reinterpret_cast(&union_storage_)->~unexpected_type(); + try { + new (&union_storage_) T(*other); + has_value_ = true; + } catch (...) { + new (&union_storage_) unexpected_type(std::move(tmp)); + } + } + } + + return *this; + } // Observer functions. + constexpr const T* operator->() const { + if (!has_value_) throw bad_expected_access(error()); + return reinterpret_cast(&union_storage_); + } + + constexpr T* operator->() { + if (!has_value_) throw bad_expected_access(error()); + return reinterpret_cast(&union_storage_); + } + constexpr const T& operator*() const& { if (!has_value_) throw bad_expected_access(error()); return *reinterpret_cast(&union_storage_); @@ -252,6 +315,14 @@ class expected { return std::move(*reinterpret_cast(&union_storage_)); } + constexpr explicit operator bool() const noexcept { return has_value_; } + constexpr bool has_value() const noexcept { return has_value_; } + + constexpr const T& value() const& { return **this; } + constexpr T& value() & { return **this; } + constexpr const T&& value() const&& { return std::move(**this); } + constexpr T&& value() && { return std::move(**this); } + constexpr const E& error() const& { assert(!has_value_ && "expected must not have a value when taking an error!"); diff --git a/netlib/expected_test.cc b/netlib/expected_test.cc index 94728ac..7efdc53 100644 --- a/netlib/expected_test.cc +++ b/netlib/expected_test.cc @@ -51,8 +51,14 @@ TEST(ExpectedTest, Unexpected) { ASSERT_THROW(*e2, bad_expected_access); auto e3 = g(true, 1, 2); - ASSERT_FALSE(e2); + ASSERT_FALSE(e3); EXPECT_THAT(e3.error(), Eq(errors::undefined)); + ASSERT_THROW(*e3, bad_expected_access); + + e3 = g(false, 2, 3); + ASSERT_TRUE(e3); + ASSERT_THAT(e3->first, Eq(2)); + ASSERT_THAT(e3->second, Eq(3)); } } // namespace From b2dd56e35eb2a91fbf93ff105ad5ba3a387012bf Mon Sep 17 00:00:00 2001 From: Dean Berris Date: Fri, 15 Feb 2019 13:57:09 +1100 Subject: [PATCH 5/6] fixup: add some assignment implementations. --- netlib/expected.h | 114 ++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 96 insertions(+), 18 deletions(-) diff --git a/netlib/expected.h b/netlib/expected.h index a6bc662..419ed18 100644 --- a/netlib/expected.h +++ b/netlib/expected.h @@ -259,31 +259,109 @@ class expected { return *this; } - if (!has_value_ and other.has_value_) { - if constexpr (std::is_nothrow_copy_constructible_v) { - reinterpret_cast(&union_storage_)->~unexpected_type(); - new (&union_storage_) T(other.value()); + assert(!has_value_ and other.has_value_); + if constexpr (std::is_nothrow_copy_constructible_v) { + reinterpret_cast(&union_storage_)->~unexpected_type(); + new (&union_storage_) T(other.value()); + has_value_ = true; + } else if constexpr (std::is_nothrow_move_constructible_v) { + T tmp = *other; + reinterpret_cast(&union_storage_)->~unexpected_type(); + new (&union_storage_) T(tmp); + has_value_ = true; + } else if constexpr (std::is_nothrow_move_constructible_v) { + unexpected_type tmp = unexpected(std::move(error())); + reinterpret_cast(&union_storage_)->~unexpected_type(); + try { + new (&union_storage_) T(*other); has_value_ = true; - } else if constexpr (std::is_nothrow_move_constructible_v) { - T tmp = *other; - reinterpret_cast(&union_storage_)->~unexpected_type(); - new (&union_storage_) T(tmp); + } catch (...) { + new (&union_storage_) unexpected_type(std::move(tmp)); + throw; + } + } + + return *this; + } + + expected& operator=(expected&& other) noexcept( + std::is_nothrow_move_assignable_vand + std::is_nothrow_move_constructible_v) { + if (has_value_ and other.has_value_) { + **this = std::move(*other); + return *this; + } + + if (!has_value_ and !other.has_value_) { + (*reinterpret_cast(&union_storage_)) = + unexpected(std::move(other.error())); + return *this; + } + + if (has_value_ and !other.has_value_) { + reinterpret_cast(&union_storage_)->~T(); + new (&union_storage_) unexpected_type( + std::move(std::forward>(other).error())); + has_value_ = false; + return *this; + } + + assert(!has_value_ and other.has_value_); + if constexpr (std::is_nothrow_move_constructible_v) { + reinterpret_cast(&union_storage_)->~T(); + new (&union_storage_) T(*std::move(other)); + has_value_ = true; + } else if constexpr (std::is_nothrow_move_constructible_v()) { + unexpected_type tmp = unexpected(std::move(error())); + reinterpret_cast(&union_storage_)->~unexpected_type(); + try { + new (&union_storage_) T(*std::move(other)); has_value_ = true; - } else if constexpr (std::is_nothrow_move_constructible_v) { - unexpected_type tmp = unexpected(std::move(error())); - reinterpret_cast(&union_storage_)->~unexpected_type(); - try { - new (&union_storage_) T(*other); - has_value_ = true; - } catch (...) { - new (&union_storage_) unexpected_type(std::move(tmp)); - } + } catch (...) { + new (&union_storage_) unexpected_type(std::move(tmp)); + throw; } } return *this; } + template and + std::is_assignable_v, + bool> = false> + expected& operator=(const unexpected& e) noexcept( + std::is_nothrow_copy_assignable_vand + std::is_nothrow_copy_constructible_v) { + if (!has_value_) { + (*reinterpret_cast(&union_storage_)) = e; + return *this; + } + + reinterpret_cast(&union_storage_)->~T(); + new (&union_storage_) + unexpected_type(unexpected(std::forward(e))); + has_value_ = false; + return *this; + } + + template and + std::is_nothrow_move_assignable_v, + bool> = false> + expected& operator=(unexpected&& e) noexcept( + std::is_nothrow_move_assignable_v) { + if (!has_value_) { + (*reinterpret_cast(&union_storage_)) = + unexpected(std::move(std::forward>(e))); + return *this; + } + + reinterpret_cast(&union_storage_)->~T(); + new (&union_storage_) + unexpected_type(unexpected(std::forward(e))); + has_value_ = false; + return *this; + } + // Observer functions. constexpr const T* operator->() const { if (!has_value_) throw bad_expected_access(error()); @@ -346,7 +424,7 @@ class expected { assert(!has_value_ && "expected must not have a value when taking an error!"); return std::move(*reinterpret_cast*>(&union_storage_)) - ->value(); + .value(); }; private: From e9ae15cc8d30262479734a3cfcffddb31d00bb93 Mon Sep 17 00:00:00 2001 From: Dean Berris Date: Fri, 15 Feb 2019 15:07:03 +1100 Subject: [PATCH 6/6] fixup: add more test cases for assignment, fix default construction --- netlib/expected.h | 83 +++++++++++++++++++++++++++++------------ netlib/expected_test.cc | 37 ++++++++++++++++++ 2 files changed, 97 insertions(+), 23 deletions(-) diff --git a/netlib/expected.h b/netlib/expected.h index 419ed18..514b3f3 100644 --- a/netlib/expected.h +++ b/netlib/expected.h @@ -90,8 +90,10 @@ class bad_expected_access : public bad_expected_access { /// This is a minimal implementation of the proposed P0323R3 /// std::expected<...> API. +// TODO: Implement a specilisation of expected. +// TODO: Implement more member functions as needed. template -class expected { +class [[nodiscard]] expected { public: using value_type = T; using error_type = E; @@ -102,16 +104,18 @@ class expected { using type = expected; }; - constexpr expected() : has_value_(false) {} + constexpr expected() : has_value_(false) { + new (&union_storage_) T(); + has_value_ = true; + } constexpr expected(const expected& other) : union_storage_(other.union_storage_), has_value_(other.has_value_) {} - constexpr expected( - expected&& other, - std::enable_if_t && - std::is_move_constructible_v>* = - 0) noexcept(std::is_nothrow_move_constructible_v&& - std::is_nothrow_move_constructible_v) + constexpr expected(expected && other, + std::enable_if_t < std::is_move_constructible_v && + std::is_move_constructible_v> * = + 0) noexcept(std::is_nothrow_move_constructible_v && + std::is_nothrow_move_constructible_v) : has_value_(other.has_value_) { if (other.has_value_) new (union_storage_) @@ -180,7 +184,7 @@ class expected { template , bool> = false> explicit constexpr expected( - U&& value, + U && value, std::enable_if_t and !std::is_same_v, std::in_place_t> and !std::is_same_v, std::decay_t> and @@ -192,7 +196,7 @@ class expected { template , bool> = false> constexpr expected( - U&& value, + U && value, std::enable_if_t and !std::is_same_v, std::in_place_t> and !std::is_same_v, std::decay_t> and @@ -215,7 +219,7 @@ class expected { template , bool> = false> - explicit constexpr expected(unexpected&& e) noexcept( + explicit constexpr expected(unexpected && e) noexcept( std::is_nothrow_move_constructible_v) : has_value_(false) { new (&union_storage_) unexpected(std::move(e.value())); @@ -223,8 +227,8 @@ class expected { template , bool> = false> - constexpr expected(unexpected&& e) noexcept( - std::is_nothrow_constructible_v) + constexpr expected(unexpected && + e) noexcept(std::is_nothrow_constructible_v) : has_value_(false) { new (&union_storage_) unexpected(std::move(e.value())); } @@ -285,8 +289,8 @@ class expected { } expected& operator=(expected&& other) noexcept( - std::is_nothrow_move_assignable_vand - std::is_nothrow_move_constructible_v) { + std::is_nothrow_move_assignable_v and + std::is_nothrow_move_constructible_v) { if (has_value_ and other.has_value_) { **this = std::move(*other); return *this; @@ -330,8 +334,8 @@ class expected { std::is_assignable_v, bool> = false> expected& operator=(const unexpected& e) noexcept( - std::is_nothrow_copy_assignable_vand - std::is_nothrow_copy_constructible_v) { + std::is_nothrow_copy_assignable_v and + std::is_nothrow_copy_constructible_v) { if (!has_value_) { (*reinterpret_cast(&union_storage_)) = e; return *this; @@ -362,6 +366,39 @@ class expected { return *this; } + template < + class U, + std::enable_if_t< + !std::is_same_v, std::decay_t> and + !std::conjunction_v, + std::is_same>> and + std::is_constructible_v and std::is_assignable_v and + std::is_nothrow_move_constructible_v, + bool> = false> + expected& operator=(U&& value) { + if (has_value_) { + this->value() = std::forward(value); + return *this; + } + + if constexpr (std::is_nothrow_constructible_v) { + reinterpret_cast(&union_storage_)->~unexpected_type(); + new (&union_storage_) T(std::forward(value)); + has_value_ = true; + } else if constexpr (std::is_nothrow_constructible_v) { + unexpected_type tmp(std::move(error())); + reinterpret_cast(&union_storage_)->~unexpected_type(); + try { + new (&union_storage_) T(std::forward(value)); + has_value_ = true; + } catch (...) { + new (&union_storage_) unexpected_type(std::move(tmp)); + throw; + } + } + return *this; + } + // Observer functions. constexpr const T* operator->() const { if (!has_value_) throw bad_expected_access(error()); @@ -378,7 +415,7 @@ class expected { return *reinterpret_cast(&union_storage_); } - constexpr T& operator*() & { + constexpr T& operator*()& { if (!has_value_) throw bad_expected_access(error()); return *reinterpret_cast(&union_storage_); } @@ -388,7 +425,7 @@ class expected { return std::move(*reinterpret_cast(&union_storage_)); } - constexpr T&& operator*() && { + constexpr T&& operator*()&& { if (!has_value_) throw bad_expected_access(error()); return std::move(*reinterpret_cast(&union_storage_)); } @@ -397,9 +434,9 @@ class expected { constexpr bool has_value() const noexcept { return has_value_; } constexpr const T& value() const& { return **this; } - constexpr T& value() & { return **this; } + constexpr T& value()& { return **this; } constexpr const T&& value() const&& { return std::move(**this); } - constexpr T&& value() && { return std::move(**this); } + constexpr T&& value()&& { return std::move(**this); } constexpr const E& error() const& { assert(!has_value_ && @@ -407,7 +444,7 @@ class expected { return reinterpret_cast*>(&union_storage_)->value(); } - constexpr E& error() & { + constexpr E& error()& { assert(!has_value_ && "expected must not have a value when taking an error!"); return reinterpret_cast*>(&union_storage_)->value(); @@ -420,7 +457,7 @@ class expected { ->value(); } - constexpr E&& error() && { + constexpr E&& error()&& { assert(!has_value_ && "expected must not have a value when taking an error!"); return std::move(*reinterpret_cast*>(&union_storage_)) diff --git a/netlib/expected_test.cc b/netlib/expected_test.cc index 7efdc53..8e0f14e 100644 --- a/netlib/expected_test.cc +++ b/netlib/expected_test.cc @@ -61,5 +61,42 @@ TEST(ExpectedTest, Unexpected) { ASSERT_THAT(e3->second, Eq(3)); } +TEST(ExpectedTest, AssignmentSimple) { + expected e; + ASSERT_NO_THROW(e = 1); + EXPECT_THAT(e.value(), Eq(1)); + ASSERT_NO_THROW(e = {}); + EXPECT_THAT(e.value(), Eq(int{})); + ASSERT_NO_THROW(e = unexpected{errors::undefined}); + ASSERT_THROW(*e, bad_expected_access); + EXPECT_THAT(e.error(), Eq(errors::undefined)); +} + +struct throwing_copy_exception : public std::exception { + const char* what() const noexcept override { + return "throwing copy exception"; + } +}; + +struct throwing_copy { + throwing_copy() = default; + throwing_copy(const throwing_copy& e) { throw throwing_copy_exception(); } + throwing_copy& operator=(const throwing_copy&) { + throw throwing_copy_exception(); + } + throwing_copy(throwing_copy&&) = default; + throwing_copy& operator=(throwing_copy&&) = default; + ~throwing_copy() = default; +}; + +TEST(ExpectedTest, AssignmentThrowingCopy) { + expected instance; + ASSERT_NO_THROW(instance = throwing_copy{}); + throwing_copy tmp; + ASSERT_NO_THROW(instance = std::move(tmp)); + ASSERT_THROW(instance = tmp, throwing_copy_exception); + ASSERT_THROW(*instance = tmp, throwing_copy_exception); +} + } // namespace } // namespace cppnetlib