Hi everyone,

there is https://issues.dlang.org/show_bug.cgi?id=21180 bug, anyone knows how to avoid it?

Test case:
-------------
import std;

class Silly {
    bool opEquals(const Silly silly) const @safe {
        return silly is this;
    }

    alias opEquals = Object.opEquals;
}

bool comp(T)() @safe {
    return new T() == new T();
}

void main()
{
    comp!Silly.writeln;
    comp!(const Silly).writeln;
    comp!(immutable Silly).writeln;
}
-------------

It always tries to call Object.opEquals, when narrower overload should've been selected.

- Alex.

On Friday, 28 August 2020 at 08:16:01 UTC, Alexandru Ermicioi wrote:
> Hi everyone,
> ....

Would be glad at least to pointers, where in dmd is logic for operator overloading happens, as well as for overloading rules, so I could fix it myself, if no-one is able to pick up it.

On Friday, 28 August 2020 at 08:16:01 UTC, Alexandru Ermicioi wrote:
> Hi everyone,
>
> there is https://issues.dlang.org/show_bug.cgi?id=21180 bug, anyone knows how to avoid it?
>
> Test case:
> -------------
> import std;
>
> class Silly {
>     bool opEquals(const Silly silly) const @safe {
>         return silly is this;
>     }
>
>     alias opEquals = Object.opEquals;
> }
>
> bool comp(T)() @safe {
>     return new T() == new T();
> }
>
> void main()
> {
>     comp!Silly.writeln;
>     comp!(const Silly).writeln;
>     comp!(immutable Silly).writeln;
> }
> -------------
>
> It always tries to call Object.opEquals, when narrower overload should've been selected.
>
> - Alex.

Essentially, this boils down to the issues described in https://issues.dlang.org/show_bug.cgi?id=1824, and a host of other bugzilla issues.

When you do a == b with a and b being class objects, it's lowered to object.opEquals(a, b)[0], which casts both a and b to Object before doing the comparison. So, you'll need to override Object.opEquals to have the right function called:

class Silly {
    int field;
    this(int f) {
        field = f;
    }
    override bool opEquals(Object o) {
        auto silly = cast(Silly)o;

        // If cast returns null, it's not a Silly instance
        if (!silly) return false;

        // Compare Silly objects
        return field == silly.field;
    }
}

unittest {
    Silly a = new Silly(1);
    assert(a == new Silly(1));
    assert(a != new Silly(2));
}

That takes care of choosing the correct overload, but as you may have noticed, there's another issue: your @safe function comp() can't call the @system function object.opEquals. Since object.opEquals operates on Object instances, not your specific subclass, it has to assume the worst, and is @system. There's no real good solution to that in the language as of now, and some of us have been pulling our hair for years because of it.

What you'll need to do is mark every function that does compare two class objects with == as @trusted or @system.

--
  Simen


[0]: https://github.com/dlang/druntime/blob/master/src/object.d#L166

On Friday, 28 August 2020 at 10:28:07 UTC, Simen Kjærås wrote:

> What you'll need to do is mark every function that does compare two class objects with == as @trusted or @system.

No that is not a solution at all, in template code that requires safety. You basically will have to sacrifice safety for rest of types, such as structs, unions & enums for the sake of objects being able to compare.

Could we just template that opEquals in this manner:
-------
bool opEquals(T : Object, X : Object)(T lhs, X rhs)
{
    if (lhs is rhs) return true;

    if (lhs is null || rhs is null) return false;

    if (!lhs.opEquals(rhs)) return false;

    if (typeid(lhs) is typeid(rhs) ||
        !__ctfe && typeid(lhs).opEquals(typeid(rhs)))
    {
        return true;
    }

    return rhs.opEquals(lhs);
}
-------

That would at least allow us to define an overload which is safe and would be picked up by new implementation.

I'm wondering why it wasn't done yet, are there any reasons for that?

- Alex.

On Friday, 28 August 2020 at 10:42:09 UTC, Alexandru Ermicioi wrote:
> ...
Also, why it is limited to just objects? It seems that this function enforces symmetry between two objects. What about rest of the possible types, such as structs, unions?

On Friday, 28 August 2020 at 10:42:09 UTC, Alexandru Ermicioi wrote:
> No that is not a solution at all, in template code that requires safety. You basically will have to sacrifice safety for rest of types, such as structs, unions & enums for the sake of objects being able to compare.

Yup. There's a reason I'm saying it's suboptimal. FWIW, you can limit the taint by using @trusted lambdas.

One of the reasons this hasn't been fixed is idiomatic D code very rarely uses classes, but that does not mean they're always the wrong tool.


> Could we just template that opEquals in this manner:
> -------
> bool opEquals(T : Object, X : Object)(T lhs, X rhs)
> {
>     if (lhs is rhs) return true;
>
>     if (lhs is null || rhs is null) return false;
>
>     if (!lhs.opEquals(rhs)) return false;
>
>     if (typeid(lhs) is typeid(rhs) ||
>         !__ctfe && typeid(lhs).opEquals(typeid(rhs)))
>     {
>         return true;
>     }
>
>     return rhs.opEquals(lhs);
> }
> -------
>
> That would at least allow us to define an overload which is safe and would be picked up by new implementation.
>
> I'm wondering why it wasn't done yet, are there any reasons for that?

The template solution may look like it solves every problem, but it really doesn't. Consider this code:

class A {
    bool opEquals(A) { return false; }
}
class B : A {
    bool opEquals(B) { return true; }
}

unittest {
    B b1 = new B();
    B b2 = new B();
    A a1 = b1;
    A a2 = b2;
    assert(b1 == b2);
    assert(a1 != a2); // WTF?
}

With the template solution, the function in B would be ignored when stored in a variable with static type A. This solution would sometimes do the right thing, other times it would silently do the wrong thing.




> Also, why it is limited to just objects? It seems that this function enforces symmetry between two objects. What about rest of the possible types, such as structs, unions?

That's an issue. The spec clearly states (https://dlang.org/spec/operatoroverloading.html#equals):

2. [T]he expressions a.opEquals(b) and b.opEquals(a) are tried. If both resolve to the same opEquals function, then the expression is rewritten to be a.opEquals(b).
3. If one is a better match than the other, or one compiles and the other does not, the first is selected.
4. Otherwise, an error results.

This is clearly not the case:

struct S1 {
    bool opEquals(S2 a) {
        return true;
    }
}
struct S2 {
    bool opEquals(S1 a) {
        return false;
    }
}

unittest {
    S1 a;
    S2 b;
    assert((a == b) == (b == a)); // Fails
}

I didn't find a bugzilla entry on this, but I'm pretty sure there is one.

As for why there's no global function like for classes, that's because there's no need - there is no disconnect between the static and dynamic types of non-class variables (or, if there is, it's explicitly programmed in, like in std.variant.Variant).

--
  Simen

On Friday, 28 August 2020 at 12:29:20 UTC, Simen Kjærås wrote:
> ....

Seems that these methods should be rooted out from Object, and placed in respective interfaces like:

-----
interface Equatable(T) {
    bool opEquals(T value);
}
-----

Then it would be a lot more simple. People who want equality check, will implement interface with right type, for example Equatable!Object.

- Alex

On Friday, 28 August 2020 at 13:35:43 UTC, Alexandru Ermicioi wrote:
> On Friday, 28 August 2020 at 12:29:20 UTC, Simen Kjærås wrote:
>> ....
>
> Seems that these methods should be rooted out from Object, and placed in respective interfaces like:
>
> -----
> interface Equatable(T) {
>     bool opEquals(T value);
> }
> -----
>
> Then it would be a lot more simple. People who want equality check, will implement interface with right type, for example Equatable!Object.

Yup, it's been proposed, but nothing's come of it yet. Here's Andrei's DIP on ProtoObject, which apparently is untouched for over two years now: https://github.com/andralex/DIPs/blob/ProtoObject/DIPs/DIPxxxx.md

The main problem with solving this issue is doing so will break all code that uses the current system. This is clearly suboptimal.

--
  Simen