Comment #0 by monkeyworks12 — 2015-07-17T15:30:08Z
Currently, comparing two Nullables does not check if either is null. If either of them are, Nullable.get will assert.
import std.typecons;
void main()
{
Nullable!int n1 = 0;
Nullable!int n2;
Nullable!int n3;
assert(n1 == n2); //Nullable.get asserts
assert(n2 == n3); //Nullable.get asserts
}
Instead a custom opEquals should be implemented that checks if either Nullable is null before comparing their values. They should behave as shown below:
Nullable!int n4 = 0;
assert(n1 == n2); //n2 is null; returns false
assert(n2 == n3); //n2 and n3 are null; returns false
//Both n1 and n4 and non-null, so compare their values;
//Returns true
assert(n1 == n4);
Comment #1 by b2.temp — 2015-12-25T07:41:58Z
I was about to propose a PR that changes the behavior of Nullable but actually I think your report is invalid:
1/ it only throws in asseert mode, which allows to detect a wrong usage when testing the software and maybe to add some isNull() before comparing if necessary.
2/ if a custom opEquals() is added, even if it works fine, it becomes quite unclear how opCmp() should behave then.
see the changes:
https://github.com/BBasile/phobos/commit/f458ad1318e5536c91882c02397df262961c63a2#diff-4e008aedb3026d4a84f58323e53bf017R1930
so I suggest you to close.
Comment #2 by monkeyworks12 — 2015-12-27T02:32:57Z
I agree opCmp is a little weird to implement for Nullable, but it's really not much different from NaN. If we follow what the floating point numbers do:
Nullable!int n1;
Nullable!int n2 = 0;
assert(!(n1 < n2) && !(n1 == n2) && !(n1 > n2));
Of course the semantics of NaN are somewhat confusing at first and possibly bug-prone, so it may not be something we want to duplicate in another type if possible. Interestingly, this is how the built-in "nullable" types behave when compared:
Object o1 = null;
Object o2 = new Object();
assert(o1 > o2); //Segfault
int* i1 = null;
int* i2 = new int(0);
assert(i1 <= i2); //One of these will pass
assert(i1 > i2);
int[] a1 = null;
int[] a2 = [0];
assert(a1 <= a2); //I didn't know this was valid code;
assert(a1 > a2); //it must compare the array pointers
//and thus work similarly to case 2
So it looks like we have our choice of semantics to choose from.
Either way, the issue of opCmp is completely separate from opEquals, so I don't agree that this bug should be closed. Every built-in nullable type in D works as I described, and I don't believe that Nullable should be any different.
Object o1 = null;
Object o2 = new Object();
assert(o1 != o2); //Passes
int* i1 = null;
int* i2 = new int(0);
assert(i1 != i2); //Passes
int[] a1 = null;
int[] a2 = [0];
assert(a1 != a2); //Passes
The fact that these issues pop up when you actually try to use Nullable in any serious way suggest to me that its design is deeply flawed, and that it should be deprecated and replaced at some point. However, until that happens, we should aim to improve it as much as possible without breaking existing code.
If you don't want to open a defect for Nullable.opCmp, that's fine with me, but let's not close this one for opEquals.
Comment #3 by b2.temp — 2015-12-28T08:44:40Z
(In reply to monkeyworks12 from comment #2)
> but let's not close this one for opEquals.
Without great convictionI've opened the PR, but let's the official maintainers take the decision ;)
https://github.com/D-Programming-Language/phobos/pull/3887
Comment #4 by luis — 2016-03-30T14:41:06Z
I've also stumbled on this limitation, but the behavior I was looking forward to was:
Nullable!int a;
Nullable!int b;
assert(a == b);
Nullable!int c;
Nullable!int d = 42;
assert(c != d);
Since I can see the benefit of using Nullable in these different ways, that probably means this should be a template parameter.
Comment #5 by jack — 2016-04-05T00:13:29Z
*** This issue has been marked as a duplicate of issue 13017 ***