[Bug c++/105200] New: user-defined operator <=> for enumerated types is ignored

[Bug c++/105200] New: user-defined operator <=> for enumerated types is ignored

[falbrechtskirchinger at gmail dot com]

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

            Bug ID: 105200
           Summary: user-defined operator <=> for enumerated types is
                    ignored
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: falbrechtskirchinger at gmail dot com
  Target Milestone: ---

Version: g++
(Compiler-Explorer-Build-gcc-45fb78c9c456ace1d914c836d15af38ae345b902-binutils-2.36.1)
12.0.1 20220407 (experimental)

Code:

#include <compare>
#include <iostream>

enum class foo { a, b, c, d };

inline std::partial_ordering operator<=>(const foo lhs, const foo rhs) {
        std::cout << "foo <=> called\n";
        return std::partial_ordering::less;
}

int main() {
        std::cout << std::boolalpha;

        bool b1 = foo::b < foo::a;
        std::cout << "foo::b < foo::a := " << b1 << "\n";

        std::cout << "=============\n";

        bool b2 = std::is_lt(foo::b <=> foo::a);
        std::cout << "std::is_lt(foo::b <=> foo::a) := " << b2 << "\n";
}

Comparison of GCC, Clang, MSVC, ICC:
https://godbolt.org/z/n7GnqhMvn


GCC (apparently) always selects the builtin operator <=> for enumerated types
and ignores the user-defined one. GCC's behavior differs from Clang, MSVC, and
ICC (although the latter, despite calling user-defined <=>, still produces the
wrong result).

Of course defining the regular comparison operators works as intended and so
this different behavior with user-defined <=> seems inconsistent, even if it
can be argued that it is to letter of the standard (see [expr.spaceship] on
comparing enumerated types). I disagree with this reading of the standard over
all.

Discussing this on IRC and referring to sections [expr.spaceship],
[over.built], [over.match.oper], and [over.match.best] of the standard no
consensus was reached as to which implementation is correct and I was asked to
open this bug report.

As a reminder to myself: Trying to find a workaround, I deleted operator <
which resulted in a segfault. I should try to re-create this ICE and file a
separate bug.

[Bug c++/105200] user-defined operator <=> for enumerated types is ignored

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
The reason why g++ selects the built-in https://eel.is/c++draft/over.built#15
operator over the user supplied spaceship candidate is the
https://eel.is/c++draft/over.match.best#general-2.8 rule, that candidate is
rewritten while the builtin one is not.
What rule do you think would make the rewritten candidate better than the
built-in?

[Bug c++/105200] user-defined operator <=> for enumerated types is ignored

[jakub at gcc dot gnu.org]

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

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
If one defines instead say bool operator<(const foo, const foo);
then the built-in candidate isn't considered because of
https://eel.is/c++draft/over.match.oper#3.3
But for the user operator<=> vs. built-in operator<, they don't have the same
operator name, so the built-in operator< is in the candidate set.

[Bug c++/105200] user-defined operator <=> for enumerated types is ignored

[falbrechtskirchinger at gmail dot com]

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

