Skip to content

Fix v8::External::Value/New for the new ExternalPointerTypeTag API #1015

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
gdavidkov wants to merge 1 commit into
base: main
Choose a base branch
from
+101 −59
Open

Fix v8::External::Value/New for the new ExternalPointerTypeTag API #1015

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
129 changes: 73 additions & 56 deletions nan_callbacks_12_inl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,13 +179,30 @@ class PropertyCallbackInfo {
};

namespace imp {

// Recent V8 versions (currently Chromium/Electron's V8 snapshot; not yet in
// any released Node) gained an ExternalPointerTypeTag parameter on
// v8::External::Value(). Detect that API via the V8_EXTERNAL_POINTER_TAG_COUNT
// macro (defined in <v8-internal.h> alongside the new signature) rather than a
// V8_MAJOR_VERSION cutoff, since Node and Chromium ship divergent V8
// snapshots under the same major version. Externals created by nan use
// kExternalPointerTypeTagDefault (see imp::NewExternal in
// nan_implementation_12_inl.h), so the same tag is used here on read.
inline void* GetExternalPointer(v8::Local<v8::External> ext) {
#ifdef V8_EXTERNAL_POINTER_TAG_COUNT
return ext->Value(v8::kExternalPointerTypeTagDefault);
#else
return ext->Value();
#endif
}

static
void FunctionCallbackWrapper(const v8::FunctionCallbackInfo<v8::Value> &info) {
v8::Local<v8::Object> obj = info.Data().As<v8::Object>();
FunctionCallback callback = reinterpret_cast<FunctionCallback>(
reinterpret_cast<intptr_t>(
obj->GetInternalField(kFunctionIndex)
.As<v8::Value>().As<v8::External>()->Value()));
imp::GetExternalPointer(obj->GetInternalField(kFunctionIndex)
.As<v8::Value>().As<v8::External>())));
FunctionCallbackInfo<v8::Value>
cbinfo(info, obj->GetInternalField(kDataIndex).As<v8::Value>());
callback(cbinfo);
Expand All @@ -203,8 +220,8 @@ void GetterCallbackWrapper(
cbinfo(info, obj->GetInternalField(kDataIndex).As<v8::Value>());
GetterCallback callback = reinterpret_cast<GetterCallback>(
reinterpret_cast<intptr_t>(
obj->GetInternalField(kGetterIndex)
.As<v8::Value>().As<v8::External>()->Value()));
imp::GetExternalPointer(obj->GetInternalField(kGetterIndex)
.As<v8::Value>().As<v8::External>())));
callback(property.As<v8::String>(), cbinfo);
}

Expand All @@ -221,8 +238,8 @@ void SetterCallbackWrapper(
cbinfo(info, obj->GetInternalField(kDataIndex).As<v8::Value>());
SetterCallback callback = reinterpret_cast<SetterCallback>(
reinterpret_cast<intptr_t>(
obj->GetInternalField(kSetterIndex)
.As<v8::Value>().As<v8::External>()->Value()));
imp::GetExternalPointer(obj->GetInternalField(kSetterIndex)
.As<v8::Value>().As<v8::External>())));
callback(property.As<v8::String>(), value, cbinfo);
}

Expand All @@ -240,8 +257,8 @@ void GetterCallbackWrapper(
cbinfo(info, obj->GetInternalField(kDataIndex).As<v8::Value>());
GetterCallback callback = reinterpret_cast<GetterCallback>(
reinterpret_cast<intptr_t>(
obj->GetInternalField(kGetterIndex)
.As<v8::Value>().As<v8::External>()->Value()));
imp::GetExternalPointer(obj->GetInternalField(kGetterIndex)
.As<v8::Value>().As<v8::External>())));
callback(property, cbinfo);
}

Expand All @@ -258,8 +275,8 @@ void SetterCallbackWrapper(
cbinfo(info, obj->GetInternalField(kDataIndex).As<v8::Value>());
SetterCallback callback = reinterpret_cast<SetterCallback>(
reinterpret_cast<intptr_t>(
obj->GetInternalField(kSetterIndex)
.As<v8::Value>().As<v8::External>()->Value()));
imp::GetExternalPointer(obj->GetInternalField(kSetterIndex)
.As<v8::Value>().As<v8::External>())));
callback(property, value, cbinfo);
}

