Fix Issue 22159 - == causes error for array of classes in safe method

[RazvanN7]

If the type of classes that are compared does not override opEquals, Object.opEquals ends up, eventually, being called. Since Object.opEquals is @System, @safe user code is going to issue a confusing "Incompatible types for comparison: LeClass and LeClass". To fix this, I'm simply checking if Object.opEquals is being called and I'm trusting it (it's just doing an is comparison anyway).

"But why aren't you marking Object.opEquals as @safe?" -> that will impose the restriction that all classes define a @safe opEquals (which would normally make sense, but it would break tons of code).

"This makes the code ugly" -> I agree, but I have no other idea on how to fix this, other than ProtoObject. Maybe other better solutions exist, feel free to enlighten me.

"This does not fix the underlying issue that the compiler gags errors when analyzing the hook, therefore it's still possible to have a confusing error message, for example when a class defines a system opEquals that is called from safe code" -> Yes, but that matter can be explored in a different PR.

cc @adamdruppe as you have been looking at this.

[dlang-bot]

Thanks for your pull request and interest in making D better, @RazvanN7! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

• My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
• My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
• I have provided a detailed rationale explaining my changes
• New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

---

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	22159	regression	"==" causeses error for array of classes in safe method

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "stable + dmd#14988"

[adamdruppe]

It looks to me that the original bug is a diagnostic bug - the compiler is not giving a helpful error message, but the behavior is correct. The opEquals is system so it is not supposed to be called from safe code.

The cast in here is not correct, since the method may be overridden by child classes and violate the safety guarantees.

Add this as a unittest:

class C { int a; this(int) @safe {} }
class D : C {
        this(int a) @safe { super(a); }
        override bool opEquals(Object rhs) const {
                // obviously not safe
                *(cast(int*) 0x5afe) = 0xdead5afe;
                return true;
        }
}

@safe void main()
{
    C c = new D(1);
    C[] a = [c, c, c];
    assert(a == [c, c, c]);
}

It should definitely fail to compile. I believe it will compile with this PR.

[dkorpel]

@RazvanN7 Do you agree with Adam?

[RazvanN7]

Yes, it seems that @adamdruppe is right.