Breaking change in C++20 or regression in clang-trunk/gcc-trunk when overloading equality comparison with non-Boolean return value?

Question

The following code compiles fine with clang-trunk in c++17 mode but breaks in c++2a (upcoming c++20) mode:

// Meta struct describing the result of a comparison
struct Meta {};

struct Foo {
    Meta operator==(const Foo&) { return Meta{}; }
    Meta operator!=(const Foo&) { return Meta{}; }
};

int main()
{
    Meta res = (Foo{} != Foo{});
}

It also compiles fine with gcc-trunk or clang-9.0.0: https://godbolt.org/z/8GGT78

The error with clang-trunk and -std=c++2a:

<source>:12:19: error: use of overloaded operator '!=' is ambiguous (with operand types 'Foo' and 'Foo')
    Meta res = (f != g);
                ~ ^  ~
<source>:6:10: note: candidate function
    Meta operator!=(const Foo&) {return Meta{};}
         ^
<source>:5:10: note: candidate function
    Meta operator==(const Foo&) {return Meta{};}
         ^
<source>:5:10: note: candidate function (with reversed parameter order)

I understand that C++20 will make it possible to only overload operator== and the compiler will automatically generate operator!= by negating the result of operator==. As far as I understand, this only works as long as the return type is bool.

The source of the problem is that in Eigen we declare a set of operators ==, !=, <, ... between Array objects or Array and Scalars, which return (an expression of) an Array of bool (which can then be accessed element-wise, or used otherwise). E.g.,

#include <Eigen/Core>
int main()
{
    Eigen::ArrayXd a(10);
    a.setRandom();
    return (a != 0.0).any();
}

In contrast to my example above this even fails with gcc-trunk: https://godbolt.org/z/RWktKs. I have not managed yet to reduce this to a non-Eigen example, which fails in both clang-trunk and gcc-trunk (the example at the top is quite simplified).

Related issue report: https://gitlab.com/libeigen/eigen/issues/1833

My actual question: Is this actually a breaking change in C++20 (and is there a possibility to overload the comparison operators to return Meta-objects), or is it more likely a regression in clang/gcc?

Trahurn

Related: #58320428 – Trahurn

Answer 1

The Eigen issue appears to reduce to the following:

using Scalar = double;

template<class Derived>
struct Base {
    friend inline int operator==(const Scalar&, const Derived&) { return 1; }
    int operator!=(const Scalar&) const;
};

struct X : Base<X> {};

int main() {
    X{} != 0.0;
}

The two candidates for the expression are

1. the rewritten candidate from operator==(const Scalar&, const Derived&)
2. Base<X>::operator!=(const Scalar&) const

Per [over.match.funcs]/4, as operator!= was not imported into the scope of X by a using-declaration, the type of the implicit object parameter for #2 is const Base<X>&. As a result, #1 has a better implicit conversion sequence for that argument (exact match, rather than derived-to-base conversion). Selecting #1 then renders the program ill-formed.

Possible fixes:

• Add using Base::operator!=; to Derived, or
• Change the operator== to take a const Base& instead of a const Derived&.

Juxtapose

Is there a reason why the actual code couldn't return a bool from their operator==? Because that seems to be the only reason the code is ill-formed under the new rules. – Waterer

The actual code involves an operator==(Array, Scalar) that does element-wise comparison and return an Array of bool. You can't turn that into a bool without breaking everything else. – Juxtapose

This seems a bit like a defect in the standard. The rules for rewriting operator== weren't supposed to affect existing code, yet they do in this case, because the check for a bool return value is not part of selecting candidates for rewriting. – Waterer

@NicolBolas: The general principle being followed is that the check is for whether you can do something (e.g., invoke the operator), not whether you should, to avoid having implementation changes silently affect the interpretation of other code. It turns out that rewritten comparisons break a lot of things, but mostly things that were already questionable and are easy to fix. So, for better or for worse, these rules were adopted anyway. – Mise

