Skip to content

Commit

Permalink
test: Tighten up compiler warnings
Browse files Browse the repository at this point in the history
PR-URL: #315
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Gabriel Schulhof <[email protected]>
  • Loading branch information
mcheshkov authored and Gabriel Schulhof committed Sep 10, 2018
1 parent 7cdd787 commit 622ffae
Show file tree
Hide file tree
Showing 7 changed files with 57 additions and 44 deletions.
81 changes: 46 additions & 35 deletions napi-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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) \
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -867,21 +878,21 @@ template <typename ValueType>
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 <typename ValueType>
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 <typename ValueType>
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 <typename ValueType>
Expand Down Expand Up @@ -929,7 +940,7 @@ template <typename ValueType>
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) {
Expand All @@ -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<const napi_property_descriptor*>(&property));
NAPI_THROW_IF_FAILED(_env, status);
NAPI_THROW_IF_FAILED_VOID(_env, status);
}

inline void Object::DefineProperties(const std::initializer_list<PropertyDescriptor>& properties) {
napi_status status = napi_define_properties(_env, _value, properties.size(),
reinterpret_cast<const napi_property_descriptor*>(properties.begin()));
NAPI_THROW_IF_FAILED(_env, status);
NAPI_THROW_IF_FAILED_VOID(_env, status);
}

inline void Object::DefineProperties(const std::vector<PropertyDescriptor>& properties) {
napi_status status = napi_define_properties(_env, _value, properties.size(),
reinterpret_cast<const napi_property_descriptor*>(properties.data()));
NAPI_THROW_IF_FAILED(_env, status);
NAPI_THROW_IF_FAILED_VOID(_env, status);
}

inline bool Object::InstanceOf(const Function& constructor) const {
Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -1466,7 +1477,7 @@ inline TypedArrayOf<T>::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<void**>(&_data), nullptr, nullptr);
NAPI_THROW_IF_FAILED(_env, status);
NAPI_THROW_IF_FAILED_VOID(_env, status);
}

template <typename T>
Expand Down Expand Up @@ -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 {
Expand All @@ -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) {
Expand Down Expand Up @@ -1752,7 +1763,7 @@ inline void Buffer<T>::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<T*>(voidData);
}
Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -2077,7 +2088,7 @@ template <typename T>
inline void Reference<T>::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;
}
}
Expand All @@ -2090,7 +2101,7 @@ inline void Reference<T>::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);
}
}

Expand Down Expand Up @@ -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
Expand All @@ -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);
}
}

