Fix bugs in in ctl::optional (#1203)

Manually manage the lifetime of `value_` by using an anonymous
`union`. This fixes a bunch of double-frees and double-constructs.

Additionally move the `present_` flag last. When `T` has padding
`present_` will be placed there saving `alignof(T)` bytes from
`sizeof(optional<T>)`.
This commit is contained in:
Alkis Evlogimenos 2024-06-08 02:47:24 +02:00 committed by GitHub
parent 2ba6b0158f
commit d44a7dc603
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 32 additions and 12 deletions

View file

@ -14,38 +14,45 @@ class optional
public: public:
using value_type = T; using value_type = T;
~optional() = default; ~optional()
{
if (present_)
value_.~T();
}
optional() noexcept : present_(false) optional() noexcept : present_(false)
{ {
} }
optional(const T& value) : present_(true), value_(value) optional(const T& value) : present_(true)
{ {
new (&value_) T(value);
} }
optional(T&& value) : present_(true), value_(std::move(value)) optional(T&& value) : present_(true)
{ {
new (&value_) T(std::move(value));
} }
optional(const optional& other) : present_(other.present_) optional(const optional& other) : present_(other.present_)
{ {
if (present_) if (other.present_)
new (&value_) T(other.value_); new (&value_) T(other.value_);
} }
optional(optional&& other) noexcept : present_(other.present_) optional(optional&& other) noexcept : present_(other.present_)
{ {
if (present_) if (other.present_)
value_ = std::move(other.value_); new (&value_) T(std::move(other.value_));
} }
optional& operator=(const optional& other) optional& operator=(const optional& other)
{ {
if (this != &other) { if (this != &other) {
reset();
if (other.present_)
new (&value_) T(other.value_);
present_ = other.present_; present_ = other.present_;
if (present_)
value_ = other.value_;
} }
return *this; return *this;
} }
@ -53,9 +60,10 @@ class optional
optional& operator=(optional&& other) noexcept optional& operator=(optional&& other) noexcept
{ {
if (this != &other) { if (this != &other) {
reset();
if (other.present_)
new (&value_) T(std::move(other.value_));
present_ = other.present_; present_ = other.present_;
if (present_)
value_ = std::move(other.value_);
} }
return *this; return *this;
} }
@ -102,8 +110,9 @@ class optional
template<typename... Args> template<typename... Args>
void emplace(Args&&... args) void emplace(Args&&... args)
{ {
reset();
present_ = true; present_ = true;
value_ = T(std::forward<Args>(args)...); new (&value_) T(std::forward<Args>(args)...);
} }
void swap(optional& other) noexcept void swap(optional& other) noexcept
@ -120,8 +129,10 @@ class optional
} }
private: private:
union {
T value_;
};
bool present_; bool present_;
T value_;
}; };
} // namespace ctl } // namespace ctl

View file

@ -26,6 +26,8 @@
// #include <string> // #include <string>
// #define ctl std // #define ctl std
static int g = 0;
int int
main() main()
{ {
@ -114,6 +116,13 @@ main()
return 24; return 24;
} }
{
struct A { int* p = &g; A() {++*p; } };
ctl::optional<A> x;
if (g != 0)
return 25;
}
CheckForMemoryLeaks(); CheckForMemoryLeaks();
return 0; return 0;
} }