Expand All @@ -282,8 +299,8 @@ v8::Intercepted PropertyGetterCallbackWrapper(
cbinfo(info, obj->GetInternalField(kDataIndex).As<v8::Value>());
PropertyGetterCallback callback = reinterpret_cast<PropertyGetterCallback>(
reinterpret_cast<intptr_t>(
obj->GetInternalField(kPropertyGetterIndex)
.As<v8::Value>().As<v8::External>()->Value()));
imp::GetExternalPointer(obj->GetInternalField(kPropertyGetterIndex)
.As<v8::Value>().As<v8::External>())));
return callback(property.As<v8::String>(), cbinfo);
}

Expand All @@ -300,8 +317,8 @@ v8::Intercepted PropertySetterCallbackWrapper(
cbinfo(info, obj->GetInternalField(kDataIndex).As<v8::Value>());
PropertySetterCallback callback = reinterpret_cast<PropertySetterCallback>(
reinterpret_cast<intptr_t>(
obj->GetInternalField(kPropertySetterIndex)
.As<v8::Value>().As<v8::External>()->Value()));
imp::GetExternalPointer(obj->GetInternalField(kPropertySetterIndex)
.As<v8::Value>().As<v8::External>())));
return callback(property.As<v8::String>(), value, cbinfo);
}

Expand All @@ -320,8 +337,8 @@ void PropertyGetterCallbackWrapper(
cbinfo(info, obj->GetInternalField(kDataIndex).As<v8::Value>());
PropertyGetterCallback callback = reinterpret_cast<PropertyGetterCallback>(
reinterpret_cast<intptr_t>(
obj->GetInternalField(kPropertyGetterIndex)
.As<v8::Value>().As<v8::External>()->Value()));
imp::GetExternalPointer(obj->GetInternalField(kPropertyGetterIndex)
.As<v8::Value>().As<v8::External>())));
callback(property.As<v8::String>(), cbinfo);
}

Expand All @@ -338,8 +355,8 @@ void PropertySetterCallbackWrapper(
cbinfo(info, obj->GetInternalField(kDataIndex).As<v8::Value>());
PropertySetterCallback callback = reinterpret_cast<PropertySetterCallback>(
reinterpret_cast<intptr_t>(
obj->GetInternalField(kPropertySetterIndex)
.As<v8::Value>().As<v8::External>()->Value()));
imp::GetExternalPointer(obj->GetInternalField(kPropertySetterIndex)
.As<v8::Value>().As<v8::External>())));
callback(property.As<v8::String>(), value, cbinfo);
}

Expand All @@ -357,8 +374,8 @@ void PropertyEnumeratorCallbackWrapper(
cbinfo(info, obj->GetInternalField(kDataIndex).As<v8::Value>());
PropertyEnumeratorCallback callback =
reinterpret_cast<PropertyEnumeratorCallback>(reinterpret_cast<intptr_t>(
obj->GetInternalField(kPropertyEnumeratorIndex)
.As<v8::Value>().As<v8::External>()->Value()));
imp::GetExternalPointer(obj->GetInternalField(kPropertyEnumeratorIndex)
.As<v8::Value>().As<v8::External>())));
callback(cbinfo);
}

Expand All @@ -376,8 +393,8 @@ v8::Intercepted PropertyDeleterCallbackWrapper(
cbinfo(info, obj->GetInternalField(kDataIndex).As<v8::Value>());
PropertyDeleterCallback callback = reinterpret_cast<PropertyDeleterCallback>(
reinterpret_cast<intptr_t>(
obj->GetInternalField(kPropertyDeleterIndex)
.As<v8::Value>().As<v8::External>()->Value()));
imp::GetExternalPointer(obj->GetInternalField(kPropertyDeleterIndex)
.As<v8::Value>().As<v8::External>())));
return callback(property.As<v8::String>(), cbinfo);
}

Expand All @@ -394,8 +411,8 @@ v8::Intercepted PropertyQueryCallbackWrapper(
cbinfo(info, obj->GetInternalField(kDataIndex).As<v8::Value>());
PropertyQueryCallback callback = reinterpret_cast<PropertyQueryCallback>(
reinterpret_cast<intptr_t>(
obj->GetInternalField(kPropertyQueryIndex)
.As<v8::Value>().As<v8::External>()->Value()));
imp::GetExternalPointer(obj->GetInternalField(kPropertyQueryIndex)
.As<v8::Value>().As<v8::External>())));
return callback(property.As<v8::String>(), cbinfo);
}

