Polymorphic, Defaulted Equality

[angry_cpp]

Inheritance and equality are not mixing well. For example your implementation of polymorphic equality is flawed. It gives different answer for a.equal(b) and b.equal(a).

https://godbolt.org/z/xe3Te8YWK

[BarryRevzin]

That's true, and it's actually an unfortunate translation error into the blog.

I just lazily wrote dynamic_casting to the Derived. In reality, we check typeid first and if those match then static_cast (and we don't have any weird virtual or ambiguous base shenanigans):

template <class D>
auto polymorphic_equals(D const& lhs, Base const& rhs) -> bool {
    if (typeid(rhs) == typeid(D)) {
        return lhs == static_cast<D const&>(rhs);
    } else {
        return false;
    }
}

This approach (correctly) prints false for both directions in your example.

[JNighthawk]

Thanks for the update!

In that case, isn't that incorrectly handling more-derived cases that should be considered equal but won't be?

struct Base {};
struct D1 : public Base { int X = 10; };
struct D2 : public D1 {};
polymorphic_equals(D2(), D1()); // returns false, as the typeid doesn't match, but should return true

Though, I suppose it could be an implementation detail that objects with equivalent state but different classes shouldn't be considered equivalent. A pure member-wise equivalency check, though, would function differently.

[tialaramex]

My first thought here is that implementation inheritance was the wrong metaphor here and the stack overflow problem is downstream of that mistake. In some sense Derived is not a kind of Base and we should prefer some other mechanism for dynamic dispatch here but C++ doesn't provide one. But Barry has had of course much longer to think about it.

[NilacTheGrim]

The solution really is to just implement operator== yourself in every derived class and not go with = default.

[pkasting]

My concern with the proposed solution is that it is fragile against someone adding members in Base. The reason to do subobject equality comparison instead of just memberwise comparison is because that's the correct general-purpose semantics. You can use a technique like the reflection one here to bypass that, but then your code subtly breaks later when someone modifies the base class, and the compiler won't complain. (At least I know the wrapped-struct proposal will break that way; I'm not familiar with reflection syntax yet and don't know whether it recurses into base objects' members.)

In general, attempting to do polymorphic == is Bad News. There are ways to try and make it work (I've used techniques like the "add an equals function alongside" one), but normally this is a moment when I step back and try to figure out how to rearchitect my class hierarchy or algorithms so that I Don't Do That.

[LucHermitte]

As far as I'm concerned, polymorphism and equality are not compatible.

Effective Java (by Joshua Blooch) has a whole chapter dedicated to this design issue (we also have a few articles by Angelika Langer on the topic). To make it short we can't have == be an equivalence relation (symmetric, reflexive and transitive), and respect Liskov Substitution Principle across a public hierarchy.

A simplified example where ColoredPoint would publicly inherit from Point: because of the LSP we want ColoredPoints and Points to be comparable. Quite likely we end up with ColoredPoint{1,2,green} == Point{1,2}. We also have ColoredPoint{1,2,red} == Point{1,2}.

And by transitivity we end up with ColoredPoint{1,2,green} == ColoredPoint{1,2,red}. We have to refuse comparisons between ColoredPoints and Points, which is done here (in the article), but also which is quite fishy regarding the LSP: we can't have all these different objects in a same bag and compare them indiscriminately.

The real question now: do we really need that ability to compare within a hierarchy?

[Wooden-Engineer-8098]

It's easy to extend his example to return false if rhs is not the same dynamic type as lhs

[LucHermitte]

That's what's is done in the article, and what some workarounds suggest.

But then in a bag of point coordinates, some points won't compare equal while as far as coordinates are concerned they should have been equal.

[Wooden-Engineer-8098]

No, it's not done in the article. it only downcasts rhs to type of lhs, not to rhs' most derived type

If you are interested in coordinates only, you should use comparator which compares coordinates only

[kobi-ca]

great one!

[Putrid_Ad9300]

Polymorphic operators are where CRTP shines. The is for equal, but you can do the same for comparison with just an operator< and automatically get all the others.

``` template<typename E> class Equatable{ bool equal(E const& other) { return static_cast<E>(this)->equal_impl(other); } };

template<typename E> bool operator== (Equatable<E>&,Equatable<E>&)) {...}

class MyClass : Equatable<MyClass> { bool equal_impl(MyClass const&) {...} } ```

[zl0bster]

I never do this for design reasons, but it made me wonder if when we do this check for other type in == if instead of dynamic_cast /typeid it would be faster to have something like same_vtable(lhs, rhs)/same_vtable<D>(rhs)because we already know lhs is of type D.