Bug 1410252 - MakeNotNull<PointerType, OptionalPointeeType>(Args...) - r?njn draft
authorGerald Squelart <gsquelart@mozilla.com>
Fri, 20 Oct 2017 14:56:01 +1100
changeset 684539 9af8b06dd01aa7c90f0c15d5259c45a88f489266
parent 684511 ce1a86d3b4db161c95d1147676bbed839d7a4732
child 684540 2a458dd36dc5dff87858e2a853b168de826a36bb
push id85634
push usergsquelart@mozilla.com
push dateSun, 22 Oct 2017 23:53:05 +0000
reviewersnjn
bugs1410252
milestone58.0a1
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
mfbt/NotNull.h
mfbt/tests/TestNotNull.cpp
--- a/mfbt/NotNull.h
+++ b/mfbt/NotNull.h
@@ -58,16 +58,17 @@
 // - When the handle is rebound to another object. References don't allow this.
 //
 // - When the handle has type |void|. |void&| is not allowed.
 //
 // NotNull is an alternative that can be used in any of the above cases except
 // 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 {
 
 // NotNull can be used to wrap a "base" pointer (raw or smart) to indicate it
 // is not null. Some examples:
 //
 // - NotNull<char*>
@@ -80,37 +81,39 @@ namespace mozilla {
 //
 // - It must be initialized explicitly. There is no default initialization.
 //
 // - It auto-converts to the base pointer type.
 //
 // - 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.
 //
 // - |operator bool()| is deleted. This means you cannot check a NotNull in a
 //   boolean context, which eliminates the possibility of unnecessary null
 //   checks.
 //
 // NotNull currently doesn't work with UniquePtr. See
 // https://github.com/Microsoft/GSL/issues/89 for some discussion.
 //
 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) {}
 
 public:
   // Disallow default construction.
   NotNull() = delete;
 
   // Construct/assign from another NotNull with a compatible base pointer type.
@@ -147,16 +150,32 @@ template <typename T>
 NotNull<T>
 WrapNotNull(const T aBasePtr)
 {
   NotNull<T> notNull(aBasePtr);
   MOZ_RELEASE_ASSERT(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
 operator==(const NotNull<T>& aLhs, const NotNull<U>& aRhs)
 {
   return aLhs.get() == aRhs.get();
 }
 template <typename T, typename U>
--- a/mfbt/tests/TestNotNull.cpp
+++ b/mfbt/tests/TestNotNull.cpp
@@ -4,20 +4,21 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "mozilla/NotNull.h"
 #include "mozilla/RefPtr.h"
 #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
 
 class Blah
 {
 public:
   Blah() : mX(0) {}
   void blah() {};
@@ -298,17 +299,78 @@ TestNotNullWithRefPtr()
   f_ref(r2);
 
   // At this point the refcount is 4.
 
   // At function's end all RefPtrs are destroyed and the refcount drops to 0
   // 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;
 }