Expand All @@ -411,8 +428,8 @@ void PropertyDeleterCallbackWrapper(
cbinfo(info, obj->GetInternalField(kDataIndex).As<v8::Value>());
PropertyDeleterCallback callback = reinterpret_cast<PropertyDeleterCallback>(
reinterpret_cast<intptr_t>(
obj->GetInternalField(kPropertyDeleterIndex)
.As<v8::Value>().As<v8::External>()->Value()));
imp::GetExternalPointer(obj->GetInternalField(kPropertyDeleterIndex)
.As<v8::Value>().As<v8::External>())));
callback(property.As<v8::String>(), cbinfo);
}

Expand All @@ -428,8 +445,8 @@ void PropertyQueryCallbackWrapper(
cbinfo(info, obj->GetInternalField(kDataIndex).As<v8::Value>());
PropertyQueryCallback callback = reinterpret_cast<PropertyQueryCallback>(
reinterpret_cast<intptr_t>(
obj->GetInternalField(kPropertyQueryIndex)
.As<v8::Value>().As<v8::External>()->Value()));
imp::GetExternalPointer(obj->GetInternalField(kPropertyQueryIndex)
.As<v8::Value>().As<v8::External>())));
callback(property.As<v8::String>(), cbinfo);
}

Expand All @@ -446,8 +463,8 @@ void PropertyGetterCallbackWrapper(
cbinfo(info, obj->GetInternalField(kDataIndex).As<v8::Value>());
PropertyGetterCallback callback = reinterpret_cast<PropertyGetterCallback>(
reinterpret_cast<intptr_t>(
obj->GetInternalField(kPropertyGetterIndex)
.As<v8::Value>().As<v8::External>()->Value()));
imp::GetExternalPointer(obj->GetInternalField(kPropertyGetterIndex)
.As<v8::Value>().As<v8::External>())));
callback(property, cbinfo);
}

Expand All @@ -464,8 +481,8 @@ void PropertySetterCallbackWrapper(
cbinfo(info, obj->GetInternalField(kDataIndex).As<v8::Value>());
PropertySetterCallback callback = reinterpret_cast<PropertySetterCallback>(
reinterpret_cast<intptr_t>(
obj->GetInternalField(kPropertySetterIndex)
.As<v8::Value>().As<v8::External>()->Value()));
imp::GetExternalPointer(obj->GetInternalField(kPropertySetterIndex)
.As<v8::Value>().As<v8::External>())));
callback(property, value, cbinfo);
}

Expand All @@ -482,8 +499,8 @@ void PropertyEnumeratorCallbackWrapper(
cbinfo(info, obj->GetInternalField(kDataIndex).As<v8::Value>());
PropertyEnumeratorCallback callback =
reinterpret_cast<PropertyEnumeratorCallback>(reinterpret_cast<intptr_t>(
obj->GetInternalField(kPropertyEnumeratorIndex)
.As<v8::Value>().As<v8::External>()->Value()));
imp::GetExternalPointer(obj->GetInternalField(kPropertyEnumeratorIndex)
.As<v8::Value>().As<v8::External>())));
callback(cbinfo);
}

Expand All @@ -499,8 +516,8 @@ void PropertyDeleterCallbackWrapper(
cbinfo(info, obj->GetInternalField(kDataIndex).As<v8::Value>());
PropertyDeleterCallback callback = reinterpret_cast<PropertyDeleterCallback>(
reinterpret_cast<intptr_t>(
obj->GetInternalField(kPropertyDeleterIndex)
.As<v8::Value>().As<v8::External>()->Value()));
imp::GetExternalPointer(obj->GetInternalField(kPropertyDeleterIndex)
.As<v8::Value>().As<v8::External>())));
callback(property, cbinfo);
}

Expand All @@ -516,8 +533,8 @@ void PropertyQueryCallbackWrapper(
cbinfo(info, obj->GetInternalField(kDataIndex).As<v8::Value>());
PropertyQueryCallback callback = reinterpret_cast<PropertyQueryCallback>(
reinterpret_cast<intptr_t>(
obj->GetInternalField(kPropertyQueryIndex)
.As<v8::Value>().As<v8::External>()->Value()));
imp::GetExternalPointer(obj->GetInternalField(kPropertyQueryIndex)
.As<v8::Value>().As<v8::External>())));
callback(property, cbinfo);
}

