Skip to content
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

Jump to bottom

src: start annotating native code side effect #21458

Closed
wants to merge 5 commits into from
Closed

src: start annotating native code side effect #21458

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7132853
src: start annotating native code side effect
TimothyGu Jun 22, 2018
c5f4cb5
fixup! src: start annotating native code side effect
TimothyGu Jun 22, 2018
d418bb8
fixup! fixup! src: start annotating native code side effect
TimothyGu Jun 22, 2018
a129a6f
fixup! src: start annotating native code side effect
TimothyGu Jun 23, 2018
c43658d
fixup! fixup! src: start annotating native code side effect
TimothyGu Jun 24, 2018
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
6 changes: 3 additions & 3 deletions src/cares_wrap.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2107,8 +2107,8 @@ void Initialize(Local<Object> target,

env->SetMethod(target, "getaddrinfo", GetAddrInfo);
env->SetMethod(target, "getnameinfo", GetNameInfo);
env->SetMethod(target, "isIPv6", IsIPv6);
env->SetMethod(target, "canonicalizeIP", CanonicalizeIP);
env->SetMethodNoSideEffect(target, "isIPv6", IsIPv6);
env->SetMethodNoSideEffect(target, "canonicalizeIP", CanonicalizeIP);

env->SetMethod(target, "strerror", StrError);

Expand Down Expand Up @@ -2165,7 +2165,7 @@ void Initialize(Local<Object> target,
env->SetProtoMethod(channel_wrap, "querySoa", Query<QuerySoaWrap>);
env->SetProtoMethod(channel_wrap, "getHostByAddr", Query<GetHostByAddrWrap>);

env->SetProtoMethod(channel_wrap, "getServers", GetServers);
env->SetProtoMethodNoSideEffect(channel_wrap, "getServers", GetServers);
env->SetProtoMethod(channel_wrap, "setServers", SetServers);
env->SetProtoMethod(channel_wrap, "cancel", Cancel);

Expand Down
70 changes: 65 additions & 5 deletions src/env-inl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -675,17 +675,41 @@ inline void Environment::ThrowUVException(int errorno,
inline v8::Local<v8::FunctionTemplate>
Environment::NewFunctionTemplate(v8::FunctionCallback callback,
v8::Local<v8::Signature> signature,
v8::ConstructorBehavior behavior) {
v8::ConstructorBehavior behavior,
v8::SideEffectType side_effect_type) {
v8::Local<v8::External> external = as_external();
return v8::FunctionTemplate::New(isolate(), callback, external,
signature, 0, behavior);
signature, 0, behavior, side_effect_type);
}

inline void Environment::SetMethod(v8::Local<v8::Object> that,
const char* name,
v8::FunctionCallback callback) {
v8::Local<v8::Function> function =
NewFunctionTemplate(callback)->GetFunction();
NewFunctionTemplate(callback,
v8::Local<v8::Signature>(),
// TODO(TimothyGu): Investigate if SetMethod is ever
// used for constructors.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's easy to check, isn't it? Or do you suspect make test will miss instances?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather leave that to a separate PR is what I meant.

v8::ConstructorBehavior::kAllow,
v8::SideEffectType::kHasSideEffect)->GetFunction();
// kInternalized strings are created in the old space.
const v8::NewStringType type = v8::NewStringType::kInternalized;
v8::Local<v8::String> name_string =
v8::String::NewFromUtf8(isolate(), name, type).ToLocalChecked();
that->Set(name_string, function);
function->SetName(name_string); // NODE_SET_METHOD() compatibility.
}

inline void Environment::SetMethodNoSideEffect(v8::Local<v8::Object> that,
const char* name,
v8::FunctionCallback callback) {
v8::Local<v8::Function> function =
NewFunctionTemplate(callback,
v8::Local<v8::Signature>(),
// TODO(TimothyGu): Investigate if SetMethod is ever
// used for constructors.
v8::ConstructorBehavior::kAllow,
v8::SideEffectType::kHasNoSideEffect)->GetFunction();
// kInternalized strings are created in the old space.
const v8::NewStringType type = v8::NewStringType::kInternalized;
v8::Local<v8::String> name_string =
Expand All @@ -699,7 +723,24 @@ inline void Environment::SetProtoMethod(v8::Local<v8::FunctionTemplate> that,
v8::FunctionCallback callback) {
v8::Local<v8::Signature> signature = v8::Signature::New(isolate(), that);
v8::Local<v8::FunctionTemplate> t =
NewFunctionTemplate(callback, signature, v8::ConstructorBehavior::kThrow);
NewFunctionTemplate(callback, signature, v8::ConstructorBehavior::kThrow,
v8::SideEffectType::kHasSideEffect);
// kInternalized strings are created in the old space.
const v8::NewStringType type = v8::NewStringType::kInternalized;
v8::Local<v8::String> name_string =
v8::String::NewFromUtf8(isolate(), name, type).ToLocalChecked();
that->PrototypeTemplate()->Set(name_string, t);
t->SetClassName(name_string); // NODE_SET_PROTOTYPE_METHOD() compatibility.
}

inline void Environment::SetProtoMethodNoSideEffect(
v8::Local<v8::FunctionTemplate> that,
const char* name,
v8::FunctionCallback callback) {
v8::Local<v8::Signature> signature = v8::Signature::New(isolate(), that);
v8::Local<v8::FunctionTemplate> t =
NewFunctionTemplate(callback, signature, v8::ConstructorBehavior::kThrow,
v8::SideEffectType::kHasNoSideEffect);
// kInternalized strings are created in the old space.
const v8::NewStringType type = v8::NewStringType::kInternalized;
v8::Local<v8::String> name_string =
Expand All @@ -711,7 +752,26 @@ inline void Environment::SetProtoMethod(v8::Local<v8::FunctionTemplate> that,
inline void Environment::SetTemplateMethod(v8::Local<v8::FunctionTemplate> that,
const char* name,
v8::FunctionCallback callback) {
v8::Local<v8::FunctionTemplate> t = NewFunctionTemplate(callback);
v8::Local<v8::FunctionTemplate> t =
NewFunctionTemplate(callback, v8::Local<v8::Signature>(),
v8::ConstructorBehavior::kAllow,
v8::SideEffectType::kHasSideEffect);
// kInternalized strings are created in the old space.
const v8::NewStringType type = v8::NewStringType::kInternalized;
v8::Local<v8::String> name_string =
v8::String::NewFromUtf8(isolate(), name, type).ToLocalChecked();
that->Set(name_string, t);
t->SetClassName(name_string); // NODE_SET_METHOD() compatibility.
}

inline void Environment::SetTemplateMethodNoSideEffect(
v8::Local<v8::FunctionTemplate> that,
const char* name,
v8::FunctionCallback callback) {
v8::Local<v8::FunctionTemplate> t =
NewFunctionTemplate(callback, v8::Local<v8::Signature>(),
v8::ConstructorBehavior::kAllow,
v8::SideEffectType::kHasNoSideEffect);
// kInternalized strings are created in the old space.
const v8::NewStringType type = v8::NewStringType::kInternalized;
v8::Local<v8::String> name_string =
Expand Down
17 changes: 16 additions & 1 deletion src/env.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -752,19 +752,34 @@ class Environment {
v8::Local<v8::Signature> signature =
v8::Local<v8::Signature>(),
v8::ConstructorBehavior behavior =
v8::ConstructorBehavior::kAllow);
v8::ConstructorBehavior::kAllow,
v8::SideEffectType side_effect =
v8::SideEffectType::kHasSideEffect);

// Convenience methods for NewFunctionTemplate().
inline void SetMethod(v8::Local<v8::Object> that,
const char* name,
v8::FunctionCallback callback);

inline void SetProtoMethod(v8::Local<v8::FunctionTemplate> that,
const char* name,
v8::FunctionCallback callback);
inline void SetTemplateMethod(v8::Local<v8::FunctionTemplate> that,
const char* name,
v8::FunctionCallback callback);

// Safe variants denote the function has no side effects.
inline void SetMethodNoSideEffect(v8::Local<v8::Object> that,
const char* name,
v8::FunctionCallback callback);
inline void SetProtoMethodNoSideEffect(v8::Local<v8::FunctionTemplate> that,
const char* name,
v8::FunctionCallback callback);
inline void SetTemplateMethodNoSideEffect(
v8::Local<v8::FunctionTemplate> that,
const char* name,
v8::FunctionCallback callback);

void BeforeExit(void (*cb)(void* arg), void* arg);
void RunBeforeExitCallbacks();
void AtExit(void (*cb)(void* arg), void* arg);
Expand Down
4 changes: 2 additions & 2 deletions src/inspector_js_api.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ void Initialize(Local<Object> target, Local<Value> unused,
if (agent->IsWaitingForConnect())
env->SetMethod(target, "callAndPauseOnStart", CallAndPauseOnStart);
env->SetMethod(target, "open", Open);
env->SetMethod(target, "url", Url);
env->SetMethodNoSideEffect(target, "url", Url);

env->SetMethod(target, "asyncTaskScheduled", AsyncTaskScheduledWrapper);
env->SetMethod(target, "asyncTaskCanceled",
Expand All @@ -300,7 +300,7 @@ void Initialize(Local<Object> target, Local<Value> unused,
InvokeAsyncTaskFnWithId<&Agent::AsyncTaskFinished>);

env->SetMethod(target, "registerAsyncHook", RegisterAsyncHookWrapper);
env->SetMethod(target, "isEnabled", IsEnabled);
env->SetMethodNoSideEffect(target, "isEnabled", IsEnabled);

auto conn_str = FIXED_ONE_BYTE_STRING(env->isolate(), "Connection");
Local<FunctionTemplate> tmpl =
Expand Down
10 changes: 5 additions & 5 deletions src/module_wrap.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -789,11 +789,11 @@ void ModuleWrap::Initialize(Local<Object> target,
env->SetProtoMethod(tpl, "link", Link);
env->SetProtoMethod(tpl, "instantiate", Instantiate);
env->SetProtoMethod(tpl, "evaluate", Evaluate);
env->SetProtoMethod(tpl, "namespace", Namespace);
env->SetProtoMethod(tpl, "getStatus", GetStatus);
env->SetProtoMethod(tpl, "getError", GetError);
env->SetProtoMethod(tpl, "getStaticDependencySpecifiers",
GetStaticDependencySpecifiers);
env->SetProtoMethodNoSideEffect(tpl, "namespace", Namespace);
env->SetProtoMethodNoSideEffect(tpl, "getStatus", GetStatus);
env->SetProtoMethodNoSideEffect(tpl, "getError", GetError);
env->SetProtoMethodNoSideEffect(tpl, "getStaticDependencySpecifiers",
GetStaticDependencySpecifiers);

target->Set(FIXED_ONE_BYTE_STRING(isolate, "ModuleWrap"), tpl->GetFunction());
env->SetMethod(target, "resolve", Resolve);
Expand Down
20 changes: 12 additions & 8 deletions src/node.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ using v8::Promise;
using v8::PropertyCallbackInfo;
using v8::ScriptOrigin;
using v8::SealHandleScope;
using v8::SideEffectType;
using v8::String;
using v8::TryCatch;
using v8::Undefined;
Expand Down Expand Up @@ -1947,7 +1948,10 @@ void SetupProcessObject(Environment* env,
title_string,
ProcessTitleGetter,
env->is_main_thread() ? ProcessTitleSetter : nullptr,
env->as_external()).FromJust());
env->as_external(),
v8::DEFAULT,
v8::None,
SideEffectType::kHasNoSideEffect).FromJust());

// process.version
READONLY_PROPERTY(process,
Expand Down Expand Up @@ -2252,17 +2256,17 @@ void SetupProcessObject(Environment* env,
env->SetMethod(process, "_getActiveHandles", GetActiveHandles);
env->SetMethod(process, "_kill", Kill);

env->SetMethod(process, "cwd", Cwd);
env->SetMethodNoSideEffect(process, "cwd", Cwd);
env->SetMethod(process, "dlopen", DLOpen);
env->SetMethod(process, "reallyExit", Exit);
env->SetMethod(process, "uptime", Uptime);
env->SetMethodNoSideEffect(process, "uptime", Uptime);

#if defined(__POSIX__) && !defined(__ANDROID__) && !defined(__CloudABI__)
env->SetMethod(process, "getuid", GetUid);
env->SetMethod(process, "geteuid", GetEUid);
env->SetMethod(process, "getgid", GetGid);
env->SetMethod(process, "getegid", GetEGid);
env->SetMethod(process, "getgroups", GetGroups);
env->SetMethodNoSideEffect(process, "getuid", GetUid);
env->SetMethodNoSideEffect(process, "geteuid", GetEUid);
env->SetMethodNoSideEffect(process, "getgid", GetGid);
env->SetMethodNoSideEffect(process, "getegid", GetEGid);
env->SetMethodNoSideEffect(process, "getgroups", GetGroups);
#endif // __POSIX__ && !defined(__ANDROID__) && !defined(__CloudABI__)
}

Expand Down
28 changes: 14 additions & 14 deletions src/node_buffer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1083,12 +1083,12 @@ void SetupBufferJS(const FunctionCallbackInfo<Value>& args) {
Local<Object> proto = args[0].As<Object>();
env->set_buffer_prototype_object(proto);

env->SetMethod(proto, "asciiSlice", StringSlice<ASCII>);
env->SetMethod(proto, "base64Slice", StringSlice<BASE64>);
env->SetMethod(proto, "latin1Slice", StringSlice<LATIN1>);
env->SetMethod(proto, "hexSlice", StringSlice<HEX>);
env->SetMethod(proto, "ucs2Slice", StringSlice<UCS2>);
env->SetMethod(proto, "utf8Slice", StringSlice<UTF8>);
env->SetMethodNoSideEffect(proto, "asciiSlice", StringSlice<ASCII>);
env->SetMethodNoSideEffect(proto, "base64Slice", StringSlice<BASE64>);
env->SetMethodNoSideEffect(proto, "latin1Slice", StringSlice<LATIN1>);
env->SetMethodNoSideEffect(proto, "hexSlice", StringSlice<HEX>);
env->SetMethodNoSideEffect(proto, "ucs2Slice", StringSlice<UCS2>);
env->SetMethodNoSideEffect(proto, "utf8Slice", StringSlice<UTF8>);

env->SetMethod(proto, "asciiWrite", StringWrite<ASCII>);
env->SetMethod(proto, "base64Write", StringWrite<BASE64>);
Expand Down Expand Up @@ -1116,22 +1116,22 @@ void Initialize(Local<Object> target,
Environment* env = Environment::GetCurrent(context);

env->SetMethod(target, "setupBufferJS", SetupBufferJS);
env->SetMethod(target, "createFromString", CreateFromString);
env->SetMethodNoSideEffect(target, "createFromString", CreateFromString);

env->SetMethod(target, "byteLengthUtf8", ByteLengthUtf8);
env->SetMethodNoSideEffect(target, "byteLengthUtf8", ByteLengthUtf8);
env->SetMethod(target, "copy", Copy);
env->SetMethod(target, "compare", Compare);
env->SetMethod(target, "compareOffset", CompareOffset);
env->SetMethodNoSideEffect(target, "compare", Compare);
env->SetMethodNoSideEffect(target, "compareOffset", CompareOffset);
env->SetMethod(target, "fill", Fill);
env->SetMethod(target, "indexOfBuffer", IndexOfBuffer);
env->SetMethod(target, "indexOfNumber", IndexOfNumber);
env->SetMethod(target, "indexOfString", IndexOfString);
env->SetMethodNoSideEffect(target, "indexOfBuffer", IndexOfBuffer);
env->SetMethodNoSideEffect(target, "indexOfNumber", IndexOfNumber);
env->SetMethodNoSideEffect(target, "indexOfString", IndexOfString);

env->SetMethod(target, "swap16", Swap16);
env->SetMethod(target, "swap32", Swap32);
env->SetMethod(target, "swap64", Swap64);

env->SetMethod(target, "encodeUtf8String", EncodeUtf8String);
env->SetMethodNoSideEffect(target, "encodeUtf8String", EncodeUtf8String);

target->Set(env->context(),
FIXED_ONE_BYTE_STRING(env->isolate(), "kMaxLength"),
Expand Down
Loading