-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[Diagnostics] Diagnose comparisons with '.nan' and suggest using '.isNan' instead #33860
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
Conversation
I'm fine with the diagnostics generated. Pavel or Holly may have feedback on the implementation details, so please let one of them review as well. |
Thank you! While I wait for them to take a look as well, let me run some tests: @swift-ci please smoke test |
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.
This is a good warning to have! Thanks for tackling it.
…onformance and update some comments
Heh, I didn't notice I wrote |
@swift-ci please smoke test |
@swift-ci please smoke test macOS platform |
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! I have left a couple of improvement suggestions inline.
… and use it in ConstraintSystem
…'isStandardComparisonOperator' method
… to check for FloatingPoint conformance
@swift-ci please smoke test |
|
This is based on a suggestion from @stephentyrone on Twitter: https://twitter.com/stephentyrone/status/1302702935831384071
It is a pretty common mistake to compare values to the
.nan
static property, instead of using the.isNaN
instance property when checking for the presence or absence of NaN.According to isNaN documentation:
So, diagnose comparisons using
==
,>=
and etc when either of the arguments is.nan
fromFloatingPoint
protocol. For example:and suggest using
isNaN
when applicable.