Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

assert: Check typed array view type in deepEqual (backport to 4.x) #6147

Closed
wants to merge 1 commit into from

Conversation

addaleax
Copy link
Member

Checklist
  • tests and code linting passes
  • a test and/or benchmark is included
  • the commit message follows commit guidelines
Affected core subsystem(s)

assert

Description of change

This is a backport of #5910 and is identical to what landed in master up to s/Buffer.from/new Buffer/.


Do not convert typed arrays to Buffer for deepEqual since their values may not be accurately represented by 8-bit ints. Instead perform binary comparison of underlying ArrayBuffers, but only when the array types match.

Never apply any kind of optimization for floating-point typed arrays since bit pattern equality is not the right kind of check for them.

Fixes: #5907 (introduced in #4330)

Do not convert typed arrays to `Buffer` for deepEqual since
their values may not be accurately represented by 8-bit ints.
Instead perform binary comparison of underlying `ArrayBuffer`s,
but only when the array types match.

Never apply any kind of optimization for floating-point typed
arrays since bit pattern equality is not the right kind of check
for them.

PR-URL: nodejs#5910
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Fixes: nodejs#5907
@mscdex mscdex added assert Issues and PRs related to the assert subsystem. v4.x labels Apr 11, 2016
@MylesBorins
Copy link
Contributor

@addaleax thanks so much for this! I'm loving seeing the little verified tag next to your commit sha.

Just so you know our cherry-pick style will lose your signature as we create releases... that being said super cool it was included!!!

@MylesBorins
Copy link
Contributor

landed as f949c27

@addaleax
Copy link
Member Author

@thealphanerd No problem! 😄 And yep, I know the signature will be lost – it’s just a default I have set up because it’s a nice feature of git and it doesn’t really hurt anyone :)

@addaleax addaleax deleted the backport-5910-to-4.x branch April 11, 2016 16:46
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
assert Issues and PRs related to the assert subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants