I encountered a problem in the implementation of std.xml.Document.opEquals (yes, I've reported an issue). The problem is demonstrated with this example:

class Base
{
    int a;

    override bool opEquals (Object o)
    {
        if (auto base = cast(Base) o)
            return base.a == a;
        else
            return false;
    }
}

class Foo : Base
{
    int b;

    override bool opEquals (Object o)
    {
        if (auto foo = cast(Foo) o)
            return super == cast(Base) foo && foo.b == b;
        else
            return false;
    }
}

void main()
{
    auto f1 = new Foo;
    auto f2 = new Foo;
    assert(f1 == f2);
}

This code will result in an infinite recursion. I think the problem is in the super call, due to == being rewritten to call object.opEquals. The implementation of object.opEquals will call opEquals on the actual instances. The call will be dynamically resolved and end up calling Foo.opEquals instead of Base.opEquals.

Is this really good behavior, something a developer would expect? I mean, in every other case calling super.someMethod will actually call the method in the base class.

In this case the solution/workaround is to explicitly call super.opEquals, but that will miss some optimizations implemented in object.opEquals.

-- 
/Jacob Carlborg

On Wednesday, 2 September 2015 at 18:57:11 UTC, Jacob Carlborg wrote:
> I encountered a problem in the implementation of std.xml.Document.opEquals (yes, I've reported an issue). The problem is demonstrated with this example:
>
> class Base
> {
>     int a;
>
>     override bool opEquals (Object o)
>     {
>         if (auto base = cast(Base) o)
>             return base.a == a;
>         else
>             return false;
>     }
> }
>
> class Foo : Base
> {
>     int b;
>
>     override bool opEquals (Object o)
>     {
>         if (auto foo = cast(Foo) o)
>             return super == cast(Base) foo && foo.b == b;
>         else
>             return false;
>     }
> }
>
> void main()
> {
>     auto f1 = new Foo;
>     auto f2 = new Foo;
>     assert(f1 == f2);
> }
>
> This code will result in an infinite recursion. I think the problem is in the super call, due to == being rewritten to call object.opEquals. The implementation of object.opEquals will call opEquals on the actual instances. The call will be dynamically resolved and end up calling Foo.opEquals instead of Base.opEquals.
>
> Is this really good behavior, something a developer would expect? I mean, in every other case calling super.someMethod will actually call the method in the base class.
>
> In this case the solution/workaround is to explicitly call super.opEquals, but that will miss some optimizations implemented in object.opEquals.

Yeah, I would just call super.opEquals, like so.

class Base {
    int a;

    override bool opEquals(Object o) {
        if (auto other = cast(Base) o)
            return a == other.a;

        return false;
    }
}

class Foo : Base {
    int b;

    override bool opEquals(Object o) {
        if (!super.opEquals(o))
            return false;

        if (auto other = cast(Foo) o)
            return b == other.b;

        return false;
    }
}

void main()
{
    auto f1 = new Foo;
    auto f2 = new Foo;
    assert(f1 == f2);
}

If some optimisations are missed by structuring the methods in this way, then maybe that's something the compiler should be programmed to handle.

On 2015-09-02 22:25, w0rp wrote:

> Yeah, I would just call super.opEquals, like so.

I know that's the workaround, but the question is if it's a good implementation/behavior of opEquals.

-- 
/Jacob Carlborg

On 9/2/15 2:57 PM, Jacob Carlborg wrote:

> In this case the solution/workaround is to explicitly call
> super.opEquals, but that will miss some optimizations implemented in
> object.opEquals.

Those optimizations have already been exploited by the time you get to Foo.opEquals, so I wouldn't worry about that.

However, the avoidance of casting would be a good goal. One of the things I don't like about the current == implementation for objects is it cannot take any advantage of type knowledge at the call site.

-Steve

On Thursday, 3 September 2015 at 13:05:49 UTC, Steven Schveighoffer wrote:
> On 9/2/15 2:57 PM, Jacob Carlborg wrote:
>
>> In this case the solution/workaround is to explicitly call
>> super.opEquals, but that will miss some optimizations implemented in
>> object.opEquals.
>
> Those optimizations have already been exploited by the time you get to Foo.opEquals, so I wouldn't worry about that.
>
> However, the avoidance of casting would be a good goal. One of the things I don't like about the current == implementation for objects is it cannot take any advantage of type knowledge at the call site.

Every time I've tried to templatize the free function opEquals, I've run into compiler bugs, but we'll get there eventually. It looks like Kenji has a PR now to fix one of the issues:

https://issues.dlang.org/show_bug.cgi?id=12537

So, I'll have to make another stab at it soon.

- Jonathan M Davis

On Fri, Sep 04, 2015 at 07:10:25PM +0000, Jonathan M Davis via Digitalmars-d wrote:
> On Thursday, 3 September 2015 at 13:05:49 UTC, Steven Schveighoffer wrote:
> >On 9/2/15 2:57 PM, Jacob Carlborg wrote:
> >
> >>In this case the solution/workaround is to explicitly call super.opEquals, but that will miss some optimizations implemented in object.opEquals.
> >
> >Those optimizations have already been exploited by the time you get to Foo.opEquals, so I wouldn't worry about that.
> >
> >However, the avoidance of casting would be a good goal. One of the things I don't like about the current == implementation for objects is it cannot take any advantage of type knowledge at the call site.
> 
> Every time I've tried to templatize the free function opEquals, [...]
[...]

Wait, wait, did I miss something? Since when was operator overloading allowed as free functions? Or is opEquals a special case?


T

-- 
If it tastes good, it's probably bad for you.

On 09/04/2015 09:21 PM, H. S. Teoh via Digitalmars-d wrote:
>
> Wait, wait, did I miss something? Since when was operator overloading
> allowed as free functions?

Since UFCS, but DMD does not implement it.

> Or is opEquals a special case?
>

Yup, quite special: http://dlang.org/operatoroverloading.html#equals

On Friday, 4 September 2015 at 20:39:14 UTC, Timon Gehr wrote:
> On 09/04/2015 09:21 PM, H. S. Teoh via Digitalmars-d wrote:
>>
>> Wait, wait, did I miss something? Since when was operator overloading
>> allowed as free functions?
>
> Since UFCS, but DMD does not implement it.

There is nothing in the spec about supporting operator overloading with free functions, so I don't know where you get the idea that it's even intended to be a feature. UFCS applies to functions which use the member function call syntax, and operators aren't used that way. There is no plan whatsoever to support operator overloading via free functions.

- Jonathan M Davis

On Friday, 4 September 2015 at 19:25:35 UTC, H. S. Teoh wrote:
> Wait, wait, did I miss something? Since when was operator overloading allowed as free functions? Or is opEquals a special case?

Clearly, you haven't read TDPL recently enough. ;)

There is a free function, opEquals, in object.d which gets called for classes, and _it_ is what == gets translated to for classes, and it calls the member function version of opEquals on classes:

https://github.com/D-Programming-Language/druntime/blob/master/src/object.d#L143

This allows us to avoid a number of fun bugs with opEquals that you get in languages like Java and makes it completely unnecessary to do stuff like check whether the argument to opEquals is null. Timon gave the link to the explanation in the spec:

http://dlang.org/operatoroverloading.html#equals

but TDPL also talks about it. But as part of the fix for

https://issues.dlang.org/show_bug.cgi?id=9769

we need to templatize the free function opEquals so that it doesn't require Object. Then opEquals on a class will take whatever the most base class in your class hierarchy is that has opEquals rather than Object, and we can actually deprecate opEquals on Object (though that may require some additional compiler help rather than simply deprecating it; I'm not sure). Regardless, templatizing the free function version of opEquals is the first step towards that.

- Jonathan M Davis

On 2015-09-05 08:18, Jonathan M Davis wrote:

> There is nothing in the spec about supporting operator overloading with
> free functions, so I don't know where you get the idea that it's even
> intended to be a feature. UFCS applies to functions which use the member
> function call syntax, and operators aren't used that way. There is no
> plan whatsoever to support operator overloading via free functions.

Since "a == b" would be lowered to "a.opEquals(b)" one could argue that the compile would also try UFCS since it would do that if the code had been "a.opEquals(b)" from the beginning.

-- 
/Jacob Carlborg