From 55383998b1d6440ea2653b9efd4286ab4eb11805 Mon Sep 17 00:00:00 2001 From: Jonne Ransijn Date: Mon, 25 Nov 2024 12:54:43 +0100 Subject: [PATCH] AK: Use `Noncopyable.h` in `Optional` This makes them trivially copyable/movable, silencing > "parameter is copied for each invocation" warnings on `Optional`, which are unnecessairy, since `Optional` is just a trivially copyable pointer. This creates a slight change in behaviour when moving out of an `Optional`, since the moved-from optional no longer gets cleared. Moved-from values should be considered to be in an undefined state, and if clearing a moved-from `Optional` is desired, you should be using `Optional::release_value()` instead. --- AK/Optional.h | 33 +++------------------------------ Tests/AK/TestOptional.cpp | 1 - 2 files changed, 3 insertions(+), 31 deletions(-) diff --git a/AK/Optional.h b/AK/Optional.h index 7519072a8ad..4453f51d875 100644 --- a/AK/Optional.h +++ b/AK/Optional.h @@ -336,6 +336,9 @@ private: template requires(IsLvalueReference) class [[nodiscard]] Optional { + AK_MAKE_DEFAULT_COPYABLE(Optional); + AK_MAKE_DEFAULT_MOVABLE(Optional); + template friend class Optional; @@ -369,17 +372,6 @@ public: { } - ALWAYS_INLINE Optional(Optional const& other) - : m_pointer(other.m_pointer) - { - } - - ALWAYS_INLINE Optional(Optional&& other) - : m_pointer(other.m_pointer) - { - other.m_pointer = nullptr; - } - template ALWAYS_INLINE Optional(Optional& other) requires(CanBePlacedInOptional) @@ -402,25 +394,6 @@ public: other.m_pointer = nullptr; } - ALWAYS_INLINE Optional& operator=(Optional& other) - { - m_pointer = other.m_pointer; - return *this; - } - - ALWAYS_INLINE Optional& operator=(Optional const& other) - { - m_pointer = other.m_pointer; - return *this; - } - - ALWAYS_INLINE Optional& operator=(Optional&& other) - { - m_pointer = other.m_pointer; - other.m_pointer = nullptr; - return *this; - } - template ALWAYS_INLINE Optional& operator=(Optional& other) requires(CanBePlacedInOptional) diff --git a/Tests/AK/TestOptional.cpp b/Tests/AK/TestOptional.cpp index 2c0f3f49a57..a30e9ec3893 100644 --- a/Tests/AK/TestOptional.cpp +++ b/Tests/AK/TestOptional.cpp @@ -247,7 +247,6 @@ TEST_CASE(move_optional_reference) y = move(x); EXPECT_EQ(y.has_value(), true); EXPECT_EQ(y.value(), 3); - EXPECT_EQ(x.has_value(), false); } TEST_CASE(short_notation_reference)