7

I have a number of structs that are derived from the same base for the purpose of code reuse, but I do not want any form of polymorphism.

struct B {
    int field;
    void doStuff() {}
    bool operator==(const B& b) {
        return field == b.field;
    }
};

struct D1 : public B {
    D1(int field) : B{field} {}
};
struct D2 : public B {
    D2(int field) : B{field} {}
};

Structs D1 and D2 (and more similar structs) derive from B to share common fields and methods, so that I would not need to duplicate those fields and methods in each of the derived classes.

Struct B is never instantiated; I only use instances of D1 and D2. Furthermore, D1 and D2 are not supposed to interact with each other at all. Essentially, I do not want any polymorphic behaviour: D1 and D2, for all purposes, should act as unrelated structs.

I would like any D1 to be compared with other D1s for equality, and any D2 to be compared with other D2s for equality. Since D1 and D2 don't contain any fields, it would seem appropriate to define an equality operator in struct B.

However, (as expected) I get the following interaction between D1 and D2:

int main() {
    D1 d1a(1);
    D1 d1b(1);
    D2 d2(1);

    assert(d1a == d1b); // good

    assert(d1a == d2); // oh no, it compiles!
}

I don't want to be able to compare D1 with D2 objects, because for all purposes, they should act as if they are unrelated.

How can I make the last assertion be a compile error without duplicating code? Defining the equality operator separately for D1 and D2 (and all the other similar structs) would mean code duplication, so I wish to avoid that if possible.

Bernard
  • 5,209
  • 1
  • 34
  • 64

4 Answers4

7

"Structs D1 and D2 (and more similar structs) derive from B to share common fields and methods"

Then make B a private base class. Obviously, D1 and D2 shouldn't share their equality operator, since the two operators take different arguments. You can of course share part of the implementation as bool B::equal(B const&) const, as this won't be accessible to outside users.

MSalters
  • 173,980
  • 10
  • 155
  • 350
  • That means `doStuff()` can't be called by other classes, though. So I'll have to write forwarding functions in all my derived classes. – Bernard Jul 16 '18 at 16:14
  • 1
    @Bernard: That's what `using` declarations for members are for. You can publicly `using` a member of a privately inherited class. – Nicol Bolas Jul 16 '18 at 19:13
4

You can use CRTP to define operator == only on the reference to the base class of the final type:

template<typename T>
struct B {
    int field;
    void doStuff() {}
    bool operator==(const B<T>& b) {
        return field == b.field;
    }
};

struct D1 : public B<D1> {
    D1(int field) : B{field} {}
};
struct D2 : public B<D2> {
    D2(int field) : B{field} {}
};

This causes the first assert to compile and the second one to be rejected.

