[Bug c++/100330] operator bool() is used when operator<() is available to do comparison

[redi at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100330

Jonathan Wakely  changed:

   What|Removed |Added

 Status|UNCONFIRMED |RESOLVED
 Resolution|--- |INVALID

--- Comment #2 from Jonathan Wakely  ---
The crux of the problem is that bool is comparable using any of == != < > <=
and >= and your type is implicitly convertible to bool. That means your type is
comparable using any of == != < > <= >= too, but with weird results:

  vertex_description v1((void*)1), v2((void*)2);
  assert(v1 < v2);
  assert(v1 == v2);  // OK, but v1 < v2 also true!
  assert(v1 >= v2);  // OK, but v1 < v2 also true!

This means your class' comparisons are unusable. The only one with sensible
behaviour is operator< which doesn't use the bool conversion, but the result of
that operator is inconsistent with the results of the others.

In C++17 you could get away with this, and using the class as a key in an
associative container worked because it only ever used operator< and ignored
the others.

In C++20 comparisons work differently, and a type with broken/inconsistent
comparisons will cause problems in many places. Comparisons for std::pair (and
std::tuple, std::optional, and containers) assume your type has consistent
comparisons, and if it doesn't, things will break.

There are two ways to fix your class to work in C++20:

Make the conversion to bool explicit:

  explicit operator bool() const;

This has been recommended practice for a decade now, since C++11. Even before
then, implicit conversions to bool were known to be a problem (see
https://www.artima.com/articles/the-safe-bool-idiom which was written in 2004
and stated that an implicit conversion to bool makes your type comparable).

Or define all the comparisons for the type so that they behave consistently:

  bool operator<(const vertex_descriptor) const;
  bool operator>(const vertex_descriptor) const;
  bool operator<=(const vertex_descriptor) const;
  bool operator>=(const vertex_descriptor) const;
  bool operator==(const vertex_descriptor) const;
  bool operator!=(const vertex_descriptor) const;

For many types, this can be done very simply:

  bool operator==(const vertex_descriptor) const = default;
  auto operator<=>(const vertex_descriptor) const = default;

This will fix your type, and make pair have sensible
comparisons too.

[Bug c++/100330] operator bool() is used when operator<() is available to do comparison

[redi at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100330

--- Comment #1 from Jonathan Wakely  ---
Reduced:

#include 

extern "C" int puts(const char*);

struct vertex_descriptor {

operator bool() const {
puts(__func__);
return i;
}

bool operator<(const vertex_descriptor b) const {
puts(__func__);
return i < b.i;
}

int i = 0;
};

int main()
{
std::pair p1({1}, 0);
std::pair p2({2}, 0);
if (!(p1 < p2))
  throw 1;
}

This aborts in C++20. I think GCC is correct according to the standard.

In C++20 the operator< for std:pair is synthesized from operator<=>.

That uses the synth-three-way helper defined in
http://wg21.link/expos.only.func which determines that vertex_descriptor is
three-way-comparable, and so uses <=> to compare them. But that uses your
implicit conversion to bool, i.e. it is equivalent to p1.first==true <=>
p2.first==true

An explicit operator bool avoids the problem, because synth-three-way will not
implicitly convert the operands to bool, and so will do:

  if (p1.first < p2.first) return weak_ordering::less;
  if (p2.first < p1.first) return weak_ordering::greater;
  return weak_ordering::equivalent;

This uses vertex_descrioptor::operator< as intended.