finish renaming async finalizer to basic

KevinEady · KevinEady · commit 5ca963397ef8 · 2024-07-25T15:31:33.000+02:00
diff --git a/napi-inl.h b/napi-inl.h
@@ -35,9 +35,9 @@ namespace details {
 constexpr int napi_no_external_buffers_allowed = 22;
 
 template <typename FreeType>
-inline void default_finalizer(node_api_nogc_env /*env*/,
-                              void* data,
-                              void* /*hint*/) {
+inline void default_basic_finalizer(node_api_nogc_env /*env*/,
+                                    void* data,
+                                    void* /*hint*/) {
   delete static_cast<FreeType*>(data);
 }
 
@@ -46,7 +46,7 @@ inline void default_finalizer(node_api_nogc_env /*env*/,
 // TODO: Replace this code with `napi_add_finalizer()` whenever it becomes
 // available on all supported versions of Node.js.
 template <typename FreeType,
-          node_api_nogc_finalize finalizer = default_finalizer<FreeType>>
+          node_api_nogc_finalize finalizer = default_basic_finalizer<FreeType>>
 inline napi_status AttachData(napi_env env,
                               napi_value obj,
                               FreeType* data,
@@ -427,7 +427,7 @@ inline std::string StringFormat(const char* format, ...) {
 }
 
 template <typename T>
-class HasAsyncFinalizer {
+class HasExtendedFinalizer {
  private:
   template <typename U, void (U::*)(Napi::Env)>
   struct SFINAE {};
@@ -441,7 +441,7 @@ class HasAsyncFinalizer {
 };
 
 template <typename T>
-class HasSyncFinalizer {
+class HasBasicFinalizer {
  private:
   template <typename U, void (U::*)(Napi::BasicEnv)>
   struct SFINAE {};
@@ -653,22 +653,20 @@ void BasicEnv::CleanupHook<Hook, Arg>::WrapperWithArg(void* data)
 #endif  // NAPI_VERSION > 2
 
 #if NAPI_VERSION > 5
-template <typename T, BasicEnv::AsyncFinalizer<T> async_fini>
+template <typename T, BasicEnv::Finalizer<T> fini>
 inline void BasicEnv::SetInstanceData(T* data) const {
   napi_status status = napi_set_instance_data(
       _env,
       data,
-      [](napi_env env, void* data, void*) {
-        async_fini(env, static_cast<T*>(data));
-      },
+      [](napi_env env, void* data, void*) { fini(env, static_cast<T*>(data)); },
       nullptr);
   NAPI_FATAL_IF_FAILED(
       status, "BasicEnv::SetInstanceData", "invalid arguments");
 }
 
 template <typename DataType,
           typename HintType,
-          Napi::BasicEnv::AsyncFinalizerWithHint<DataType, HintType> fini>
+          Napi::BasicEnv::FinalizerWithHint<DataType, HintType> fini>
 inline void BasicEnv::SetInstanceData(DataType* data, HintType* hint) const {
   napi_status status = napi_set_instance_data(
       _env,
@@ -693,12 +691,12 @@ inline T* BasicEnv::GetInstanceData() const {
 }
 
 template <typename T>
-void BasicEnv::DefaultAsyncFini(Env, T* data) {
+void BasicEnv::DefaultFini(Env, T* data) {
   delete data;
 }
 
 template <typename DataType, typename HintType>
-void BasicEnv::DefaultAsyncFiniWithHint(Env, DataType* data, HintType*) {
+void BasicEnv::DefaultFiniWithHint(Env, DataType* data, HintType*) {
   delete data;
 }
 #endif  // NAPI_VERSION > 5
@@ -5018,18 +5016,18 @@ inline void ObjectWrap<T>::FinalizeCallback(node_api_nogc_env env,
   // Prevent ~ObjectWrap from calling napi_remove_wrap
   instance->_ref = nullptr;
 
-  // If class overrides the synchronous finalizer, execute it.
-  if constexpr (details::HasSyncFinalizer<T>::value) {
+  // If class overrides the basic finalizer, execute it.
+  if constexpr (details::HasBasicFinalizer<T>::value) {
 #ifndef NODE_API_EXPERIMENTAL_HAS_POST_FINALIZER
     HandleScope scope(env);
 #endif
 
     instance->Finalize(Napi::BasicEnv(env));
   }
 
-  // If class overrides the asynchronous finalizer, either schedule it or
+  // If class overrides the (extended) finalizer, either schedule it or
   // execute it immediately (depending on experimental features enabled).
-  if constexpr (details::HasAsyncFinalizer<T>::value) {
+  if constexpr (details::HasExtendedFinalizer<T>::value) {
 #ifdef NODE_API_EXPERIMENTAL_HAS_POST_FINALIZER
     // In experimental, attach via node_api_post_finalizer.
     // `PostFinalizeCallback` is responsible for deleting the `T* instance`,
@@ -5040,16 +5038,16 @@ inline void ObjectWrap<T>::FinalizeCallback(node_api_nogc_env env,
                          "ObjectWrap<T>::FinalizeCallback",
                          "node_api_post_finalizer failed");
 #else
-    // In non-experimental, this `FinalizeCallback` already executes from
-    // outside the garbage collector. Execute the override directly.
+    // In non-experimental, this `FinalizeCallback` already executes from a
+    // non-basic environment. Execute the override directly.
     // `PostFinalizeCallback` is responsible for deleting the `T* instance`,
     // after calling the user-provided finalizer.
     HandleScope scope(env);
     PostFinalizeCallback(env, data, static_cast<void*>(nullptr));
 #endif
   }
-  // If the instance does _not_ have an asynchronous finalizer, delete the `T*
-  // instance` immediately.
+  // If the instance does _not_ override the (extended) finalizer, delete the
+  // `T* instance` immediately.
   else {
     delete instance;
   }
diff --git a/napi.h b/napi.h
@@ -318,9 +318,9 @@ class BasicEnv {
   node_api_nogc_env _env;
 #if NAPI_VERSION > 5
   template <typename T>
-  static void DefaultAsyncFini(Env, T* data);
+  static void DefaultFini(Env, T* data);
   template <typename DataType, typename HintType>
-  static void DefaultAsyncFiniWithHint(Env, DataType* data, HintType* hint);
+  static void DefaultFiniWithHint(Env, DataType* data, HintType* hint);
 #endif  // NAPI_VERSION > 5
  public:
   BasicEnv(node_api_nogc_env env);
@@ -355,17 +355,16 @@ class BasicEnv {
   T* GetInstanceData() const;
 
   template <typename T>
-  using AsyncFinalizer = void (*)(Env, T*);
-  template <typename T,
-            AsyncFinalizer<T> async_fini = BasicEnv::DefaultAsyncFini<T>>
+  using Finalizer = void (*)(Env, T*);
+  template <typename T, Finalizer<T> fini = BasicEnv::DefaultFini<T>>
   void SetInstanceData(T* data) const;
 
   template <typename DataType, typename HintType>
-  using AsyncFinalizerWithHint = void (*)(Env, DataType*, HintType*);
+  using FinalizerWithHint = void (*)(Env, DataType*, HintType*);
   template <typename DataType,
             typename HintType,
-            AsyncFinalizerWithHint<DataType, HintType> fini =
-                BasicEnv::DefaultAsyncFiniWithHint<DataType, HintType>>
+            FinalizerWithHint<DataType, HintType> fini =
+                BasicEnv::DefaultFiniWithHint<DataType, HintType>>
   void SetInstanceData(DataType* data, HintType* hint) const;
 #endif  // NAPI_VERSION > 5
 
diff --git a/test/finalizer_order.cc b/test/finalizer_order.cc
@@ -4,8 +4,8 @@ namespace {
 class Test : public Napi::ObjectWrap<Test> {
  public:
   Test(const Napi::CallbackInfo& info) : Napi::ObjectWrap<Test>(info) {
-    syncFinalizerCalled = false;
-    asyncFinalizerCalled = false;
+    basicFinalizerCalled = false;
+    finalizerCalled = false;
 
     if (info.Length() > 0) {
       finalizeCb_ = Napi::Persistent(info[0].As<Napi::Function>());
@@ -17,71 +17,71 @@ class Test : public Napi::ObjectWrap<Test> {
                 DefineClass(env,
                             "Test",
                             {
-                                StaticAccessor("isSyncFinalizerCalled",
-                                               &IsSyncFinalizerCalled,
+                                StaticAccessor("isBasicFinalizerCalled",
+                                               &IsBasicFinalizerCalled,
                                                nullptr,
                                                napi_default),
-                                StaticAccessor("isAsyncFinalizerCalled",
-                                               &IsAsyncFinalizerCalled,
+                                StaticAccessor("isFinalizerCalled",
+                                               &IsFinalizerCalled,
                                                nullptr,
                                                napi_default),
                             }));
   }
 
-  void Finalize(Napi::BasicEnv /*env*/) { syncFinalizerCalled = true; }
+  void Finalize(Napi::BasicEnv /*env*/) { basicFinalizerCalled = true; }
 
   void Finalize(Napi::Env /*env*/) {
-    asyncFinalizerCalled = true;
+    finalizerCalled = true;
     if (!finalizeCb_.IsEmpty()) {
       finalizeCb_.Call({});
     }
   }
 
-  static Napi::Value IsSyncFinalizerCalled(const Napi::CallbackInfo& info) {
-    return Napi::Boolean::New(info.Env(), syncFinalizerCalled);
+  static Napi::Value IsBasicFinalizerCalled(const Napi::CallbackInfo& info) {
+    return Napi::Boolean::New(info.Env(), basicFinalizerCalled);
   }
 
-  static Napi::Value IsAsyncFinalizerCalled(const Napi::CallbackInfo& info) {
-    return Napi::Boolean::New(info.Env(), asyncFinalizerCalled);
+  static Napi::Value IsFinalizerCalled(const Napi::CallbackInfo& info) {
+    return Napi::Boolean::New(info.Env(), finalizerCalled);
   }
 
  private:
   Napi::FunctionReference finalizeCb_;
 
-  static bool syncFinalizerCalled;
-  static bool asyncFinalizerCalled;
+  static bool basicFinalizerCalled;
+  static bool finalizerCalled;
 };
 
-bool Test::syncFinalizerCalled = false;
-bool Test::asyncFinalizerCalled = false;
+bool Test::basicFinalizerCalled = false;
+bool Test::finalizerCalled = false;
 
-bool externalSyncFinalizerCalled = false;
-bool externalAsyncFinalizerCalled = false;
+bool externalBasicFinalizerCalled = false;
+bool externalFinalizerCalled = false;
 
-Napi::Value CreateExternalSyncFinalizer(const Napi::CallbackInfo& info) {
-  externalSyncFinalizerCalled = false;
+Napi::Value CreateExternalBasicFinalizer(const Napi::CallbackInfo& info) {
+  externalBasicFinalizerCalled = false;
   return Napi::External<int>::New(
       info.Env(), new int(1), [](Napi::BasicEnv /*env*/, int* data) {
-        externalSyncFinalizerCalled = true;
+        externalBasicFinalizerCalled = true;
         delete data;
       });
 }
 
-Napi::Value CreateExternalAsyncFinalizer(const Napi::CallbackInfo& info) {
-  externalAsyncFinalizerCalled = false;
+Napi::Value CreateExternalFinalizer(const Napi::CallbackInfo& info) {
+  externalFinalizerCalled = false;
   return Napi::External<int>::New(
       info.Env(), new int(1), [](Napi::Env /*env*/, int* data) {
-        externalAsyncFinalizerCalled = true;
+        externalFinalizerCalled = true;
         delete data;
       });
 }
 
-Napi::Value IsExternalSyncFinalizerCalled(const Napi::CallbackInfo& info) {
-  return Napi::Boolean::New(info.Env(), externalSyncFinalizerCalled);
+Napi::Value isExternalBasicFinalizerCalled(const Napi::CallbackInfo& info) {
+  return Napi::Boolean::New(info.Env(), externalBasicFinalizerCalled);
 }
 
-Napi::Value IsExternalAsyncFinalizerCalled(const Napi::CallbackInfo& info) {
-  return Napi::Boolean::New(info.Env(), externalAsyncFinalizerCalled);
+Napi::Value IsExternalFinalizerCalled(const Napi::CallbackInfo& info) {
+  return Napi::Boolean::New(info.Env(), externalFinalizerCalled);
 }
 
 #ifdef NODE_API_EXPERIMENTAL_HAS_POST_FINALIZER
@@ -132,14 +132,14 @@ Napi::Value PostFinalizerWithDataAndHint(const Napi::CallbackInfo& info) {
 Napi::Object InitFinalizerOrder(Napi::Env env) {
   Napi::Object exports = Napi::Object::New(env);
   Test::Initialize(env, exports);
-  exports["createExternalSyncFinalizer"] =
-      Napi::Function::New(env, CreateExternalSyncFinalizer);
-  exports["createExternalAsyncFinalizer"] =
-      Napi::Function::New(env, CreateExternalAsyncFinalizer);
-  exports["isExternalSyncFinalizerCalled"] =
-      Napi::Function::New(env, IsExternalSyncFinalizerCalled);
-  exports["isExternalAsyncFinalizerCalled"] =
-      Napi::Function::New(env, IsExternalAsyncFinalizerCalled);
+  exports["createExternalBasicFinalizer"] =
+      Napi::Function::New(env, CreateExternalBasicFinalizer);
+  exports["createExternalFinalizer"] =
+      Napi::Function::New(env, CreateExternalFinalizer);
+  exports["isExternalBasicFinalizerCalled"] =
+      Napi::Function::New(env, isExternalBasicFinalizerCalled);
+  exports["isExternalFinalizerCalled"] =
+      Napi::Function::New(env, IsExternalFinalizerCalled);
 
 #ifdef NODE_API_EXPERIMENTAL_HAS_POST_FINALIZER
   exports["PostFinalizer"] = Napi::Function::New(env, PostFinalizer);
diff --git a/test/finalizer_order.js b/test/finalizer_order.js
@@ -22,47 +22,46 @@ function test (binding) {
       global.gc();
 
       if (isExperimental) {
-        assert.strictEqual(binding.finalizer_order.Test.isSyncFinalizerCalled, true, 'Expected sync finalizer to be called [before ticking]');
-        assert.strictEqual(binding.finalizer_order.Test.isAsyncFinalizerCalled, false, 'Expected async finalizer to not be called [before ticking]');
+        assert.strictEqual(binding.finalizer_order.Test.isBasicFinalizerCalled, true, 'Expected basic finalizer to be called [before ticking]');
+        assert.strictEqual(binding.finalizer_order.Test.isFinalizerCalled, false, 'Expected (extended) finalizer to not be called [before ticking]');
         assert.strictEqual(isCallbackCalled, false, 'Expected callback to not be called [before ticking]');
       } else {
-        assert.strictEqual(binding.finalizer_order.Test.isSyncFinalizerCalled, false, 'Expected sync finalizer to not be called [before ticking]');
-        assert.strictEqual(binding.finalizer_order.Test.isAsyncFinalizerCalled, false, 'Expected async finalizer to not be called [before ticking]');
+        assert.strictEqual(binding.finalizer_order.Test.isBasicFinalizerCalled, false, 'Expected basic finalizer to not be called [before ticking]');
+        assert.strictEqual(binding.finalizer_order.Test.isFinalizerCalled, false, 'Expected (extended) finalizer to not be called [before ticking]');
         assert.strictEqual(isCallbackCalled, false, 'Expected callback to not be called [before ticking]');
       }
     },
     () => {
-      assert.strictEqual(binding.finalizer_order.Test.isSyncFinalizerCalled, true, 'Expected sync finalizer to be called [after ticking]');
-      assert.strictEqual(binding.finalizer_order.Test.isAsyncFinalizerCalled, true, 'Expected async finalizer to be called [after ticking]');
+      assert.strictEqual(binding.finalizer_order.Test.isBasicFinalizerCalled, true, 'Expected basic finalizer to be called [after ticking]');
+      assert.strictEqual(binding.finalizer_order.Test.isFinalizerCalled, true, 'Expected (extended) finalizer to be called [after ticking]');
       assert.strictEqual(isCallbackCalled, true, 'Expected callback to be called [after ticking]');
     },
 
-    'Finalizer Order - External with Sync Finalizer',
+    'Finalizer Order - External with Basic Finalizer',
     () => {
-      console.log(binding.finalizer_order);
-      let ext = binding.finalizer_order.createExternalSyncFinalizer();
+      let ext = binding.finalizer_order.createExternalBasicFinalizer();
       ext = null;
       global.gc();
 
       if (isExperimental) {
-        assert.strictEqual(binding.finalizer_order.isExternalSyncFinalizerCalled(), true, 'Expected External sync finalizer to be called [before ticking]');
+        assert.strictEqual(binding.finalizer_order.isExternalBasicFinalizerCalled(), true, 'Expected External basic finalizer to be called [before ticking]');
       } else {
-        assert.strictEqual(binding.finalizer_order.isExternalSyncFinalizerCalled(), false, 'Expected External sync finalizer to not be called [before ticking]');
+        assert.strictEqual(binding.finalizer_order.isExternalBasicFinalizerCalled(), false, 'Expected External basic finalizer to not be called [before ticking]');
       }
     },
     () => {
-      assert.strictEqual(binding.finalizer_order.isExternalSyncFinalizerCalled(), true, 'Expected External sync finalizer to be called [after ticking]');
+      assert.strictEqual(binding.finalizer_order.isExternalBasicFinalizerCalled(), true, 'Expected External basic finalizer to be called [after ticking]');
     },
 
-    'Finalizer Order - External with Async Finalizer',
+    'Finalizer Order - External with Finalizer',
     () => {
-      let ext = binding.finalizer_order.createExternalAsyncFinalizer();
+      let ext = binding.finalizer_order.createExternalFinalizer();
       ext = null;
       global.gc();
-      assert.strictEqual(binding.finalizer_order.isExternalAsyncFinalizerCalled(), false, 'Expected External async finalizer to not be called [before ticking]');
+      assert.strictEqual(binding.finalizer_order.isExternalFinalizerCalled(), false, 'Expected External extended finalizer to not be called [before ticking]');
     },
     () => {
-      assert.strictEqual(binding.finalizer_order.isExternalAsyncFinalizerCalled(), true, 'Expected External async finalizer to be called [after ticking]');
+      assert.strictEqual(binding.finalizer_order.isExternalFinalizerCalled(), true, 'Expected External extended finalizer to be called [after ticking]');
     }
   ];
 
