From 07652f6d7c039825845961756959e6dbb4a3da8c Mon Sep 17 00:00:00 2001 From: Roy Wright Date: Tue, 14 Aug 2018 16:51:51 +0300 Subject: [PATCH] test: Tighten up compiler warnings PR-URL: https://github.com/nodejs/node-addon-api/pull/315 Reviewed-By: Michael Dawson Reviewed-By: Gabriel Schulhof --- napi-inl.h | 81 +++++++++++++++++++++++++-------------------- test/arraybuffer.cc | 4 +-- test/binding.gyp | 2 ++ test/buffer.cc | 4 +-- test/error.cc | 2 +- test/external.cc | 4 +-- test/objectwrap.cc | 4 +-- 7 files changed, 57 insertions(+), 44 deletions(-) diff --git a/napi-inl.h b/napi-inl.h index 620cac5..59362e7 100644 --- a/napi-inl.h +++ b/napi-inl.h @@ -27,6 +27,9 @@ namespace details { #define NAPI_THROW_IF_FAILED(env, status, ...) \ if ((status) != napi_ok) throw Error::New(env); +#define NAPI_THROW_IF_FAILED_VOID(env, status) \ + if ((status) != napi_ok) throw Error::New(env); + #else // NAPI_CPP_EXCEPTIONS #define NAPI_THROW(e) (e).ThrowAsJavaScriptException(); @@ -40,6 +43,14 @@ namespace details { return __VA_ARGS__; \ } +// We need a _VOID version of this macro to avoid warnings resulting from +// leaving the NAPI_THROW_IF_FAILED `...` argument empty. +#define NAPI_THROW_IF_FAILED_VOID(env, status) \ + if ((status) != napi_ok) { \ + Error::New(env).ThrowAsJavaScriptException(); \ + return; \ + } + #endif // NAPI_CPP_EXCEPTIONS #define NAPI_FATAL_IF_FAILED(status, location, message) \ @@ -160,7 +171,7 @@ struct AccessorCallbackData { napi_value exports) { \ return Napi::RegisterModule(env, exports, regfunc); \ } \ - NAPI_MODULE(modname, __napi_ ## regfunc); + NAPI_MODULE(modname, __napi_ ## regfunc) // Adapt the NAPI_MODULE registration function: // - Wrap the arguments in NAPI wrappers. @@ -867,21 +878,21 @@ template inline void Object::Set(napi_value key, const ValueType& value) { napi_status status = napi_set_property(_env, _value, key, Value::From(_env, value)); - NAPI_THROW_IF_FAILED(_env, status); + NAPI_THROW_IF_FAILED_VOID(_env, status); } template inline void Object::Set(Value key, const ValueType& value) { napi_status status = napi_set_property(_env, _value, key, Value::From(_env, value)); - NAPI_THROW_IF_FAILED(_env, status); + NAPI_THROW_IF_FAILED_VOID(_env, status); } template inline void Object::Set(const char* utf8name, const ValueType& value) { napi_status status = napi_set_named_property(_env, _value, utf8name, Value::From(_env, value)); - NAPI_THROW_IF_FAILED(_env, status); + NAPI_THROW_IF_FAILED_VOID(_env, status); } template @@ -929,7 +940,7 @@ template inline void Object::Set(uint32_t index, const ValueType& value) { napi_status status = napi_set_element(_env, _value, index, Value::From(_env, value)); - NAPI_THROW_IF_FAILED(_env, status); + NAPI_THROW_IF_FAILED_VOID(_env, status); } inline bool Object::Delete(uint32_t index) { @@ -949,19 +960,19 @@ inline Array Object::GetPropertyNames() { inline void Object::DefineProperty(const PropertyDescriptor& property) { napi_status status = napi_define_properties(_env, _value, 1, reinterpret_cast(&property)); - NAPI_THROW_IF_FAILED(_env, status); + NAPI_THROW_IF_FAILED_VOID(_env, status); } inline void Object::DefineProperties(const std::initializer_list& properties) { napi_status status = napi_define_properties(_env, _value, properties.size(), reinterpret_cast(properties.begin())); - NAPI_THROW_IF_FAILED(_env, status); + NAPI_THROW_IF_FAILED_VOID(_env, status); } inline void Object::DefineProperties(const std::vector& properties) { napi_status status = napi_define_properties(_env, _value, properties.size(), reinterpret_cast(properties.data())); - NAPI_THROW_IF_FAILED(_env, status); + NAPI_THROW_IF_FAILED_VOID(_env, status); } inline bool Object::InstanceOf(const Function& constructor) const { @@ -1171,7 +1182,7 @@ inline void ArrayBuffer::EnsureInfo() const { // since they can never change during the lifetime of the ArrayBuffer. if (_data == nullptr) { napi_status status = napi_get_arraybuffer_info(_env, _value, &_data, &_length); - NAPI_THROW_IF_FAILED(_env, status); + NAPI_THROW_IF_FAILED_VOID(_env, status); } } @@ -1222,7 +1233,7 @@ inline DataView::DataView(napi_env env, napi_value value) : Object(env, value) { &_data /* data */, nullptr /* arrayBuffer */, nullptr /* byteOffset */); - NAPI_THROW_IF_FAILED(_env, status); + NAPI_THROW_IF_FAILED_VOID(_env, status); } inline Napi::ArrayBuffer DataView::ArrayBuffer() const { @@ -1466,7 +1477,7 @@ inline TypedArrayOf::TypedArrayOf(napi_env env, napi_value value) : TypedArray(env, value), _data(nullptr) { napi_status status = napi_get_typedarray_info( _env, _value, &_type, &_length, reinterpret_cast(&_data), nullptr, nullptr); - NAPI_THROW_IF_FAILED(_env, status); + NAPI_THROW_IF_FAILED_VOID(_env, status); } template @@ -1615,7 +1626,7 @@ inline Promise::Deferred Promise::Deferred::New(napi_env env) { inline Promise::Deferred::Deferred(napi_env env) : _env(env) { napi_status status = napi_create_promise(_env, &_deferred, &_promise); - NAPI_THROW_IF_FAILED(_env, status); + NAPI_THROW_IF_FAILED_VOID(_env, status); } inline Promise Promise::Deferred::Promise() const { @@ -1628,12 +1639,12 @@ inline Napi::Env Promise::Deferred::Env() const { inline void Promise::Deferred::Resolve(napi_value value) const { napi_status status = napi_resolve_deferred(_env, _deferred, value); - NAPI_THROW_IF_FAILED(_env, status); + NAPI_THROW_IF_FAILED_VOID(_env, status); } inline void Promise::Deferred::Reject(napi_value value) const { napi_status status = napi_reject_deferred(_env, _deferred, value); - NAPI_THROW_IF_FAILED(_env, status); + NAPI_THROW_IF_FAILED_VOID(_env, status); } inline Promise::Promise(napi_env env, napi_value value) : Object(env, value) { @@ -1752,7 +1763,7 @@ inline void Buffer::EnsureInfo() const { size_t byteLength; void* voidData; napi_status status = napi_get_buffer_info(_env, _value, &voidData, &byteLength); - NAPI_THROW_IF_FAILED(_env, status); + NAPI_THROW_IF_FAILED_VOID(_env, status); _length = byteLength / sizeof (T); _data = static_cast(voidData); } @@ -1888,7 +1899,7 @@ inline void Error::ThrowAsJavaScriptException() const { HandleScope scope(_env); if (!IsEmpty()) { napi_status status = napi_throw(_env, Value()); - NAPI_THROW_IF_FAILED(_env, status); + NAPI_THROW_IF_FAILED_VOID(_env, status); } } @@ -2077,7 +2088,7 @@ template inline void Reference::Reset() { if (_ref != nullptr) { napi_status status = napi_delete_reference(_env, _ref); - NAPI_THROW_IF_FAILED(_env, status); + NAPI_THROW_IF_FAILED_VOID(_env, status); _ref = nullptr; } } @@ -2090,7 +2101,7 @@ inline void Reference::Reset(const T& value, uint32_t refcount) { napi_value val = value; if (val != nullptr) { napi_status status = napi_create_reference(_env, value, refcount, &_ref); - NAPI_THROW_IF_FAILED(_env, status); + NAPI_THROW_IF_FAILED_VOID(_env, status); } } @@ -2364,7 +2375,7 @@ inline CallbackInfo::CallbackInfo(napi_env env, napi_callback_info info) _argc = _staticArgCount; _argv = _staticArgs; napi_status status = napi_get_cb_info(env, info, &_argc, _argv, &_this, &_data); - NAPI_THROW_IF_FAILED(_env, status); + NAPI_THROW_IF_FAILED_VOID(_env, status); if (_argc > _staticArgCount) { // Use either a fixed-size array (on the stack) or a dynamically-allocated @@ -2373,7 +2384,7 @@ inline CallbackInfo::CallbackInfo(napi_env env, napi_callback_info info) _argv = _dynamicArgs; status = napi_get_cb_info(env, info, &_argc, _argv, nullptr, nullptr); - NAPI_THROW_IF_FAILED(_env, status); + NAPI_THROW_IF_FAILED_VOID(_env, status); } } @@ -2430,7 +2441,7 @@ inline PropertyDescriptor PropertyDescriptor::Accessor(const char* utf8name, Getter getter, napi_property_attributes attributes, - void* data) { + void* /*data*/) { typedef details::CallbackData CbData; // TODO: Delete when the function is destroyed auto callbackData = new CbData({ getter, nullptr }); @@ -2459,7 +2470,7 @@ template inline PropertyDescriptor PropertyDescriptor::Accessor(napi_value name, Getter getter, napi_property_attributes attributes, - void* data) { + void* /*data*/) { typedef details::CallbackData CbData; // TODO: Delete when the function is destroyed auto callbackData = new CbData({ getter, nullptr }); @@ -2490,7 +2501,7 @@ inline PropertyDescriptor PropertyDescriptor::Accessor(const char* utf8name, Getter getter, Setter setter, napi_property_attributes attributes, - void* data) { + void* /*data*/) { typedef details::AccessorCallbackData CbData; // TODO: Delete when the function is destroyed auto callbackData = new CbData({ getter, setter }); @@ -2521,7 +2532,7 @@ inline PropertyDescriptor PropertyDescriptor::Accessor(napi_value name, Getter getter, Setter setter, napi_property_attributes attributes, - void* data) { + void* /*data*/) { typedef details::AccessorCallbackData CbData; // TODO: Delete when the function is destroyed auto callbackData = new CbData({ getter, setter }); @@ -2552,7 +2563,7 @@ template inline PropertyDescriptor PropertyDescriptor::Function(const char* utf8name, Callable cb, napi_property_attributes attributes, - void* data) { + void* /*data*/) { typedef decltype(cb(CallbackInfo(nullptr, nullptr))) ReturnType; typedef details::CallbackData CbData; // TODO: Delete when the function is destroyed @@ -2582,7 +2593,7 @@ template inline PropertyDescriptor PropertyDescriptor::Function(napi_value name, Callable cb, napi_property_attributes attributes, - void* data) { + void* /*data*/) { typedef decltype(cb(CallbackInfo(nullptr, nullptr))) ReturnType; typedef details::CallbackData CbData; // TODO: Delete when the function is destroyed @@ -2663,7 +2674,7 @@ inline ObjectWrap::ObjectWrap(const Napi::CallbackInfo& callbackInfo) { napi_ref ref; T* instance = static_cast(this); status = napi_wrap(env, wrapper, instance, FinalizeCallback, nullptr, &ref); - NAPI_THROW_IF_FAILED(env, status) + NAPI_THROW_IF_FAILED_VOID(env, status); Reference* instanceRef = instance; *instanceRef = Reference(env, ref); @@ -3031,7 +3042,7 @@ inline HandleScope::HandleScope(napi_env env, napi_handle_scope scope) inline HandleScope::HandleScope(Napi::Env env) : _env(env) { napi_status status = napi_open_handle_scope(_env, &_scope); - NAPI_THROW_IF_FAILED(_env, status); + NAPI_THROW_IF_FAILED_VOID(_env, status); } inline HandleScope::~HandleScope() { @@ -3056,7 +3067,7 @@ inline EscapableHandleScope::EscapableHandleScope( inline EscapableHandleScope::EscapableHandleScope(Napi::Env env) : _env(env) { napi_status status = napi_open_escapable_handle_scope(_env, &_scope); - NAPI_THROW_IF_FAILED(_env, status); + NAPI_THROW_IF_FAILED_VOID(_env, status); } inline EscapableHandleScope::~EscapableHandleScope() { @@ -3124,11 +3135,11 @@ inline AsyncWorker::AsyncWorker(const Object& receiver, napi_value resource_id; napi_status status = napi_create_string_latin1( _env, resource_name, NAPI_AUTO_LENGTH, &resource_id); - NAPI_THROW_IF_FAILED(_env, status); + NAPI_THROW_IF_FAILED_VOID(_env, status); status = napi_create_async_work(_env, resource, resource_id, OnExecute, OnWorkComplete, this, &_work); - NAPI_THROW_IF_FAILED(_env, status); + NAPI_THROW_IF_FAILED_VOID(_env, status); } inline AsyncWorker::~AsyncWorker() { @@ -3169,12 +3180,12 @@ inline Napi::Env AsyncWorker::Env() const { inline void AsyncWorker::Queue() { napi_status status = napi_queue_async_work(_env, _work); - NAPI_THROW_IF_FAILED(_env, status); + NAPI_THROW_IF_FAILED_VOID(_env, status); } inline void AsyncWorker::Cancel() { napi_status status = napi_cancel_async_work(_env, _work); - NAPI_THROW_IF_FAILED(_env, status); + NAPI_THROW_IF_FAILED_VOID(_env, status); } inline ObjectReference& AsyncWorker::Receiver() { @@ -3197,7 +3208,7 @@ inline void AsyncWorker::SetError(const std::string& error) { _error = error; } -inline void AsyncWorker::OnExecute(napi_env env, void* this_pointer) { +inline void AsyncWorker::OnExecute(napi_env /*env*/, void* this_pointer) { AsyncWorker* self = static_cast(this_pointer); #ifdef NAPI_CPP_EXCEPTIONS try { @@ -3211,7 +3222,7 @@ inline void AsyncWorker::OnExecute(napi_env env, void* this_pointer) { } inline void AsyncWorker::OnWorkComplete( - napi_env env, napi_status status, void* this_pointer) { + napi_env /*env*/, napi_status status, void* this_pointer) { AsyncWorker* self = static_cast(this_pointer); if (status != napi_cancelled) { HandleScope scope(self->_env); diff --git a/test/arraybuffer.cc b/test/arraybuffer.cc index b03492d..27bb993 100644 --- a/test/arraybuffer.cc +++ b/test/arraybuffer.cc @@ -63,7 +63,7 @@ Value CreateExternalBufferWithFinalize(const CallbackInfo& info) { info.Env(), data, testLength, - [](Env env, void* finalizeData) { + [](Env /*env*/, void* finalizeData) { delete[] static_cast(finalizeData); finalizeCount++; }); @@ -92,7 +92,7 @@ Value CreateExternalBufferWithFinalizeHint(const CallbackInfo& info) { info.Env(), data, testLength, - [](Env env, void* finalizeData, char* finalizeHint) { + [](Env /*env*/, void* finalizeData, char* /*finalizeHint*/) { delete[] static_cast(finalizeData); finalizeCount++; }, diff --git a/test/binding.gyp b/test/binding.gyp index 698eb45..52cb7c2 100644 --- a/test/binding.gyp +++ b/test/binding.gyp @@ -28,6 +28,8 @@ ], 'include_dirs': ["::New(info.Env(), new int(1), - [](Env env, int* data) { + [](Env /*env*/, int* data) { delete data; finalizeCount++; }); @@ -25,7 +25,7 @@ Value CreateExternalWithFinalizeHint(const CallbackInfo& info) { finalizeCount = 0; char* hint = nullptr; return External::New(info.Env(), new int(1), - [](Env env, int* data, char* hint) { + [](Env /*env*/, int* data, char* /*hint*/) { delete data; finalizeCount++; }, diff --git a/test/objectwrap.cc b/test/objectwrap.cc index 7dc5a8f..fae7b26 100644 --- a/test/objectwrap.cc +++ b/test/objectwrap.cc @@ -32,11 +32,11 @@ class Test : public Napi::ObjectWrap { return Napi::Number::New(info.Env(), value); } - Napi::Value Iter(const Napi::CallbackInfo& info) { + Napi::Value Iter(const Napi::CallbackInfo& /*info*/) { return Constructor.New({}); } - void Setter(const Napi::CallbackInfo& info, const Napi::Value& new_value) { + void Setter(const Napi::CallbackInfo& /*info*/, const Napi::Value& new_value) { value = new_value.As(); }