From 9d303a74e3197c82fe48171e9189912c9e0e2a22 Mon Sep 17 00:00:00 2001
From: Johannes Altmanninger <aclopte@gmail.com>
Date: Wed, 26 Oct 2022 14:20:38 +0200
Subject: [PATCH] maybe_t: make maybe_t<T> trivially copyable if T is

When passing a value of type maybe_t<size_t>, clangd complains:

    Parameter 'cursor' is passed by value and only copied once; consider
    moving it to avoid unnecessary copies (fix available)

We get this warning because maybe_t<size_t> is not trivially copyable
because it has a user-defined destructor and copy-constructor.  Let's remove
them if the contained type is trivially copyable, to avoid such warnings.
No functional change.
---
 src/maybe.h | 52 ++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 36 insertions(+), 16 deletions(-)

diff --git a/src/maybe.h b/src/maybe.h
index a451787a4..29e15350a 100644
--- a/src/maybe.h
+++ b/src/maybe.h
@@ -7,10 +7,11 @@
 #include <utility>
 
 namespace maybe_detail {
-// Template magic to make maybe_t<T> copyable iff T is copyable.
-// maybe_impl_t is the "too aggressive" implementation: it is always copyable.
+// Template magic to make maybe_t<T> (trivially) copyable iff T is.
+
+// This is an unsafe implementation: it is always trivially copyable.
 template <typename T>
-struct maybe_impl_t {
+struct maybe_impl_trivially_copyable_t {
     alignas(T) char storage[sizeof(T)];
     bool filled = false;
 
@@ -38,11 +39,13 @@ struct maybe_impl_t {
         return res;
     }
 
-    maybe_impl_t() = default;
+    maybe_impl_trivially_copyable_t() = default;
 
     // Move construction/assignment from a T.
-    explicit maybe_impl_t(T &&v) : filled(true) { new (storage) T(std::forward<T>(v)); }
-    maybe_impl_t &operator=(T &&v) {
+    explicit maybe_impl_trivially_copyable_t(T &&v) : filled(true) {
+        new (storage) T(std::forward<T>(v));
+    }
+    maybe_impl_trivially_copyable_t &operator=(T &&v) {
         if (filled) {
             value() = std::move(v);
         } else {
@@ -53,8 +56,8 @@ struct maybe_impl_t {
     }
 
     // Copy construction/assignment from a T.
-    explicit maybe_impl_t(const T &v) : filled(true) { new (storage) T(v); }
-    maybe_impl_t &operator=(const T &v) {
+    explicit maybe_impl_trivially_copyable_t(const T &v) : filled(true) { new (storage) T(v); }
+    maybe_impl_trivially_copyable_t &operator=(const T &v) {
         if (filled) {
             value() = v;
         } else {
@@ -63,14 +66,26 @@ struct maybe_impl_t {
         }
         return *this;
     }
+};
 
-    // Move construction/assignment from a maybe_impl.
-    maybe_impl_t(maybe_impl_t &&v) : filled(v.filled) {
+// This is an unsafe implementation: it is always copyable.
+template <typename T>
+struct maybe_impl_not_trivially_copyable_t : public maybe_impl_trivially_copyable_t<T> {
+    using base_t = maybe_impl_trivially_copyable_t<T>;
+    using base_t::maybe_impl_trivially_copyable_t;
+    using base_t::operator=;
+    using base_t::filled;
+    using base_t::reset;
+    using base_t::storage;
+
+    // Move construction/assignment from another instance.
+    maybe_impl_not_trivially_copyable_t(maybe_impl_not_trivially_copyable_t &&v) {
+        filled = v.filled;
         if (filled) {
             new (storage) T(std::move(v.value()));
         }
     }
-    maybe_impl_t &operator=(maybe_impl_t &&v) {
+    maybe_impl_not_trivially_copyable_t &operator=(maybe_impl_not_trivially_copyable_t &&v) {
         if (!v.filled) {
             reset();
         } else {
@@ -79,13 +94,14 @@ struct maybe_impl_t {
         return *this;
     }
 
-    // Copy construction/assignment from a maybe_impl.
-    maybe_impl_t(const maybe_impl_t &v) : filled(v.filled) {
+    // Copy construction/assignment from another instance.
+    maybe_impl_not_trivially_copyable_t(const maybe_impl_not_trivially_copyable_t &v) : base_t() {
+        filled = v.filled;
         if (v.filled) {
             new (storage) T(v.value());
         }
     }
-    maybe_impl_t &operator=(const maybe_impl_t &v) {
+    maybe_impl_not_trivially_copyable_t &operator=(const maybe_impl_not_trivially_copyable_t &v) {
         if (&v == this) return *this;
         if (!v.filled) {
             reset();
@@ -95,7 +111,7 @@ struct maybe_impl_t {
         return *this;
     }
 
-    ~maybe_impl_t() { reset(); }
+    ~maybe_impl_not_trivially_copyable_t() { reset(); }
 };
 
 struct copyable_t {};
@@ -128,7 +144,11 @@ inline constexpr none_t none() { return none_t::none; }
 // This is a value-type class that stores a value of T in aligned storage.
 template <typename T>
 class maybe_t : private maybe_detail::conditionally_copyable_t<T> {
-    maybe_detail::maybe_impl_t<T> impl_;
+    using maybe_impl_t =
+        typename std::conditional<std::is_trivially_copyable<T>::value,
+                                  maybe_detail::maybe_impl_trivially_copyable_t<T>,
+                                  maybe_detail::maybe_impl_not_trivially_copyable_t<T>>::type;
+    maybe_impl_t impl_;
 
    public:
     // return whether the receiver contains a value.