Implementation of optional using std::aligned_storage

Issue #300
-- test for no default constructor required enabled
-- Passes all tests locally

include/xlnt/utils/optional.hpp

@@ -40,8 +40,8 @@ public:
     /// <summary>
     /// Default contructor. is_set() will be false initially.
     /// </summary>
-    optional() noexcept(std::is_nothrow_default_constructible<T>{})
-        : has_value_(false), value_(T())
+    optional() noexcept
+        : has_value_(false)
     {
     }
 
@@ -50,8 +50,89 @@ public:
     /// noexcept if T copy ctor is noexcept
     /// </summary>
     optional(const T &value) noexcept(std::is_nothrow_copy_constructible<T>{})
-        : has_value_(true), value_(value)
+        : has_value_(true)
     {
+        new (&storage_) T(value);
+    }
+
+    /// <summary>
+    /// Constructs this optional with a value.
+    /// noexcept if T move ctor is noexcept
+    /// </summary>
+    optional(T &&value) noexcept(std::is_nothrow_move_constructible<T>{})
+        : has_value_(true)
+    {
+        new (&storage_) T(std::move(value));
+    }
+
+    /// <summary>
+    /// Copy constructs this optional from other
+    /// noexcept if T copy ctor is noexcept
+    /// </summary>
+    optional(const optional& other) noexcept(std::is_nothrow_copy_constructible<T>{})
+        : has_value_(other.has_value_)
+    {
+        if (has_value_)
+        {
+            new (&storage_) T(other.value_ref());
+        }
+    }
+
+    /// <summary>
+    /// Move constructs this optional from other. Clears the value from other if set
+    /// noexcept if T move ctor is noexcept
+    /// </summary>
+    optional(optional &&other) noexcept(std::is_nothrow_move_constructible<T>{})
+        : has_value_(other.has_value_)
+    {
+        if (has_value_)
+        {
+            new (&storage_) T(std::move(other.value_ref()));
+            other.clear();
+        }
+    }
+
+    /// <summary>
+    /// Copy assignment of this optional from other
+    /// noexcept if set and clear are noexcept for T&
+    /// </summary>
+    optional &operator=(const optional &other) noexcept(noexcept(set(std::declval<const T &>())) && noexcept(clear()))
+    {
+        if (other.has_value_)
+        {
+            set(other.value_ref());
+        }
+        else
+        {
+            clear();
+        }
+        return *this;
+    }
+
+    /// <summary>
+    /// Move assignment of this optional from other
+    /// noexcept if set and clear are noexcept for T&&
+    /// </summary>
+    optional &operator=(optional &&other) noexcept(noexcept(set(std::declval<T&&>())) && noexcept(clear()))
+    {
+        if (other.has_value_)
+        {
+            set(std::move(other.value_ref()));
+            other.clear();
+        }
+        else
+        {
+            clear();
+        }
+        return *this;
+    }
+
+    /// <summary>
+    /// Destructor cleans up the T instance if set
+    /// </summary>
+    ~optional() noexcept // note:: unconditional because msvc freaks out otherwise
+    {
+        clear();
     }
 
     /// <summary>
@@ -64,12 +145,69 @@ public:
     }
 
     /// <summary>
-    /// Sets the value to value.
+    /// Copies the value into the stored value
     /// </summary>
-    void set(const T &value) noexcept(std::is_nothrow_assignable<T, T>{})
+    void set(const T &value) noexcept(std::is_nothrow_copy_constructible<T>{} && std::is_nothrow_assignable<T, T>{})
     {
-        value_ = value;
-        has_value_ = true;
+        if (has_value_)
+        {
+            value_ref() = value;
+        }
+        else
+        {
+            new (&storage_) T(value);
+            has_value_ = true;
+        }
+    }
+
+    /// <summary>
+    /// Moves the value into the stored value
+    /// </summary>
+    void set(T &&value) noexcept(std::is_nothrow_move_constructible<T>{} && std::is_nothrow_move_assignable<T>{})
+    {
+        // 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;
+        }
+    }
+
+    /// <summary>
+    /// Assignment operator overload. Equivalent to setting the value using optional::set.
+    /// </summary>
+    optional &operator=(const T &rhs) noexcept(noexcept(set(std::declval<const T &>())))
+    {
+        set(rhs);
+        return *this;
+    }
+
+    /// <summary>
+    /// Assignment operator overload. Equivalent to setting the value using optional::set.
+    /// </summary>
+    optional &operator=(T &&rhs) noexcept(noexcept(set(std::declval<T &&>())))
+    {
+        set(std::move(rhs));
+        return *this;
+    }
+
+    /// <summary>
+    /// After this is called, is_set() will return false until a new value is provided.
+    /// </summary>
+    void clear() noexcept(std::is_nothrow_destructible<T>{})
+    {
+        if (has_value_)
+        {
+            reinterpret_cast<T *>(&storage_)->~T();
+        }
+        has_value_ = false;
     }
 
     /// <summary>
@@ -83,7 +221,7 @@ public:
             throw invalid_attribute();
         }
 
-        return value_;
+        return value_ref();
     }
 
     /// <summary>
@@ -97,26 +235,7 @@ public:
             throw invalid_attribute();
         }
 
-        return value_;
-    }
-
-    /// <summary>
-    /// Resets the internal value using its default constructor. After this is
-    /// called, is_set() will return false until a new value is provided.
-    /// </summary>
-    void clear() noexcept(std::is_nothrow_assignable<T, T>{} && std::is_nothrow_default_constructible<T>{})
-    {
-        value_ = T();
-        has_value_ = false;
-    }
-
-    /// <summary>
-    /// Assignment operator. Equivalent to setting the value using optional::set.
-    /// </summary>
-    optional &operator=(const T &rhs) noexcept(noexcept(set(std::declval<T &>())))
-    {
-        set(rhs);
-        return *this;
+        return value_ref();
     }
 
     /// <summary>
@@ -134,7 +253,7 @@ public:
         {
             return true;
         }
-        return value_ == other.value_;
+        return value_ref() == other.value_ref();
     }
 
     /// <summary>
@@ -148,8 +267,19 @@ public:
     }
 
 private:
+    // helpers for getting a T out of storage
+    T & value_ref()
+    {
+        return *reinterpret_cast<T *>(&storage_);
+    }
+
+    const T &value_ref() const
+    {
+        return *reinterpret_cast<const T *>(&storage_);
+    }
+
     bool has_value_;
-    T value_;
+    typename std::aligned_storage<sizeof(T), alignof(T)>::type storage_;
 };
 
 } // namespace xlnt

tests/utils/optional_tests.cpp

@@ -109,8 +109,8 @@ public:
         xlnt::optional<convertible> opt3(test_val);
         xlnt_assert(opt3.is_set());
         xlnt_assert_equals(opt3.get().val, test_val);
-        //// no default ctor
-        //xlnt::optional<no_default> no_def_opt;
+        // no default ctor
+        xlnt::optional<no_default> no_def_opt;
     }
 
     void test_copy_ctor()