Skip to content

doc: advise against using randomFill on floats#38150

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.

Sign up for GitHub

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

Jump to bottom
Closed
tniessen wants to merge 1 commit into from
Closed

doc: advise against using randomFill on floats #38150

tniessen wants to merge 1 commit into from

Conversation

@tniessen
Copy link
Member

@tniessentniessen commented Apr 8, 2021

The docs not only allow using randomFill on Float32Array and Float64Array, but they also include examples of doing that. There are virtually no valid use cases for this, and it can cause incorrect behavior due to the extremely low, but non-negligible, chance of producing NaN, +Infinity, and -Infinity in the TypedArray.

Refs: #38137

@nodejs-github-botnodejs-github-bot added crypto Issues and PRs related to the crypto subsystem. doc Issues and PRs related to the documentations. labels Apr 8, 2021
@aduh95aduh95 added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 8, 2021
jasnell pushed a commit that referenced this pull request Apr 12, 2021
@tniessen@jasnell
doc: advise against using randomFill on floats
f46d293
Refs: #38137 PR-URL: #38150 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]>
@jasnell
Copy link
Member

jasnell commented Apr 12, 2021

Landed in f46d293

@jasnelljasnell closed this Apr 12, 2021
BethGriggs pushed a commit that referenced this pull request Apr 15, 2021
@tniessen@BethGriggs
doc: advise against using randomFill on floats
42ca710
Refs: #38137 PR-URL: #38150 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]>
@BethGriggsBethGriggs mentioned this pull request Apr 16, 2021
Merged
@tniessentniessen deleted the doc-advise-against-randomfill-sync branch October 7, 2021 16:50
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnelljasnelljasnell approved these changes

@addaleaxaddaleaxaddaleax approved these changes

@benjamingrbenjamingrbenjamingr approved these changes

@cjihrigcjihrigcjihrig approved these changes

@aduh95aduh95aduh95 approved these changes

Assignees

No one assigned

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.cryptoIssues and PRs related to the crypto subsystem.docIssues and PRs related to the documentations.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@tniessen@jasnell@addaleax@benjamingr@cjihrig@aduh95@targos@nodejs-github-bot