user4815162342
  • 141,790
  • 18
  • 296
  • 355
  • It turns out that I could also make other simplifications with my code using CRTP. I'm going with this solution. A possible danger with CRTP is that I can't put a `static_assert` in class `B` to make sure `T` indeed derives from `B`, which means that carelessness here might result in some bugs at run-time. – Bernard Jul 16 '18 at 16:12
  • Any reason for the downvotes? This seems to be the best answer and I am keen to accept this. – Bernard Jul 16 '18 at 16:21
  • @Bernard If I understand your concern correctly, you would like to prevent someone from passing `B::operator==` a `const X&` instead of a `const B&`, which would compile as long as `X` happened to have a field named `field`. If so, you can change `field == b.field` to `field == static_cast&>(b).field`. It's a bit more verbose, but `static_cast` should ensure `T` derives from `B`. Here is the code amended with a `==` that [reproduces the issue](https://pastebin.com/EuV3FRnE). – user4815162342 Jul 16 '18 at 20:18
  • @Bernard The downvoters chose to remain silent, but I guess the downvotes come from the general preference for a function operator as seen in other answers. Also, is the `static_cast` variant useful? If so, I'll include it in the answer. – user4815162342 Jul 17 '18 at 07:18
  • Yes, it is useful :) A class-wide solution would be preferred (because inheritance is really a property of the class instead of `operator==`), but in the lack of other alternatives this would have to suffice. – Bernard Jul 17 '18 at 08:18
  • @Bernard I've amended the answer with a simpler solution, constraining the type of the *argument* of `operator ==` to `const B&`, which automatically rejects `T&`. I'm not sure that a class-wide solution would apply - the problem was in the _argument_ of `operator ==`, which is a property of the operator. – user4815162342 Jul 17 '18 at 08:54
  • I think I might have misunderstood you. When I said "I can't put a `static_assert` in class `B` to make sure `T` indeed derives from `B`", I meant that I want to protect against accidentally writing `class D3 : public B` instead of `class D3 : public B`, meaning that ideally the former should be a compilation error. – Bernard Jul 17 '18 at 09:17
  • @Bernard Now I see what you mean. The solution in [this answer](https://stackoverflow.com/a/11241079/1600898) covers that, I think. – user4815162342 Jul 17 '18 at 10:40
  • Thanks for your help, but I've already read that and it would only work for a inheritance that is only one level deep. Unfortunately, in my use case I have about ten leaf classes (i.e. those that will be actually instantiated and used by other parts of the code) and the inheritance goes about three levels deep. I've looked at all the other answers as well and all the class-wide solutions are hacks in one way or another. Of all the imperfect choices over there, I think the macro answer is the cleanest choice (but it is still really yucky). – Bernard Jul 17 '18 at 11:55
  • @Bernard FYI, my answer solves this leaf class issue (and still only requires one template function. – jonspaceharper Jul 18 '18 at 07:56
  • @JonHarper However, your answer doesn't use CRTP, which the OP discovered fits his needs for other things. The "leaf class issue" comes from attempting to prevent accidentally writing `class D3: public B` (regardless of the use of `operator ==`). – user4815162342 Jul 18 '18 at 08:00
  • Darn it, I missed that comment. Thanks. – jonspaceharper Jul 18 '18 at 08:12
  • 1
    @JonHarper Yeah, both your answer and lubgr's answer solves this issue satisfactorily as well. But I've come to realize that CRTP is more extensible - in particular, I can have other member functions written in a similar way to the member `operator==`. – Bernard Jul 18 '18 at 08:59
3

Instead of defining your equality operator as part of the base class, you usually need two functions in the derived classes:

struct B {
    int field;
    void doStuff() {}
};

struct D1 : public B {
    D1(int field) : B{field} {}
    bool operator==(const D1& d) {
        return field == d.field;
    }
};
struct D2 : public B {
    D2(int field) : B{field} {}
    bool operator==(const D2& d) {
        return field == d.field;
    }
};

Or, as is generally preferred, you could make them free functions:

bool operator==(const D1 &lhs, const D1 &rhs)
{
    return lhs.field == rhs.field;
}

bool operator==(const D2 &lhs, const D2 &rhs)
{
    return lhs.field == rhs.field;
}

Note: If field was not a public member, you would need to declare the free function version as a friend.

Handling a large number of arbitrary types

Okay, so maybe you have D3 through D99, as well, some of which are indirect descendant of B. You'll need to use templates:

template <class T>
bool operator==(const T &lhs, const T &rhs)
{
    return lhs.field == rhs.field;
}

Great! But this grabs everything, which is bad for unrelated types that are not supposed to be comparable. So we need constraints.

TL;DR; Solution

Here's a trivial implementation without any code duplication (i.e. works for an arbitrary number of derived types):

template <class T, class = std::enable_if<std::is_base_of<B,T>()
                       && !std::is_same<B, std::remove_cv_t<std::remove_reference_t<T>>>()>>
bool operator==(const T &lhs, const T &rhs)
{
    return lhs.field == rhs.field;
}

The enable_if checks first that T inherits from B then ensures it's not B. You stated in your question that B is basically an abstract type and is never directly implemented, but it's a compile-time test, so why not be paranoid?

As you later noted in comments, not all D# are derived directly from B. This will still work.

Why you are having this issue

Given the following:

D1 d1(1);
D2 d2(2);
d1 == d2;

The compiler has to find a comparison operator, whether a free function or member of D1 (not D2). Thankfully, you've defined one in class B. The third line above can equivalently be stated:

d1.operator==(d2)

operator==, however, is part of B, so we're basically calling B::operator==(const B &). Why does this work when D2 is not B? A language lawyer would clarify if it's technically argument dependent lookup (ADL) or overload resolution, but the effect is that D2 is silently cast to B as part of the function call, making this equivalent to the above:

d1.operator==(static_cast<B>(d2));

This happens because no better comparison function can be found. Since there's no alternative, the compiler selects B::operator==(const B &) and makes the cast.

jonspaceharper
  • 4,207
  • 2
  • 22
  • 42
2

You can remove the equality operator from your original definition of the struct and replace it with a function template accepting two identical parameter types:

template <class T> bool operator == (const T& lhs, const T& rhs)
{
    return lhs.field == rhs.field;
}

Note that this function is somewhat "greedy", it might be best to put it in a namespace (together with the structs, to enable ADL) or to further constrain the types like this:

#include <type_traits>

template <class T, std::enable_if_t<std::is_base_of_v<B, T>, int> = 0>
bool operator == (const T& lhs, const T& rhs)
{
    return lhs.field == rhs.field;
}

(note that std::is_base_of_v requires C++17, but the verbose counterpart exists since C++11).

As a last tweak, in order to prevent such an explicit instantiation:

operator == <B>(d1a,  d2); // ultra-weird usage scenario, but compiles!

or (as @Aconcagua pointed out in the comments) type deduction with base-class references to the derived structs,

B& b1 = d1a;
B& b2 = d2;

assert(b1 == b2); // Compiles, but see below.

you also might want to add

template <> bool operator == <B>(const B&, const B&) = delete;
lubgr
  • 37,368
  • 3
  • 66
  • 117
  • 1
    More important: if not deleting, we could do: `D1 d1; D2 d2; B& b1 = d1; B& b2 = d2; bool eq = b1 == b2;`. – Aconcagua Jul 16 '18 at 13:45
  • @Aconcagua Right, thanks for the hint! I'll incorporate that. – lubgr Jul 16 '18 at 13:48
  • Your template function lacks constraints. This means that `struct Unrelated { char field; };` could participate in overload resolution. That's probably being *too* greedy. – jonspaceharper Jul 16 '18 at 13:53
  • Add `std::enable_if`? – jonspaceharper Jul 16 '18 at 13:57
  • With the right SFINAE to prevent this function from being too greedy, this would also work as good as CRTP (assuming I only need operators, which can be declared outside the class definition). – Bernard Jul 16 '18 at 16:18
  • @Bernard Thanks for your suggestions, I added such a constraint to the answer. – lubgr Jul 16 '18 at 19:55