From 515e2d9734b9d6f3a1c8e5b2764662b39cef97ce Mon Sep 17 00:00:00 2001 From: Daniel Bertalan Date: Fri, 2 Jul 2021 17:42:50 +0200 Subject: [PATCH] AK: Use conditionally trivial special member functions This commit makes use of the conditionally trivial special member functions introduced in C++20. Basically, `Optional` and `Variant` inherits whether its wrapped type is trivially copy constructible, trivially copy assignable or trivially destructible. This lets the compiler optimize optimize a large number of their use cases. The constraints have been applied to `Optional`'s converting constructors too in order to make the API more explicit. This feature is not supported by Clang yet, so we use conditional compilation so that Lagom can be built on macOS. Once Clang has P0848R3 support, these can be removed. --- AK/Optional.h | 71 ++++++++++++++++++++++++--------------------------- AK/Platform.h | 4 +++ AK/Variant.h | 32 +++++++++++++++++++++++ 3 files changed, 70 insertions(+), 37 deletions(-) diff --git a/AK/Optional.h b/AK/Optional.h index 7348fe9a78c..54067eb5b70 100644 --- a/AK/Optional.h +++ b/AK/Optional.h @@ -1,5 +1,6 @@ /* * Copyright (c) 2018-2021, Andreas Kling + * Copyright (c) 2021, Daniel Bertalan * * SPDX-License-Identifier: BSD-2-Clause */ @@ -20,23 +21,30 @@ public: ALWAYS_INLINE Optional() = default; - ALWAYS_INLINE Optional(const T& value) - : m_has_value(true) - { - new (&m_storage) T(value); - } +#ifdef AK_HAS_CONDITIONALLY_TRIVIAL + Optional(const Optional& other) requires(!IsCopyConstructible) = delete; + Optional(const Optional& other) = default; - template - ALWAYS_INLINE Optional(const U& value) - : m_has_value(true) - { - new (&m_storage) T(value); - } + Optional(Optional&& other) requires(!IsMoveConstructible) = delete; - ALWAYS_INLINE Optional(T&& value) - : m_has_value(true) + Optional& operator=(const Optional&) requires(!IsCopyConstructible || !IsDestructible) = delete; + Optional& operator=(const Optional&) = default; + + Optional& operator=(Optional&& other) requires(!IsMoveConstructible || !IsDestructible) = delete; + + ~Optional() requires(!IsDestructible) = delete; + ~Optional() = default; +#endif + + ALWAYS_INLINE Optional(const Optional& other) +#ifdef AK_HAS_CONDITIONALLY_TRIVIAL + requires(!IsTriviallyCopyConstructible) +#endif + : m_has_value(other.m_has_value) { - new (&m_storage) T(move(value)); + if (other.has_value()) { + new (&m_storage) T(other.value()); + } } ALWAYS_INLINE Optional(Optional&& other) @@ -44,39 +52,25 @@ public: { if (other.has_value()) { new (&m_storage) T(other.release_value()); - other.m_has_value = false; } } - ALWAYS_INLINE Optional(const Optional& other) - : m_has_value(other.m_has_value) - { - if (m_has_value) { - new (&m_storage) T(other.value()); - } - } - - ALWAYS_INLINE Optional(Optional& other) - : m_has_value(other.m_has_value) - { - if (m_has_value) { - new (&m_storage) T(other.value()); - } - } - - template - ALWAYS_INLINE Optional(U& value) + template + ALWAYS_INLINE explicit(!IsConvertible) Optional(U&& value) requires(!IsSame, Optional> && IsConstructible) : m_has_value(true) { - new (&m_storage) T(value); + new (&m_storage) T(forward(value)); } ALWAYS_INLINE Optional& operator=(const Optional& other) +#ifdef AK_HAS_CONDITIONALLY_TRIVIAL + requires(!IsTriviallyCopyConstructible || !IsTriviallyDestructible) +#endif { if (this != &other) { clear(); m_has_value = other.m_has_value; - if (m_has_value) { + if (other.has_value()) { new (&m_storage) T(other.value()); } } @@ -88,8 +82,9 @@ public: if (this != &other) { clear(); m_has_value = other.m_has_value; - if (other.has_value()) + if (other.has_value()) { new (&m_storage) T(other.release_value()); + } } return *this; } @@ -107,6 +102,9 @@ public: } ALWAYS_INLINE ~Optional() +#ifdef AK_HAS_CONDITIONALLY_TRIVIAL + requires(!IsTriviallyDestructible) +#endif { clear(); } @@ -167,7 +165,6 @@ private: alignas(T) u8 m_storage[sizeof(T)]; bool m_has_value { false }; }; - } using AK::Optional; diff --git a/AK/Platform.h b/AK/Platform.h index 56aa4a1c5b3..699b4dffbe4 100644 --- a/AK/Platform.h +++ b/AK/Platform.h @@ -25,6 +25,10 @@ #define ARCH(arch) (defined(AK_ARCH_##arch) && AK_ARCH_##arch) +#if !defined(__clang__) +# define AK_HAS_CONDITIONALLY_TRIVIAL +#endif + #ifdef ALWAYS_INLINE # undef ALWAYS_INLINE #endif diff --git a/AK/Variant.h b/AK/Variant.h index 9263439ec8c..afe116d9895 100644 --- a/AK/Variant.h +++ b/AK/Variant.h @@ -210,7 +210,27 @@ public: template friend struct Variant; +#ifdef AK_HAS_CONDITIONALLY_TRIVIAL + Variant(const Variant&) requires(!(IsCopyConstructible && ...)) = delete; + Variant(const Variant&) = default; + + Variant(Variant&&) requires(!(IsMoveConstructible && ...)) = delete; + Variant(Variant&&) = default; + + ~Variant() requires(!(IsDestructible && ...)) = delete; + ~Variant() = default; + + Variant& operator=(const Variant&) requires(!(IsCopyConstructible && ...) || !(IsDestructible && ...)) = delete; + Variant& operator=(const Variant&) = default; + + Variant& operator=(Variant&&) requires(!(IsMoveConstructible && ...) || !(IsDestructible && ...)) = delete; + Variant& operator=(Variant&&) = default; +#endif + ALWAYS_INLINE Variant(const Variant& old) +#ifdef AK_HAS_CONDITIONALLY_TRIVIAL + requires(!(IsTriviallyCopyConstructible && ...)) +#endif : Detail::MergeAndDeduplicatePacks>...>() , m_data {} , m_index(old.m_index) @@ -223,6 +243,9 @@ public: // and if a variant with a nontrivial move ctor is moved from, it may or may not be valid // but it will still contain the "moved-from" state of the object it previously contained. ALWAYS_INLINE Variant(Variant&& old) +#ifdef AK_HAS_CONDITIONALLY_TRIVIAL + requires(!(IsTriviallyMoveConstructible && ...)) +#endif : Detail::MergeAndDeduplicatePacks>...>() , m_data {} , m_index(old.m_index) @@ -231,11 +254,17 @@ public: } ALWAYS_INLINE ~Variant() +#ifdef AK_HAS_CONDITIONALLY_TRIVIAL + requires(!(IsTriviallyDestructible && ...)) +#endif { Helper::delete_(m_index, m_data); } ALWAYS_INLINE Variant& operator=(const Variant& other) +#ifdef AK_HAS_CONDITIONALLY_TRIVIAL + requires(!(IsTriviallyCopyConstructible && ...) || !(IsTriviallyDestructible && ...)) +#endif { m_index = other.m_index; Helper::copy_(other.m_index, other.m_data, m_data); @@ -243,6 +272,9 @@ public: } ALWAYS_INLINE Variant& operator=(Variant&& other) +#ifdef AK_HAS_CONDITIONALLY_TRIVIAL + requires(!(IsTriviallyMoveConstructible && ...) || !(IsTriviallyDestructible && ...)) +#endif { m_index = other.m_index; Helper::move_(other.m_index, other.m_data, m_data);