Wow, thanks a lot, I guess your solution will resolve our issue (I don't have time to install gcc/clang trunk with reasonable effort at the moment, so I will just check if this breaks anything up to the latest stable compiler versions). – Trahurn

Answer 2

Yes, the code in fact breaks in C++20.

The expression Foo{} != Foo{} has three candidates in C++20 (whereas there was only one in C++17):

Meta operator!=(Foo& /*this*/, const Foo&); // #1
Meta operator==(Foo& /*this*/, const Foo&); // #2
Meta operator==(const Foo&, Foo& /*this*/); // #3 - which is #2 reversed

This comes from the new rewritten candidate rules in [over.match.oper]/3.4. All of those candidates are viable, since our Foo arguments are not const. In order to find the best viable candidate, we have to go through our tiebreakers.

The relevant rules for best viable function are, from [over.match.best]/2:

Given these definitions, a viable function F1 is defined to be a better function than another viable function F2 if for all arguments i, ICSi(F1) is not a worse conversion sequence than ICSi(F2), and then

• [... lots of irrelevant cases for this example ...] or, if not that, then
• F2 is a rewritten candidate ([over.match.oper]) and F1 is not
• F1 and F2 are rewritten candidates, and F2 is a synthesized candidate with reversed order of parameters and F1 is not

#2 and #3 are rewritten candidates, and #3 has reversed order of parameters, while #1 is not rewritten. But in order to get to that tiebreaker, we need to first get through that initial condition: for all arguments the conversion sequences are not worse.

#1 is better than #2 because all the conversion sequences are the same (trivially, because the function parameters are the same) and #2 is a rewritten candidate while #1 is not.

But... both pairs #1/#3 and #2/#3 get stuck on that first condition. In both cases, the first parameter has a better conversion sequence for #1/#2 while the second parameter has a better conversion sequence for #3 (the parameter that is const has to undergo an extra const qualification, so it has a worse conversion sequence). This const flip-flop causes us to not be able to prefer either one.

As a result, the whole overload resolution is ambiguous.

As far as I understand, this only works as long as the return type is bool.

That's not correct. We unconditionally consider rewritten and reversed candidates. The rule we have is, from [over.match.oper]/9:

If a rewritten operator== candidate is selected by overload resolution for an operator @, its return type shall be cv bool

That is, we still consider these candidates. But if the best viable candidate is an operator== that returns, say, Meta - the result is basically the same as if that candidate was deleted.

We did not want to be in a state where overload resolution would have to consider the return type. And in any case, the fact that the code here returns Meta is immaterial - the problem would also exist if it returned bool.

---

Thankfully, the fix here is easy:

struct Foo {
    Meta operator==(const Foo&) const;
    Meta operator!=(const Foo&) const;
    //                         ^^^^^^
};

Once you make both comparison operators const, there is no more ambiguity. All the parameters are the same, so all the conversion sequences are trivially the same. #1 would now beat #3 by not by rewritten and #2 would now beat #3 by not being reversed - which makes #1 the best viable candidate. Same outcome that we had in C++17, just a few more steps to get there.

Umbel

"We did not want to be in a state where overload resolution would have to consider the return type." Just to be clear, while overload resolution itself does not consider the return type, the subsequent rewritten operations do. One's code is ill-formed if overload resolution would select a rewritten ==and the return type of the selected function isn't bool. But this culling doesn't happen during overload resolution itself. – Waterer

Its actually only ill-formed if the return type is something that does not support operator !... – Laszlo

@ChrisDodd No, it has to be exactly cv bool (and before this change, the requirement was contextual conversion to bool - still not !) – Umbel

Unfortunately, this does not solve my actual problem, but that was because I failed to provide a MRE which actually describes my problem. I'll accept this and when I'm able to reduce my problem properly I'll ask a new question ... – Trahurn

Looks like a proper reduction for the original issue is gcc.godbolt.org/z/tFy4qz – Juxtapose

Answer 3

[over.match.best]/2 lists how valid overloads in a set are prioritized. Section 2.8 tells us that F1 is better than F2 if (among many other things):

F2 is a rewritten candidate ([over.match.oper]) and F1 is not

The example there shows an explicit operator< being called even though operator<=> is there.

And [over.match.oper]/3.4.3 tells us that the candidacy of operator== in this circumstance is a rewritten candidate.

However, your operators forget one crucial thing: they should be const functions. And making them not const causes earlier aspects of overload resolution to come into play. Neither function is an exact match, as non-const-to-const conversions need to happen for different arguments. That causes the ambiguity in question.

Once you make them const, Clang trunk compiles.

I can't speak to the rest of Eigen, as I don't know the code, it's very large, and thus can't fit in an MCVE.

Waterer

We only get to the tiebreaker you listed if there are equally good conversions for all arguments. But there aren't: due to the missing const, the non-reversed candidates have a better conversion sequence for the second argument and the reversed candidate has a better conversion sequence for the first argument. – Howlett

@RichardSmith: Yeah, that was the sort of complexity I was talking about. But I didn't want to have to actually go through and read/internalize those rules ;) – Waterer

Indeed, I forgot the const in the minimal example. I'm pretty certain that Eigen uses const everywhere (or outside class definitions, also with const references), but I need to check. I try to break down the overall mechanism Eigen uses to a minimal example, when I find the time. – Trahurn

Answer 4

We have similar issues with our Goopax header files. Compiling the following with clang-10 and -std=c++2a produces a compiler error.

template<typename T> class gpu_type;

using gpu_bool = gpu_type<bool>;
using gpu_int  = gpu_type<int>;

template<typename T>
class gpu_type
{
    friend inline gpu_bool operator==(T a, const gpu_type& b);
    friend inline gpu_bool operator!=(T a, const gpu_type& b);
};

int main()
{
    gpu_int a;
    gpu_bool b = (a == 0);
}

Providing these additional operators seems to solve the problem:

template<typename T>
class gpu_type
{
    ...
    friend inline gpu_bool operator==(const gpu_type& b, T a);
    friend inline gpu_bool operator!=(const gpu_type& b, T a);
};

Wellington

Was this not something that would be useful to have done beforehand? Otherwise, how would a == 0 have compiled? – Waterer

This isn't really a similar issue. As Nicol pointed out, this already didn't compile in C++17. It continues to not compile in C++20, just for a different reason. – Umbel

I forgot to mention: We also provide member operators: gpu_bool gpu_type<T>::operator==(T a) const; and gpu_bool gpu_type<T>::operator!=(T a) const; With C++-17, this works fine. But now with clang-10 and C++-20, these are not found anymore, and instead the compiler tries to generate its own operators by swapping the arguments, and it fails, because the return type is not bool. – Wellington