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

async_wrap: pass uid to JS as double #7096

Merged
merged 1 commit into from
Jun 1, 2016

Conversation

trevnorris
Copy link
Contributor

@trevnorris trevnorris commented Jun 1, 2016

  • tests and code linting passes
  • the commit message follows commit guidelines
Affected core subsystem(s)

async_wrap

Description of change

Passing the uid via v8::Integer::New() converts it to a uint32_t. Which
will trim the value early. Instead use v8::Number::New() to convert the
int64_t to a double so that JS can see the full 2^53 range of uid's.

No test was included because the only way to increase the uid is to create new handles, and creating more than a uin32_t's worth of handles would take a while.

R=@bnoordhuis
R=@AndreasMadsen

CI: https://ci.nodejs.org/job/node-test-pull-request/2895/

@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Jun 1, 2016
@trevnorris trevnorris mentioned this pull request Jun 1, 2016
3 tasks
@addaleax
Copy link
Member

addaleax commented Jun 1, 2016

LGTM

2 similar comments
@cjihrig
Copy link
Contributor

cjihrig commented Jun 1, 2016

LGTM

@AndreasMadsen
Copy link
Member

AndreasMadsen commented Jun 1, 2016

LGTM

Passing the uid via v8::Integer::New() converts it to a uint32_t. Which
will trim the value early. Instead use v8::Number::New() to convert the
int64_t to a double so that JS can see the full 2^53 range of uid's.

PR-URL: nodejs#7096
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
@trevnorris trevnorris force-pushed the asyncwrap-uid-to-double branch from c686a88 to f81f0c3 Compare June 1, 2016 21:40
@trevnorris trevnorris closed this Jun 1, 2016
@trevnorris trevnorris deleted the asyncwrap-uid-to-double branch June 1, 2016 21:41
@trevnorris trevnorris merged commit f81f0c3 into nodejs:master Jun 1, 2016
@trevnorris
Copy link
Contributor Author

Thanks much! Landed in f81f0c3.

Fishrock123 pushed a commit that referenced this pull request Jun 1, 2016
Passing the uid via v8::Integer::New() converts it to a uint32_t. Which
will trim the value early. Instead use v8::Number::New() to convert the
int64_t to a double so that JS can see the full 2^53 range of uid's.

PR-URL: #7096
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
rvagg pushed a commit that referenced this pull request Jun 2, 2016
Passing the uid via v8::Integer::New() converts it to a uint32_t. Which
will trim the value early. Instead use v8::Number::New() to convert the
int64_t to a double so that JS can see the full 2^53 range of uid's.

PR-URL: #7096
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jun 7, 2016
Passing the uid via v8::Integer::New() converts it to a uint32_t. Which
will trim the value early. Instead use v8::Number::New() to convert the
int64_t to a double so that JS can see the full 2^53 range of uid's.

Ref: #7048
PR-URL: #7096
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jun 24, 2016
Passing the uid via v8::Integer::New() converts it to a uint32_t. Which
will trim the value early. Instead use v8::Number::New() to convert the
int64_t to a double so that JS can see the full 2^53 range of uid's.

Ref: #7048
PR-URL: #7096
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jun 28, 2016
Passing the uid via v8::Integer::New() converts it to a uint32_t. Which
will trim the value early. Instead use v8::Number::New() to convert the
int64_t to a double so that JS can see the full 2^53 range of uid's.

Ref: #7048
PR-URL: #7096
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 12, 2016
Passing the uid via v8::Integer::New() converts it to a uint32_t. Which
will trim the value early. Instead use v8::Number::New() to convert the
int64_t to a double so that JS can see the full 2^53 range of uid's.

Ref: #7048
PR-URL: #7096
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Jul 12, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants