Skip to content

Implement opCmp for Nullable objects #10762

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

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
53 changes: 53 additions & 0 deletions std/typecons.d
Original file line number Diff line number Diff line change
Expand Up @@ -3675,6 +3675,59 @@ struct Nullable(T)

private bool _isNull = true;

/**
* Compares two Nullable values.
* - If one is null and the other is not, the null one is considered smaller.
* - If both are null, they are equal.
* - If both are non-null, compares the payloads.
*
* Returns:
* Negative if `this < rhs`, zero if equal, positive if `this > rhs`.
*/
int opCmp(Rhs)(auto ref Rhs rhs) const
if (is(typeof(_value.payload < rhs.get)) && is(typeof(_value.payload > rhs.get)))
Copy link
Contributor

Choose a reason for hiding this comment

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

This test will return true for any type with a get method that returns a value comparable to the payload, even if it is not a Nullable type. The correct way to check whether Rhs is a Nullable type whose payload is comparable to this Nullable's payload is like this:

    if (is(Rhs == Nullable!U, U) && is(typeof(this.get < rhs.get)))

(Strictly speaking, it doesn't matter whether you use payload or get in the typeof(...) expression, but you should use the same on both sides for consistency.)

{
static if (is(Rhs == Nullable))
{
if (_isNull)
return rhs._isNull ? 0 : -1;
else if (rhs._isNull)
return 1;
else
return _value.payload < rhs._value.payload ? -1 :
_value.payload > rhs._value.payload ? 1 : 0;
}
else
{
static if (is(typeof(rhs.isNull)))
{
if (_isNull)
return rhs.isNull ? 0 : -1;
else if (rhs.isNull)
return 1;
else
return _value.payload < rhs.get ? -1 :
_value.payload > rhs.get ? 1 : 0;
}
Comment on lines +3690 to +3711
Copy link
Contributor

Choose a reason for hiding this comment

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

Once you've fixed the template constraint, you can get rid of the static if (is(Rhs == Nullable)) statement and combine these two branches.

else
{
return _isNull ? -1 : (_value.payload < rhs ? -1 : (_value.payload > rhs ? 1 : 0));
}
Comment on lines +3712 to +3715
Copy link
Contributor

Choose a reason for hiding this comment

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

This branch is unreachable and should be deleted.

}
}

// Basic Null Comparison
@safe unittest
{
Nullable!int a = 5;
Nullable!int b = Nullable!int.init;

assert(a > b);
assert(b < a);
assert(a == a);
assert(b == b);
}

/**
* Constructor initializing `this` with `value`.
*
Expand Down
Loading