Bug 1410252 - MakeNotNull<PointerType, OptionalPointeeType>(Args...) - r=njn

MakeNotNull is similar to UniquePtr, in that it combines the infallible
allocation and construction of an object on the heap and wraps the (raw or
smart) pointer into a NotNull.
It skips the unnecessary null check from WrapNotNull, and removes the usual
naked 'new' used in many WrapNotNull calls.

MozReview-Commit-ID: UwCrhDnkUg

--HG--
extra : rebase_source : 5a027165fc17ed748783c7ffda03eb421865ad6e

mfbt/NotNull.h

@@ -63,6 +63,7 @@
 // for the last one, where the handle type is |void|. See below.
 
 #include "mozilla/Assertions.h"
+#include "mozilla/Move.h"
 #include <stddef.h>
 
 namespace mozilla {
@@ -85,8 +86,8 @@ namespace mozilla {
 // - It does not auto-convert from a base pointer. Implicit conversion from a
 //   less-constrained type (e.g. T*) to a more-constrained type (e.g.
 //   NotNull<T*>) is dangerous. Creation and assignment from a base pointer can
-//   only be done with WrapNotNull(), which makes them impossible to overlook,
-//   both when writing and reading code.
+//   only be done with WrapNotNull() or MakeNotNull<>(), which makes them
+//   impossible to overlook, both when writing and reading code.
 //
 // - When initialized (or assigned) it is checked, and if it is null we abort.
 //   This guarantees that it cannot be null.
@@ -102,10 +103,12 @@ template <typename T>
 class NotNull
 {
   template <typename U> friend NotNull<U> WrapNotNull(U aBasePtr);
+  template<typename U, typename... Args>
+  friend NotNull<U> MakeNotNull(Args&&... aArgs);
 
   T mBasePtr;
 
-  // This constructor is only used by WrapNotNull().
+  // This constructor is only used by WrapNotNull() and MakeNotNull<U>().
   template <typename U>
   explicit NotNull(U aBasePtr) : mBasePtr(aBasePtr) {}
 
@@ -152,6 +155,22 @@ WrapNotNull(const T aBasePtr)
   return notNull;
 }
 
+// Allocate an object with infallible new, and wrap its pointer in NotNull.
+// |MakeNotNull<Ptr<Ob>>(args...)| will run |new Ob(args...)|
+// and return NotNull<Ptr<Ob>>.
+template<typename T, typename... Args>
+NotNull<T>
+MakeNotNull(Args&&... aArgs)
+{
+  // Extract the pointee type from what T's dereferencing operator returns
+  // (which could be a reference to a const type).
+  using Pointee = typename mozilla::RemoveConst<
+    typename mozilla::RemoveReference<decltype(*DeclVal<T>())>::Type>::Type;
+  static_assert(!IsArray<Pointee>::value,
+                "MakeNotNull cannot construct an array");
+  return NotNull<T>(new Pointee(Forward<Args>(aArgs)...));
+}
+
 // Compare two NotNulls.
 template <typename T, typename U>
 inline bool

mfbt/tests/TestNotNull.cpp

@@ -9,10 +9,11 @@
 #include "mozilla/UniquePtr.h"
 #include "mozilla/Unused.h"
 
-using mozilla::WrapNotNull;
+using mozilla::MakeNotNull;
 using mozilla::MakeUnique;
 using mozilla::NotNull;
 using mozilla::UniquePtr;
+using mozilla::WrapNotNull;
 
 #define CHECK MOZ_RELEASE_ASSERT
 
@@ -303,11 +304,72 @@ TestNotNullWithRefPtr()
   // and the MyRefType is destroyed.
 }
 
+void
+TestMakeNotNull()
+{
+  // Raw pointer.
+  auto nni = MakeNotNull<int*>(11);
+  static_assert(mozilla::IsSame<NotNull<int*>, decltype(nni)>::value,
+                "MakeNotNull<int*> should return NotNull<int*>");
+  CHECK(*nni == 11);
+  delete nni;
+
+  // Raw pointer to const.
+  auto nnci = MakeNotNull<const int*>(12);
+  static_assert(mozilla::IsSame<NotNull<const int*>, decltype(nnci)>::value,
+                "MakeNotNull<const int*> should return NotNull<const int*>");
+  CHECK(*nnci == 12);
+  delete nnci;
+
+  // Create a derived object and store its base pointer.
+  struct Base
+  {
+    virtual ~Base() = default;
+    virtual bool IsDerived() const { return false; }
+  };
+  struct Derived : Base
+  {
+    bool IsDerived() const override { return true; }
+  };
+  auto nnd = MakeNotNull<Derived*>();
+  static_assert(mozilla::IsSame<NotNull<Derived*>, decltype(nnd)>::value,
+                "MakeNotNull<Derived*> should return NotNull<Derived*>");
+  CHECK(nnd->IsDerived());
+  delete nnd;
+  NotNull<Base*> nnb = MakeNotNull<Derived*>();
+  static_assert(mozilla::IsSame<NotNull<Base*>, decltype(nnb)>::value,
+                "MakeNotNull<Derived*> should be assignable to NotNull<Base*>");
+  // Check that we have really built a Derived object.
+  CHECK(nnb->IsDerived());
+  delete nnb;
+
+  // Allow smart pointers.
+  auto nnmi = MakeNotNull<MyPtr<int>>(23);
+  static_assert(mozilla::IsSame<NotNull<MyPtr<int>>, decltype(nnmi)>::value,
+                "MakeNotNull<MyPtr<int>> should return NotNull<MyPtr<int>>");
+  CHECK(*nnmi == 23);
+  delete nnmi.get().get();
+
+  auto nnui = MakeNotNull<UniquePtr<int>>(24);
+  static_assert(
+    mozilla::IsSame<NotNull<UniquePtr<int>>, decltype(nnui)>::value,
+    "MakeNotNull<UniquePtr<int>> should return NotNull<UniquePtr<int>>");
+  CHECK(*nnui == 24);
+
+  // Expect only 1 RefCnt (from construction).
+  auto nnr = MakeNotNull<RefPtr<MyRefType>>(1);
+  static_assert(
+    mozilla::IsSame<NotNull<RefPtr<MyRefType>>, decltype(nnr)>::value,
+    "MakeNotNull<RefPtr<MyRefType>> should return NotNull<RefPtr<MyRefType>>");
+  mozilla::Unused << nnr;
+}
+
 int
 main()
 {
   TestNotNullWithMyPtr();
   TestNotNullWithRefPtr();
+  TestMakeNotNull();
 
   return 0;
 }