From 2a942d445826eef8a0c2b8f2e8666431190c55ee Mon Sep 17 00:00:00 2001 From: Vladimir Morozov Date: Mon, 23 Jan 2023 07:15:34 -0800 Subject: [PATCH 1/8] node-api: deprecate napi_module_register --- src/node_api.h | 66 +++---------------- test/node-api/test_null_init/binding.gyp | 8 --- test/node-api/test_null_init/test.js | 7 -- test/node-api/test_null_init/test_null_init.c | 3 - 4 files changed, 8 insertions(+), 76 deletions(-) delete mode 100644 test/node-api/test_null_init/binding.gyp delete mode 100644 test/node-api/test_null_init/test.js delete mode 100644 test/node-api/test_null_init/test_null_init.c diff --git a/src/node_api.h b/src/node_api.h index caf987dbd8dd8b..be0002b52cf51d 100644 --- a/src/node_api.h +++ b/src/node_api.h @@ -31,6 +31,7 @@ struct uv_loop_s; // Forward declaration. typedef napi_value(NAPI_CDECL* napi_addon_register_func)(napi_env env, napi_value exports); +// Used by deprecated registration method napi_module_register. typedef struct napi_module { int nm_version; unsigned int nm_flags; @@ -43,70 +44,15 @@ typedef struct napi_module { #define NAPI_MODULE_VERSION 1 -#if defined(_MSC_VER) -#if defined(__cplusplus) -#define NAPI_C_CTOR(fn) \ - static void NAPI_CDECL fn(void); \ - namespace { \ - struct fn##_ { \ - fn##_() { fn(); } \ - } fn##_v_; \ - } \ - static void NAPI_CDECL fn(void) -#else // !defined(__cplusplus) -#pragma section(".CRT$XCU", read) -// The NAPI_C_CTOR macro defines a function fn that is called during CRT -// initialization. -// C does not support dynamic initialization of static variables and this code -// simulates C++ behavior. Exporting the function pointer prevents it from being -// optimized. See for details: -// https://docs.microsoft.com/en-us/cpp/c-runtime-library/crt-initialization?view=msvc-170 -#define NAPI_C_CTOR(fn) \ - static void NAPI_CDECL fn(void); \ - __declspec(dllexport, allocate(".CRT$XCU")) void(NAPI_CDECL * fn##_)(void) = \ - fn; \ - static void NAPI_CDECL fn(void) -#endif // defined(__cplusplus) -#else -#define NAPI_C_CTOR(fn) \ - static void fn(void) __attribute__((constructor)); \ - static void fn(void) -#endif - -#define NAPI_MODULE_X(modname, regfunc, priv, flags) \ - EXTERN_C_START \ - static napi_module _module = { \ - NAPI_MODULE_VERSION, \ - flags, \ - __FILE__, \ - regfunc, \ - #modname, \ - priv, \ - {0}, \ - }; \ - NAPI_C_CTOR(_register_##modname) { napi_module_register(&_module); } \ - EXTERN_C_END - #define NAPI_MODULE_INITIALIZER_X(base, version) \ NAPI_MODULE_INITIALIZER_X_HELPER(base, version) #define NAPI_MODULE_INITIALIZER_X_HELPER(base, version) base##version #ifdef __wasm32__ -#define NAPI_WASM_INITIALIZER \ - NAPI_MODULE_INITIALIZER_X(napi_register_wasm_v, NAPI_MODULE_VERSION) -#define NAPI_MODULE(modname, regfunc) \ - EXTERN_C_START \ - NAPI_MODULE_EXPORT napi_value NAPI_WASM_INITIALIZER(napi_env env, \ - napi_value exports) { \ - return regfunc(env, exports); \ - } \ - EXTERN_C_END +#define NAPI_MODULE_INITIALIZER_BASE napi_register_wasm_v #else -#define NAPI_MODULE(modname, regfunc) \ - NAPI_MODULE_X(modname, regfunc, NULL, 0) // NOLINT (readability/null_usage) -#endif - #define NAPI_MODULE_INITIALIZER_BASE napi_register_module_v +#endif #define NAPI_MODULE_INITIALIZER \ NAPI_MODULE_INITIALIZER_X(NAPI_MODULE_INITIALIZER_BASE, NAPI_MODULE_VERSION) @@ -116,11 +62,15 @@ typedef struct napi_module { NAPI_MODULE_EXPORT napi_value NAPI_MODULE_INITIALIZER(napi_env env, \ napi_value exports); \ EXTERN_C_END \ - NAPI_MODULE(NODE_GYP_MODULE_NAME, NAPI_MODULE_INITIALIZER) \ napi_value NAPI_MODULE_INITIALIZER(napi_env env, napi_value exports) +#define NAPI_MODULE(modname, regfunc) \ + NAPI_MODULE_INIT() { return regfunc(env, exports); } + EXTERN_C_START +// Deprecated. Replaced by symbol-based registration defined by NAPI_MODULE +// and NAPI_MODULE_INIT macros. NAPI_EXTERN void NAPI_CDECL napi_module_register(napi_module* mod); NAPI_EXTERN NAPI_NO_RETURN void NAPI_CDECL diff --git a/test/node-api/test_null_init/binding.gyp b/test/node-api/test_null_init/binding.gyp deleted file mode 100644 index 27701616e338db..00000000000000 --- a/test/node-api/test_null_init/binding.gyp +++ /dev/null @@ -1,8 +0,0 @@ -{ - 'targets': [ - { - 'target_name': 'test_null_init', - 'sources': [ 'test_null_init.c' ] - } - ] -} diff --git a/test/node-api/test_null_init/test.js b/test/node-api/test_null_init/test.js deleted file mode 100644 index 6e6bf51839bed0..00000000000000 --- a/test/node-api/test_null_init/test.js +++ /dev/null @@ -1,7 +0,0 @@ -'use strict'; -const common = require('../../common'); -const assert = require('assert'); - -assert.throws( - () => require(`./build/${common.buildType}/test_null_init`), - /Module has no declared entry point[.]/); diff --git a/test/node-api/test_null_init/test_null_init.c b/test/node-api/test_null_init/test_null_init.c deleted file mode 100644 index d9d2200488ce41..00000000000000 --- a/test/node-api/test_null_init/test_null_init.c +++ /dev/null @@ -1,3 +0,0 @@ -#include - -NAPI_MODULE(NODE_GYP_MODULE_NAME, NULL) From 1b072eaacfaaf0fb954181c978a913bd600f4597 Mon Sep 17 00:00:00 2001 From: Vladimir Morozov Date: Mon, 23 Jan 2023 15:58:18 -0800 Subject: [PATCH 2/8] minimize changes to node_api.h --- src/node_api.h | 63 +++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 57 insertions(+), 6 deletions(-) diff --git a/src/node_api.h b/src/node_api.h index be0002b52cf51d..a5f78ff8c62167 100644 --- a/src/node_api.h +++ b/src/node_api.h @@ -31,7 +31,6 @@ struct uv_loop_s; // Forward declaration. typedef napi_value(NAPI_CDECL* napi_addon_register_func)(napi_env env, napi_value exports); -// Used by deprecated registration method napi_module_register. typedef struct napi_module { int nm_version; unsigned int nm_flags; @@ -44,16 +43,68 @@ typedef struct napi_module { #define NAPI_MODULE_VERSION 1 +#if defined(_MSC_VER) +#if defined(__cplusplus) +#define NAPI_C_CTOR(fn) \ + static void NAPI_CDECL fn(void); \ + namespace { \ + struct fn##_ { \ + fn##_() { fn(); } \ + } fn##_v_; \ + } \ + static void NAPI_CDECL fn(void) +#else // !defined(__cplusplus) +#pragma section(".CRT$XCU", read) +// The NAPI_C_CTOR macro defines a function fn that is called during CRT +// initialization. +// C does not support dynamic initialization of static variables and this code +// simulates C++ behavior. Exporting the function pointer prevents it from being +// optimized. See for details: +// https://docs.microsoft.com/en-us/cpp/c-runtime-library/crt-initialization?view=msvc-170 +#define NAPI_C_CTOR(fn) \ + static void NAPI_CDECL fn(void); \ + __declspec(dllexport, allocate(".CRT$XCU")) void(NAPI_CDECL * fn##_)(void) = \ + fn; \ + static void NAPI_CDECL fn(void) +#endif // defined(__cplusplus) +#else +#define NAPI_C_CTOR(fn) \ + static void fn(void) __attribute__((constructor)); \ + static void fn(void) +#endif + +#define NAPI_MODULE_X(modname, regfunc, priv, flags) \ + EXTERN_C_START \ + static napi_module _module = { \ + NAPI_MODULE_VERSION, \ + flags, \ + __FILE__, \ + regfunc, \ + #modname, \ + priv, \ + {0}, \ + }; \ + NAPI_C_CTOR(_register_##modname) { napi_module_register(&_module); } \ + EXTERN_C_END + #define NAPI_MODULE_INITIALIZER_X(base, version) \ NAPI_MODULE_INITIALIZER_X_HELPER(base, version) #define NAPI_MODULE_INITIALIZER_X_HELPER(base, version) base##version #ifdef __wasm32__ -#define NAPI_MODULE_INITIALIZER_BASE napi_register_wasm_v -#else -#define NAPI_MODULE_INITIALIZER_BASE napi_register_module_v +#define NAPI_WASM_INITIALIZER \ + NAPI_MODULE_INITIALIZER_X(napi_register_wasm_v, NAPI_MODULE_VERSION) +#define NAPI_MODULE(modname, regfunc) \ + EXTERN_C_START \ + NAPI_MODULE_EXPORT napi_value NAPI_WASM_INITIALIZER(napi_env env, \ + napi_value exports) { \ + return regfunc(env, exports); \ + } \ + EXTERN_C_END #endif +#define NAPI_MODULE_INITIALIZER_BASE napi_register_module_v + #define NAPI_MODULE_INITIALIZER \ NAPI_MODULE_INITIALIZER_X(NAPI_MODULE_INITIALIZER_BASE, NAPI_MODULE_VERSION) @@ -64,13 +115,13 @@ typedef struct napi_module { EXTERN_C_END \ napi_value NAPI_MODULE_INITIALIZER(napi_env env, napi_value exports) +#ifndef __wasm32__ #define NAPI_MODULE(modname, regfunc) \ NAPI_MODULE_INIT() { return regfunc(env, exports); } +#endif EXTERN_C_START -// Deprecated. Replaced by symbol-based registration defined by NAPI_MODULE -// and NAPI_MODULE_INIT macros. NAPI_EXTERN void NAPI_CDECL napi_module_register(napi_module* mod); NAPI_EXTERN NAPI_NO_RETURN void NAPI_CDECL From 398107540bdbb8868940efeefa57595a11dce7f0 Mon Sep 17 00:00:00 2001 From: Vladimir Morozov Date: Thu, 26 Jan 2023 09:22:15 -0800 Subject: [PATCH 3/8] restore original change + NAPI_MODULE_X --- src/node_api.h | 67 ++++++++------------------------------------------ 1 file changed, 10 insertions(+), 57 deletions(-) diff --git a/src/node_api.h b/src/node_api.h index a5f78ff8c62167..d769d2eb6f75c2 100644 --- a/src/node_api.h +++ b/src/node_api.h @@ -31,6 +31,7 @@ struct uv_loop_s; // Forward declaration. typedef napi_value(NAPI_CDECL* napi_addon_register_func)(napi_env env, napi_value exports); +// Used by deprecated registration method napi_module_register. typedef struct napi_module { int nm_version; unsigned int nm_flags; @@ -43,67 +44,15 @@ typedef struct napi_module { #define NAPI_MODULE_VERSION 1 -#if defined(_MSC_VER) -#if defined(__cplusplus) -#define NAPI_C_CTOR(fn) \ - static void NAPI_CDECL fn(void); \ - namespace { \ - struct fn##_ { \ - fn##_() { fn(); } \ - } fn##_v_; \ - } \ - static void NAPI_CDECL fn(void) -#else // !defined(__cplusplus) -#pragma section(".CRT$XCU", read) -// The NAPI_C_CTOR macro defines a function fn that is called during CRT -// initialization. -// C does not support dynamic initialization of static variables and this code -// simulates C++ behavior. Exporting the function pointer prevents it from being -// optimized. See for details: -// https://docs.microsoft.com/en-us/cpp/c-runtime-library/crt-initialization?view=msvc-170 -#define NAPI_C_CTOR(fn) \ - static void NAPI_CDECL fn(void); \ - __declspec(dllexport, allocate(".CRT$XCU")) void(NAPI_CDECL * fn##_)(void) = \ - fn; \ - static void NAPI_CDECL fn(void) -#endif // defined(__cplusplus) -#else -#define NAPI_C_CTOR(fn) \ - static void fn(void) __attribute__((constructor)); \ - static void fn(void) -#endif - -#define NAPI_MODULE_X(modname, regfunc, priv, flags) \ - EXTERN_C_START \ - static napi_module _module = { \ - NAPI_MODULE_VERSION, \ - flags, \ - __FILE__, \ - regfunc, \ - #modname, \ - priv, \ - {0}, \ - }; \ - NAPI_C_CTOR(_register_##modname) { napi_module_register(&_module); } \ - EXTERN_C_END - #define NAPI_MODULE_INITIALIZER_X(base, version) \ NAPI_MODULE_INITIALIZER_X_HELPER(base, version) #define NAPI_MODULE_INITIALIZER_X_HELPER(base, version) base##version #ifdef __wasm32__ -#define NAPI_WASM_INITIALIZER \ - NAPI_MODULE_INITIALIZER_X(napi_register_wasm_v, NAPI_MODULE_VERSION) -#define NAPI_MODULE(modname, regfunc) \ - EXTERN_C_START \ - NAPI_MODULE_EXPORT napi_value NAPI_WASM_INITIALIZER(napi_env env, \ - napi_value exports) { \ - return regfunc(env, exports); \ - } \ - EXTERN_C_END -#endif - +#define NAPI_MODULE_INITIALIZER_BASE napi_register_wasm_v +#else #define NAPI_MODULE_INITIALIZER_BASE napi_register_module_v +#endif #define NAPI_MODULE_INITIALIZER \ NAPI_MODULE_INITIALIZER_X(NAPI_MODULE_INITIALIZER_BASE, NAPI_MODULE_VERSION) @@ -115,13 +64,17 @@ typedef struct napi_module { EXTERN_C_END \ napi_value NAPI_MODULE_INITIALIZER(napi_env env, napi_value exports) -#ifndef __wasm32__ #define NAPI_MODULE(modname, regfunc) \ NAPI_MODULE_INIT() { return regfunc(env, exports); } -#endif + +// Deprecated. Use NAPI_MODULE. +#define NAPI_MODULE_X(modname, regfunc, priv, flags) \ + NAPI_MODULE(modname, regfunc) EXTERN_C_START +// Deprecated. Replaced by symbol-based registration defined by NAPI_MODULE +// and NAPI_MODULE_INIT macros. NAPI_EXTERN void NAPI_CDECL napi_module_register(napi_module* mod); NAPI_EXTERN NAPI_NO_RETURN void NAPI_CDECL From 7e53683ee34bd472297edefdcf7e67e448b13fe4 Mon Sep 17 00:00:00 2001 From: Vladimir Morozov Date: Thu, 2 Feb 2023 20:55:20 -0800 Subject: [PATCH 4/8] restore test_null_init test --- test/node-api/test_null_init/binding.gyp | 8 ++++ test/node-api/test_null_init/test.js | 7 +++ test/node-api/test_null_init/test_null_init.c | 47 +++++++++++++++++++ 3 files changed, 62 insertions(+) create mode 100644 test/node-api/test_null_init/binding.gyp create mode 100644 test/node-api/test_null_init/test.js create mode 100644 test/node-api/test_null_init/test_null_init.c diff --git a/test/node-api/test_null_init/binding.gyp b/test/node-api/test_null_init/binding.gyp new file mode 100644 index 00000000000000..27701616e338db --- /dev/null +++ b/test/node-api/test_null_init/binding.gyp @@ -0,0 +1,8 @@ +{ + 'targets': [ + { + 'target_name': 'test_null_init', + 'sources': [ 'test_null_init.c' ] + } + ] +} diff --git a/test/node-api/test_null_init/test.js b/test/node-api/test_null_init/test.js new file mode 100644 index 00000000000000..6e6bf51839bed0 --- /dev/null +++ b/test/node-api/test_null_init/test.js @@ -0,0 +1,7 @@ +'use strict'; +const common = require('../../common'); +const assert = require('assert'); + +assert.throws( + () => require(`./build/${common.buildType}/test_null_init`), + /Module has no declared entry point[.]/); diff --git a/test/node-api/test_null_init/test_null_init.c b/test/node-api/test_null_init/test_null_init.c new file mode 100644 index 00000000000000..ed0dad0c452e90 --- /dev/null +++ b/test/node-api/test_null_init/test_null_init.c @@ -0,0 +1,47 @@ +#include + +#if defined(_MSC_VER) +#if defined(__cplusplus) +#define NAPI_C_CTOR(fn) \ + static void NAPI_CDECL fn(void); \ + namespace { \ + struct fn##_ { \ + fn##_() { fn(); } \ + } fn##_v_; \ + } \ + static void NAPI_CDECL fn(void) +#else // !defined(__cplusplus) +#pragma section(".CRT$XCU", read) +// The NAPI_C_CTOR macro defines a function fn that is called during CRT +// initialization. +// C does not support dynamic initialization of static variables and this code +// simulates C++ behavior. Exporting the function pointer prevents it from being +// optimized. See for details: +// https://docs.microsoft.com/en-us/cpp/c-runtime-library/crt-initialization?view=msvc-170 +#define NAPI_C_CTOR(fn) \ + static void NAPI_CDECL fn(void); \ + __declspec(dllexport, allocate(".CRT$XCU")) void(NAPI_CDECL * fn##_)(void) = \ + fn; \ + static void NAPI_CDECL fn(void) +#endif // defined(__cplusplus) +#else +#define NAPI_C_CTOR(fn) \ + static void fn(void) __attribute__((constructor)); \ + static void fn(void) +#endif + +#define NAPI_MODULE_TEST(modname, regfunc) \ + EXTERN_C_START \ + static napi_module _module = { \ + NAPI_MODULE_VERSION, \ + 0, \ + __FILE__, \ + regfunc, \ + #modname, \ + NULL, \ + {0}, \ + }; \ + NAPI_C_CTOR(_register_##modname) { napi_module_register(&_module); } \ + EXTERN_C_END + +NAPI_MODULE_TEST(NODE_GYP_MODULE_NAME, NULL) From d0af42d9509c2904f08f82c6863392e020d6e38c Mon Sep 17 00:00:00 2001 From: Vladimir Morozov Date: Fri, 10 Mar 2023 07:17:53 -0800 Subject: [PATCH 5/8] Address PR feedback --- src/api/environment.cc | 30 +++++++++- src/node.h | 8 ++- src/node_api.h | 3 + test/cctest/test_linked_binding.cc | 56 +++++++++++++++++++ test/node-api/test_null_init/test_null_init.c | 6 ++ 5 files changed, 100 insertions(+), 3 deletions(-) diff --git a/src/api/environment.cc b/src/api/environment.cc index f56ee8d12bd1c5..93c12adda910f5 100644 --- a/src/api/environment.cc +++ b/src/api/environment.cc @@ -833,7 +833,9 @@ void AddLinkedBinding(Environment* env, const node_module& mod) { } void AddLinkedBinding(Environment* env, const napi_module& mod) { - AddLinkedBinding(env, napi_module_to_node_module(&mod)); + node_module node_mod = napi_module_to_node_module(&mod); + node_mod.nm_flags = NM_F_LINKED; + AddLinkedBinding(env, node_mod); } void AddLinkedBinding(Environment* env, @@ -854,6 +856,32 @@ void AddLinkedBinding(Environment* env, AddLinkedBinding(env, mod); } +void AddLinkedBinding(Environment* env, + const char* name, + napi_addon_register_func fn) { + node_module mod = { + -1, + NM_F_LINKED, + nullptr, // nm_dso_handle + nullptr, // nm_filename + nullptr, // nm_register_func + [](v8::Local exports, + v8::Local module, + v8::Local context, + void* priv) { + napi_module_register_by_symbol( + exports, + module, + context, + reinterpret_cast(priv)); + }, + name, + fn, + nullptr // nm_link + }; + AddLinkedBinding(env, mod); +} + static std::atomic next_thread_id{0}; ThreadId AllocateEnvironmentThreadId() { diff --git a/src/node.h b/src/node.h index fc2531f8677e70..e5b23f30d9530b 100644 --- a/src/node.h +++ b/src/node.h @@ -75,6 +75,9 @@ #include "v8-platform.h" // NOLINT(build/include_order) #include "node_version.h" // NODE_MODULE_VERSION +#define NAPI_EXPERIMENTAL +#include "node_api.h" + #include #include #include @@ -121,8 +124,6 @@ // Forward-declare libuv loop struct uv_loop_s; -struct napi_module; - // Forward-declare these functions now to stop MSVS from becoming // terminally confused when it's done in node_internals.h namespace node { @@ -1235,6 +1236,9 @@ NODE_EXTERN void AddLinkedBinding(Environment* env, const char* name, addon_context_register_func fn, void* priv); +NODE_EXTERN void AddLinkedBinding(Environment* env, + const char* name, + napi_addon_register_func fn); /* Registers a callback with the passed-in Environment instance. The callback * is called after the event loop exits, but before the VM is disposed. diff --git a/src/node_api.h b/src/node_api.h index d769d2eb6f75c2..0e41aa79ad4d62 100644 --- a/src/node_api.h +++ b/src/node_api.h @@ -75,6 +75,9 @@ EXTERN_C_START // Deprecated. Replaced by symbol-based registration defined by NAPI_MODULE // and NAPI_MODULE_INIT macros. +#if defined(__cplusplus) && __cplusplus >= 201402L +[[deprecated]] +#endif NAPI_EXTERN void NAPI_CDECL napi_module_register(napi_module* mod); NAPI_EXTERN NAPI_NO_RETURN void NAPI_CDECL diff --git a/test/cctest/test_linked_binding.cc b/test/cctest/test_linked_binding.cc index 7e40068b5db799..b6c304e32843da 100644 --- a/test/cctest/test_linked_binding.cc +++ b/test/cctest/test_linked_binding.cc @@ -127,6 +127,29 @@ TEST_F(LinkedBindingTest, LocallyDefinedLinkedBindingNapiTest) { CHECK_EQ(strcmp(*utf8val, "world"), 0); } +TEST_F(LinkedBindingTest, LocallyDefinedLinkedBindingNapiCallbackTest) { + const v8::HandleScope handle_scope(isolate_); + const Argv argv; + Env test_env {handle_scope, argv}; + + AddLinkedBinding(*test_env, "local_linked_napi", InitializeLocalNapiBinding); + + v8::Local context = isolate_->GetCurrentContext(); + + const char* run_script = + "process._linkedBinding('local_linked_napi').hello"; + v8::Local script = v8::Script::Compile( + context, + v8::String::NewFromOneByte(isolate_, + reinterpret_cast(run_script)) + .ToLocalChecked()) + .ToLocalChecked(); + v8::Local completion_value = script->Run(context).ToLocalChecked(); + v8::String::Utf8Value utf8val(isolate_, completion_value); + CHECK_NOT_NULL(*utf8val); + CHECK_EQ(strcmp(*utf8val, "world"), 0); +} + napi_value NapiLinkedWithInstanceData(napi_env env, napi_value exports) { int* instance_data = new int(0); CHECK_EQ( @@ -191,6 +214,39 @@ TEST_F(LinkedBindingTest, LocallyDefinedLinkedBindingNapiInstanceDataTest) { delete instance_data; } +TEST_F(LinkedBindingTest, LocallyDefinedLinkedBindingNapiCallbackInstanceDataTest) { + const v8::HandleScope handle_scope(isolate_); + int* instance_data = nullptr; + + { + const Argv argv; + Env test_env {handle_scope, argv}; + + AddLinkedBinding(*test_env, "local_linked_napi_id", NapiLinkedWithInstanceData); + + v8::Local context = isolate_->GetCurrentContext(); + + const char* run_script = + "process._linkedBinding('local_linked_napi_id').hello"; + v8::Local script = v8::Script::Compile( + context, + v8::String::NewFromOneByte(isolate_, + reinterpret_cast(run_script)) + .ToLocalChecked()) + .ToLocalChecked(); + v8::Local completion_value = + script->Run(context).ToLocalChecked(); + CHECK(completion_value->IsExternal()); + instance_data = static_cast( + completion_value.As()->Value()); + CHECK_NE(instance_data, nullptr); + CHECK_EQ(*instance_data, 0); + } + + CHECK_EQ(*instance_data, 1); + delete instance_data; +} + TEST_F(LinkedBindingTest, ManyBindingsTest) { const v8::HandleScope handle_scope(isolate_); const Argv argv; diff --git a/test/node-api/test_null_init/test_null_init.c b/test/node-api/test_null_init/test_null_init.c index ed0dad0c452e90..28c283b89240f7 100644 --- a/test/node-api/test_null_init/test_null_init.c +++ b/test/node-api/test_null_init/test_null_init.c @@ -1,5 +1,11 @@ #include +// This test uses old module initialization style deprecated in current code. +// The goal is to see that all previously compiled code continues to work the +// same way as before. +// The test has a copy of previous macro definitions which are removed from +// the node_api.h file. + #if defined(_MSC_VER) #if defined(__cplusplus) #define NAPI_C_CTOR(fn) \ From 6e19806429db3e974ee92ecbe35295b3bde53136 Mon Sep 17 00:00:00 2001 From: Vladimir Morozov Date: Fri, 10 Mar 2023 11:26:05 -0800 Subject: [PATCH 6/8] format test_linked_binding.cc --- test/cctest/test_linked_binding.cc | 227 +++++++++++++++-------------- 1 file changed, 115 insertions(+), 112 deletions(-) diff --git a/test/cctest/test_linked_binding.cc b/test/cctest/test_linked_binding.cc index b6c304e32843da..1a4e6a838b5d72 100644 --- a/test/cctest/test_linked_binding.cc +++ b/test/cctest/test_linked_binding.cc @@ -1,19 +1,19 @@ -#include "node_test_fixture.h" #include "node_api.h" +#include "node_test_fixture.h" void InitializeBinding(v8::Local exports, v8::Local module, v8::Local context, void* priv) { v8::Isolate* isolate = context->GetIsolate(); - exports->Set( - context, - v8::String::NewFromOneByte(isolate, - reinterpret_cast("key")) - .ToLocalChecked(), - v8::String::NewFromOneByte(isolate, - reinterpret_cast("value")) - .ToLocalChecked()) + exports + ->Set(context, + v8::String::NewFromOneByte(isolate, + reinterpret_cast("key")) + .ToLocalChecked(), + v8::String::NewFromOneByte( + isolate, reinterpret_cast("value")) + .ToLocalChecked()) .FromJust(); } @@ -24,18 +24,18 @@ class LinkedBindingTest : public EnvironmentTestFixture {}; TEST_F(LinkedBindingTest, SimpleTest) { const v8::HandleScope handle_scope(isolate_); const Argv argv; - Env test_env {handle_scope, argv}; + Env test_env{handle_scope, argv}; v8::Local context = isolate_->GetCurrentContext(); - const char* run_script = - "process._linkedBinding('cctest_linkedbinding').key"; - v8::Local script = v8::Script::Compile( - context, - v8::String::NewFromOneByte(isolate_, - reinterpret_cast(run_script)) - .ToLocalChecked()) - .ToLocalChecked(); + const char* run_script = "process._linkedBinding('cctest_linkedbinding').key"; + v8::Local script = + v8::Script::Compile( + context, + v8::String::NewFromOneByte( + isolate_, reinterpret_cast(run_script)) + .ToLocalChecked()) + .ToLocalChecked(); v8::Local completion_value = script->Run(context).ToLocalChecked(); v8::String::Utf8Value utf8val(isolate_, completion_value); CHECK_NOT_NULL(*utf8val); @@ -48,35 +48,35 @@ void InitializeLocalBinding(v8::Local exports, void* priv) { ++*static_cast(priv); v8::Isolate* isolate = context->GetIsolate(); - exports->Set( - context, - v8::String::NewFromOneByte(isolate, - reinterpret_cast("key")) - .ToLocalChecked(), - v8::String::NewFromOneByte(isolate, - reinterpret_cast("value")) - .ToLocalChecked()) + exports + ->Set(context, + v8::String::NewFromOneByte(isolate, + reinterpret_cast("key")) + .ToLocalChecked(), + v8::String::NewFromOneByte( + isolate, reinterpret_cast("value")) + .ToLocalChecked()) .FromJust(); } TEST_F(LinkedBindingTest, LocallyDefinedLinkedBindingTest) { const v8::HandleScope handle_scope(isolate_); const Argv argv; - Env test_env {handle_scope, argv}; + Env test_env{handle_scope, argv}; int calls = 0; AddLinkedBinding(*test_env, "local_linked", InitializeLocalBinding, &calls); v8::Local context = isolate_->GetCurrentContext(); - const char* run_script = - "process._linkedBinding('local_linked').key"; - v8::Local script = v8::Script::Compile( - context, - v8::String::NewFromOneByte(isolate_, - reinterpret_cast(run_script)) - .ToLocalChecked()) - .ToLocalChecked(); + const char* run_script = "process._linkedBinding('local_linked').key"; + v8::Local script = + v8::Script::Compile( + context, + v8::String::NewFromOneByte( + isolate_, reinterpret_cast(run_script)) + .ToLocalChecked()) + .ToLocalChecked(); v8::Local completion_value = script->Run(context).ToLocalChecked(); v8::String::Utf8Value utf8val(isolate_, completion_value); CHECK_NOT_NULL(*utf8val); @@ -86,41 +86,41 @@ TEST_F(LinkedBindingTest, LocallyDefinedLinkedBindingTest) { napi_value InitializeLocalNapiBinding(napi_env env, napi_value exports) { napi_value key, value; - CHECK_EQ( - napi_create_string_utf8(env, "hello", NAPI_AUTO_LENGTH, &key), napi_ok); - CHECK_EQ( - napi_create_string_utf8(env, "world", NAPI_AUTO_LENGTH, &value), napi_ok); + CHECK_EQ(napi_create_string_utf8(env, "hello", NAPI_AUTO_LENGTH, &key), + napi_ok); + CHECK_EQ(napi_create_string_utf8(env, "world", NAPI_AUTO_LENGTH, &value), + napi_ok); CHECK_EQ(napi_set_property(env, exports, key, value), napi_ok); return nullptr; } static napi_module local_linked_napi = { - NAPI_MODULE_VERSION, - node::ModuleFlags::kLinked, - nullptr, - InitializeLocalNapiBinding, - "local_linked_napi", - nullptr, - {0}, + NAPI_MODULE_VERSION, + node::ModuleFlags::kLinked, + nullptr, + InitializeLocalNapiBinding, + "local_linked_napi", + nullptr, + {0}, }; TEST_F(LinkedBindingTest, LocallyDefinedLinkedBindingNapiTest) { const v8::HandleScope handle_scope(isolate_); const Argv argv; - Env test_env {handle_scope, argv}; + Env test_env{handle_scope, argv}; AddLinkedBinding(*test_env, local_linked_napi); v8::Local context = isolate_->GetCurrentContext(); - const char* run_script = - "process._linkedBinding('local_linked_napi').hello"; - v8::Local script = v8::Script::Compile( - context, - v8::String::NewFromOneByte(isolate_, - reinterpret_cast(run_script)) - .ToLocalChecked()) - .ToLocalChecked(); + const char* run_script = "process._linkedBinding('local_linked_napi').hello"; + v8::Local script = + v8::Script::Compile( + context, + v8::String::NewFromOneByte( + isolate_, reinterpret_cast(run_script)) + .ToLocalChecked()) + .ToLocalChecked(); v8::Local completion_value = script->Run(context).ToLocalChecked(); v8::String::Utf8Value utf8val(isolate_, completion_value); CHECK_NOT_NULL(*utf8val); @@ -130,20 +130,20 @@ TEST_F(LinkedBindingTest, LocallyDefinedLinkedBindingNapiTest) { TEST_F(LinkedBindingTest, LocallyDefinedLinkedBindingNapiCallbackTest) { const v8::HandleScope handle_scope(isolate_); const Argv argv; - Env test_env {handle_scope, argv}; + Env test_env{handle_scope, argv}; AddLinkedBinding(*test_env, "local_linked_napi", InitializeLocalNapiBinding); v8::Local context = isolate_->GetCurrentContext(); - const char* run_script = - "process._linkedBinding('local_linked_napi').hello"; - v8::Local script = v8::Script::Compile( - context, - v8::String::NewFromOneByte(isolate_, - reinterpret_cast(run_script)) - .ToLocalChecked()) - .ToLocalChecked(); + const char* run_script = "process._linkedBinding('local_linked_napi').hello"; + v8::Local script = + v8::Script::Compile( + context, + v8::String::NewFromOneByte( + isolate_, reinterpret_cast(run_script)) + .ToLocalChecked()) + .ToLocalChecked(); v8::Local completion_value = script->Run(context).ToLocalChecked(); v8::String::Utf8Value utf8val(isolate_, completion_value); CHECK_NOT_NULL(*utf8val); @@ -152,33 +152,32 @@ TEST_F(LinkedBindingTest, LocallyDefinedLinkedBindingNapiCallbackTest) { napi_value NapiLinkedWithInstanceData(napi_env env, napi_value exports) { int* instance_data = new int(0); - CHECK_EQ( - napi_set_instance_data( - env, - instance_data, - [](napi_env env, void* data, void* hint) { - ++*static_cast(data); - }, nullptr), - napi_ok); + CHECK_EQ(napi_set_instance_data( + env, + instance_data, + [](napi_env env, void* data, void* hint) { + ++*static_cast(data); + }, + nullptr), + napi_ok); napi_value key, value; - CHECK_EQ( - napi_create_string_utf8(env, "hello", NAPI_AUTO_LENGTH, &key), napi_ok); - CHECK_EQ( - napi_create_external(env, instance_data, nullptr, nullptr, &value), - napi_ok); + CHECK_EQ(napi_create_string_utf8(env, "hello", NAPI_AUTO_LENGTH, &key), + napi_ok); + CHECK_EQ(napi_create_external(env, instance_data, nullptr, nullptr, &value), + napi_ok); CHECK_EQ(napi_set_property(env, exports, key, value), napi_ok); return nullptr; } static napi_module local_linked_napi_id = { - NAPI_MODULE_VERSION, - node::ModuleFlags::kLinked, - nullptr, - NapiLinkedWithInstanceData, - "local_linked_napi_id", - nullptr, - {0}, + NAPI_MODULE_VERSION, + node::ModuleFlags::kLinked, + nullptr, + NapiLinkedWithInstanceData, + "local_linked_napi_id", + nullptr, + {0}, }; TEST_F(LinkedBindingTest, LocallyDefinedLinkedBindingNapiInstanceDataTest) { @@ -187,7 +186,7 @@ TEST_F(LinkedBindingTest, LocallyDefinedLinkedBindingNapiInstanceDataTest) { { const Argv argv; - Env test_env {handle_scope, argv}; + Env test_env{handle_scope, argv}; AddLinkedBinding(*test_env, local_linked_napi_id); @@ -195,17 +194,18 @@ TEST_F(LinkedBindingTest, LocallyDefinedLinkedBindingNapiInstanceDataTest) { const char* run_script = "process._linkedBinding('local_linked_napi_id').hello"; - v8::Local script = v8::Script::Compile( - context, - v8::String::NewFromOneByte(isolate_, - reinterpret_cast(run_script)) - .ToLocalChecked()) - .ToLocalChecked(); + v8::Local script = + v8::Script::Compile( + context, + v8::String::NewFromOneByte( + isolate_, reinterpret_cast(run_script)) + .ToLocalChecked()) + .ToLocalChecked(); v8::Local completion_value = script->Run(context).ToLocalChecked(); CHECK(completion_value->IsExternal()); - instance_data = static_cast( - completion_value.As()->Value()); + instance_data = + static_cast(completion_value.As()->Value()); CHECK_NE(instance_data, nullptr); CHECK_EQ(*instance_data, 0); } @@ -214,31 +214,34 @@ TEST_F(LinkedBindingTest, LocallyDefinedLinkedBindingNapiInstanceDataTest) { delete instance_data; } -TEST_F(LinkedBindingTest, LocallyDefinedLinkedBindingNapiCallbackInstanceDataTest) { +TEST_F(LinkedBindingTest, + LocallyDefinedLinkedBindingNapiCallbackInstanceDataTest) { const v8::HandleScope handle_scope(isolate_); int* instance_data = nullptr; { const Argv argv; - Env test_env {handle_scope, argv}; + Env test_env{handle_scope, argv}; - AddLinkedBinding(*test_env, "local_linked_napi_id", NapiLinkedWithInstanceData); + AddLinkedBinding( + *test_env, "local_linked_napi_id", NapiLinkedWithInstanceData); v8::Local context = isolate_->GetCurrentContext(); const char* run_script = "process._linkedBinding('local_linked_napi_id').hello"; - v8::Local script = v8::Script::Compile( - context, - v8::String::NewFromOneByte(isolate_, - reinterpret_cast(run_script)) - .ToLocalChecked()) - .ToLocalChecked(); + v8::Local script = + v8::Script::Compile( + context, + v8::String::NewFromOneByte( + isolate_, reinterpret_cast(run_script)) + .ToLocalChecked()) + .ToLocalChecked(); v8::Local completion_value = script->Run(context).ToLocalChecked(); CHECK(completion_value->IsExternal()); - instance_data = static_cast( - completion_value.As()->Value()); + instance_data = + static_cast(completion_value.As()->Value()); CHECK_NE(instance_data, nullptr); CHECK_EQ(*instance_data, 0); } @@ -250,7 +253,7 @@ TEST_F(LinkedBindingTest, LocallyDefinedLinkedBindingNapiCallbackInstanceDataTes TEST_F(LinkedBindingTest, ManyBindingsTest) { const v8::HandleScope handle_scope(isolate_); const Argv argv; - Env test_env {handle_scope, argv}; + Env test_env{handle_scope, argv}; int calls = 0; AddLinkedBinding(*test_env, "local_linked1", InitializeLocalBinding, &calls); @@ -265,16 +268,16 @@ TEST_F(LinkedBindingTest, ManyBindingsTest) { const char* run_script = "for (let i = 1; i <= 5; i++)process._linkedBinding(`local_linked${i}`);" "process._linkedBinding('local_linked_napi').hello"; - v8::Local script = v8::Script::Compile( - context, - v8::String::NewFromOneByte(isolate_, - reinterpret_cast(run_script)) - .ToLocalChecked()) - .ToLocalChecked(); + v8::Local script = + v8::Script::Compile( + context, + v8::String::NewFromOneByte( + isolate_, reinterpret_cast(run_script)) + .ToLocalChecked()) + .ToLocalChecked(); v8::Local completion_value = script->Run(context).ToLocalChecked(); v8::String::Utf8Value utf8val(isolate_, completion_value); CHECK_NOT_NULL(*utf8val); CHECK_EQ(strcmp(*utf8val, "world"), 0); CHECK_EQ(calls, 5); } - From 16121e3c77a4e135bb2f43b40140a4c9ea6ec2fb Mon Sep 17 00:00:00 2001 From: Vladimir Morozov Date: Fri, 10 Mar 2023 16:08:06 -0800 Subject: [PATCH 7/8] Format node_api.h --- src/node_api.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/node_api.h b/src/node_api.h index 0e41aa79ad4d62..4d1b50414e2e02 100644 --- a/src/node_api.h +++ b/src/node_api.h @@ -78,7 +78,8 @@ EXTERN_C_START #if defined(__cplusplus) && __cplusplus >= 201402L [[deprecated]] #endif -NAPI_EXTERN void NAPI_CDECL napi_module_register(napi_module* mod); +NAPI_EXTERN void NAPI_CDECL +napi_module_register(napi_module* mod); NAPI_EXTERN NAPI_NO_RETURN void NAPI_CDECL napi_fatal_error(const char* location, From 96681889abe5465143d47523fb9d18aec899dd00 Mon Sep 17 00:00:00 2001 From: Vladimir Morozov Date: Sat, 11 Mar 2023 06:17:31 -0800 Subject: [PATCH 8/8] attempt to fix compilation on Linux --- src/api/environment.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/api/environment.cc b/src/api/environment.cc index 93c12adda910f5..239b14e44add88 100644 --- a/src/api/environment.cc +++ b/src/api/environment.cc @@ -876,7 +876,7 @@ void AddLinkedBinding(Environment* env, reinterpret_cast(priv)); }, name, - fn, + reinterpret_cast(fn), nullptr // nm_link }; AddLinkedBinding(env, mod);