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<RefPtr<T>> acts more like
   RefPtr<T> 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

@@ -147,11 +147,13 @@ class NotNull {
   NotNull() = delete;
 
   // Construct/assign from another NotNull with a compatible base pointer type.
-  template <typename U>
+  template <typename U,
+            typename = std::enable_if_t<std::is_convertible_v<const U&, T>>>
   constexpr MOZ_IMPLICIT NotNull(const NotNull<U>& aOther)
       : mBasePtr(aOther.mBasePtr) {}
 
-  template <typename U>
+  template <typename U,
+            typename = std::enable_if_t<std::is_convertible_v<U&&, T>>>
   constexpr MOZ_IMPLICIT NotNull(MovingNotNull<U>&& 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> -> T* compatibility).
+  template <typename U,
+            std::enable_if_t<!std::is_pointer_v<T> &&
+                                 std::is_convertible_v<const T&, U*>,
+                             int> = 0>
+  constexpr operator U*() const& {
+    return get();
+  }
+
+  // Don't allow implicit conversions to raw pointers from rvalue-references.
+  template <typename U,
+            std::enable_if_t<!std::is_pointer_v<T> &&
+                                 std::is_convertible_v<const T&, U*> &&
+                                 !std::is_convertible_v<const T&&, U*>,
+                             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<T*> {
   NotNull() = delete;
 
   // Construct/assign from another NotNull with a compatible base pointer type.
-  template <typename U>
+  template <typename U,
+            typename = std::enable_if_t<std::is_convertible_v<const U&, T*>>>
   constexpr MOZ_IMPLICIT NotNull(const NotNull<U>& aOther)
       : mBasePtr(aOther.get()) {
     static_assert(sizeof(T*) == sizeof(NotNull<T*>),
@@ -213,7 +234,8 @@ class NotNull<T*> {
                   "mBasePtr must have zero offset.");
   }
 
-  template <typename U>
+  template <typename U,
+            typename = std::enable_if_t<std::is_convertible_v<U&&, T*>>>
   constexpr MOZ_IMPLICIT NotNull(MovingNotNull<U>&& aOther)
       : mBasePtr(NotNull{std::move(aOther)}) {}
 
@@ -298,10 +320,12 @@ class MOZ_NON_AUTOABLE MovingNotNull {
 
   MOZ_IMPLICIT MovingNotNull(const NotNull<T>& aSrc) : mBasePtr(aSrc.get()) {}
 
-  template <typename U>
+  template <typename U,
+            typename = std::enable_if_t<std::is_convertible_v<U, T>>>
   MOZ_IMPLICIT MovingNotNull(const NotNull<U>& aSrc) : mBasePtr(aSrc.get()) {}
 
-  template <typename U>
+  template <typename U,
+            typename = std::enable_if_t<std::is_convertible_v<U, T>>>
   MOZ_IMPLICIT MovingNotNull(MovingNotNull<U>&& aSrc)
       : mBasePtr(std::move(aSrc).unwrapBasePtr()) {}
 

mfbt/RefPtr.h

@@ -24,6 +24,10 @@ class nsISupports;
 
 namespace mozilla {
 template <class T>
+class MovingNotNull;
+template <class T>
+class NotNull;
+template <class T>
 class OwningNonNull;
 template <class T>
 class StaticLocalRefPtr;
@@ -144,6 +148,22 @@ class MOZ_IS_REFPTR RefPtr {
   // construct from |Move(RefPtr<SomeSubclassOfT>)|.
   {}
 
+  template <typename I,
+            typename = std::enable_if_t<!std::is_same_v<I, RefPtr<T>> &&
+                                        std::is_convertible_v<I, RefPtr<T>>>>
+  MOZ_IMPLICIT RefPtr(const mozilla::NotNull<I>& aSmartPtr)
+      : mRawPtr(RefPtr<T>(aSmartPtr.get()).forget().take())
+  // construct from |mozilla::NotNull|.
+  {}
+
+  template <typename I,
+            typename = std::enable_if_t<!std::is_same_v<I, RefPtr<T>> &&
+                                        std::is_convertible_v<I, RefPtr<T>>>>
+  MOZ_IMPLICIT RefPtr(mozilla::MovingNotNull<I>&& aSmartPtr)
+      : mRawPtr(RefPtr<T>(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 <typename I,
+            typename = std::enable_if_t<std::is_convertible_v<I, RefPtr<T>>>>
+  RefPtr<T>& operator=(const mozilla::NotNull<I>& aSmartPtr)
+  // assign from |mozilla::NotNull|.
+  {
+    assign_assuming_AddRef(RefPtr<T>(aSmartPtr.get()).forget().take());
+    return *this;
+  }
+
+  template <typename I,
+            typename = std::enable_if_t<std::is_convertible_v<I, RefPtr<T>>>>
+  RefPtr<T>& operator=(mozilla::MovingNotNull<I>&& aSmartPtr)
+  // assign from |mozilla::MovingNotNull|.
+  {
+    assign_assuming_AddRef(
+        RefPtr<T>(std::move(aSmartPtr).unwrapBasePtr()).forget().take());
+    return *this;
+  }
+
   // Defined in OwningNonNull.h
   template <class U>
   RefPtr<T>& operator=(const mozilla::OwningNonNull<U>& aOther);

xpcom/base/nsCOMPtr.h

@@ -653,6 +653,22 @@ class MOZ_IS_REFPTR nsCOMPtr final
     NSCAP_ASSERT_NO_QUERY_NEEDED();
   }
 
+  // construct from |mozilla::NotNull|.
+  template <typename I,
+            typename = std::enable_if_t<!std::is_same_v<I, nsCOMPtr<T>> &&
+                                        std::is_convertible_v<I, nsCOMPtr<T>>>>
+  MOZ_IMPLICIT nsCOMPtr(const mozilla::NotNull<I>& aSmartPtr)
+      : NSCAP_CTOR_BASE(nsCOMPtr<T>(aSmartPtr.get()).forget().take()) {}
+
+  // construct from |mozilla::MovingNotNull|.
+  template <typename I,
+            typename = std::enable_if_t<!std::is_same_v<I, nsCOMPtr<T>> &&
+                                        std::is_convertible_v<I, nsCOMPtr<T>>>>
+  MOZ_IMPLICIT nsCOMPtr(mozilla::MovingNotNull<I>&& aSmartPtr)
+      : NSCAP_CTOR_BASE(
+            nsCOMPtr<T>(std::move(aSmartPtr).unwrapBasePtr()).forget().take()) {
+  }
+
   // Defined in OwningNonNull.h
   template <class U>
   MOZ_IMPLICIT nsCOMPtr(const mozilla::OwningNonNull<U>& aOther);
@@ -789,6 +805,23 @@ class MOZ_IS_REFPTR nsCOMPtr final
     return *this;
   }
 
+  // Assign from |mozilla::NotNull|.
+  template <typename I,
+            typename = std::enable_if_t<std::is_convertible_v<I, nsCOMPtr<T>>>>
+  nsCOMPtr<T>& operator=(const mozilla::NotNull<I>& aSmartPtr) {
+    assign_assuming_AddRef(nsCOMPtr<T>(aSmartPtr.get()).forget().take());
+    return *this;
+  }
+
+  // Assign from |mozilla::MovingNotNull|.
+  template <typename I,
+            typename = std::enable_if_t<std::is_convertible_v<I, nsCOMPtr<T>>>>
+  nsCOMPtr<T>& operator=(mozilla::MovingNotNull<I>&& aSmartPtr) {
+    assign_assuming_AddRef(
+        nsCOMPtr<T>(std::move(aSmartPtr).unwrapBasePtr()).forget().take());
+    return *this;
+  }
+
   // Defined in OwningNonNull.h
   template <class U>
   nsCOMPtr<T>& operator=(const mozilla::OwningNonNull<U>& aOther);