diff --git a/include/etl/optional.h b/include/etl/optional.h index ec998479..c696a66d 100644 --- a/include/etl/optional.h +++ b/include/etl/optional.h @@ -169,21 +169,19 @@ namespace etl } //*************************************************************************** - /// Constructor from value type. + /// Converting constructor from value type. + /// Constructs T in-place from U&&, without requiring T to be + /// copy/move constructible. //*************************************************************************** + template ::value && + !etl::is_same::type, etl::in_place_t>::value && + !etl::is_same::type, optional_impl>::value, int>::type = 0> ETL_CONSTEXPR20_STL - optional_impl(const T& value_) + optional_impl(U&& value_) { - storage.construct(value_); - } - - //*************************************************************************** - /// Constructor from value type. - //*************************************************************************** - ETL_CONSTEXPR20_STL - optional_impl(T&& value_) - { - storage.construct(etl::move(value_)); + storage.construct(etl::forward(value_)); } //*************************************************************************** @@ -280,23 +278,24 @@ namespace etl //*************************************************************************** /// Assignment operator from value type. //*************************************************************************** +#if ETL_USING_CPP11 + template ::value && + !etl::is_same::type, optional_impl>::value, int>::type = 0> + ETL_CONSTEXPR20_STL + optional_impl& operator =(U&& value_) + { + storage.construct(etl::forward(value_)); + + return *this; + } +#else ETL_CONSTEXPR20_STL optional_impl& operator =(const T& value_) { storage.construct(value_); - return *this; - } - -#if ETL_USING_CPP11 - //*************************************************************************** - /// Assignment operator from value type. - //*************************************************************************** - ETL_CONSTEXPR20_STL - optional_impl& operator =(T&& value_) - { - storage.construct(etl::move(value_)); - return *this; } #endif @@ -1442,22 +1441,36 @@ namespace etl #if ETL_USING_CPP11 //*************************************************************************** - /// Construct from value type. + /// Converting constructor from value type. + /// Constructs T in-place from U&&, without requiring T to be + /// copy/move constructible. //*************************************************************************** - template + template ::value && + !etl::is_same::type, etl::optional>::value && + !etl::is_same::type, etl::in_place_t>::value && + !etl::is_same::type, etl::nullopt_t>::value && + etl::is_pod::type>::value, int>::type = 0> ETL_CONSTEXPR14 - optional(const T& value_) - : impl_t(value_) + optional(U&& value_) + : impl_t(etl::forward(value_)) { } //*************************************************************************** - /// Construct from value type. + /// Converting constructor from value type. //*************************************************************************** - template + template ::value && + !etl::is_same::type, etl::optional>::value && + !etl::is_same::type, etl::in_place_t>::value && + !etl::is_same::type, etl::nullopt_t>::value && + !etl::is_pod::type>::value, int>::type = 0> ETL_CONSTEXPR20_STL - optional(const T& value_) - : impl_t(value_) + optional(U&& value_) + : impl_t(etl::forward(value_)) { } #else @@ -1470,29 +1483,6 @@ namespace etl } #endif - -#if ETL_USING_CPP11 - //*************************************************************************** - /// Move construct from value type. - //*************************************************************************** - template - ETL_CONSTEXPR14 - optional(T&& value_) - : impl_t(etl::move(value_)) - { - } - - //*************************************************************************** - /// Move construct from value type. - //*************************************************************************** - template - ETL_CONSTEXPR20_STL - optional(T&& value_) - : impl_t(etl::move(value_)) - { - } -#endif - #if ETL_USING_CPP11 //*************************************************************************** /// Emplace construct from arguments. @@ -1641,25 +1631,35 @@ namespace etl #if ETL_USING_CPP11 //*************************************************************************** - /// Assignment operator from value type. + /// Converting assignment operator from value type. //*************************************************************************** - template + template ::value && + !etl::is_same::type, etl::optional>::value && + !etl::is_same::type, etl::nullopt_t>::value && + etl::is_pod::type>::value, int>::type = 0> ETL_CONSTEXPR14 - optional& operator =(const T& value_) + optional& operator =(U&& value_) { - impl_t::operator=(value_); + impl_t::operator=(etl::forward(value_)); return *this; } //*************************************************************************** - /// Assignment operator from value type. + /// Converting assignment operator from value type. //*************************************************************************** - template + template ::value && + !etl::is_same::type, etl::optional>::value && + !etl::is_same::type, etl::nullopt_t>::value && + !etl::is_pod::type>::value, int>::type = 0> ETL_CONSTEXPR20_STL - optional& operator =(const T& value_) + optional& operator =(U&& value_) { - impl_t::operator=(value_); + impl_t::operator=(etl::forward(value_)); return *this; } @@ -1675,32 +1675,6 @@ namespace etl } #endif -#if ETL_USING_CPP11 - //*************************************************************************** - /// Move assignment operator from value type. - //*************************************************************************** - template - ETL_CONSTEXPR14 - optional& operator =(T&& value_) - { - impl_t::operator=(etl::move(value_)); - - return *this; - } - - //*************************************************************************** - /// Move assignment operator from value type. - //*************************************************************************** - template - ETL_CONSTEXPR20_STL - optional& operator =(T&& value_) - { - impl_t::operator=(etl::move(value_)); - - return *this; - } -#endif - //*************************************************************************** /// Returns an iterator to the beginning of the optional. //*************************************************************************** diff --git a/test/test_optional.cpp b/test/test_optional.cpp index 6ca762a9..8968efd7 100644 --- a/test/test_optional.cpp +++ b/test/test_optional.cpp @@ -1118,5 +1118,46 @@ namespace CHECK_EQUAL(42, *opt); } + + //************************************************************************* + // GitHub issue #146: etl::optional doesn't compile with deleted copy constructor + //************************************************************************* +#if ETL_USING_CPP11 + struct Issue146_NonCopyable + { + Issue146_NonCopyable(int some) : _some(some) {} + Issue146_NonCopyable(const Issue146_NonCopyable&) = delete; + Issue146_NonCopyable(Issue146_NonCopyable&&) = delete; + Issue146_NonCopyable& operator=(const Issue146_NonCopyable&) = delete; + + int _some; + }; + + struct Issue146_Container + { + Issue146_Container(int a_val) : a(a_val) {} + Issue146_Container() : a(etl::nullopt) {} + + etl::optional a; + }; + + TEST(test_optional_issue_146_deleted_copy_ctor) + { + // etl::optional should compile when T has deleted copy/move constructors, + // as long as T is constructible from the given arguments. + Issue146_Container with_value(42); + Issue146_Container without_value; + + CHECK_TRUE(with_value.a.has_value()); + CHECK_EQUAL(42, with_value.a->_some); + + CHECK_FALSE(without_value.a.has_value()); + + // in_place construction should also work + etl::optional opt(etl::in_place_t{}, 99); + CHECK_TRUE(opt.has_value()); + CHECK_EQUAL(99, opt->_some); + } +#endif } }