Expand All @@ -535,8 +552,8 @@ v8::Intercepted IndexGetterCallbackWrapper(
cbinfo(info, obj->GetInternalField(kDataIndex).As<v8::Value>());
IndexGetterCallback callback = reinterpret_cast<IndexGetterCallback>(
reinterpret_cast<intptr_t>(
obj->GetInternalField(kIndexPropertyGetterIndex)
.As<v8::Value>().As<v8::External>()->Value()));
imp::GetExternalPointer(obj->GetInternalField(kIndexPropertyGetterIndex)
.As<v8::Value>().As<v8::External>())));
return callback(index, cbinfo);
}

Expand All @@ -553,8 +570,8 @@ v8::Intercepted IndexSetterCallbackWrapper(
cbinfo(info, obj->GetInternalField(kDataIndex).As<v8::Value>());
IndexSetterCallback callback = reinterpret_cast<IndexSetterCallback>(
reinterpret_cast<intptr_t>(
obj->GetInternalField(kIndexPropertySetterIndex)
.As<v8::Value>().As<v8::External>()->Value()));
imp::GetExternalPointer(obj->GetInternalField(kIndexPropertySetterIndex)
.As<v8::Value>().As<v8::External>())));
return callback(index, value, cbinfo);
}

Expand All @@ -572,8 +589,8 @@ void IndexGetterCallbackWrapper(
cbinfo(info, obj->GetInternalField(kDataIndex).As<v8::Value>());
IndexGetterCallback callback = reinterpret_cast<IndexGetterCallback>(
reinterpret_cast<intptr_t>(
obj->GetInternalField(kIndexPropertyGetterIndex)
.As<v8::Value>().As<v8::External>()->Value()));
imp::GetExternalPointer(obj->GetInternalField(kIndexPropertyGetterIndex)
.As<v8::Value>().As<v8::External>())));
callback(index, cbinfo);
}

Expand All @@ -589,8 +606,8 @@ void IndexSetterCallbackWrapper(
cbinfo(info, obj->GetInternalField(kDataIndex).As<v8::Value>());
IndexSetterCallback callback = reinterpret_cast<IndexSetterCallback>(
reinterpret_cast<intptr_t>(
obj->GetInternalField(kIndexPropertySetterIndex)
.As<v8::Value>().As<v8::External>()->Value()));
imp::GetExternalPointer(obj->GetInternalField(kIndexPropertySetterIndex)
.As<v8::Value>().As<v8::External>())));
callback(index, value, cbinfo);
}

Expand All @@ -609,9 +626,9 @@ void IndexEnumeratorCallbackWrapper(
cbinfo(info, obj->GetInternalField(kDataIndex).As<v8::Value>());
IndexEnumeratorCallback callback = reinterpret_cast<IndexEnumeratorCallback>(
reinterpret_cast<intptr_t>(
obj->GetInternalField(
imp::GetExternalPointer(obj->GetInternalField(
kIndexPropertyEnumeratorIndex)
.As<v8::Value>().As<v8::External>()->Value()));
.As<v8::Value>().As<v8::External>())));
callback(cbinfo);
}

Expand All @@ -628,8 +645,8 @@ v8::Intercepted IndexDeleterCallbackWrapper(
cbinfo(info, obj->GetInternalField(kDataIndex).As<v8::Value>());
IndexDeleterCallback callback = reinterpret_cast<IndexDeleterCallback>(
reinterpret_cast<intptr_t>(
obj->GetInternalField(kIndexPropertyDeleterIndex)
.As<v8::Value>().As<v8::External>()->Value()));
imp::GetExternalPointer(obj->GetInternalField(kIndexPropertyDeleterIndex)
.As<v8::Value>().As<v8::External>())));
return callback(index, cbinfo);
}

Expand All @@ -644,8 +661,8 @@ v8::Intercepted IndexQueryCallbackWrapper(
cbinfo(info, obj->GetInternalField(kDataIndex).As<v8::Value>());
IndexQueryCallback callback = reinterpret_cast<IndexQueryCallback>(
reinterpret_cast<intptr_t>(
obj->GetInternalField(kIndexPropertyQueryIndex)
.As<v8::Value>().As<v8::External>()->Value()));
imp::GetExternalPointer(obj->GetInternalField(kIndexPropertyQueryIndex)
.As<v8::Value>().As<v8::External>())));
return callback(index, cbinfo);
}

