fix: preserve optional value lifetime

.github/workflows/ci-linux.yml

@@ -387,6 +387,13 @@ jobs:
     - uses: ./.github/actions/init-ut-make-config
       with:
         options: --with-bthread-tracer --with-asan
+    - name: Test C++14 optional with sanitizers
+      run: |
+        clang++-12 -std=c++14 -DBRPC_TEST_OPTIONAL_CXX14 \
+          -fsanitize=address,undefined -fno-omit-frame-pointer -I src \
+          test/optional_unittest.cpp -lgtest_main -lgtest -pthread \
+          -o "$RUNNER_TEMP/optional-cxx14-unittest"
+        "$RUNNER_TEMP/optional-cxx14-unittest"
     - name: compile tests
       run: |
         cat config.mk

src/butil/containers/optional.h

@@ -119,10 +119,136 @@ struct is_constructible_convertible_assignable_from_optional
         is_convertible_from_optional<T, U>::value ||
         is_assignable_from_optional<T, U>::value> {};
 
+template <bool enabled>
+struct OptionalCopyConstructible {};
+
+template <>
+struct OptionalCopyConstructible<false> {
+    OptionalCopyConstructible() = default;
+    OptionalCopyConstructible(const OptionalCopyConstructible&) = delete;
+    OptionalCopyConstructible(OptionalCopyConstructible&&) = default;
+    OptionalCopyConstructible& operator=(const OptionalCopyConstructible&) = default;
+    OptionalCopyConstructible& operator=(OptionalCopyConstructible&&) = default;
+};
+
+template <bool enabled>
+struct OptionalMoveConstructible {};
+
+template <>
+struct OptionalMoveConstructible<false> {
+    OptionalMoveConstructible() = default;
+    OptionalMoveConstructible(const OptionalMoveConstructible&) = default;
+    OptionalMoveConstructible(OptionalMoveConstructible&&) = delete;
+    OptionalMoveConstructible& operator=(const OptionalMoveConstructible&) = default;
+    OptionalMoveConstructible& operator=(OptionalMoveConstructible&&) = default;
+};
+
+template <bool enabled>
+struct OptionalCopyAssignable {};
+
+template <>
+struct OptionalCopyAssignable<false> {
+    OptionalCopyAssignable() = default;
+    OptionalCopyAssignable(const OptionalCopyAssignable&) = default;
+    OptionalCopyAssignable(OptionalCopyAssignable&&) = default;
+    OptionalCopyAssignable& operator=(const OptionalCopyAssignable&) = delete;
+    OptionalCopyAssignable& operator=(OptionalCopyAssignable&&) = default;
+};
+
+template <bool enabled>
+struct OptionalMoveAssignable {};
+
+template <>
+struct OptionalMoveAssignable<false> {
+    OptionalMoveAssignable() = default;
+    OptionalMoveAssignable(const OptionalMoveAssignable&) = default;
+    OptionalMoveAssignable(OptionalMoveAssignable&&) = default;
+    OptionalMoveAssignable& operator=(const OptionalMoveAssignable&) = default;
+    OptionalMoveAssignable& operator=(OptionalMoveAssignable&&) = delete;
+};
+
+template <typename T>
+class OptionalBase {
+public:
+    OptionalBase() : _engaged(false) {}
+
+    OptionalBase(const OptionalBase& rhs) : _engaged(false) {
+        if (rhs._engaged) {
+            _storage.Init(*rhs._storage);
+            _engaged = true;
+        }
+    }
+
+    OptionalBase(OptionalBase&& rhs) noexcept(
+        std::is_nothrow_move_constructible<T>::value) : _engaged(false) {
+        if (rhs._engaged) {
+            _storage.Init(std::move(*rhs._storage));
+            _engaged = true;
+        }
+    }
+
+    ~OptionalBase() { reset(); }
+
+    OptionalBase& operator=(const OptionalBase& rhs) {
+        if (this != &rhs) {
+            if (rhs._engaged) {
+                if (_engaged) {
+                    *_storage = *rhs._storage;
+                } else {
+                    _storage.Init(*rhs._storage);
+                    _engaged = true;
+                }
+            } else {
+                reset();
+            }
+        }
+        return *this;
+    }
+
+    OptionalBase& operator=(OptionalBase&& rhs) noexcept(
+        std::is_nothrow_move_constructible<T>::value &&
+        std::is_nothrow_move_assignable<T>::value) {
+        if (this != &rhs) {
+            if (rhs._engaged) {
+                if (_engaged) {
+                    *_storage = std::move(*rhs._storage);
+                } else {
+                    _storage.Init(std::move(*rhs._storage));
+                    _engaged = true;
+                }
+            } else {
+                reset();
+            }
+        }
+        return *this;
+    }
+
+protected:
+    void reset() {
+        if (_engaged) {
+            _storage.Destroy();
+            _engaged = false;
+        }
+    }
+
+    bool _engaged;
+    ManualConstructor<T> _storage;
+};
+
 } // namespace internal
 
 template<typename T>
