From 284f94353a622b2cdb928491d6ea514667f074b8 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Wed, 1 May 2019 15:44:48 +0200 Subject: [PATCH 1/2] Add missing methods to Nan::Maybe Nan::Maybe was an alias for v8::Maybe with most Node.js versions but that's problematic because v8::Maybe has grown new methods over time that are missing in older versions. From now on we use our custom Maybe implementation with all Node.js versions. This commit also adds the following missing methods: * Maybe::Check() const * Maybe::To(T*) const * Maybe::ToChecked() const Fixes: https://github.com/nodejs/nan/issues/851 --- Makefile | 1 + nan.h | 64 ++++++++++++++++++++++++++++++++++++++++++ nan_maybe_43_inl.h | 13 --------- nan_maybe_pre_43_inl.h | 48 ------------------------------- test/binding.gyp | 4 +++ test/cpp/maybe.cpp | 39 +++++++++++++++++++++++++ test/js/maybe-test.js | 16 +++++++++++ 7 files changed, 124 insertions(+), 61 deletions(-) create mode 100644 test/cpp/maybe.cpp create mode 100644 test/js/maybe-test.js diff --git a/Makefile b/Makefile index 551c06de..45125242 100644 --- a/Makefile +++ b/Makefile @@ -55,6 +55,7 @@ LINT_SOURCES = \ test/cpp/json-parse.cpp \ test/cpp/json-stringify.cpp \ test/cpp/makecallback.cpp \ + test/cpp/maybe.cpp \ test/cpp/morenews.cpp \ test/cpp/multifile1.cpp \ test/cpp/multifile2.cpp \ diff --git a/nan.h b/nan.h index f8a5554a..f1f19cc9 100644 --- a/nan.h +++ b/nan.h @@ -213,6 +213,70 @@ template > class Persistent; #endif // NODE_MODULE_VERSION +template +class Maybe { + public: + inline bool IsNothing() const { return !has_value_; } + inline bool IsJust() const { return has_value_; } + + inline T ToChecked() const { return FromJust(); } + inline void Check() const { FromJust(); } + + inline bool To(T* out) const { + if (IsJust()) *out = value_; + return IsJust(); + } + + inline T FromJust() const { +#if defined(V8_ENABLE_CHECKS) + assert(IsJust() && "FromJust is Nothing"); +#endif // V8_ENABLE_CHECKS + return value_; + } + + inline T FromMaybe(const T& default_value) const { + return has_value_ ? value_ : default_value; + } + + inline bool operator==(const Maybe &other) const { + return (IsJust() == other.IsJust()) && + (!IsJust() || FromJust() == other.FromJust()); + } + + inline bool operator!=(const Maybe &other) const { + return !operator==(other); + } + +#if defined(V8_MAJOR_VERSION) && (V8_MAJOR_VERSION > 4 || \ + (V8_MAJOR_VERSION == 4 && defined(V8_MINOR_VERSION) && V8_MINOR_VERSION >= 3)) + // Allow implicit conversions from v8::Maybe to Nan::Maybe. + Maybe(const v8::Maybe& that) // NOLINT(runtime/explicit) + : has_value_(that.IsJust()) + , value_(that.FromMaybe(T())) {} +#endif + + private: + Maybe() : has_value_(false) {} + explicit Maybe(const T& t) : has_value_(true), value_(t) {} + bool has_value_; + T value_; + + template + friend Maybe Nothing(); + template + friend Maybe Just(const U& u); +}; + +template +inline Maybe Nothing() { + return Maybe(); +} + +template +inline Maybe Just(const T& t) { + return Maybe(t); +} + #if defined(V8_MAJOR_VERSION) && (V8_MAJOR_VERSION > 4 || \ (V8_MAJOR_VERSION == 4 && defined(V8_MINOR_VERSION) && V8_MINOR_VERSION >= 3)) # include "nan_maybe_43_inl.h" // NOLINT(build/include) diff --git a/nan_maybe_43_inl.h b/nan_maybe_43_inl.h index 91783da6..c04ce30d 100644 --- a/nan_maybe_43_inl.h +++ b/nan_maybe_43_inl.h @@ -12,19 +12,6 @@ template using MaybeLocal = v8::MaybeLocal; -template -using Maybe = v8::Maybe; - -template -inline Maybe Nothing() { - return v8::Nothing(); -} - -template -inline Maybe Just(const T& t) { - return v8::Just(t); -} - inline MaybeLocal ToDetailString(v8::Local val) { v8::Isolate *isolate = v8::Isolate::GetCurrent(); diff --git a/nan_maybe_pre_43_inl.h b/nan_maybe_pre_43_inl.h index 3847296d..83325ae0 100644 --- a/nan_maybe_pre_43_inl.h +++ b/nan_maybe_pre_43_inl.h @@ -48,54 +48,6 @@ class MaybeLocal { v8::Local val_; }; -template -class Maybe { - public: - inline bool IsNothing() const { return !has_value_; } - inline bool IsJust() const { return has_value_; } - - inline T FromJust() const { -#if defined(V8_ENABLE_CHECKS) - assert(IsJust() && "FromJust is Nothing"); -#endif // V8_ENABLE_CHECKS - return value_; - } - - inline T FromMaybe(const T& default_value) const { - return has_value_ ? value_ : default_value; - } - - inline bool operator==(const Maybe &other) const { - return (IsJust() == other.IsJust()) && - (!IsJust() || FromJust() == other.FromJust()); - } - - inline bool operator!=(const Maybe &other) const { - return !operator==(other); - } - - private: - Maybe() : has_value_(false) {} - explicit Maybe(const T& t) : has_value_(true), value_(t) {} - bool has_value_; - T value_; - - template - friend Maybe Nothing(); - template - friend Maybe Just(const U& u); -}; - -template -inline Maybe Nothing() { - return Maybe(); -} - -template -inline Maybe Just(const T& t) { - return Maybe(t); -} - inline MaybeLocal ToDetailString(v8::Handle val) { return MaybeLocal(val->ToDetailString()); diff --git a/test/binding.gyp b/test/binding.gyp index c2928357..09c4d326 100644 --- a/test/binding.gyp +++ b/test/binding.gyp @@ -90,6 +90,10 @@ "target_name" : "makecallback" , "sources" : [ "cpp/makecallback.cpp" ] } + , { + "target_name" : "maybe" + , "sources" : [ "cpp/maybe.cpp" ] + } , { "target_name" : "asyncresource" , "sources" : [ "cpp/asyncresource.cpp" ] diff --git a/test/cpp/maybe.cpp b/test/cpp/maybe.cpp new file mode 100644 index 00000000..d7a2729d --- /dev/null +++ b/test/cpp/maybe.cpp @@ -0,0 +1,39 @@ +/********************************************************************* + * NAN - Native Abstractions for Node.js + * + * Copyright (c) 2019 NAN contributors + * + * MIT License + ********************************************************************/ + +#include + +using namespace Nan; // NOLINT(build/namespaces) + +NAN_METHOD(Test) { + info.GetReturnValue().Set(false); + { + Maybe mb = Nothing(); + if (mb.IsJust()) return; + if (!mb.IsNothing()) return; + if (mb.To(NULL)) return; + } + { + Maybe mb = Just(42); + if (!mb.IsJust()) return; + if (mb.IsNothing()) return; + if (42 != mb.FromJust()) return; + if (42 != mb.ToChecked()) return; + mb.Check(); + int v; + if (!mb.To(&v)) return; + if (42 != v) return; + } + info.GetReturnValue().Set(true); +} + +NAN_MODULE_INIT(Init) { + SetMethod(target, "test", Test); +} + +NODE_MODULE(maybe, Init) diff --git a/test/js/maybe-test.js b/test/js/maybe-test.js new file mode 100644 index 00000000..fc653aa1 --- /dev/null +++ b/test/js/maybe-test.js @@ -0,0 +1,16 @@ +/********************************************************************* + * NAN - Native Abstractions for Node.js + * + * Copyright (c) 2019 NAN contributors + * + * MIT License + ********************************************************************/ + +const test = require('tap').test + , testRoot = require('path').resolve(__dirname, '..') + , bindings = require('bindings')({ module_root: testRoot, bindings: 'maybe' }) + +test('maybe', function (t) { + t.plan(1); + t.ok(bindings.test()); +}); From d1131c7a8a163f16360276df8467e51d647689e4 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Thu, 16 May 2019 10:11:55 +0200 Subject: [PATCH 2/2] Fix cpplint warnings. The code base is now `make lint`-clean again. --- nan.h | 3 ++- nan_implementation_12_inl.h | 3 ++- test/cpp/morenews.cpp | 12 ++++++++---- test/cpp/multifile2.cpp | 2 +- test/cpp/news.cpp | 21 ++++++++++++++------- test/cpp/setcallhandler.cpp | 3 ++- test/cpp/threadlocal.cpp | 3 ++- 7 files changed, 31 insertions(+), 16 deletions(-) diff --git a/nan.h b/nan.h index f1f19cc9..eea80164 100644 --- a/nan.h +++ b/nan.h @@ -1143,7 +1143,8 @@ class Utf8String { const int flags = v8::String::NO_NULL_TERMINATION | imp::kReplaceInvalidUtf8; #if NODE_MAJOR_VERSION >= 11 - length_ = string->WriteUtf8(v8::Isolate::GetCurrent(), str_, static_cast(len), 0, flags); + length_ = string->WriteUtf8(v8::Isolate::GetCurrent(), str_, + static_cast(len), 0, flags); #else // See https://github.com/nodejs/nan/issues/832. // Disable the warning as there is no way around it. diff --git a/nan_implementation_12_inl.h b/nan_implementation_12_inl.h index 8756e6ee..255293ac 100644 --- a/nan_implementation_12_inl.h +++ b/nan_implementation_12_inl.h @@ -347,7 +347,8 @@ Factory::return_t Factory::New(v8::Local value) { // V8 > 7.0 #if V8_MAJOR_VERSION > 7 || (V8_MAJOR_VERSION == 7 && V8_MINOR_VERSION > 0) - return v8::StringObject::New(v8::Isolate::GetCurrent(), value).As(); + return v8::StringObject::New(v8::Isolate::GetCurrent(), value) + .As(); #else #ifdef _MSC_VER #pragma warning(push) diff --git a/test/cpp/morenews.cpp b/test/cpp/morenews.cpp index 47780224..1e972a6f 100644 --- a/test/cpp/morenews.cpp +++ b/test/cpp/morenews.cpp @@ -72,11 +72,13 @@ NAN_MODULE_INIT(Init) { ); Set(target , New("newNegativeInteger").ToLocalChecked() - , GetFunction(New(NewNegativeInteger)).ToLocalChecked() + , GetFunction(New(NewNegativeInteger)) + .ToLocalChecked() ); Set(target , New("newPositiveInteger").ToLocalChecked() - , GetFunction(New(NewPositiveInteger)).ToLocalChecked() + , GetFunction(New(NewPositiveInteger)) + .ToLocalChecked() ); Set(target , New("newUtf8String").ToLocalChecked() @@ -92,11 +94,13 @@ NAN_MODULE_INIT(Init) { ); Set(target , New("newExternalStringResource").ToLocalChecked() - , GetFunction(New(NewExternalStringResource)).ToLocalChecked() + , GetFunction(New(NewExternalStringResource)) + .ToLocalChecked() ); Set(target , New("newExternalAsciiStringResource").ToLocalChecked() - , GetFunction(New(NewExternalAsciiStringResource)).ToLocalChecked() + , GetFunction(New(NewExternalAsciiStringResource)) + .ToLocalChecked() ); } diff --git a/test/cpp/multifile2.cpp b/test/cpp/multifile2.cpp index aca579b7..978fdffe 100644 --- a/test/cpp/multifile2.cpp +++ b/test/cpp/multifile2.cpp @@ -7,7 +7,7 @@ ********************************************************************/ #include -#include "multifile2.h" +#include "multifile2.h" // NOLINT(build/include) using namespace Nan; // NOLINT(build/namespaces) diff --git a/test/cpp/news.cpp b/test/cpp/news.cpp index c0b4b380..5b54c0ce 100644 --- a/test/cpp/news.cpp +++ b/test/cpp/news.cpp @@ -169,31 +169,38 @@ NAN_MODULE_INIT(Init) { ); Set(target , New("newNegativeInteger").ToLocalChecked() - , GetFunction(New(NewNegativeInteger)).ToLocalChecked() + , GetFunction(New(NewNegativeInteger)) + .ToLocalChecked() ); Set(target , New("newPositiveInteger").ToLocalChecked() - , GetFunction(New(NewPositiveInteger)).ToLocalChecked() + , GetFunction(New(NewPositiveInteger)) + .ToLocalChecked() ); Set(target , New("newUnsignedInteger").ToLocalChecked() - , GetFunction(New(NewUnsignedInteger)).ToLocalChecked() + , GetFunction(New(NewUnsignedInteger)) + .ToLocalChecked() ); Set(target , New("newInt32FromPositive").ToLocalChecked() - , GetFunction(New(NewInt32FromPositive)).ToLocalChecked() + , GetFunction(New(NewInt32FromPositive)) + .ToLocalChecked() ); Set(target , New("newInt32FromNegative").ToLocalChecked() - , GetFunction(New(NewInt32FromNegative)).ToLocalChecked() + , GetFunction(New(NewInt32FromNegative)) + .ToLocalChecked() ); Set(target , New("newUint32FromPositive").ToLocalChecked() - , GetFunction(New(NewUint32FromPositive)).ToLocalChecked() + , GetFunction(New(NewUint32FromPositive)) + .ToLocalChecked() ); Set(target , New("newUint32FromNegative").ToLocalChecked() - , GetFunction(New(NewUint32FromNegative)).ToLocalChecked() + , GetFunction(New(NewUint32FromNegative)) + .ToLocalChecked() ); Set(target , New("newUtf8String").ToLocalChecked() diff --git a/test/cpp/setcallhandler.cpp b/test/cpp/setcallhandler.cpp index b9c1dbcd..cd569c6b 100644 --- a/test/cpp/setcallhandler.cpp +++ b/test/cpp/setcallhandler.cpp @@ -37,7 +37,8 @@ NAN_MODULE_INIT(Init) { ); Set(target , New("b").ToLocalChecked() - , GetFunction(New(CallAsFunctionHandlerSetter)).ToLocalChecked() + , GetFunction(New(CallAsFunctionHandlerSetter)) + .ToLocalChecked() ); } diff --git a/test/cpp/threadlocal.cpp b/test/cpp/threadlocal.cpp index a3fc5875..67578bc1 100644 --- a/test/cpp/threadlocal.cpp +++ b/test/cpp/threadlocal.cpp @@ -63,7 +63,8 @@ NAN_METHOD(thread_local_storage) { NAN_MODULE_INIT(Init) { Set(target , New("thread_local_storage").ToLocalChecked() - , GetFunction(New(thread_local_storage)).ToLocalChecked() + , GetFunction(New(thread_local_storage)) + .ToLocalChecked() ); }