Why is `opequals` for objects not `@safe` by default?

[Jonathan M Davis]

On Sunday, April 7, 2024 3:02:58 AM MDT Liam McGillivray via Digitalmars-d 
wrote:
> If you were willing to do breaking changes, perhaps the base
> `opEquals` function can be set to `@safe`. Of course, that would
> be a breaking change if anyone has an override that isn't
> compatible with `@safe`. However, perhaps there can be a
> deprecation warning for any non-safe code defined inside
> `opEquals`, and then a few years later the base function can be
> declared as `@safe`. Given that `@trusted` exists, this would be
> a very easy fix for anyone to make.

No, we really don't want to force @safe on opEquals, because that means that
you can't write one that's @system, meaning that you're potentially forced
to use @trusted on code that really shouldn't be treated as @safe.

In many cases, @safe makes sense, but it doesn't always. Ideally, we
wouldn't force _any_ attributes on opEquals, opCmp, toHash, or toString, and
they would be left entirely up to programmers to decide. But given the
nature of inheritance, that means not having them on Object and instead
putting them on the derived class. And it's already the case that if you're
doing anything with inheritance, you're designing your own class hierarchy
with your own set of requirements - which could include attributes like
@safe or pure, or it could exclude them. So, it would simply mean that you
couldn't compare Objects, which is just fine, since that isn't going to be a
meaningful comparison unless the two class objects are from the same class
hierarchy, and as long as the appropriate druntime code is templatized, it
can handle whatever attributes you put on those functions in your class
hierarchy instead of needing to do anything with Object.

> Does it currently work if a `@safe` declaration is added for
> `opEquals`, but no definition? If not I think this should work.
> It would just default to the existing `opEquals` function, but
> with the `@safe` attribute.

Adding declarations with no definitions is just going to result in linker
errors when code tries to call the function - or nothing if the function is
never called.

No, if you want to add more attributes onto opEquals and actually be able to
take advantage of them with ==, then you need to provide a new overload.
E.G. if you have

class C
{
    bool opEquals(C rhs) @safe
    {
        ...
    }

    override bool opEquals(Object rhs) @safe
    {
        ...
    }
}

then the first overload will be called when comparing class references of
type C - or class references derived from C, whereas if you're comparing
class references of type Object (or class references which aren't C or
derived from C), then they'll be compared as Object, and the base class
opEquals will be called, resulting in your second overload being called
thanks to polymorphism, but because the base class version is @system, ==
will still be treated as @system in that case. E.G.

C lhs = new C;
C rhs = new C;

// Can be used in @safe code, because they're both C.
auto result == lhs == rhs;

but

Object lhs = new C;
Object rhs = new C;

// Can't be used in @safe code, because they're both Object.
auto result == lhs == rhs;

> Anyway, I discovered that `==` can be replaced with `is`, and it
> seems to do the same thing while working within a `@safe`
> function.

The is operator and == are _not_ the same. When you use is on class
references, it's true if and only if the two references point to the same
object. It's basically doing a pointer comparison. In contrast, == calls the
free function, opEquals, which will call opEquals on the class references to
compare them if they're not null and they're not pointing to the same
object. And then whether they're considered equal or not depends on the
implementation of opEquals.

https://dlang.org/spec/expression.html#identity_expressions

So, if you had something like

class C
{
    private int _value;

    this(int value)
    {
        _value = value;
    }

    bool opEquals(C rhs) @safe
    {
        return _value == rhs._value;
    }

    override bool opEquals(Object rhs)
    {
        if(auto c = cast(C)rhs)
            return _value == c._value;
        return false;
    }
}

auto a = new C(42);
auto b = new C(42);

then

assert(a is b);

would fail, whereas

assert(a == b);

would be pass.

> On Friday, 5 April 2024 at 05:56:05 UTC, Jonathan M Davis wrote:
> > Ideally, Object wouldn't have functions like opEquals, and
> > they'd only be added by derived classes so that programs could
> > put whatever attributes on them make the most sense, but that's
> > a breaking change, so it hasn't happened.
> > - Jonathan M Davis
>
> That sounds like a horrible idea. For something as basic as
> opEquals, one shouldn't need to do an operator override.

You already need to do an override to get opEquals unless you want it to
just compare the references, which is borderline useless. All that opEquals
on Object does is compare the addresses of the references with the is
operator, and that's almost never what you want when doing an equality
check. In almost all cases, you want to be comparing the values of the
member variables, and that means writing your own opEquals.

- Jonathan M Davis