Skip to content

Commit

Permalink
Refactor SetMethod() to deal with v8::Templates (#566)
Browse files Browse the repository at this point in the history
Latest v8 disallows setting non-primitive values on v8::Template and
derived types. So we no longer can assign v8::Functions directly since
it results in a v8 runtime warning reported in #558 [0]. Instead we
assign v8::FunctionTemplates. The initial approach of using a bunch of
overloads does not cover all cases as was reported in #564 [1] which
also discusses the technical reason behind this regression.

The new approach employs two auxiliary functions that take an
additional argument to discriminate v8::Templates from non-templates.
It deals with derived types correctly since the discriminating argument
(a pointer) is subject to "normal" conversion rules.

The entry point SetMethod(...) "peels off" the handle type using a
template-template parameter. This gives access to the "inner type" T.
It also deals nicely with the Local<> vs. Handle<> schism which is
still an issue with historic node/v8 versions.

It also adds tests for both the runtime warning in #558 and the
regression from #564. Finally, it tunes "npm test" to show v8 runtime
deprecation warnings.

This closes #564.

[0] #558
[1] #564
  • Loading branch information
agnat authored and kkoopa committed May 3, 2016
1 parent 6c5677f commit b9083cf
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 56 deletions.
82 changes: 27 additions & 55 deletions nan.h
Original file line number Diff line number Diff line change
Expand Up @@ -1852,75 +1852,47 @@ NAN_INLINE void SetInstanceTemplate(
SetTemplate(templ->InstanceTemplate(), name, value, attributes);
}

#if NODE_MODULE_VERSION < IOJS_3_0_MODULE_VERSION
NAN_INLINE void SetMethod(
v8::Handle<v8::Object> recv
, const char *name
, FunctionCallback callback) {
HandleScope scope;
v8::Local<v8::Function> fn = GetFunction(New<v8::FunctionTemplate>(
callback)).ToLocalChecked();
v8::Local<v8::String> fn_name = New(name).ToLocalChecked();
fn->SetName(fn_name);
recv->Set(fn_name, fn);
}
namespace imp {

NAN_INLINE void SetMethod(
v8::Handle<v8::FunctionTemplate> templ
, const char *name
, FunctionCallback callback) {
HandleScope scope;
v8::Local<v8::FunctionTemplate> t = New<v8::FunctionTemplate>(callback);
v8::Local<v8::String> fn_name = New(name).ToLocalChecked();
t->SetClassName(fn_name);
templ->Set(fn_name, t);
// Note(@agnat): Helper to distinguish different receiver types. The first
// version deals with receivers derived from v8::Template. The second version
// handles everything else. The final argument only serves as discriminator and
// is unused.
template <typename T>
NAN_INLINE
void
SetMethodAux(T recv,
v8::Local<v8::String> name,
v8::Local<v8::FunctionTemplate> tpl,
v8::Template *) {
recv->Set(name, tpl);
}

NAN_INLINE void SetMethod(
v8::Handle<v8::ObjectTemplate> templ
, const char *name
, FunctionCallback callback) {
HandleScope scope;
v8::Local<v8::FunctionTemplate> t = New<v8::FunctionTemplate>(callback);
v8::Local<v8::String> fn_name = New(name).ToLocalChecked();
t->SetClassName(fn_name);
templ->Set(fn_name, t);
}
#else
NAN_INLINE void SetMethod(
v8::Local<v8::Object> recv
, const char *name
, FunctionCallback callback) {
HandleScope scope;
v8::Local<v8::Function> fn = GetFunction(New<v8::FunctionTemplate>(
callback)).ToLocalChecked();
v8::Local<v8::String> fn_name = New(name).ToLocalChecked();
fn->SetName(fn_name);
recv->Set(fn_name, fn);
template <typename T>
NAN_INLINE
void
SetMethodAux(T recv,
v8::Local<v8::String> name,
v8::Local<v8::FunctionTemplate> tpl,
...) {
recv->Set(name, GetFunction(tpl).ToLocalChecked());
}

NAN_INLINE void SetMethod(
v8::Local<v8::FunctionTemplate> templ
, const char *name
, FunctionCallback callback) {
HandleScope scope;
v8::Local<v8::FunctionTemplate> t = New<v8::FunctionTemplate>(callback);
v8::Local<v8::String> fn_name = New(name).ToLocalChecked();
t->SetClassName(fn_name);
templ->Set(fn_name, t);
}
} // end of namespace imp

template <typename T, template <typename> class HandleType>
NAN_INLINE void SetMethod(
v8::Local<v8::ObjectTemplate> templ
HandleType<T> recv
, const char *name
, FunctionCallback callback) {
HandleScope scope;
v8::Local<v8::FunctionTemplate> t = New<v8::FunctionTemplate>(callback);
v8::Local<v8::String> fn_name = New(name).ToLocalChecked();
t->SetClassName(fn_name);
templ->Set(fn_name, t);
// Note(@agnat): Pass an empty T* as discriminator. See note on
// SetMethodAux(...) above
imp::SetMethodAux(recv, fn_name, t, static_cast<T*>(0));
}
#endif

NAN_INLINE void SetPrototypeMethod(
v8::Local<v8::FunctionTemplate> recv
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
"url": "git://github.com/nodejs/nan.git"
},
"scripts": {
"test": "tap --gc test/js/*-test.js",
"test": "tap --gc --stderr test/js/*-test.js",
"rebuild-tests": "node-gyp rebuild --msvs_version=2013 --directory test",
"docs": "doc/.build.sh"
},
Expand Down
15 changes: 15 additions & 0 deletions test/cpp/settemplate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ MyObject::MyObject() {
MyObject::~MyObject() {
}

void Foo(FunctionCallbackInfo<v8::Value> const&) {}

NAN_MODULE_INIT(MyObject::Init) {
// Prepare constructor template
v8::Local<v8::FunctionTemplate> tpl = Nan::New<v8::FunctionTemplate>(New);
Expand Down Expand Up @@ -74,6 +76,19 @@ NAN_MODULE_INIT(MyObject::Init) {
Set(target
, Nan::New<v8::String>("MyObject").ToLocalChecked()
, tpl->GetFunction());


//=== SetMethod ==============================================================

v8::Local<v8::Object> obj = Nan::New<v8::Object>();
SetMethod(obj, "foo", Foo);

// https://github.com/nodejs/nan/issues/564
v8::Local<v8::Function> func = Nan::New<v8::Function>(Foo);
SetMethod(func, "foo", Foo);

v8::Local<v8::FunctionTemplate> t = Nan::New<v8::FunctionTemplate>(Foo);
SetMethod(t, "foo", Foo);
}

NAN_METHOD(MyObject::New) {
Expand Down

0 comments on commit b9083cf

Please sign in to comment.