-
-
Notifications
You must be signed in to change notification settings - Fork 702
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
Fix Issue 19226 - std.typecons.Nullable(T, T nullValue) doesn't fully handle non-self-equal nullValue #6693
Conversation
Thanks for your pull request, @n8sh! Bugzilla references
Testing this PR locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub fetch digger
dub run digger -- build "master + phobos#6693" |
Is this different/related to #6583? |
Oh nevermind, this isn't about opEquals. |
It is related though since |
Opened bug report https://issues.dlang.org/show_bug.cgi?id=19227 |
Is there a codegen bug on Linux 32-bit? Seems odd that the test case would only fail on that platform. |
It seems like this must be a bug. |
75a9b04
to
03746ac
Compare
c825de9
to
4ebe3de
Compare
f12eaac
to
e4a8c07
Compare
@@ -3476,7 +3476,7 @@ Returns: | |||
} | |||
//Need to use 'is' if T is a float type | |||
//because NaN != NaN | |||
else static if (isFloatingPoint!T) | |||
else static if (__traits(isFloating, T) || __traits(compiles, { static assert(!(nullValue == nullValue)); })) |
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.
Q: Why is there the seemingly-extraneous __traits(isFloating, T)
?
A: So this patch doesn't change the behavior of Nullable!(double, -0.0)
.
f1643ea
to
0ebd89f
Compare
The wrong code generation on 32-bit linux was still happening last I checked so I expect this not to go through. |
3 days have been passed already. |
Yup, it's still there. |
I've been unable to replicate the Linux_32 / Linux_64_32 problem locally so far so I'm unable to tell what the problem is or make a useful bug report. |
… handle non-self-equal nullValue
I am changing the test case for this PR. Whatever is wrong with 32-bit linux is a separate issue. |
Nullable(T, T nullValue)
special-cases floating point numbers but this doesn't account for all types wherenullValue != nullValue
. For instance:https://run.dlang.io/is/hiLncI