-
Notifications
You must be signed in to change notification settings - Fork 461
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
Leave platform-specific type in operator def.n #341
Leave platform-specific type in operator def.n #341
Conversation
After discussion with Gabriel, it seems that changing the type only shifts which cases we'd have to cast so it better to just add a cast to the test. |
ba4ad76
to
d7e10e9
Compare
Explicitly casting to `uint32_t` prevents a template resolution ambiguity on 32-bit platforms. Fixes: nodejs#337
d7e10e9
to
186c492
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Explicitly casting to `uint32_t` prevents a template resolution ambiguity on 32-bit platforms. Fixes: #337 PR-URL: #341 Fixes: #337 Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Nicola Del Gobbo <[email protected]>
Landed as 2ad47a8 |
Explicitly casting to `uint32_t` prevents a template resolution ambiguity on 32-bit platforms. Fixes: nodejs/node-addon-api#337 PR-URL: nodejs/node-addon-api#341 Fixes: nodejs/node-addon-api#337 Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Nicola Del Gobbo <[email protected]>
Explicitly casting to `uint32_t` prevents a template resolution ambiguity on 32-bit platforms. Fixes: nodejs/node-addon-api#337 PR-URL: nodejs/node-addon-api#341 Fixes: nodejs/node-addon-api#337 Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Nicola Del Gobbo <[email protected]>
Explicitly casting to `uint32_t` prevents a template resolution ambiguity on 32-bit platforms. Fixes: nodejs/node-addon-api#337 PR-URL: nodejs/node-addon-api#341 Fixes: nodejs/node-addon-api#337 Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Nicola Del Gobbo <[email protected]>
Explicitly casting to `uint32_t` prevents a template resolution ambiguity on 32-bit platforms. Fixes: nodejs/node-addon-api#337 PR-URL: nodejs/node-addon-api#341 Fixes: nodejs/node-addon-api#337 Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Nicola Del Gobbo <[email protected]>
The property get/set sugar for numerical indices has to stay platform-
agnostic. That is, we cannot use
uint32_t
, but must instead useint
in the operator definition, because
int
resolves exactly on both32-bit and 64-bit platforms.
Fixes: #337