Bug 1631709 - NotNull must not be movable. r=jwalden
authorSimon Giesecke <sgiesecke@mozilla.com>
Tue, 28 Apr 2020 11:23:05 +0000
changeset 526472 08e2ec70d06d30ce99568ce6a473bc6d698dbc70
parent 526471 2550dcabbedf1e95ba9c8d71cae32aea0b7a7a29
child 526473 c8a063cf794b600e01d9ff452465e8ee6b3ceeaf
push id114281
push usersgiesecke@mozilla.com
push dateTue, 28 Apr 2020 11:54:35 +0000
treeherderautoland@08e2ec70d06d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwalden
bugs1631709
milestone77.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1631709 - NotNull must not be movable. r=jwalden Differential Revision: https://phabricator.services.mozilla.com/D71721
mfbt/NotNull.h
mfbt/tests/TestNotNull.cpp
--- a/mfbt/NotNull.h
+++ b/mfbt/NotNull.h
@@ -139,16 +139,23 @@ class NotNull {
   // Implicit conversion to a base pointer. Preferable to get().
   constexpr operator const T&() const { return get(); }
 
   // Dereference operators.
   constexpr auto* operator->() const MOZ_NONNULL_RETURN {
     return mBasePtr.operator->();
   }
   constexpr decltype(*mBasePtr) operator*() const { return *mBasePtr; }
+
+  // NotNull can be copied, but not moved. Moving a NotNull with a smart base
+  // pointer would leave a nullptr NotNull behind. The move operations must not
+  // be explicitly deleted though, since that would cause overload resolution to
+  // fail in situations where a copy is possible.
+  NotNull(const NotNull&) = default;
+  NotNull& operator=(const NotNull&) = default;
 };
 
 // Specialization for T* to allow adding MOZ_NONNULL_RETURN attributes.
 template <typename T>
 class NotNull<T*> {
   template <typename U>
   friend constexpr NotNull<U> WrapNotNull(U aBasePtr);
   template <typename U>
--- a/mfbt/tests/TestNotNull.cpp
+++ b/mfbt/tests/TestNotNull.cpp
@@ -283,16 +283,24 @@ void TestNotNullWithRefPtr() {
   // At this point the refcount is 4.
 
   // No change to the refcount occurs because of the argument passing. Within
   // f_ref() the refcount temporarily hits 5, due to the local RefPtr.
   f_ref(r2);
 
   // At this point the refcount is 4.
 
+  NotNull<RefPtr<MyRefType>> r6 = std::move(r2);
+  mozilla::Unused << r6;
+
+  CHECK(r2.get());
+  CHECK(r6.get());
+
+  // At this point the refcount is 5 again, since NotNull is not movable.
+
   // 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(std::is_same_v<NotNull<int*>, decltype(nni)>,