From 2e85e1b37bb9bd6f432614873ed74c9a5d53d45f Mon Sep 17 00:00:00 2001 From: Lloyd Pique Date: Tue, 23 Apr 2024 18:33:17 -0700 Subject: ftl: non_null improvements * Disallow NonNull by checking for nullptr, instead of false. * Add an "operator bool()" to work like other smart pointers. * Add [[nodiscard]] where it makes sense. * Implement the full set of comparison operations, and allow comparisons between NonNull

and another type Q. This allows the use of NonNull

in sorted containers like std::set, and allows easier lookup via unwrapped pointers. * Specialize std::hash for NonNull. This allows the use of NonNull

in hashed containers like std::unordered_set. Test: atest ftl_test Bug: 185536303 Change-Id: Ib1090e393ea5f5641be2cd9c61011049039ea615 --- include/ftl/non_null.h | 112 +++++++++++++++++++++++++++++++++++++++++---- libs/ftl/non_null_test.cpp | 86 +++++++++++++++++++++++++++++++++- 2 files changed, 189 insertions(+), 9 deletions(-) diff --git a/include/ftl/non_null.h b/include/ftl/non_null.h index 35d09d71de..4a5d8bffd0 100644 --- a/include/ftl/non_null.h +++ b/include/ftl/non_null.h @@ -68,26 +68,28 @@ class NonNull final { constexpr NonNull(const NonNull&) = default; constexpr NonNull& operator=(const NonNull&) = default; - constexpr const Pointer& get() const { return pointer_; } - constexpr explicit operator const Pointer&() const { return get(); } + [[nodiscard]] constexpr const Pointer& get() const { return pointer_; } + [[nodiscard]] constexpr explicit operator const Pointer&() const { return get(); } // Move operations. These break the invariant, so care must be taken to avoid subsequent access. constexpr NonNull(NonNull&&) = default; constexpr NonNull& operator=(NonNull&&) = default; - constexpr Pointer take() && { return std::move(pointer_); } - constexpr explicit operator Pointer() && { return take(); } + [[nodiscard]] constexpr Pointer take() && { return std::move(pointer_); } + [[nodiscard]] constexpr explicit operator Pointer() && { return take(); } // Dereferencing. - constexpr decltype(auto) operator*() const { return *get(); } - constexpr decltype(auto) operator->() const { return get(); } + [[nodiscard]] constexpr decltype(auto) operator*() const { return *get(); } + [[nodiscard]] constexpr decltype(auto) operator->() const { return get(); } + + [[nodiscard]] constexpr explicit operator bool() const { return !(pointer_ == nullptr); } // Private constructor for ftl::as_non_null. Excluded from candidate constructors for conversions // through the passkey idiom, for clear compilation errors. template constexpr NonNull(Passkey, P&& pointer) : pointer_(std::forward

(pointer)) { - if (!pointer_) std::abort(); + if (pointer_ == nullptr) std::abort(); } private: @@ -98,11 +100,13 @@ class NonNull final { }; template -constexpr auto as_non_null(P&& pointer) -> NonNull> { +[[nodiscard]] constexpr auto as_non_null(P&& pointer) -> NonNull> { using Passkey = typename NonNull>::Passkey; return {Passkey{}, std::forward

(pointer)}; } +// NonNull

<=> NonNull + template constexpr bool operator==(const NonNull

& lhs, const NonNull& rhs) { return lhs.get() == rhs.get(); @@ -113,4 +117,96 @@ constexpr bool operator!=(const NonNull

& lhs, const NonNull& rhs) { return !operator==(lhs, rhs); } +template +constexpr bool operator<(const NonNull

& lhs, const NonNull& rhs) { + return lhs.get() < rhs.get(); +} + +template +constexpr bool operator<=(const NonNull

& lhs, const NonNull& rhs) { + return lhs.get() <= rhs.get(); +} + +template +constexpr bool operator>=(const NonNull

& lhs, const NonNull& rhs) { + return lhs.get() >= rhs.get(); +} + +template +constexpr bool operator>(const NonNull

& lhs, const NonNull& rhs) { + return lhs.get() > rhs.get(); +} + +// NonNull

<=> Q + +template +constexpr bool operator==(const NonNull

& lhs, const Q& rhs) { + return lhs.get() == rhs; +} + +template +constexpr bool operator!=(const NonNull

& lhs, const Q& rhs) { + return lhs.get() != rhs; +} + +template +constexpr bool operator<(const NonNull

& lhs, const Q& rhs) { + return lhs.get() < rhs; +} + +template +constexpr bool operator<=(const NonNull

& lhs, const Q& rhs) { + return lhs.get() <= rhs; +} + +template +constexpr bool operator>=(const NonNull

& lhs, const Q& rhs) { + return lhs.get() >= rhs; +} + +template +constexpr bool operator>(const NonNull

& lhs, const Q& rhs) { + return lhs.get() > rhs; +} + +// P <=> NonNull + +template +constexpr bool operator==(const P& lhs, const NonNull& rhs) { + return lhs == rhs.get(); +} + +template +constexpr bool operator!=(const P& lhs, const NonNull& rhs) { + return lhs != rhs.get(); +} + +template +constexpr bool operator<(const P& lhs, const NonNull& rhs) { + return lhs < rhs.get(); +} + +template +constexpr bool operator<=(const P& lhs, const NonNull& rhs) { + return lhs <= rhs.get(); +} + +template +constexpr bool operator>=(const P& lhs, const NonNull& rhs) { + return lhs >= rhs.get(); +} + +template +constexpr bool operator>(const P& lhs, const NonNull& rhs) { + return lhs > rhs.get(); +} + } // namespace android::ftl + +// Specialize std::hash for ftl::NonNull +template +struct std::hash> { + std::size_t operator()(const android::ftl::NonNull

& ptr) const { + return std::hash

()(ptr.get()); + } +}; diff --git a/libs/ftl/non_null_test.cpp b/libs/ftl/non_null_test.cpp index bd0462b3b6..367b398915 100644 --- a/libs/ftl/non_null_test.cpp +++ b/libs/ftl/non_null_test.cpp @@ -14,12 +14,17 @@ * limitations under the License. */ +#include #include #include #include +#include #include #include +#include +#include +#include namespace android::test { namespace { @@ -47,7 +52,7 @@ Pair dupe_if(ftl::NonNull> non_null_ptr, bool condition) { // Keep in sync with example usage in header file. TEST(NonNull, Example) { const auto string_ptr = ftl::as_non_null(std::make_shared("android")); - std::size_t size; + std::size_t size{}; get_length(string_ptr, ftl::as_non_null(&size)); EXPECT_EQ(size, 7u); @@ -71,5 +76,84 @@ constexpr StringViewPtr longest(StringViewPtr ptr1, StringViewPtr ptr2) { static_assert(longest(kApplePtr, kOrangePtr) == kOrangePtr); +static_assert(static_cast(kApplePtr)); + +static_assert(std::is_same_v())), + ftl::NonNull>); + } // namespace + +TEST(NonNull, SwapRawPtr) { + int i1 = 123; + int i2 = 456; + auto ptr1 = ftl::as_non_null(&i1); + auto ptr2 = ftl::as_non_null(&i2); + + std::swap(ptr1, ptr2); + + EXPECT_EQ(*ptr1, 456); + EXPECT_EQ(*ptr2, 123); +} + +TEST(NonNull, SwapSmartPtr) { + auto ptr1 = ftl::as_non_null(std::make_shared(123)); + auto ptr2 = ftl::as_non_null(std::make_shared(456)); + + std::swap(ptr1, ptr2); + + EXPECT_EQ(*ptr1, 456); + EXPECT_EQ(*ptr2, 123); +} + +TEST(NonNull, VectorOfRawPtr) { + int i = 1; + std::vector> vpi; + vpi.push_back(ftl::as_non_null(&i)); + EXPECT_FALSE(ftl::contains(vpi, nullptr)); + EXPECT_TRUE(ftl::contains(vpi, &i)); + EXPECT_TRUE(ftl::contains(vpi, vpi.front())); +} + +TEST(NonNull, VectorOfSmartPtr) { + std::vector>> vpi; + vpi.push_back(ftl::as_non_null(std::make_shared(2))); + EXPECT_FALSE(ftl::contains(vpi, nullptr)); + EXPECT_TRUE(ftl::contains(vpi, vpi.front().get())); + EXPECT_TRUE(ftl::contains(vpi, vpi.front())); +} + +TEST(NonNull, SetOfRawPtr) { + int i = 1; + std::set> spi; + spi.insert(ftl::as_non_null(&i)); + EXPECT_FALSE(ftl::contains(spi, nullptr)); + EXPECT_TRUE(ftl::contains(spi, &i)); + EXPECT_TRUE(ftl::contains(spi, *spi.begin())); +} + +TEST(NonNull, SetOfSmartPtr) { + std::set>> spi; + spi.insert(ftl::as_non_null(std::make_shared(2))); + EXPECT_FALSE(ftl::contains(spi, nullptr)); + EXPECT_TRUE(ftl::contains(spi, spi.begin()->get())); + EXPECT_TRUE(ftl::contains(spi, *spi.begin())); +} + +TEST(NonNull, UnorderedSetOfRawPtr) { + int i = 1; + std::unordered_set> spi; + spi.insert(ftl::as_non_null(&i)); + EXPECT_FALSE(ftl::contains(spi, nullptr)); + EXPECT_TRUE(ftl::contains(spi, &i)); + EXPECT_TRUE(ftl::contains(spi, *spi.begin())); +} + +TEST(NonNull, UnorderedSetOfSmartPtr) { + std::unordered_set>> spi; + spi.insert(ftl::as_non_null(std::make_shared(2))); + EXPECT_FALSE(ftl::contains(spi, nullptr)); + EXPECT_TRUE(ftl::contains(spi, spi.begin()->get())); + EXPECT_TRUE(ftl::contains(spi, *spi.begin())); +} + } // namespace android::test -- cgit v1.2.3-59-g8ed1b