Expand All @@ -660,8 +677,8 @@ void IndexDeleterCallbackWrapper(
cbinfo(info, obj->GetInternalField(kDataIndex).As<v8::Value>());
IndexDeleterCallback callback = reinterpret_cast<IndexDeleterCallback>(
reinterpret_cast<intptr_t>(
obj->GetInternalField(kIndexPropertyDeleterIndex)
.As<v8::Value>().As<v8::External>()->Value()));
imp::GetExternalPointer(obj->GetInternalField(kIndexPropertyDeleterIndex)
.As<v8::Value>().As<v8::External>())));
callback(index, cbinfo);
}

Expand All @@ -676,8 +693,8 @@ void IndexQueryCallbackWrapper(
cbinfo(info, obj->GetInternalField(kDataIndex).As<v8::Value>());
IndexQueryCallback callback = reinterpret_cast<IndexQueryCallback>(
reinterpret_cast<intptr_t>(
obj->GetInternalField(kIndexPropertyQueryIndex)
.As<v8::Value>().As<v8::External>()->Value()));
imp::GetExternalPointer(obj->GetInternalField(kIndexPropertyQueryIndex)
.As<v8::Value>().As<v8::External>())));
callback(index, cbinfo);
}

Expand Down
22 changes: 19 additions & 3 deletions nan_implementation_12_inl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,22 @@

namespace imp {

// Recent V8 versions (currently Chromium/Electron's V8 snapshot; not yet in
// any released Node) gained an ExternalPointerTypeTag parameter on
// v8::External::New(). Detect that API via the V8_EXTERNAL_POINTER_TAG_COUNT
// macro (defined in <v8-internal.h> alongside the new signature) rather than
// a V8_MAJOR_VERSION cutoff, since Node and Chromium ship divergent V8
// snapshots under the same major version. Externals created via this helper
// are read back through imp::GetExternalPointer in nan_callbacks_12_inl.h,
// which uses the matching kExternalPointerTypeTagDefault tag.
inline v8::Local<v8::External> NewExternal(v8::Isolate* isolate, void* value) {
#ifdef V8_EXTERNAL_POINTER_TAG_COUNT
return v8::External::New(isolate, value, v8::kExternalPointerTypeTagDefault);
#else
return v8::External::New(isolate, value);
#endif
}

//=== Array ====================================================================

Factory<v8::Array>::return_t
Expand Down Expand Up @@ -76,7 +92,7 @@ Factory<v8::Date>::New(double value) {

Factory<v8::External>::return_t
Factory<v8::External>::New(void * value) {
return v8::External::New(v8::Isolate::GetCurrent(), value);
return imp::NewExternal(v8::Isolate::GetCurrent(), value);
}

//=== Function =================================================================
Expand All @@ -92,7 +108,7 @@ Factory<v8::Function>::New( FunctionCallback callback

obj->SetInternalField(
imp::kFunctionIndex
, v8::External::New(isolate, reinterpret_cast<void *>(callback)));
, imp::NewExternal(isolate, reinterpret_cast<void *>(callback)));

v8::Local<v8::Value> val = v8::Local<v8::Value>::New(isolate, data);

Expand Down Expand Up @@ -128,7 +144,7 @@ Factory<v8::FunctionTemplate>::New( FunctionCallback callback

obj->SetInternalField(
imp::kFunctionIndex
, v8::External::New(isolate, reinterpret_cast<void *>(callback)));
, imp::NewExternal(isolate, reinterpret_cast<void *>(callback)));
v8::Local<v8::Value> val = v8::Local<v8::Value>::New(isolate, data);

if (!val.IsEmpty()) {
Expand Down
4 changes: 4 additions & 0 deletions test/cpp/nannew.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,11 @@ NAN_METHOD(testExternal) {

t.plan(2);

#ifdef V8_EXTERNAL_POINTER_TAG_COUNT
t.ok(_(New<External>(&ttt)->Value(v8::kExternalPointerTypeTagDefault) == &ttt));
#else
t.ok(_(New<External>(&ttt)->Value() == &ttt));
#endif
t.ok(_( assertType<External>(New<External>(&ttt))));

info.GetReturnValue().SetUndefined();
Expand Down
Loading
Loading