diff --git a/include/xlnt/utils/optional.hpp b/include/xlnt/utils/optional.hpp
index 03415304..75fd4ac5 100644
--- a/include/xlnt/utils/optional.hpp
+++ b/include/xlnt/utils/optional.hpp
@@ -40,8 +40,8 @@ public:
///
/// Default contructor. is_set() will be false initially.
///
- optional() noexcept(std::is_nothrow_default_constructible{})
- : has_value_(false), value_(T())
+ optional() noexcept
+ : has_value_(false)
{
}
@@ -50,8 +50,89 @@ public:
/// noexcept if T copy ctor is noexcept
///
optional(const T &value) noexcept(std::is_nothrow_copy_constructible{})
- : has_value_(true), value_(value)
+ : has_value_(true)
{
+ new (&storage_) T(value);
+ }
+
+ ///
+ /// Constructs this optional with a value.
+ /// noexcept if T move ctor is noexcept
+ ///
+ optional(T &&value) noexcept(std::is_nothrow_move_constructible{})
+ : has_value_(true)
+ {
+ new (&storage_) T(std::move(value));
+ }
+
+ ///
+ /// Copy constructs this optional from other
+ /// noexcept if T copy ctor is noexcept
+ ///
+ optional(const optional& other) noexcept(std::is_nothrow_copy_constructible{})
+ : has_value_(other.has_value_)
+ {
+ if (has_value_)
+ {
+ new (&storage_) T(other.value_ref());
+ }
+ }
+
+ ///
+ /// Move constructs this optional from other. Clears the value from other if set
+ /// noexcept if T move ctor is noexcept
+ ///
+ optional(optional &&other) noexcept(std::is_nothrow_move_constructible{})
+ : has_value_(other.has_value_)
+ {
+ if (has_value_)
+ {
+ new (&storage_) T(std::move(other.value_ref()));
+ other.clear();
+ }
+ }
+
+ ///
+ /// Copy assignment of this optional from other
+ /// noexcept if set and clear are noexcept for T&
+ ///
+ optional &operator=(const optional &other) noexcept(noexcept(set(std::declval())) && noexcept(clear()))
+ {
+ if (other.has_value_)
+ {
+ set(other.value_ref());
+ }
+ else
+ {
+ clear();
+ }
+ return *this;
+ }
+
+ ///
+ /// Move assignment of this optional from other
+ /// noexcept if set and clear are noexcept for T&&
+ ///
+ optional &operator=(optional &&other) noexcept(noexcept(set(std::declval())) && noexcept(clear()))
+ {
+ if (other.has_value_)
+ {
+ set(std::move(other.value_ref()));
+ other.clear();
+ }
+ else
+ {
+ clear();
+ }
+ return *this;
+ }
+
+ ///
+ /// Destructor cleans up the T instance if set
+ ///
+ ~optional() noexcept // note:: unconditional because msvc freaks out otherwise
+ {
+ clear();
}
///
@@ -64,12 +145,69 @@ public:
}
///
- /// Sets the value to value.
+ /// Copies the value into the stored value
///
- void set(const T &value) noexcept(std::is_nothrow_assignable{})
+ void set(const T &value) noexcept(std::is_nothrow_copy_constructible{} && std::is_nothrow_assignable{})
{
- value_ = value;
- has_value_ = true;
+ if (has_value_)
+ {
+ value_ref() = value;
+ }
+ else
+ {
+ new (&storage_) T(value);
+ has_value_ = true;
+ }
+ }
+
+ ///
+ /// Moves the value into the stored value
+ ///
+ void set(T &&value) noexcept(std::is_nothrow_move_constructible{} && std::is_nothrow_move_assignable{})
+ {
+ // note seperate overload for two reasons (as opposed to perfect forwarding)
+ // 1. have to deal with implicit conversions internally with perfect forwarding
+ // 2. have to deal with the noexcept specfiers for all the different variations
+ // overload is just far and away the simpler solution
+ if (has_value_)
+ {
+ value_ref() = std::move(value);
+ }
+ else
+ {
+ new (&storage_) T(std::move(value));
+ has_value_ = true;
+ }
+ }
+
+ ///
+ /// Assignment operator overload. Equivalent to setting the value using optional::set.
+ ///
+ optional &operator=(const T &rhs) noexcept(noexcept(set(std::declval())))
+ {
+ set(rhs);
+ return *this;
+ }
+
+ ///
+ /// Assignment operator overload. Equivalent to setting the value using optional::set.
+ ///
+ optional &operator=(T &&rhs) noexcept(noexcept(set(std::declval())))
+ {
+ set(std::move(rhs));
+ return *this;
+ }
+
+ ///
+ /// After this is called, is_set() will return false until a new value is provided.
+ ///
+ void clear() noexcept(std::is_nothrow_destructible{})
+ {
+ if (has_value_)
+ {
+ reinterpret_cast(&storage_)->~T();
+ }
+ has_value_ = false;
}
///
@@ -83,7 +221,7 @@ public:
throw invalid_attribute();
}
- return value_;
+ return value_ref();
}
///
@@ -97,26 +235,7 @@ public:
throw invalid_attribute();
}
- return value_;
- }
-
- ///
- /// Resets the internal value using its default constructor. After this is
- /// called, is_set() will return false until a new value is provided.
- ///
- void clear() noexcept(std::is_nothrow_assignable{} && std::is_nothrow_default_constructible{})
- {
- value_ = T();
- has_value_ = false;
- }
-
- ///
- /// Assignment operator. Equivalent to setting the value using optional::set.
- ///
- optional &operator=(const T &rhs) noexcept(noexcept(set(std::declval())))
- {
- set(rhs);
- return *this;
+ return value_ref();
}
///
@@ -134,7 +253,7 @@ public:
{
return true;
}
- return value_ == other.value_;
+ return value_ref() == other.value_ref();
}
///
@@ -148,8 +267,19 @@ public:
}
private:
+ // helpers for getting a T out of storage
+ T & value_ref()
+ {
+ return *reinterpret_cast(&storage_);
+ }
+
+ const T &value_ref() const
+ {
+ return *reinterpret_cast(&storage_);
+ }
+
bool has_value_;
- T value_;
+ typename std::aligned_storage::type storage_;
};
} // namespace xlnt
diff --git a/tests/utils/optional_tests.cpp b/tests/utils/optional_tests.cpp
index 7af72a20..1b32af87 100644
--- a/tests/utils/optional_tests.cpp
+++ b/tests/utils/optional_tests.cpp
@@ -109,8 +109,8 @@ public:
xlnt::optional opt3(test_val);
xlnt_assert(opt3.is_set());
xlnt_assert_equals(opt3.get().val, test_val);
- //// no default ctor
- //xlnt::optional no_def_opt;
+ // no default ctor
+ xlnt::optional no_def_opt;
}
void test_copy_ctor()