--- Comment #3 from Florian Albrechtskirchinger <falbrechtskirchinger at gmail dot com> ---
(In reply to Jakub Jelinek from comment #2)
> If one defines instead say bool operator<(const foo, const foo);
> then the built-in candidate isn't considered because of
> https://eel.is/c++draft/over.match.oper#3.3
> But for the user operator<=> vs. built-in operator<, they don't have the
> same operator name, so the built-in operator< is in the candidate set.

My understanding is that the candidate set consists of the builtin operators
and the rewritten operators. But as you have pointed out, according to
https://eel.is/c++draft/over.match.best#general-2.8, the rewritten operator is
a worse choice than the builtin one.

Yet the other compilers disagree. And I have trouble believing the committee
wanted this inconsistent behavior between the legacy operators and spaceship.

Where do we go from here? File bugs with the other compilers? Ask the authors
of the papers on the spaceship operator?

[Bug c++/105200] user-defined operator <=> for enumerated types is ignored

[ppalka at gcc dot gnu.org]

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

Patrick Palka <ppalka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ppalka at gcc dot gnu.org

--- Comment #4 from Patrick Palka <ppalka at gcc dot gnu.org> ---
Reduced example exhibiting implementation divergence:

enum class foo { a, b };
constexpr bool operator<=>(foo, foo) { return false; }
static_assert(foo::a < foo::b);

GCC selects the built-in operator< (thus the assert passes) whereas
Clang/MSVC/ICC select the rewritten operator<=> (thus the assert fails).

[Bug c++/105200] user-defined operator <=> for enumerated types is ignored

[ppalka at gcc dot gnu.org]

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

--- Comment #5 from Patrick Palka <ppalka at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #2)
> If one defines instead say bool operator<(const foo, const foo);
> then the built-in candidate isn't considered because of
> https://eel.is/c++draft/over.match.oper#3.3
> But for the user operator<=> vs. built-in operator<, they don't have the
> same operator name, so the built-in operator< is in the candidate set.

I came to the same conclusion but for a different reason.  According to p3.3.4
only a "non-member candidate" can prevent a built-in candidate from being
considered.  And according to p3.2 and p3.4, a user operator<=> is a "rewritten
candidate" not a "non-member candidate", and therefore a user operator<=> can't
prevent a built-in candidate from being considered (even if their
parameter-type-lists are the same).

[Bug c++/105200] user-defined operator <=> for enumerated types is ignored

[barry.revzin at gmail dot com]

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

Barry Revzin <barry.revzin at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |barry.revzin at gmail dot com

--- Comment #6 from Barry Revzin <barry.revzin at gmail dot com> ---
This strikes me as a definite wording issue rather than actual design intent.
Patrick is correct as to what the wording says - it says non-member candidate,
so the rewritten candidates don't count. But I think really we should also
consider rewritten candidates. Clang and MSVC both do - which seems much more
in line with expectation and the original design.

For class types, you can just provide <=>, but for enums, you have to provide
<, >, <=, >=, and <=>?? 

I'm opening a Core issue for this: https://github.com/cplusplus/CWG/issues/205

[Bug c++/105200] user-defined operator <=> for enumerated types is ignored

[jens.maurer at gmx dot net]

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

Jens Maurer <jens.maurer at gmx dot net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jens.maurer at gmx dot net

--- Comment #7 from Jens Maurer <jens.maurer at gmx dot net> ---
I have created core issue 2673 for this.

https://cplusplus.github.io/CWG/issues/2673.html

[Bug c++/105200] user-defined operator <=> for enumerated types is ignored

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2023-01-01
             Status|UNCONFIRMED                 |SUSPENDED
     Ever confirmed|0                           |1

--- Comment #8 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Suspending this GCC bugzilla until C++ standards has a resolution on the defect
report.

[Bug c++/105200] user-defined operator <=> for enumerated types is ignored

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|SUSPENDED                   |NEW

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I think CWG2673 has been approved as DR in Issaquah 2023.

More complete testcase could be:

#include <compare>

enum class E : int { E0 = 0, E1 = 1 };
enum class F : int { F0 = 0, F1 = 1 };

constexpr auto
operator<=> (E lhs, E rhs)
{
  return (int) rhs <=> (int) lhs;
}

static_assert ((E::E0 <=> E::E1) == std::strong_ordering::greater);

static_assert (E::E0 > E::E1);

constexpr bool
operator== (F lhs, F rhs)
{
  return (int) lhs != (int) rhs;
}

static_assert (F::F0 == F::F1);
static_assert (!(F::F0 == F::F0));
static_assert (!(F::F0 != F::F1));
static_assert (F::F0 != F::F0);

The reason we don't handle this is that joust encounters the
https://eel.is/c++draft/over.match.best#general-2.8 and
https://eel.is/c++draft/over.match.best#general-2.9 handling first:
  /* F2 is a rewritten candidate (12.4.1.2) and F1 is not, or F1 and F2 are
     rewritten candidates, and F2 is a synthesized candidate with reversed
     order of parameters and F1 is not.  */
  if (cand1->rewritten ())
    {
      if (!cand2->rewritten ())
        return -1;
      if (!cand1->reversed () && cand2->reversed ())
        return 1;
      if (cand1->reversed () && !cand2->reversed ())
        return -1;
    }
  else if (cand2->rewritten ())
    return 1;
before it handles the https://eel.is/c++draft/over.match.oper#3.3.4
  /* Check whether we can discard a builtin candidate, either because we
     have two identical ones or matching builtin and non-builtin candidates.

     (Pedantically in the latter case the builtin which matched the user
     function should not be added to the overload set, but we spot it here.

     [over.match.oper]
     ... the builtin candidates include ...
     - do not have the same parameter type list as any non-template
       non-member candidate.  */

  if (identifier_p (cand1->fn) || identifier_p (cand2->fn))
    {
      for (i = 0; i < len; ++i)
        if (!same_type_p (cand1->convs[i]->type,
                          cand2->convs[i]->type))
          break;
      if (i == cand1->num_convs)
        {
          if (cand1->fn == cand2->fn)
            /* Two built-in candidates; arbitrarily pick one.  */
            return 1;
          else if (identifier_p (cand1->fn))
            /* cand1 is built-in; prefer cand2.  */
            return -1;
          else
            /* cand2 is built-in; prefer cand1.  */
            return 1;
        }
    }

Now, I wonder if the https://eel.is/c++draft/over.match.oper#3.3.4 stuff
shouldn't be conceptually done somewhere else, not in joust because it is about
whether certain candidate is added to the list of built-in candidates at all. 
Or, if it is done in joust, shouldn't it be done first or very close to the
start?