From 9aceea71fc14b92a2dcec66141eb339c04c41be9 Mon Sep 17 00:00:00 2001
From: Gabriel Schulhof <gabriel.schulhof@intel.com>
Date: Thu, 6 Aug 2020 11:24:17 -0700
Subject: [PATCH] src: concentrate callbacks provided to core N-API

This change reduces the places where we declare private functions with
the `napi_callback` signature for the purpose of using them with C++
callbacks passed as template arguments. We basically have 4 types:

  1. static with `void` return
  2. static with `napi_value` return
  3. instance with `void` return
  4. instance with `napi_value` return

We can use one of these four calling patterns in the following places
where we accept callbacks as template arguments:
  * `Napi::Function` (1. and 2.)
  * `Napi::PropertyDescriptor` (1. for the setter, 2. for the getter)
  * `Napi::InstanceWrap<T>` (3., 4. for instance methods, 4. for
     instance getters)
  * `Napi::ObjectWrap<T>` (1., 2. for static methods, 2. for static
     getters)

In the case of `InstanceWrap<T>` and `ObjectWrap<T>` instance resp.
static property descriptors we can also remove the infrastructure
designed to allow for optional getters (`GetterTag` resp.
`StaticGetterTag`) because the API for specifying instance resp. class
property descriptors does not allow one to omit the getter.

Signed-off-by: Gabriel Schulhof <gabriel.schulhof@intel.com>
PR-URL: https://github.com/nodejs/node-addon-api/pull/786
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
---
 napi-inl.h | 162 ++++++++++++++++++++++++-----------------------------
 napi.h     |  23 --------
 2 files changed, 72 insertions(+), 113 deletions(-)

diff --git a/napi-inl.h b/napi-inl.h
index 4f0636a08..3b2c558d5 100644
--- a/napi-inl.h
+++ b/napi-inl.h
@@ -135,6 +135,48 @@ struct CallbackData<Callable, void> {
   void* data;
 };
 
