diff --git a/libc/src/__support/CPP/optional.h b/libc/src/__support/CPP/optional.h index 02e8395b871ba..2c0f639a2b342 100644 --- a/libc/src/__support/CPP/optional.h +++ b/libc/src/__support/CPP/optional.h @@ -25,7 +25,7 @@ struct nullopt_t { LIBC_INLINE_VAR constexpr nullopt_t nullopt{}; // This is very simple implementation of the std::optional class. It makes -// several assumptions that the underlying type is trivially constructable, +// several assumptions that the underlying type is trivially constructible, // copyable, or movable. template class optional { template ::value> @@ -35,46 +35,63 @@ template class optional { U stored_value; }; - LIBC_INLINE ~OptionalStorage() { stored_value.~U(); } + bool in_use = false; + + LIBC_INLINE ~OptionalStorage() { reset(); } + LIBC_INLINE constexpr OptionalStorage() : empty() {} template LIBC_INLINE constexpr explicit OptionalStorage(in_place_t, Args &&...args) : stored_value(forward(args)...) {} + + LIBC_INLINE constexpr void reset() { + if (in_use) + stored_value.~U(); + in_use = false; + } }; + // The only difference is that this type U doesn't have a nontrivial + // destructor. template struct OptionalStorage { union { char empty; U stored_value; }; - // The only difference is that this class doesn't have a destructor. + bool in_use = false; + LIBC_INLINE constexpr OptionalStorage() : empty() {} template LIBC_INLINE constexpr explicit OptionalStorage(in_place_t, Args &&...args) : stored_value(forward(args)...) {} + + LIBC_INLINE constexpr void reset() { in_use = false; } }; OptionalStorage storage; - bool in_use = false; public: LIBC_INLINE constexpr optional() = default; LIBC_INLINE constexpr optional(nullopt_t) {} - LIBC_INLINE constexpr optional(const T &t) - : storage(in_place, t), in_use(true) {} + LIBC_INLINE constexpr optional(const T &t) : storage(in_place, t) { + storage.in_use = true; + } LIBC_INLINE constexpr optional(const optional &) = default; - LIBC_INLINE constexpr optional(T &&t) - : storage(in_place, move(t)), in_use(true) {} + LIBC_INLINE constexpr optional(T &&t) : storage(in_place, move(t)) { + storage.in_use = true; + } LIBC_INLINE constexpr optional(optional &&O) = default; template LIBC_INLINE constexpr optional(in_place_t, ArgTypes &&...Args) - : storage(in_place, forward(Args)...), in_use(true) {} + : storage(in_place, forward(Args)...) { + storage.in_use = true; + } LIBC_INLINE constexpr optional &operator=(T &&t) { storage = move(t); @@ -88,11 +105,7 @@ template class optional { } LIBC_INLINE constexpr optional &operator=(const optional &) = default; - LIBC_INLINE constexpr void reset() { - if (in_use) - storage.~OptionalStorage(); - in_use = false; - } + LIBC_INLINE constexpr void reset() { storage.reset(); } LIBC_INLINE constexpr const T &value() const & { return storage.stored_value; @@ -100,8 +113,10 @@ template class optional { LIBC_INLINE constexpr T &value() & { return storage.stored_value; } - LIBC_INLINE constexpr explicit operator bool() const { return in_use; } - LIBC_INLINE constexpr bool has_value() const { return in_use; } + LIBC_INLINE constexpr explicit operator bool() const { + return storage.in_use; + } + LIBC_INLINE constexpr bool has_value() const { return storage.in_use; } LIBC_INLINE constexpr const T *operator->() const { return &storage.stored_value; } diff --git a/libc/test/src/__support/CPP/optional_test.cpp b/libc/test/src/__support/CPP/optional_test.cpp index f9254d42c9a9e..b2c8545eb36f0 100644 --- a/libc/test/src/__support/CPP/optional_test.cpp +++ b/libc/test/src/__support/CPP/optional_test.cpp @@ -42,14 +42,18 @@ TEST(LlvmLibcOptionalTest, Tests) { // For this test case, the destructor increments the pointed-to value. int holding = 1; - optional Complicated(&holding); - // Destructor was run once as part of copying the object. - ASSERT_EQ(holding, 2); - // Destructor was run a second time as part of destruction. - Complicated.reset(); - ASSERT_EQ(holding, 3); - // Destructor was not run a third time as the object is already destroyed. - Complicated.reset(); + { + optional Complicated(&holding); + // Destructor was run once as part of copying the object. + ASSERT_EQ(holding, 2); + // Destructor was run a second time as part of destruction. + Complicated.reset(); + ASSERT_EQ(holding, 3); + // Destructor was not run a third time as the object is already destroyed. + Complicated.reset(); + ASSERT_EQ(holding, 3); + } + // Make sure the destructor isn't called when the optional is destroyed. ASSERT_EQ(holding, 3); // Test that assigning an optional to another works when set