Skip to content
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

promote math.isNaN #16627

Closed
wants to merge 4 commits into from
Closed

promote math.isNaN #16627

wants to merge 4 commits into from

Conversation

ringabout
Copy link
Member

@ringabout ringabout commented Jan 7, 2021

isNaN is more efficient than classify(x) == fcNan.

Ref timotheecour#64 (comment)

@ringabout ringabout marked this pull request as draft January 7, 2021 12:01
Copy link
Member

@timotheecour timotheecour left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM after addressing comment

compiler/semtypes.nim Outdated Show resolved Hide resolved
@timotheecour
Copy link
Member

CI failure was caused by #16646, I commited a workaround patch

@timotheecour timotheecour marked this pull request as ready for review January 9, 2021 00:01
@timotheecour timotheecour requested a review from Clyybber January 10, 2021 20:15
@Araq
Copy link
Member

Araq commented Jan 12, 2021

Sorry, but the older version is less code and this is not performance critical at all.

@timotheecour
Copy link
Member

timotheecour commented Jan 12, 2021

postponed till #16646 is fixed then, by which point there wouldn't need to have the missing isNaN workaround

timotheecour added a commit to timotheecour/Nim that referenced this pull request Apr 29, 2021
Araq pushed a commit that referenced this pull request Apr 30, 2021
…17895)

* revive #16627 now that csources_v1 was merged

* use dedent in rst.nim, refs #17257 (comment)

* fix test and improve rendering of a rst warning
PMunch pushed a commit to PMunch/Nim that referenced this pull request Mar 28, 2022
…ces_v1 (nim-lang#17895)

* revive nim-lang#16627 now that csources_v1 was merged

* use dedent in rst.nim, refs nim-lang#17257 (comment)

* fix test and improve rendering of a rst warning
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants