From f2336d0144fbbf6d72bd1eeaa34667dc4eb76760 Mon Sep 17 00:00:00 2001 From: Lenny Maiorani Date: Thu, 15 Dec 2022 20:51:55 -0700 Subject: [PATCH] AK+Everywhere: Move custom deleter capability to OwnPtr `OwnPtrWithCustomDeleter` was a decorator which provided the ability to add a custom deleter to `OwnPtr` by wrapping and taking the deleter as a run-time argument to the constructor. This solution means that no additional space is needed for the `OwnPtr` because it doesn't need to store a pointer to the deleter, but comes at the cost of having an extra type that stores a pointer for every instance. This logic is moved directly into `OwnPtr` by adding a template argument that is defaulted to the default deleter for the type. This means that the type itself stores the pointer to the deleter instead of every instance and adds some type safety by encoding the deleter in the type itself instead of taking a run-time argument. --- AK/DefaultDelete.h | 35 +++++++++++++++++++++++ AK/Forward.h | 3 +- AK/NonnullRefPtr.h | 2 -- AK/OwnPtr.h | 5 ++-- AK/OwnPtrWithCustomDeleter.h | 41 --------------------------- AK/RefPtr.h | 4 +-- Kernel/Library/LockRefPtr.h | 3 -- Kernel/Library/NonnullLockRefPtr.h | 2 -- Tests/AK/CMakeLists.txt | 1 + Tests/AK/TestOwnPtr.cpp | 23 +++++++++++++++ Userland/Libraries/LibCore/System.cpp | 6 +--- Userland/Libraries/LibCore/System.h | 12 +++++--- 12 files changed, 74 insertions(+), 63 deletions(-) create mode 100644 AK/DefaultDelete.h delete mode 100644 AK/OwnPtrWithCustomDeleter.h create mode 100644 Tests/AK/TestOwnPtr.cpp diff --git a/AK/DefaultDelete.h b/AK/DefaultDelete.h new file mode 100644 index 0000000000..be13a1018c --- /dev/null +++ b/AK/DefaultDelete.h @@ -0,0 +1,35 @@ +/* + * Copyright (c) 2022, the SerenityOS developers. + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#pragma once + +namespace AK { + +template +struct DefaultDelete { + constexpr DefaultDelete() = default; + + constexpr void operator()(T* t) + { + delete t; + } +}; + +template +struct DefaultDelete { + constexpr DefaultDelete() = default; + + constexpr void operator()(T* t) + { + delete[] t; + } +}; + +} + +#ifdef USING_AK_GLOBALLY +using AK::DefaultDelete; +#endif diff --git a/AK/Forward.h b/AK/Forward.h index 86b288a318..c92787928b 100644 --- a/AK/Forward.h +++ b/AK/Forward.h @@ -6,6 +6,7 @@ #pragma once +#include #include namespace AK { @@ -133,7 +134,7 @@ class LockRefPtr; template class RefPtr; -template +template> class OwnPtr; template diff --git a/AK/NonnullRefPtr.h b/AK/NonnullRefPtr.h index 2a1a0d5e06..7f3a054dd1 100644 --- a/AK/NonnullRefPtr.h +++ b/AK/NonnullRefPtr.h @@ -16,8 +16,6 @@ namespace AK { -template -class OwnPtr; template class RefPtr; diff --git a/AK/OwnPtr.h b/AK/OwnPtr.h index ece4c34391..c5452b8661 100644 --- a/AK/OwnPtr.h +++ b/AK/OwnPtr.h @@ -7,6 +7,7 @@ #pragma once #include +#include #include #include @@ -14,7 +15,7 @@ namespace AK { -template +template class [[nodiscard]] OwnPtr { public: OwnPtr() = default; @@ -105,7 +106,7 @@ public: void clear() { - delete m_ptr; + TDeleter {}(m_ptr); m_ptr = nullptr; } diff --git a/AK/OwnPtrWithCustomDeleter.h b/AK/OwnPtrWithCustomDeleter.h deleted file mode 100644 index f9968810ed..0000000000 --- a/AK/OwnPtrWithCustomDeleter.h +++ /dev/null @@ -1,41 +0,0 @@ -/* - * Copyright (c) 2022, Lucas Chollet - * - * SPDX-License-Identifier: BSD-2-Clause - */ - -#pragma once - -#include -#include -#include - -template -struct OwnPtrWithCustomDeleter { - AK_MAKE_NONCOPYABLE(OwnPtrWithCustomDeleter); - -public: - OwnPtrWithCustomDeleter(T* ptr, Function deleter) - : m_ptr(ptr) - , m_deleter(move(deleter)) - { - } - - OwnPtrWithCustomDeleter(OwnPtrWithCustomDeleter&& other) - { - swap(m_ptr, other.m_ptr); - swap(m_deleter, other.m_deleter); - }; - - ~OwnPtrWithCustomDeleter() - { - if (m_ptr) { - VERIFY(m_deleter); - m_deleter(m_ptr); - } - } - -private: - T* m_ptr { nullptr }; - Function m_deleter {}; -}; diff --git a/AK/RefPtr.h b/AK/RefPtr.h index 7619017347..9586e9c11e 100644 --- a/AK/RefPtr.h +++ b/AK/RefPtr.h @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -19,9 +20,6 @@ namespace AK { -template -class OwnPtr; - template class [[nodiscard]] RefPtr { template diff --git a/Kernel/Library/LockRefPtr.h b/Kernel/Library/LockRefPtr.h index 4c7aeb70f3..a124ef3d31 100644 --- a/Kernel/Library/LockRefPtr.h +++ b/Kernel/Library/LockRefPtr.h @@ -23,9 +23,6 @@ namespace AK { -template -class OwnPtr; - template struct LockRefPtrTraits { ALWAYS_INLINE static T* as_ptr(FlatPtr bits) diff --git a/Kernel/Library/NonnullLockRefPtr.h b/Kernel/Library/NonnullLockRefPtr.h index 97140bb619..6b6039a9a9 100644 --- a/Kernel/Library/NonnullLockRefPtr.h +++ b/Kernel/Library/NonnullLockRefPtr.h @@ -21,8 +21,6 @@ namespace AK { -template -class OwnPtr; template class LockRefPtr; diff --git a/Tests/AK/CMakeLists.txt b/Tests/AK/CMakeLists.txt index 7bf3e85110..27751cbaa4 100644 --- a/Tests/AK/CMakeLists.txt +++ b/Tests/AK/CMakeLists.txt @@ -52,6 +52,7 @@ set(AK_TEST_SOURCES TestNonnullRefPtr.cpp TestNumberFormat.cpp TestOptional.cpp + TestOwnPtr.cpp TestPrint.cpp TestQueue.cpp TestQuickSort.cpp diff --git a/Tests/AK/TestOwnPtr.cpp b/Tests/AK/TestOwnPtr.cpp new file mode 100644 index 0000000000..27ee97b5f1 --- /dev/null +++ b/Tests/AK/TestOwnPtr.cpp @@ -0,0 +1,23 @@ +/* + * Copyright (c) 2022, the SerenityOS developers. + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#include + +#include + +static u64 deleter_call_count = 0; + +TEST_CASE(should_call_custom_deleter) +{ + auto deleter = [](auto* p) { if (p) ++deleter_call_count; }; + auto ptr = OwnPtr {}; + ptr.clear(); + EXPECT_EQ(0u, deleter_call_count); + ptr = adopt_own_if_nonnull(&deleter_call_count); + EXPECT_EQ(0u, deleter_call_count); + ptr.clear(); + EXPECT_EQ(1u, deleter_call_count); +} diff --git a/Userland/Libraries/LibCore/System.cpp b/Userland/Libraries/LibCore/System.cpp index 4845435f42..d059353d02 100644 --- a/Userland/Libraries/LibCore/System.cpp +++ b/Userland/Libraries/LibCore/System.cpp @@ -1334,11 +1334,7 @@ ErrorOr getaddrinfo(char const* nodename, char const* servnam for (auto* result = results; result != nullptr; result = result->ai_next) TRY(addresses.try_append(*result)); - return AddressInfoVector { move(addresses), results, - [](struct addrinfo* ptr) { - if (ptr) - ::freeaddrinfo(ptr); - } }; + return AddressInfoVector { move(addresses), results }; } ErrorOr getsockopt(int sockfd, int level, int option, void* value, socklen_t* value_size) diff --git a/Userland/Libraries/LibCore/System.h b/Userland/Libraries/LibCore/System.h index 542f8d23c6..8520d278fc 100644 --- a/Userland/Libraries/LibCore/System.h +++ b/Userland/Libraries/LibCore/System.h @@ -10,7 +10,7 @@ #include #include -#include +#include #include #include #include @@ -226,14 +226,18 @@ public: private: friend ErrorOr getaddrinfo(char const* nodename, char const* servname, struct addrinfo const& hints); - AddressInfoVector(Vector&& addresses, struct addrinfo* ptr, AK::Function deleter) + AddressInfoVector(Vector&& addresses, struct addrinfo* ptr) : m_addresses(move(addresses)) - , m_ptr(ptr, move(deleter)) + , m_ptr(adopt_own_if_nonnull(ptr)) { } + struct AddrInfoDeleter { + void operator()(struct addrinfo* ptr) { ::freeaddrinfo(ptr); } + }; + Vector m_addresses {}; - OwnPtrWithCustomDeleter m_ptr; + OwnPtr m_ptr {}; }; ErrorOr getaddrinfo(char const* nodename, char const* servname, struct addrinfo const& hints);