opEquals @safe is ignored

[Jonathan M Davis]

On Sunday, May 24, 2020 2:57:28 AM MDT Luis via Digitalmars-d-learn wrote:
> Lets take this example code (https://run.dlang.io/is/Vkpx9j) :
>
> ´´´D
> import std;
>
> void main()
> {
> }
>
> class ExampleC
> {
>    int x;
>    this (int x) @safe
>    {
>      this.x = x;
>    }
>
>    override bool opEquals(Object o) const @trusted
>    {
>      if (ExampleC rhs = cast(ExampleC)o) {
>        return this.x == rhs.x;
>      }
>      return false;
>    }
> }
>
> @safe unittest
> {
>      auto c = new ExampleC(1);
>      assert(c != new ExampleC(23));
> }
> ´´´
>
> dmd ignores @trusted or @safe on opEquals, throwing this error :
>
> onlineapp.d(27): Error: @safe function
> onlineapp.__unittest_L24_C7 cannot call @system function
> object.opEquals
>
> An override @system or @trusted function can't be @safe, or I it
> a bug ?
>
> Also, how will this be affected by DIP1028 ?

The core problem is that Object does not have attributes on any of its
functions - including opEquals. And in the case of opEquals, the problem is
one layer worse, because == gets lowered to the free function opEquals which
in turn calls opEquals on the class reference itself (after doing additional
checks like that the reference isn't null and that you're not comparing a
reference to itself), and that free function has no attributes. In fact, the
only reason that using == on const objects works is because of a hack in
druntime that casts away const (which means that it's technically possible
and even downright trivial to break the type system by mutating a const
class object within opEquals).

And really, the only fix for this is to remove opEquals, opCmp, toHash, and
toString from Object, since no matter which set of attributes you pick,
there will be problems. With the addition of templates, it's no longer
necessary for any of those functions to even be on Object, and it really
doesn't make sense for them to be there, but they've been there since long
before templates or attributes were added to the language.

It was decided years ago that those functions should be removed from Object,
but _how_ to do it with minimal code breakage is a thorny problem, and it
hasn't really been a priority. The currently proposed solution (which has a
work-in-progress DIP IIRC) is to introduce a new root class to the language
below Object which has _nothing_ on it (the current name for that root class
being ProtoObject). Object would then continue to be the default base class
of any class, but it would then be derived from ProtoObject, and best
practice at that point would really be to explicitly derive your class from
ProtoObject (with Object being left in place pretty much just to avoid
breaking existing code). However, the DIP for ProtoObject has yet to be
fully sorted out, and it definitely hasn't been accepted yet, so it can't
yet fix the problem.

So, for now, you're basically forced to use @trusted when comparing class
references in @safe code. It's annoying, but because Object's opEquals is
@system, we're kind of stuck at the moment.

- Jonathan M Davis