FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

async_wrap: pass uid to JS as double by trevnorris · Pull Request #7096 · nodejs/node · GitHub

/ node Public

async_wrap: pass uid to JS as double - #7096

Merged
trevnorris merged 1 commit into
nodejs:masterfrom
trevnorris:asyncwrap-uid-to-double
Jun 1, 2016
Merged

async_wrap: pass uid to JS as double#7096
trevnorris merged 1 commit into
nodejs:masterfrom
trevnorris:asyncwrap-uid-to-double

Conversation

trevnorris commented Jun 1, 2016
edited
Loading

Copy link
Copy Markdown
Contributor
  • 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 added the c++ Issues and PRs that require attention from people who are familiar with C++. label Jun 1, 2016
trevnorris mentioned this pull request Jun 1, 2016
3 tasks

addaleax commented Jun 1, 2016

Copy link
Copy Markdown
Member

LGTM

2 similar comments

cjihrig commented Jun 1, 2016

Copy link
Copy Markdown
Contributor

LGTM

AndreasMadsen commented Jun 1, 2016
edited
Loading

Copy link
Copy Markdown
Member

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 force-pushed the asyncwrap-uid-to-double branch from c686a88 to f81f0c3 Compare June 1, 2016 21:40
trevnorris closed this Jun 1, 2016
trevnorris deleted the asyncwrap-uid-to-double branch June 1, 2016 21:41
trevnorris merged commit f81f0c3 into nodejs:master Jun 1, 2016

Copy link
Copy Markdown
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 mentioned this pull request Jul 12, 2016
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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


Back | FazBrowse Home | New Git URL