-class optional {
+class optional : private internal::OptionalBase<T>,
+                 private internal::OptionalCopyConstructible<
+                     std::is_copy_constructible<T>::value>,
+                 private internal::OptionalMoveConstructible<
+                     std::is_move_constructible<T>::value>,
+                 private internal::OptionalCopyAssignable<
+                     std::is_copy_constructible<T>::value &&
+                     std::is_copy_assignable<T>::value>,
+                 private internal::OptionalMoveAssignable<
+                     std::is_move_constructible<T>::value &&
+                     std::is_move_assignable<T>::value> {
 public:
     typedef T value_type;
 
@@ -137,22 +263,24 @@ class optional {
     static_assert(!is_array<value_type>::value,
                   "Instantiation of optional with an array type is ill-formed");
 
-    optional() : _engaged(false) {}
+    optional() = default;
 
     optional(nullopt_t) : optional() {}
 
     optional(const optional& rhs) = default;
 
-    optional(optional&& rhs) noexcept = default;
+    optional(optional&& rhs) noexcept(
+        std::is_nothrow_move_constructible<T>::value) = default;
 
     template <typename U, std::enable_if_t<
         !std::is_same<T, U>::value &&
         std::is_constructible<T, const U&>::value &&
         !internal::is_constructible_convertible_from_optional<T, U>::value &&
         std::is_convertible<const U&, T>::value, bool> = false>
-    optional(const optional<U>& rhs) : _engaged(rhs.has_value()) {
-        if (_engaged) {
+    optional(const optional<U>& rhs) {
+        if (rhs) {
             _storage.Init(*rhs);
+            _engaged = true;
         }
     }
 
@@ -161,9 +289,10 @@ class optional {
         std::is_constructible<T, const U&>::value &&
         !internal::is_constructible_convertible_from_optional<T, U>::value &&
         !std::is_convertible<const U&, T>::value, bool> = false>
-    explicit optional(const optional<U>& rhs) : _engaged(rhs.has_value()) {
-        if (_engaged) {
+    explicit optional(const optional<U>& rhs) {
+        if (rhs) {
             _storage.Init(*rhs);
+            _engaged = true;
         }
     }
 
@@ -172,10 +301,10 @@ class optional {
         std::is_constructible<T, U&&>::value &&
         !internal::is_constructible_convertible_from_optional<T, U>::value &&
         std::is_convertible<U&&, T>::value, bool> = false>
-    optional(optional<U>&& rhs) : _engaged(rhs.has_value()) {
-        if (_engaged) {
+    optional(optional<U>&& rhs) {
+        if (rhs) {
             _storage.Init(std::move(*rhs));
-            rhs.reset();
+            _engaged = true;
         }
     }
 
@@ -184,56 +313,59 @@ class optional {
         std::is_constructible<T, U&&>::value &&
         !internal::is_constructible_convertible_from_optional<T, U>::value &&
         !std::is_convertible<U&&, T>::value, bool> = false>
-    explicit optional(optional<U>&& rhs) : _engaged(rhs.has_value()) {
-        if (_engaged) {
+    explicit optional(optional<U>&& rhs) {
+        if (rhs) {
             _storage.Init(std::move(*rhs));
-            rhs.reset();
+            _engaged = true;
         }
     }
 
-    optional(const T& value) : _engaged(true) {
+    optional(const T& value) {
         _storage.Init(value);
+        _engaged = true;
     }
 
-    optional(T&& value) : _engaged(true) {
+    optional(T&& value) {
         _storage.Init(std::move(value));
+        _engaged = true;
     }
 
     template <typename... Args, std::enable_if_t<
         std::is_constructible<T, Args&&...>::value, bool> = false>
-    explicit optional(in_place_t, Args&&... args) : _engaged(true) {
+    explicit optional(in_place_t, Args&&... args) {
         _storage.Init(std::forward<Args>(args)...);
+        _engaged = true;
     }
 
     template <typename U, typename... Args, std::enable_if_t<
         std::is_constructible<T, std::initializer_list<U>&, Args&&...>::value,
         bool> = false>
-    optional(in_place_t, std::initializer_list<U> il, Args&&... args)
-        : _engaged(true) {
+    optional(in_place_t, std::initializer_list<U> il, Args&&... args) {
         _storage.Init(il, std::forward<Args>(args)...);
+        _engaged = true;
     }
 
     template <typename U = T, std::enable_if_t<
         !std::is_same<in_place_t, typename std::decay<U>::type>::value &&
         !std::is_same<optional<T>, typename std::decay<U>::type>::value &&
         std::is_constructible<T, U&&>::value &&
         std::is_convertible<U&&, T>::value, bool> = false>
-    optional(U&& v) : _engaged(true) {
+    optional(U&& v) {
         _storage.Init(std::forward<U>(v));
+        _engaged = true;
     }
 
     template <typename U = T, std::enable_if_t<
         !std::is_same<in_place_t, typename std::decay<U>::type>::value &&
         !std::is_same<optional<T>, typename std::decay<U>::type>::value &&
         std::is_constructible<T, U&&>::value &&
         !std::is_convertible<U&&, T>::value, bool> = false>
-    explicit optional(U&& v) : _engaged(true) {
+    explicit optional(U&& v) {
         _storage.Init(std::forward<U>(v));
+        _engaged = true;
     }
 
-    ~optional() {
-        reset();
-    }
+    ~optional() = default;
 
     optional& operator=(nullopt_t) noexcept {
         reset();
@@ -242,7 +374,9 @@ class optional {
 
     optional& operator=(const optional& rhs) = default;
 
-    optional& operator=(optional&& rhs) = default;
+    optional& operator=(optional&& rhs) noexcept(
+        std::is_nothrow_move_constructible<T>::value &&
+        std::is_nothrow_move_assignable<T>::value) = default;
 
     // Value assignment operators
     template <typename U = T, typename = std::enable_if_t<
@@ -362,7 +496,9 @@ class optional {
             std::move(**this) : static_cast<T>(std::forward<U>(v));
     }
 
-    void swap(optional& rhs) noexcept {
+    void swap(optional& rhs) noexcept(
+        std::is_nothrow_move_constructible<T>::value &&
+        std::is_nothrow_move_assignable<T>::value) {
         if (_engaged && rhs._engaged) {
             std::swap(**this, *rhs);
         } else if (_engaged) {
@@ -402,12 +538,14 @@ class optional {
     }
 
 private:
-    bool _engaged;
-    ManualConstructor<T> _storage;
+    using internal::OptionalBase<T>::_engaged;
+    using internal::OptionalBase<T>::_storage;
 };
 
 template <typename T>
-void swap(optional<T>& a, optional<T>& b) noexcept { a.swap(b); }
+void swap(optional<T>& a, optional<T>& b) noexcept(noexcept(a.swap(b))) {
+    a.swap(b);
+}
 
 // Creates a non-empty 'optional<T>`.
 template <typename T>