Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

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

Morty Proxy This is a proxified and sanitized view of the page, visit original site.