★ wanayoo — archive 1999 https://github.com/nodejs/node/pull/22094Nouvelle recherche | Portail wanayoo
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.

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

src: remove calls to deprecated v8 functions (NumberValue) #22094

Closed
wants to merge 3 commits into from

Conversation

Projects
None yet
6 participants
@ryzokuken
Copy link
Member

ryzokuken commented Aug 2, 2018

Remove all calls to deprecated v8 functions (here:
Value::NumberValue) inside the code (src directory only).

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

/cc @addaleax @hashseed

src: remove calls to deprecated v8 functions (NumberValue)
Remove all calls to deprecated v8 functions (here:
Value::NumberValue) inside the code (src directory only).
args[0]->IsNumber()
? args[0]
->NumberValue(args.GetIsolate()->GetCurrentContext())
.ToChecked()

This comment has been minimized.

@addaleax

addaleax Aug 2, 2018

Member

… args[0].As<Number>()->Value() …?

This comment has been minimized.

@ryzokuken

ryzokuken Aug 3, 2018

Author Member

Done.

wrap->AsyncReset(execution_async_id);
}


void AsyncWrap::QueueDestroyAsyncId(const FunctionCallbackInfo<Value>& args) {
CHECK(args[0]->IsNumber());
AsyncWrap::EmitDestroy(
Environment::GetCurrent(args), args[0]->NumberValue());
Environment::GetCurrent(args),
args[0]->NumberValue(args.GetIsolate()->GetCurrentContext()).ToChecked());

This comment has been minimized.

@addaleax

addaleax Aug 2, 2018

Member

We skip validation of the argument in JS land when async hooks checks are disabled – so we probably don’t want ToChecked()?

This comment has been minimized.

@ryzokuken

ryzokuken Aug 3, 2018

Author Member

Umm, dunno. There's literally a CHECK above, so I thought ToChecked would work here. I mean, it might still crash, sure, but nowhere it didn't crash earlier.

This comment has been minimized.

@addaleax

addaleax Aug 3, 2018

Member

Oh, right, I missed that – in that case, we don’t need NumberValue() either and can cast via .As<Number>(), right?

This comment has been minimized.

@ryzokuken

ryzokuken Aug 3, 2018

Author Member

:P You're right. Updating the PR.

@jasnell

jasnell approved these changes Aug 3, 2018

@cjihrig

cjihrig approved these changes Aug 7, 2018

@targos

targos approved these changes Aug 7, 2018

@targos

This comment has been minimized.

@ryzokuken

This comment has been minimized.

Copy link
Member Author

ryzokuken commented Aug 7, 2018

Landing this.

@ryzokuken

This comment has been minimized.

Copy link
Member Author

ryzokuken commented Aug 7, 2018 •

Landed in c6a54af

@ryzokuken ryzokuken closed this Aug 7, 2018

ryzokuken added a commit that referenced this pull request Aug 7, 2018

src: remove calls to deprecated v8 functions (NumberValue)
Remove all calls to deprecated v8 functions (here:
Value::NumberValue) inside the code (src directory only).

PR-URL: #22094
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>

devsnek added a commit to devsnek/node that referenced this pull request Aug 7, 2018

src: remove calls to deprecated v8 functions (NumberValue)
Remove all calls to deprecated v8 functions (here:
Value::NumberValue) inside the code (src directory only).

PR-URL: nodejs#22094
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>

targos added a commit that referenced this pull request Aug 11, 2018

src: remove calls to deprecated v8 functions (NumberValue)
Remove all calls to deprecated v8 functions (here:
Value::NumberValue) inside the code (src directory only).

PR-URL: #22094
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>

targos added a commit that referenced this pull request Aug 11, 2018

src: remove calls to deprecated v8 functions (NumberValue)
Remove all calls to deprecated v8 functions (here:
Value::NumberValue) inside the code (src directory only).

PR-URL: #22094
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>

@rvagg rvagg referenced this pull request Aug 13, 2018

Merged

Release proposal: v10.9.0 #22295

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.