Make optional trivially destructible if the stored type is

1 files changed, 70 insertions, 43 deletions

diff --git a/common/aloptional.h b/common/aloptional.h
index a33c4a65..0663b7cc 100644
--- a/common/aloptional.h
+++ b/common/aloptional.h
@@ -9,27 +9,59 @@
 
 namespace al {
 
-#define REQUIRES(...) bool rt_=true, std::enable_if_t<rt_ && (__VA_ARGS__),bool> = true
-
 struct nullopt_t { };
 struct in_place_t { };
 
 constexpr nullopt_t nullopt{};
 constexpr in_place_t in_place{};
 
+
+template<typename T, bool = std::is_trivially_destructible<T>::value>
+struct optional_storage;
+
 template<typename T>
-class optional {
+struct optional_storage<T, true> {
     bool mHasValue{false};
     union {
-        char mDummy[sizeof(T)]{};
+        char mDummy;
         T mValue;
     };
 
+    optional_storage() { }
+    template<typename ...Args>
+    explicit optional_storage(in_place_t, Args&& ...args)
+        : mHasValue{true}, mValue{std::forward<Args>(args)...}
+    { }
+    ~optional_storage() = default;
+};
+
+template<typename T>
+struct optional_storage<T, false> {
+    bool mHasValue{false};
+    union {
+        char mDummy;
+        T mValue;
+    };
+
+    optional_storage() { }
+    template<typename ...Args>
+    explicit optional_storage(in_place_t, Args&& ...args)
+        : mHasValue{true}, mValue{std::forward<Args>(args)...}
+    { }
+    ~optional_storage() { if(mHasValue) al::destroy_at(std::addressof(mValue)); }
+};
+
+template<typename T>
+class optional {
+    using storage_t = optional_storage<T>;
+
+    storage_t mStore;
+
     template<typename... Args>
-    void DoConstruct(Args&& ...args)
+    void doConstruct(Args&& ...args)
     {
-        ::new (std::addressof(mValue)) T{std::forward<Args>(args)...};
-        mHasValue = true;
+        ::new(std::addressof(mStore.mValue)) T{std::forward<Args>(args)...};
+        mStore.mHasValue = true;
     }
 
 public:
@@ -37,15 +69,13 @@ public:
 
     optional() noexcept = default;
     optional(nullopt_t) noexcept { }
-    template<REQUIRES(std::is_copy_constructible<T>::value)>
-    optional(const optional &rhs) { if(rhs) DoConstruct(*rhs); }
-    template<REQUIRES(std::is_move_constructible<T>::value)>
-    optional(optional&& rhs) { if(rhs) DoConstruct(std::move(*rhs)); }
-    template<typename... Args, REQUIRES(std::is_constructible<T, Args...>::value)>
-    explicit optional(in_place_t, Args&& ...args) : mHasValue{true}
-      , mValue{std::forward<Args>(args)...}
+    optional(const optional &rhs) { if(rhs) doConstruct(*rhs); }
+    optional(optional&& rhs) { if(rhs) doConstruct(std::move(*rhs)); }
+    template<typename ...Args>
+    explicit optional(in_place_t, Args&& ...args)
+        : mStore{al::in_place, std::forward<Args>(args)...}
     { }
-    ~optional() { if(mHasValue) al::destroy_at(std::addressof(mValue)); }
+    ~optional() = default;
 
     optional& operator=(nullopt_t) noexcept { reset(); return *this; }
     std::enable_if_t<std::is_copy_constructible<T>::value && std::is_copy_assignable<T>::value,
@@ -54,9 +84,9 @@ public:
         if(!rhs)
             reset();
         else if(*this)
-            mValue = *rhs;
+            mStore.mValue = *rhs;
         else
-            DoConstruct(*rhs);
+            doConstruct(*rhs);
         return *this;
     }
     std::enable_if_t<std::is_move_constructible<T>::value && std::is_move_assignable<T>::value,
@@ -65,40 +95,39 @@ public:
         if(!rhs)
             reset();
         else if(*this)
-            mValue = std::move(*rhs);
+            mStore.mValue = std::move(*rhs);
         else
-            DoConstruct(std::move(*rhs));
+            doConstruct(std::move(*rhs));
         return *this;
     }
     template<typename U=T>
-    std::enable_if_t<std::is_constructible<T, U>::value &&
-        std::is_assignable<T&, U>::value &&
-        !std::is_same<typename std::decay<U>::type, optional<T>>::value &&
-        (!std::is_same<typename std::decay<U>::type, T>::value ||
-        !std::is_scalar<U>::value),
+    std::enable_if_t<std::is_constructible<T, U>::value
+        && std::is_assignable<T&, U>::value
+        && !std::is_same<std::decay_t<U>, optional<T>>::value
+        && (!std::is_same<std::decay_t<U>, T>::value || !std::is_scalar<U>::value),
     optional&> operator=(U&& rhs)
     {
         if(*this)
-            mValue = std::forward<U>(rhs);
+            mStore.mValue = std::forward<U>(rhs);
         else
-            DoConstruct(std::forward<U>(rhs));
+            doConstruct(std::forward<U>(rhs));
         return *this;
     }
 
-    const T* operator->() const { return std::addressof(mValue); }
-    T* operator->() { return std::addressof(mValue); }
-    const T& operator*() const& { return mValue; }
-    T& operator*() & { return mValue; }
-    const T&& operator*() const&& { return std::move(mValue); }
-    T&& operator*() && { return std::move(mValue); }
+    const T* operator->() const { return std::addressof(mStore.mValue); }
+    T* operator->() { return std::addressof(mStore.mValue); }
+    const T& operator*() const& { return this->mValue; }
+    T& operator*() & { return mStore.mValue; }
+    const T&& operator*() const&& { return std::move(mStore.mValue); }
+    T&& operator*() && { return std::move(mStore.mValue); }
 
-    operator bool() const noexcept { return mHasValue; }
-    bool has_value() const noexcept { return mHasValue; }
+    operator bool() const noexcept { return mStore.mHasValue; }
+    bool has_value() const noexcept { return mStore.mHasValue; }
 
-    T& value() & { return mValue; }
-    const T& value() const& { return mValue; }
-    T&& value() && { return std::move(mValue); }
-    const T&& value() const&& { return std::move(mValue); }
+    T& value() & { return mStore.mValue; }
+    const T& value() const& { return mStore.mValue; }
+    T&& value() && { return std::move(mStore.mValue); }
+    const T&& value() const&& { return std::move(mStore.mValue); }
 
     template<typename U>
     T value_or(U&& defval) const&
@@ -109,9 +138,9 @@ public:
 
     void reset() noexcept
     {
-        if(mHasValue)
-            al::destroy_at(std::addressof(mValue));
-        mHasValue = false;
+        if(mStore.mHasValue)
+            al::destroy_at(std::addressof(mStore.mValue));
+        mStore.mHasValue = false;
     }
 };
 
@@ -127,8 +156,6 @@ template<typename T, typename U, typename... Args>
 inline optional<T> make_optional(std::initializer_list<U> il, Args&& ...args)
 { return optional<T>{in_place, il, std::forward<Args>(args)...}; }
 
-#undef REQUIRES
-
 } // namespace al
 
 #endif /* AL_OPTIONAL_H */