Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

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

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?

like image 848
chtz Avatar asked Mar 06 '20 16:03

chtz


3 Answers

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.

like image 179
Barry Avatar answered Oct 16 '22 22:10

Barry


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&.
like image 31
T.C. Avatar answered Oct 16 '22 20:10

T.C.


[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.

like image 43
Nicol Bolas Avatar answered Oct 16 '22 21:10

Nicol Bolas