+template <void (*Callback)(const CallbackInfo& info)>
+static napi_value
+TemplatedVoidCallback(napi_env env, napi_callback_info info) NAPI_NOEXCEPT {
+  return details::WrapCallback([&] {
+    CallbackInfo cbInfo(env, info);
+    Callback(cbInfo);
+    return nullptr;
+  });
+}
+
+template <Napi::Value (*Callback)(const CallbackInfo& info)>
+static napi_value
+TemplatedCallback(napi_env env, napi_callback_info info) NAPI_NOEXCEPT {
+  return details::WrapCallback([&] {
+    CallbackInfo cbInfo(env, info);
+    return Callback(cbInfo);
+  });
+}
+
+template <typename T,
+          Napi::Value (T::*UnwrapCallback)(const CallbackInfo& info)>
+static napi_value
+TemplatedInstanceCallback(napi_env env, napi_callback_info info) NAPI_NOEXCEPT {
+  return details::WrapCallback([&] {
+    CallbackInfo cbInfo(env, info);
+    T* instance = T::Unwrap(cbInfo.This().As<Object>());
+    return (instance->*UnwrapCallback)(cbInfo);
+  });
+}
+
+template <typename T, void (T::*UnwrapCallback)(const CallbackInfo& info)>
+static napi_value
+TemplatedInstanceVoidCallback(napi_env env,
+                              napi_callback_info info) NAPI_NOEXCEPT {
+  return details::WrapCallback([&] {
+    CallbackInfo cbInfo(env, info);
+    T* instance = T::Unwrap(cbInfo.This().As<Object>());
+    (instance->*UnwrapCallback)(cbInfo);
+    return nullptr;
+  });
+}
+
 template <typename T, typename Finalizer, typename Hint = void>
 struct FinalizeData {
   static inline
@@ -1845,15 +1887,12 @@ CreateFunction(napi_env env,
 template <Function::VoidCallback cb>
 inline Function Function::New(napi_env env, const char* utf8name, void* data) {
   napi_value result = nullptr;
-  napi_status status = napi_create_function(
-      env, utf8name, NAPI_AUTO_LENGTH,
-      [](napi_env env, napi_callback_info info) {
-        CallbackInfo callbackInfo(env, info);
-        return details::WrapCallback([&] {
-          cb(callbackInfo);
-          return nullptr;
-        });
-      }, data, &result);
+  napi_status status = napi_create_function(env,
+                                            utf8name,
+                                            NAPI_AUTO_LENGTH,
+                                            details::TemplatedVoidCallback<cb>,
+                                            data,
+                                            &result);
   NAPI_THROW_IF_FAILED(env, status, Function());
   return Function(env, result);
 }
@@ -1861,14 +1900,12 @@ inline Function Function::New(napi_env env, const char* utf8name, void* data) {
 template <Function::Callback cb>
 inline Function Function::New(napi_env env, const char* utf8name, void* data) {
   napi_value result = nullptr;
-  napi_status status = napi_create_function(
-      env, utf8name, NAPI_AUTO_LENGTH,
-      [](napi_env env, napi_callback_info info) {
-        CallbackInfo callbackInfo(env, info);
-        return details::WrapCallback([&] {
-          return cb(callbackInfo);
-        });
-      }, data, &result);
+  napi_status status = napi_create_function(env,
+                                            utf8name,
+                                            NAPI_AUTO_LENGTH,
+                                            details::TemplatedCallback<cb>,
+                                            data,
+                                            &result);
   NAPI_THROW_IF_FAILED(env, status, Function());
   return Function(env, result);
 }
@@ -2859,7 +2896,7 @@ PropertyDescriptor::Accessor(const char* utf8name,
   napi_property_descriptor desc = napi_property_descriptor();
 
   desc.utf8name = utf8name;
-  desc.getter = &GetterCallbackWrapper<Getter>;
+  desc.getter = details::TemplatedCallback<Getter>;
   desc.attributes = attributes;
   desc.data = data;
 
@@ -2882,7 +2919,7 @@ PropertyDescriptor::Accessor(Name name,
   napi_property_descriptor desc = napi_property_descriptor();
 
   desc.name = name;
-  desc.getter = &GetterCallbackWrapper<Getter>;
+  desc.getter = details::TemplatedCallback<Getter>;
   desc.attributes = attributes;
   desc.data = data;
 
@@ -2900,8 +2937,8 @@ PropertyDescriptor::Accessor(const char* utf8name,
   napi_property_descriptor desc = napi_property_descriptor();
 
   desc.utf8name = utf8name;
-  desc.getter = &GetterCallbackWrapper<Getter>;
-  desc.setter = &SetterCallbackWrapper<Setter>;
+  desc.getter = details::TemplatedCallback<Getter>;
+  desc.setter = details::TemplatedVoidCallback<Setter>;
   desc.attributes = attributes;
   desc.data = data;
 
@@ -2928,31 +2965,14 @@ PropertyDescriptor::Accessor(Name name,
   napi_property_descriptor desc = napi_property_descriptor();
 
   desc.name = name;
-  desc.getter = &GetterCallbackWrapper<Getter>;
-  desc.setter = &SetterCallbackWrapper<Setter>;
+  desc.getter = details::TemplatedCallback<Getter>;
+  desc.setter = details::TemplatedVoidCallback<Setter>;
   desc.attributes = attributes;
   desc.data = data;
 
   return desc;
 }
 
-template <typename PropertyDescriptor::GetterCallback Getter>
-napi_value
-PropertyDescriptor::GetterCallbackWrapper(napi_env env,
-                                          napi_callback_info info) {
-  CallbackInfo cbInfo(env, info);
-  return Getter(cbInfo);
-}
-
-template <typename PropertyDescriptor::SetterCallback Setter>
-napi_value
-PropertyDescriptor::SetterCallbackWrapper(napi_env env,
-                                          napi_callback_info info) {
-  CallbackInfo cbInfo(env, info);
-  Setter(cbInfo);
-  return nullptr;
-}
-
 template <typename Getter>
 inline PropertyDescriptor
 PropertyDescriptor::Accessor(Napi::Env env,
@@ -3283,7 +3303,7 @@ inline ClassPropertyDescriptor<T> InstanceWrap<T>::InstanceMethod(
     void* data) {
   napi_property_descriptor desc = napi_property_descriptor();
   desc.utf8name = utf8name;
-  desc.method = &InstanceWrap<T>::WrappedMethod<method>;
+  desc.method = details::TemplatedInstanceVoidCallback<T, method>;
   desc.data = data;
   desc.attributes = attributes;
   return desc;
@@ -3297,7 +3317,7 @@ inline ClassPropertyDescriptor<T> InstanceWrap<T>::InstanceMethod(
     void* data) {
   napi_property_descriptor desc = napi_property_descriptor();
   desc.utf8name = utf8name;
-  desc.method = &InstanceWrap<T>::WrappedMethod<method>;
+  desc.method = details::TemplatedInstanceCallback<T, method>;
   desc.data = data;
   desc.attributes = attributes;
   return desc;
@@ -3311,7 +3331,7 @@ inline ClassPropertyDescriptor<T> InstanceWrap<T>::InstanceMethod(
     void* data) {
   napi_property_descriptor desc = napi_property_descriptor();
   desc.name = name;
-  desc.method = &InstanceWrap<T>::WrappedMethod<method>;
+  desc.method = details::TemplatedInstanceVoidCallback<T, method>;
   desc.data = data;
   desc.attributes = attributes;
   return desc;
@@ -3325,7 +3345,7 @@ inline ClassPropertyDescriptor<T> InstanceWrap<T>::InstanceMethod(
     void* data) {
   napi_property_descriptor desc = napi_property_descriptor();
   desc.name = name;
-  desc.method = &InstanceWrap<T>::WrappedMethod<method>;
+  desc.method = details::TemplatedInstanceCallback<T, method>;
   desc.data = data;
   desc.attributes = attributes;
   return desc;
@@ -3378,7 +3398,7 @@ inline ClassPropertyDescriptor<T> InstanceWrap<T>::InstanceAccessor(
     void* data) {
   napi_property_descriptor desc = napi_property_descriptor();
   desc.utf8name = utf8name;
-  desc.getter = This::WrapGetter(This::GetterTag<getter>());
+  desc.getter = details::TemplatedInstanceCallback<T, getter>;
   desc.setter = This::WrapSetter(This::SetterTag<setter>());
   desc.data = data;
   desc.attributes = attributes;
@@ -3394,7 +3414,7 @@ inline ClassPropertyDescriptor<T> InstanceWrap<T>::InstanceAccessor(
     void* data) {
   napi_property_descriptor desc = napi_property_descriptor();
   desc.name = name;
-  desc.getter = This::WrapGetter(This::GetterTag<getter>());
+  desc.getter = details::TemplatedInstanceCallback<T, getter>;
   desc.setter = This::WrapSetter(This::SetterTag<setter>());
   desc.data = data;
   desc.attributes = attributes;
@@ -3487,27 +3507,6 @@ inline napi_value InstanceWrap<T>::InstanceSetterCallbackWrapper(
   });
 }
 
-template <typename T>
-template <typename InstanceWrap<T>::InstanceVoidMethodCallback method>
-inline napi_value InstanceWrap<T>::WrappedMethod(napi_env env, napi_callback_info info) noexcept {
-  return details::WrapCallback([&] {
-    const CallbackInfo cbInfo(env, info);
-    T* instance = T::Unwrap(cbInfo.This().As<Object>());
-    (instance->*method)(cbInfo);
-    return nullptr;
-  });
-}
-
-template <typename T>
-template <typename InstanceWrap<T>::InstanceMethodCallback method>
-inline napi_value InstanceWrap<T>::WrappedMethod(napi_env env, napi_callback_info info) noexcept {
-  return details::WrapCallback([&] {
-    const CallbackInfo cbInfo(env, info);
-    T* instance = T::Unwrap(cbInfo.This().As<Object>());
-    return (instance->*method)(cbInfo);
-  });
-}
-
 template <typename T>
 template <typename InstanceWrap<T>::InstanceSetterCallback method>
 inline napi_value InstanceWrap<T>::WrappedMethod(napi_env env, napi_callback_info info) noexcept {
@@ -3732,7 +3731,7 @@ inline ClassPropertyDescriptor<T> ObjectWrap<T>::StaticMethod(
     void* data) {
   napi_property_descriptor desc = napi_property_descriptor();
   desc.utf8name = utf8name;
-  desc.method = &ObjectWrap<T>::WrappedMethod<method>;
+  desc.method = details::TemplatedVoidCallback<method>;
   desc.data = data;
   desc.attributes = static_cast<napi_property_attributes>(attributes | napi_static);
   return desc;
@@ -3746,7 +3745,7 @@ inline ClassPropertyDescriptor<T> ObjectWrap<T>::StaticMethod(
     void* data) {
   napi_property_descriptor desc = napi_property_descriptor();
   desc.name = name;
-  desc.method = &ObjectWrap<T>::WrappedMethod<method>;
+  desc.method = details::TemplatedVoidCallback<method>;
   desc.data = data;
   desc.attributes = static_cast<napi_property_attributes>(attributes | napi_static);
   return desc;
@@ -3760,7 +3759,7 @@ inline ClassPropertyDescriptor<T> ObjectWrap<T>::StaticMethod(
     void* data) {
   napi_property_descriptor desc = napi_property_descriptor();
   desc.utf8name = utf8name;
-  desc.method = &ObjectWrap<T>::WrappedMethod<method>;
+  desc.method = details::TemplatedCallback<method>;
   desc.data = data;
   desc.attributes = static_cast<napi_property_attributes>(attributes | napi_static);
   return desc;
@@ -3774,7 +3773,7 @@ inline ClassPropertyDescriptor<T> ObjectWrap<T>::StaticMethod(
     void* data) {
   napi_property_descriptor desc = napi_property_descriptor();
   desc.name = name;
-  desc.method = &ObjectWrap<T>::WrappedMethod<method>;
+  desc.method = details::TemplatedCallback<method>;
   desc.data = data;
   desc.attributes = static_cast<napi_property_attributes>(attributes | napi_static);
   return desc;
@@ -3827,7 +3826,7 @@ inline ClassPropertyDescriptor<T> ObjectWrap<T>::StaticAccessor(
     void* data) {
   napi_property_descriptor desc = napi_property_descriptor();
   desc.utf8name = utf8name;
-  desc.getter = This::WrapStaticGetter(This::StaticGetterTag<getter>());
+  desc.getter = details::TemplatedCallback<getter>;
   desc.setter = This::WrapStaticSetter(This::StaticSetterTag<setter>());
   desc.data = data;
   desc.attributes = static_cast<napi_property_attributes>(attributes | napi_static);
@@ -3843,7 +3842,7 @@ inline ClassPropertyDescriptor<T> ObjectWrap<T>::StaticAccessor(
     void* data) {
   napi_property_descriptor desc = napi_property_descriptor();
   desc.name = name;
-  desc.getter = This::WrapStaticGetter(This::StaticGetterTag<getter>());
+  desc.getter = details::TemplatedCallback<getter>;
   desc.setter = This::WrapStaticSetter(This::StaticSetterTag<setter>());
   desc.data = data;
   desc.attributes = static_cast<napi_property_attributes>(attributes | napi_static);
@@ -3969,23 +3968,6 @@ inline void ObjectWrap<T>::FinalizeCallback(napi_env env, void* data, void* /*hi
   delete instance;
 }
 
-template <typename T>
-template <typename ObjectWrap<T>::StaticVoidMethodCallback method>
-inline napi_value ObjectWrap<T>::WrappedMethod(napi_env env, napi_callback_info info) noexcept {
-  return details::WrapCallback([&] {
-    method(CallbackInfo(env, info));
-    return nullptr;
-  });
-}
-
-template <typename T>
-template <typename ObjectWrap<T>::StaticMethodCallback method>
-inline napi_value ObjectWrap<T>::WrappedMethod(napi_env env, napi_callback_info info) noexcept {
-  return details::WrapCallback([&] {
-    return method(CallbackInfo(env, info));
-  });
-}
-
 template <typename T>
 template <typename ObjectWrap<T>::StaticSetterCallback method>
 inline napi_value ObjectWrap<T>::WrappedMethod(napi_env env, napi_callback_info info) noexcept {
diff --git a/napi.h b/napi.h
index cf0ce51e7..76a62f659 100644
--- a/napi.h
+++ b/napi.h
@@ -1623,10 +1623,6 @@ namespace Napi {
     operator const napi_property_descriptor&() const;
 
   private:
-    template <GetterCallback Getter>
-    static napi_value GetterCallbackWrapper(napi_env env, napi_callback_info info);
-    template <SetterCallback Setter>
-    static napi_value SetterCallbackWrapper(napi_env env, napi_callback_info info);
     napi_property_descriptor _desc;
   };
 
@@ -1748,16 +1744,8 @@ namespace Napi {
     template <InstanceSetterCallback method>
     static napi_value WrappedMethod(napi_env env, napi_callback_info info) noexcept;
 
-    template <InstanceGetterCallback getter> struct GetterTag {};
     template <InstanceSetterCallback setter> struct SetterTag {};
 
-    template <InstanceVoidMethodCallback method>
-    static napi_value WrappedMethod(napi_env env, napi_callback_info info) noexcept;
-    template <InstanceMethodCallback method>
-    static napi_value WrappedMethod(napi_env env, napi_callback_info info) noexcept;
-    template <InstanceGetterCallback getter>
-    static napi_callback WrapGetter(GetterTag<getter>) noexcept { return &This::WrappedMethod<getter>; }
-    static napi_callback WrapGetter(GetterTag<nullptr>) noexcept { return nullptr; }
     template <InstanceSetterCallback setter>
     static napi_callback WrapSetter(SetterTag<setter>) noexcept { return &This::WrappedMethod<setter>; }
     static napi_callback WrapSetter(SetterTag<nullptr>) noexcept { return nullptr; }
@@ -1892,22 +1880,11 @@ namespace Napi {
                                  StaticGetterCallback,
                                  StaticSetterCallback> StaticAccessorCallbackData;
 
-    template <StaticVoidMethodCallback method>
-    static napi_value WrappedMethod(napi_env env, napi_callback_info info) noexcept;
-
-    template <StaticMethodCallback method>
-    static napi_value WrappedMethod(napi_env env, napi_callback_info info) noexcept;
-
     template <StaticSetterCallback method>
     static napi_value WrappedMethod(napi_env env, napi_callback_info info) noexcept;
 
-    template <StaticGetterCallback getter> struct StaticGetterTag {};
     template <StaticSetterCallback setter> struct StaticSetterTag {};
 
-    template <StaticGetterCallback getter>
-    static napi_callback WrapStaticGetter(StaticGetterTag<getter>) noexcept { return &This::WrappedMethod<getter>; }
-    static napi_callback WrapStaticGetter(StaticGetterTag<nullptr>) noexcept { return nullptr; }
-
     template <StaticSetterCallback setter>
     static napi_callback WrapStaticSetter(StaticSetterTag<setter>) noexcept { return &This::WrappedMethod<setter>; }
     static napi_callback WrapStaticSetter(StaticSetterTag<nullptr>) noexcept { return nullptr; }