Expand Down Expand Up @@ -2430,7 +2441,7 @@ inline PropertyDescriptor
PropertyDescriptor::Accessor(const char* utf8name,
Getter getter,
napi_property_attributes attributes,
void* data) {
void* /*data*/) {
typedef details::CallbackData<Getter, Napi::Value> CbData;
// TODO: Delete when the function is destroyed
auto callbackData = new CbData({ getter, nullptr });
Expand Down Expand Up @@ -2459,7 +2470,7 @@ template <typename Getter>
inline PropertyDescriptor PropertyDescriptor::Accessor(napi_value name,
Getter getter,
napi_property_attributes attributes,
void* data) {
void* /*data*/) {
typedef details::CallbackData<Getter, Napi::Value> CbData;
// TODO: Delete when the function is destroyed
auto callbackData = new CbData({ getter, nullptr });
Expand Down Expand Up @@ -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<Getter, Setter> CbData;
// TODO: Delete when the function is destroyed
auto callbackData = new CbData({ getter, setter });
Expand Down Expand Up @@ -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<Getter, Setter> CbData;
// TODO: Delete when the function is destroyed
auto callbackData = new CbData({ getter, setter });
Expand Down Expand Up @@ -2552,7 +2563,7 @@ template <typename Callable>
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<Callable, ReturnType> CbData;
// TODO: Delete when the function is destroyed
Expand Down Expand Up @@ -2582,7 +2593,7 @@ template <typename Callable>
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<Callable, ReturnType> CbData;
// TODO: Delete when the function is destroyed
Expand Down Expand Up @@ -2663,7 +2674,7 @@ inline ObjectWrap<T>::ObjectWrap(const Napi::CallbackInfo& callbackInfo) {
napi_ref ref;
T* instance = static_cast<T*>(this);
status = napi_wrap(env, wrapper, instance, FinalizeCallback, nullptr, &ref);
NAPI_THROW_IF_FAILED(env, status)
NAPI_THROW_IF_FAILED_VOID(env, status);

Reference<Object>* instanceRef = instance;
*instanceRef = Reference<Object>(env, ref);
Expand Down Expand Up @@ -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() {
Expand All @@ -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() {
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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() {
Expand All @@ -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<AsyncWorker*>(this_pointer);
#ifdef NAPI_CPP_EXCEPTIONS
try {
Expand All @@ -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<AsyncWorker*>(this_pointer);
if (status != napi_cancelled) {
HandleScope scope(self->_env);
Expand Down
4 changes: 2 additions & 2 deletions test/arraybuffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ Value CreateExternalBufferWithFinalize(const CallbackInfo& info) {
info.Env(),
data,
testLength,
[](Env env, void* finalizeData) {
[](Env /*env*/, void* finalizeData) {
delete[] static_cast<uint8_t*>(finalizeData);
finalizeCount++;
});
Expand Down Expand Up @@ -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<uint8_t*>(finalizeData);
finalizeCount++;
},
Expand Down
2 changes: 2 additions & 0 deletions test/binding.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
],
'include_dirs': ["<!@(node -p \"require('../').include\")"],
'dependencies': ["<!(node -p \"require('../').gyp\")"],
'cflags': [ '-Werror', '-Wall', '-Wextra', '-Wpedantic', '-Wunused-parameter' ],
'cflags_cc': [ '-Werror', '-Wall', '-Wextra', '-Wpedantic', '-Wunused-parameter' ],
},
'targets': [
{
Expand Down
4 changes: 2 additions & 2 deletions test/buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ Value CreateExternalBufferWithFinalize(const CallbackInfo& info) {
info.Env(),
data,
testLength,
[](Env env, uint16_t* finalizeData) {
[](Env /*env*/, uint16_t* finalizeData) {
delete[] finalizeData;
finalizeCount++;
});
Expand Down Expand Up @@ -97,7 +97,7 @@ Value CreateExternalBufferWithFinalizeHint(const CallbackInfo& info) {
info.Env(),
data,
testLength,
[](Env env, uint16_t* finalizeData, char* finalizeHint) {
[](Env /*env*/, uint16_t* finalizeData, char* /*finalizeHint*/) {
delete[] finalizeData;
finalizeCount++;
},
Expand Down
2 changes: 1 addition & 1 deletion test/error.cc
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ void CatchAndRethrowErrorThatEscapesScope(const CallbackInfo& info) {

#endif // NAPI_CPP_EXCEPTIONS

void ThrowFatalError(const CallbackInfo& info) {
void ThrowFatalError(const CallbackInfo& /*info*/) {
Error::Fatal("Error::ThrowFatalError", "This is a fatal error");
}

Expand Down
4 changes: 2 additions & 2 deletions test/external.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ Value CreateExternal(const CallbackInfo& info) {
Value CreateExternalWithFinalize(const CallbackInfo& info) {
finalizeCount = 0;
return External<int>::New(info.Env(), new int(1),
[](Env env, int* data) {
[](Env /*env*/, int* data) {
delete data;
finalizeCount++;
});
Expand All @@ -25,7 +25,7 @@ Value CreateExternalWithFinalizeHint(const CallbackInfo& info) {
finalizeCount = 0;
char* hint = nullptr;
return External<int>::New(info.Env(), new int(1),
[](Env env, int* data, char* hint) {
[](Env /*env*/, int* data, char* /*hint*/) {
delete data;
finalizeCount++;
},
Expand Down
Loading

0 comments on commit 622ffae

Please sign in to comment.