From 2335bcd6e66f2a9ac2b1c23f6deb8e3a1e9c28b3 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Fri, 15 Feb 2019 11:57:19 +0100 Subject: [PATCH] n-api: turn NAPI_CALL_INTO_MODULE into a function These do not need to be macros. PR-URL: https://github.com/nodejs/node/pull/26128 Reviewed-By: Colin Ihrig Reviewed-By: James M Snell --- src/js_native_api_v8.cc | 7 ++++--- src/js_native_api_v8.h | 35 +++++++++++++++++++---------------- src/node_api.cc | 26 ++++++++++++++------------ 3 files changed, 37 insertions(+), 31 deletions(-) diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc index 287142e2658907..fd3ed2af6afe06 100644 --- a/src/js_native_api_v8.cc +++ b/src/js_native_api_v8.cc @@ -303,11 +303,12 @@ class Reference : private Finalizer { napi_env env = reference->_env; if (reference->_finalize_callback != nullptr) { - NAPI_CALL_INTO_MODULE_THROW(env, + NapiCallIntoModuleThrow(env, [&]() { reference->_finalize_callback( reference->_env, reference->_finalize_data, - reference->_finalize_hint)); + reference->_finalize_hint); + }); } // this is safe because if a request to delete the reference @@ -448,7 +449,7 @@ class CallbackWrapperBase : public CallbackWrapper { napi_callback cb = _bundle->*FunctionField; napi_value result; - NAPI_CALL_INTO_MODULE_THROW(env, result = cb(env, cbinfo_wrapper)); + NapiCallIntoModuleThrow(env, [&]() { result = cb(env, cbinfo_wrapper); }); if (result != nullptr) { this->SetReturnValue(result); diff --git a/src/js_native_api_v8.h b/src/js_native_api_v8.h index 094fc4afcc044c..d746660df42fd5 100644 --- a/src/js_native_api_v8.h +++ b/src/js_native_api_v8.h @@ -113,23 +113,26 @@ napi_status napi_set_last_error(napi_env env, napi_status error_code, } \ } while (0) -#define NAPI_CALL_INTO_MODULE(env, call, handle_exception) \ - do { \ - int open_handle_scopes = (env)->open_handle_scopes; \ - int open_callback_scopes = (env)->open_callback_scopes; \ - napi_clear_last_error((env)); \ - call; \ - CHECK_EQ((env)->open_handle_scopes, open_handle_scopes); \ - CHECK_EQ((env)->open_callback_scopes, open_callback_scopes); \ - if (!(env)->last_exception.IsEmpty()) { \ - handle_exception( \ - v8::Local::New((env)->isolate, (env)->last_exception)); \ - (env)->last_exception.Reset(); \ - } \ - } while (0) +template +void NapiCallIntoModule(napi_env env, T&& call, U&& handle_exception) { + int open_handle_scopes = env->open_handle_scopes; + int open_callback_scopes = env->open_callback_scopes; + napi_clear_last_error(env); + call(); + CHECK_EQ(env->open_handle_scopes, open_handle_scopes); + CHECK_EQ(env->open_callback_scopes, open_callback_scopes); + if (!env->last_exception.IsEmpty()) { + handle_exception(env->last_exception.Get(env->isolate)); + env->last_exception.Reset(); + } +} -#define NAPI_CALL_INTO_MODULE_THROW(env, call) \ - NAPI_CALL_INTO_MODULE((env), call, (env)->isolate->ThrowException) +template +void NapiCallIntoModuleThrow(napi_env env, T&& call) { + NapiCallIntoModule(env, call, [&](v8::Local value) { + env->isolate->ThrowException(value); + }); +} namespace v8impl { diff --git a/src/node_api.cc b/src/node_api.cc index 010fb3fe0ff840..282ef255dcaab4 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -34,11 +34,12 @@ class BufferFinalizer: private Finalizer { static void FinalizeBufferCallback(char* data, void* hint) { BufferFinalizer* finalizer = static_cast(hint); if (finalizer->_finalize_callback != nullptr) { - NAPI_CALL_INTO_MODULE_THROW(finalizer->_env, + NapiCallIntoModuleThrow(finalizer->_env, [&]() { finalizer->_finalize_callback( finalizer->_env, data, - finalizer->_finalize_hint)); + finalizer->_finalize_hint); + }); } Delete(finalizer); @@ -465,8 +466,9 @@ void napi_module_register_by_symbol(v8::Local exports, napi_env env = v8impl::GetEnv(context); napi_value _exports; - NAPI_CALL_INTO_MODULE_THROW(env, - _exports = init(env, v8impl::JsValueFromV8LocalValue(exports))); + NapiCallIntoModuleThrow(env, [&]() { + _exports = init(env, v8impl::JsValueFromV8LocalValue(exports)); + }); // If register function returned a non-null exports object different from // the exports object we passed it, set that as the "exports" property of @@ -874,14 +876,14 @@ class Work : public node::AsyncResource, public node::ThreadPoolWork { // stored. napi_env env = _env; - NAPI_CALL_INTO_MODULE(env, - _complete(_env, ConvertUVErrorCode(status), _data), - [env] (v8::Local local_err) { - // If there was an unhandled exception in the complete callback, - // report it as a fatal exception. (There is no JavaScript on the - // callstack that can possibly handle it.) - v8impl::trigger_fatal_exception(env, local_err); - }); + NapiCallIntoModule(env, [&]() { + _complete(_env, ConvertUVErrorCode(status), _data); + }, [env](v8::Local local_err) { + // If there was an unhandled exception in the complete callback, + // report it as a fatal exception. (There is no JavaScript on the + // callstack that can possibly handle it.) + v8impl::trigger_fatal_exception(env, local_err); + }); // Note: Don't access `work` after this point because it was // likely deleted by the complete callback.