src: add BaseObjectPtr nullptr operations

Allow comparing a `BaseObjectPtr` or implicitly construct a
`BaseObjectPtr` with `nullptr`.

PR-URL: nodejs#56585
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>

Author: legendecas

src/base_object-inl.h

@@ -250,6 +250,17 @@ BaseObjectPtrImpl<T, kIsWeak>& BaseObjectPtrImpl<T, kIsWeak>::operator=(
   return *new (this) BaseObjectPtrImpl(std::move(other));
 }
 
+template <typename T, bool kIsWeak>
+BaseObjectPtrImpl<T, kIsWeak>::BaseObjectPtrImpl(std::nullptr_t)
+    : BaseObjectPtrImpl() {}
+
+template <typename T, bool kIsWeak>
+BaseObjectPtrImpl<T, kIsWeak>& BaseObjectPtrImpl<T, kIsWeak>::operator=(
+    std::nullptr_t) {
+  this->~BaseObjectPtrImpl();
+  return *new (this) BaseObjectPtrImpl();
+}
+
 template <typename T, bool kIsWeak>
 void BaseObjectPtrImpl<T, kIsWeak>::reset(T* ptr) {
   *this = BaseObjectPtrImpl(ptr);
@@ -289,6 +300,16 @@ bool BaseObjectPtrImpl<T, kIsWeak>::operator !=(
   return get() != other.get();
 }
 
+template <typename T, bool kIsWeak>
+bool operator==(const BaseObjectPtrImpl<T, kIsWeak> ptr, const std::nullptr_t) {
+  return ptr.get() == nullptr;
+}
+
+template <typename T, bool kIsWeak>
+bool operator==(const std::nullptr_t, const BaseObjectPtrImpl<T, kIsWeak> ptr) {
+  return ptr.get() == nullptr;
+}
+
 template <typename T, typename... Args>
 BaseObjectPtr<T> MakeBaseObject(Args&&... args) {
   return BaseObjectPtr<T>(new T(std::forward<Args>(args)...));

src/base_object.h

@@ -288,6 +288,9 @@ class BaseObjectPtrImpl final {
   inline BaseObjectPtrImpl(BaseObjectPtrImpl&& other);
   inline BaseObjectPtrImpl& operator=(BaseObjectPtrImpl&& other);
 
+  inline BaseObjectPtrImpl(std::nullptr_t);
+  inline BaseObjectPtrImpl& operator=(std::nullptr_t);
+
   inline void reset(T* ptr = nullptr);
   inline T* get() const;
   inline T& operator*() const;
@@ -309,6 +312,13 @@ class BaseObjectPtrImpl final {
   inline BaseObject::PointerData* pointer_data() const;
 };
 
+template <typename T, bool kIsWeak>
+inline static bool operator==(const BaseObjectPtrImpl<T, kIsWeak>,
+                              const std::nullptr_t);
+template <typename T, bool kIsWeak>
+inline static bool operator==(const std::nullptr_t,
+                              const BaseObjectPtrImpl<T, kIsWeak>);
+
 template <typename T>
 using BaseObjectPtr = BaseObjectPtrImpl<T, false>;
 template <typename T>

src/histogram.cc

@@ -226,7 +226,7 @@ BaseObjectPtr<HistogramBase> HistogramBase::Create(
            ->InstanceTemplate()
            ->NewInstance(env->context())
            .ToLocal(&obj)) {
-    return BaseObjectPtr<HistogramBase>();
+    return nullptr;
   }
 
   return MakeBaseObject<HistogramBase>(env, obj, options);
@@ -240,7 +240,7 @@ BaseObjectPtr<HistogramBase> HistogramBase::Create(
            ->InstanceTemplate()
            ->NewInstance(env->context())
            .ToLocal(&obj)) {
-    return BaseObjectPtr<HistogramBase>();
+    return nullptr;
   }
   return MakeBaseObject<HistogramBase>(env, obj, std::move(histogram));
 }
@@ -394,7 +394,7 @@ BaseObjectPtr<IntervalHistogram> IntervalHistogram::Create(
   if (!GetConstructorTemplate(env)
           ->InstanceTemplate()
           ->NewInstance(env->context()).ToLocal(&obj)) {
-    return BaseObjectPtr<IntervalHistogram>();
+    return nullptr;
   }
 
   return MakeBaseObject<IntervalHistogram>(

src/node_blob.cc

@@ -168,11 +168,10 @@ BaseObjectPtr<Blob> Blob::Create(Environment* env,
 
   Local<Function> ctor;
   if (!GetConstructorTemplate(env)->GetFunction(env->context()).ToLocal(&ctor))
-    return BaseObjectPtr<Blob>();
+    return nullptr;
 
   Local<Object> obj;
-  if (!ctor->NewInstance(env->context()).ToLocal(&obj))
-    return BaseObjectPtr<Blob>();
+  if (!ctor->NewInstance(env->context()).ToLocal(&obj)) return nullptr;
 
   return MakeBaseObject<Blob>(env, obj, data_queue);
 }
@@ -326,7 +325,7 @@ BaseObjectPtr<Blob::Reader> Blob::Reader::Create(Environment* env,
            ->InstanceTemplate()
            ->NewInstance(env->context())
            .ToLocal(&obj)) {
-    return BaseObjectPtr<Blob::Reader>();
+    return nullptr;
   }
 
   return MakeBaseObject<Blob::Reader>(env, obj, std::move(blob));

src/node_http2.cc

@@ -846,7 +846,7 @@ void Http2Session::Close(uint32_t code, bool socket_closed) {
 // but this is faster and does not fail if the stream is not found.
 BaseObjectPtr<Http2Stream> Http2Session::FindStream(int32_t id) {
   auto s = streams_.find(id);
-  return s != streams_.end() ? s->second : BaseObjectPtr<Http2Stream>();
+  return s != streams_.end() ? s->second : nullptr;
 }
 
 bool Http2Session::CanAddStream() {

src/node_sockaddr.cc

@@ -553,7 +553,7 @@ BaseObjectPtr<SocketAddressBlockListWrap> SocketAddressBlockListWrap::New(
   if (!env->blocklist_constructor_template()
           ->InstanceTemplate()
           ->NewInstance(env->context()).ToLocal(&obj)) {
-    return BaseObjectPtr<SocketAddressBlockListWrap>();
+    return nullptr;
   }
   BaseObjectPtr<SocketAddressBlockListWrap> wrap =
       MakeBaseObject<SocketAddressBlockListWrap>(env, obj);
@@ -568,7 +568,7 @@ BaseObjectPtr<SocketAddressBlockListWrap> SocketAddressBlockListWrap::New(
   if (!env->blocklist_constructor_template()
           ->InstanceTemplate()
           ->NewInstance(env->context()).ToLocal(&obj)) {
-    return BaseObjectPtr<SocketAddressBlockListWrap>();
+    return nullptr;
   }
   BaseObjectPtr<SocketAddressBlockListWrap> wrap =
       MakeBaseObject<SocketAddressBlockListWrap>(
@@ -775,7 +775,7 @@ BaseObjectPtr<SocketAddressBase> SocketAddressBase::Create(
   if (!GetConstructorTemplate(env)
           ->InstanceTemplate()
           ->NewInstance(env->context()).ToLocal(&obj)) {
-    return BaseObjectPtr<SocketAddressBase>();
+    return nullptr;
   }
 
   return MakeBaseObject<SocketAddressBase>(env, obj, std::move(address));

src/node_sqlite.cc

@@ -2331,7 +2331,7 @@ BaseObjectPtr<StatementSync> StatementSync::Create(
            ->InstanceTemplate()
            ->NewInstance(env->context())
            .ToLocal(&obj)) {
-    return BaseObjectPtr<StatementSync>();
+    return nullptr;
   }
 
   return MakeBaseObject<StatementSync>(env, obj, std::move(db), stmt);
@@ -2485,7 +2485,7 @@ BaseObjectPtr<Session> Session::Create(Environment* env,
            ->InstanceTemplate()
            ->NewInstance(env->context())
            .ToLocal(&obj)) {
-    return BaseObjectPtr<Session>();
+    return nullptr;
   }
 
   return MakeBaseObject<Session>(env, obj, std::move(database), session);

src/quic/session.h

@@ -135,11 +135,11 @@ class Session final : public AsyncWrap, private SessionTicket::AppData::Source {
     const CID::Factory* cid_factory = &CID::Factory::random();
     // If the CID::Factory is a base object, we keep a reference to it
     // so that it cannot be garbage collected.
-    BaseObjectPtr<BaseObject> cid_factory_ref = {};
+    BaseObjectPtr<BaseObject> cid_factory_ref;
 
     // If the application provider is specified, it will be used to create
     // the underlying Application instance for the session.
-    BaseObjectPtr<ApplicationProvider> application_provider = {};
+    BaseObjectPtr<ApplicationProvider> application_provider;
 
     // When true, QLog output will be enabled for the session.
     bool qlog = false;

test/cctest/test_base_object_ptr.cc

@@ -155,6 +155,42 @@ TEST_F(BaseObjectPtrTest, Moveable) {
   EXPECT_EQ(realm->base_object_created_after_bootstrap(), 0);
 }
 
+TEST_F(BaseObjectPtrTest, Nullptr) {
+  const HandleScope handle_scope(isolate_);
+  const Argv argv;
+  Env env_{handle_scope, argv};
+  Environment* env = *env_;
+  Realm* realm = env->principal_realm();
+
+  BaseObjectPtr<DummyBaseObject> ptr = nullptr;
+  EXPECT_EQ(nullptr, ptr);
+  EXPECT_EQ(ptr, nullptr);
+  EXPECT_EQ(nullptr, ptr.get());
+
+  // Implicit constructor.
+  BaseObjectPtr<DummyBaseObject> ptr2 = []() -> BaseObjectPtr<DummyBaseObject> {
+    return nullptr;
+  }();
+  EXPECT_EQ(nullptr, ptr2);
+  EXPECT_EQ(ptr2, nullptr);
+  EXPECT_EQ(nullptr, ptr2.get());
+
+  BaseObjectWeakPtr<DummyBaseObject> weak_ptr{ptr};
+  EXPECT_EQ(nullptr, weak_ptr);
+  EXPECT_EQ(weak_ptr, nullptr);
+  EXPECT_EQ(nullptr, weak_ptr.get());
+  ptr.reset();
+  EXPECT_EQ(weak_ptr.get(), nullptr);
+
+  // No object creation with nullptr.
+  EXPECT_EQ(realm->base_object_created_after_bootstrap(), 0);
+
+  BaseObjectPtr<DummyBaseObject> ptr4 = DummyBaseObject::NewDetached(env);
+  EXPECT_NE(nullptr, ptr4);
+  EXPECT_NE(ptr4, nullptr);
+  EXPECT_EQ(realm->base_object_created_after_bootstrap(), 1);
+}
+
 TEST_F(BaseObjectPtrTest, NestedClasses) {
   class ObjectWithPtr : public BaseObject {
    public: