Skip to content

Conversation

@gengjiawen
Copy link
Member

see https://clang.llvm.org/extra/clang-tidy/checks/bugprone-incorrect-roundings.html.

Checks the usage of patterns known to produce incorrect rounding. Programmers often use:

(int)(double_expression + 0.5)
to round the double expression to an integer. The problem with this:

It is unnecessarily slow.
It is incorrect. The number 0.499999975 (smallest representable float number below 0.5) rounds to 1.0. Even worse behavior for negative numbers where both -0.5f and -1.4f both round to 0.0.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-botnodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Mar 24, 2019
@gengjiawengengjiawenforce-pushed the bugprone-incorrect-roundings branch from 5b449ce to 9ce6d45CompareMarch 24, 2019 14:45
@BridgeARBridgeAR requested a review from refackApril 3, 2019 02:06
@gengjiawengengjiawenforce-pushed the bugprone-incorrect-roundings branch from 9ce6d45 to eea7aabCompareApril 3, 2019 13:11
@nodejs-github-bot
Copy link
Collaborator

PR-URL: nodejs#26885 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Minwoo Jung <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>
@refackrefackforce-pushed the bugprone-incorrect-roundings branch from eea7aab to d711b97CompareApril 3, 2019 16:20
@refackrefack merged commit d711b97 into nodejs:masterApr 3, 2019
@gengjiawengengjiawen deleted the bugprone-incorrect-roundings branch April 3, 2019 16:41
BethGriggs pushed a commit that referenced this pull request Apr 4, 2019
PR-URL: #26885 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Minwoo Jung <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>
BethGriggs pushed a commit that referenced this pull request Apr 9, 2019
PR-URL: #26885 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Minwoo Jung <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Signed-off-by: Beth Griggs <[email protected]>
@BethGriggsBethGriggs mentioned this pull request Apr 9, 2019
BethGriggs pushed a commit that referenced this pull request Apr 10, 2019
PR-URL: #26885 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Minwoo Jung <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Signed-off-by: Beth Griggs <[email protected]>
Sign up for freeto 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.

6 participants

@gengjiawen@nodejs-github-bot@mscdex@refack@jasnell@JungMinu