From 10582b65288c03fc3d19c504ebb3d1f348c155a2 Mon Sep 17 00:00:00 2001 From: Nika Layzell Date: Mon, 20 Mar 2023 15:40:35 +0000 Subject: [PATCH] Bug 1607634 - Part 1: Improve the ergonomics of using NotNull with RefPtr and nsCOMPtr, r=glandium This does a few minor improvements: 1. Adds implicit conversions from NotNull to a raw pointer type if supported by the underlying type, to make it so NotNull> acts more like RefPtr in some situations. 2. Adds explicit conversion constructors and assignment operators for RefPtr and nsCOMPtr from NotNull, avoiding conversion ambiguity added by the first change. 3. Disable conversion constructors on NotNull with SFINAE if they should not be available, meaning that type traits like std::is_convertible_v interact with it properly. Differential Revision: https://phabricator.services.mozilla.com/D168883 --- mfbt/NotNull.h | 36 ++++++++++++++++++++++++++++++------ mfbt/RefPtr.h | 39 +++++++++++++++++++++++++++++++++++++++ xpcom/base/nsCOMPtr.h | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 102 insertions(+), 6 deletions(-) diff --git a/mfbt/NotNull.h b/mfbt/NotNull.h index b434bb64ae1b..1a12400e14df 100644 --- a/mfbt/NotNull.h +++ b/mfbt/NotNull.h @@ -147,11 +147,13 @@ class NotNull { NotNull() = delete; // Construct/assign from another NotNull with a compatible base pointer type. - template + template >> constexpr MOZ_IMPLICIT NotNull(const NotNull& aOther) : mBasePtr(aOther.mBasePtr) {} - template + template >> constexpr MOZ_IMPLICIT NotNull(MovingNotNull&& aOther) : mBasePtr(std::move(aOther).unwrapBasePtr()) {} @@ -165,6 +167,24 @@ class NotNull { // Implicit conversion to a base pointer. Preferable to get(). constexpr operator const T&() const { return get(); } + // Implicit conversion to a raw pointer from const lvalue-reference if + // supported by the base pointer (for RefPtr -> T* compatibility). + template && + std::is_convertible_v, + int> = 0> + constexpr operator U*() const& { + return get(); + } + + // Don't allow implicit conversions to raw pointers from rvalue-references. + template && + std::is_convertible_v && + !std::is_convertible_v, + int> = 0> + constexpr operator U*() const&& = delete; + // Dereference operators. constexpr auto* operator->() const MOZ_NONNULL_RETURN { return mBasePtr.mPtr.operator->(); @@ -204,7 +224,8 @@ class NotNull { NotNull() = delete; // Construct/assign from another NotNull with a compatible base pointer type. - template + template >> constexpr MOZ_IMPLICIT NotNull(const NotNull& aOther) : mBasePtr(aOther.get()) { static_assert(sizeof(T*) == sizeof(NotNull), @@ -213,7 +234,8 @@ class NotNull { "mBasePtr must have zero offset."); } - template + template >> constexpr MOZ_IMPLICIT NotNull(MovingNotNull&& aOther) : mBasePtr(NotNull{std::move(aOther)}) {} @@ -298,10 +320,12 @@ class MOZ_NON_AUTOABLE MovingNotNull { MOZ_IMPLICIT MovingNotNull(const NotNull& aSrc) : mBasePtr(aSrc.get()) {} - template + template >> MOZ_IMPLICIT MovingNotNull(const NotNull& aSrc) : mBasePtr(aSrc.get()) {} - template + template >> MOZ_IMPLICIT MovingNotNull(MovingNotNull&& aSrc) : mBasePtr(std::move(aSrc).unwrapBasePtr()) {} diff --git a/mfbt/RefPtr.h b/mfbt/RefPtr.h index b251b4b8f6fa..b36491f49b63 100644 --- a/mfbt/RefPtr.h +++ b/mfbt/RefPtr.h @@ -24,6 +24,10 @@ class nsISupports; namespace mozilla { template +class MovingNotNull; +template +class NotNull; +template class OwningNonNull; template class StaticLocalRefPtr; @@ -144,6 +148,22 @@ class MOZ_IS_REFPTR RefPtr { // construct from |Move(RefPtr)|. {} + template > && + std::is_convertible_v>>> + MOZ_IMPLICIT RefPtr(const mozilla::NotNull& aSmartPtr) + : mRawPtr(RefPtr(aSmartPtr.get()).forget().take()) + // construct from |mozilla::NotNull|. + {} + + template > && + std::is_convertible_v>>> + MOZ_IMPLICIT RefPtr(mozilla::MovingNotNull&& aSmartPtr) + : mRawPtr(RefPtr(std::move(aSmartPtr).unwrapBasePtr()).forget().take()) + // construct from |mozilla::MovingNotNull|. + {} + MOZ_IMPLICIT RefPtr(const nsQueryReferent& aHelper); MOZ_IMPLICIT RefPtr(const nsCOMPtr_helper& aHelper); #if defined(XP_WIN) @@ -220,6 +240,25 @@ class MOZ_IS_REFPTR RefPtr { return *this; } + template >>> + RefPtr& operator=(const mozilla::NotNull& aSmartPtr) + // assign from |mozilla::NotNull|. + { + assign_assuming_AddRef(RefPtr(aSmartPtr.get()).forget().take()); + return *this; + } + + template >>> + RefPtr& operator=(mozilla::MovingNotNull&& aSmartPtr) + // assign from |mozilla::MovingNotNull|. + { + assign_assuming_AddRef( + RefPtr(std::move(aSmartPtr).unwrapBasePtr()).forget().take()); + return *this; + } + // Defined in OwningNonNull.h template RefPtr& operator=(const mozilla::OwningNonNull& aOther); diff --git a/xpcom/base/nsCOMPtr.h b/xpcom/base/nsCOMPtr.h index 2acd04bbd82b..d5a1875d86b1 100644 --- a/xpcom/base/nsCOMPtr.h +++ b/xpcom/base/nsCOMPtr.h @@ -653,6 +653,22 @@ class MOZ_IS_REFPTR nsCOMPtr final NSCAP_ASSERT_NO_QUERY_NEEDED(); } + // construct from |mozilla::NotNull|. + template > && + std::is_convertible_v>>> + MOZ_IMPLICIT nsCOMPtr(const mozilla::NotNull& aSmartPtr) + : NSCAP_CTOR_BASE(nsCOMPtr(aSmartPtr.get()).forget().take()) {} + + // construct from |mozilla::MovingNotNull|. + template > && + std::is_convertible_v>>> + MOZ_IMPLICIT nsCOMPtr(mozilla::MovingNotNull&& aSmartPtr) + : NSCAP_CTOR_BASE( + nsCOMPtr(std::move(aSmartPtr).unwrapBasePtr()).forget().take()) { + } + // Defined in OwningNonNull.h template MOZ_IMPLICIT nsCOMPtr(const mozilla::OwningNonNull& aOther); @@ -789,6 +805,23 @@ class MOZ_IS_REFPTR nsCOMPtr final return *this; } + // Assign from |mozilla::NotNull|. + template >>> + nsCOMPtr& operator=(const mozilla::NotNull& aSmartPtr) { + assign_assuming_AddRef(nsCOMPtr(aSmartPtr.get()).forget().take()); + return *this; + } + + // Assign from |mozilla::MovingNotNull|. + template >>> + nsCOMPtr& operator=(mozilla::MovingNotNull&& aSmartPtr) { + assign_assuming_AddRef( + nsCOMPtr(std::move(aSmartPtr).unwrapBasePtr()).forget().take()); + return *this; + } + // Defined in OwningNonNull.h template nsCOMPtr& operator=(const mozilla::OwningNonNull& aOther);