[Bug libstdc++/98842] New: optional's spaceship operations generates wrong code when operator== is not present

[Bug libstdc++/98842] New: optional's spaceship operations generates wrong code when operator== is not present

[nunoplopes at sapo dot pt]

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

            Bug ID: 98842
           Summary: optional's spaceship operations generates wrong code
                    when operator== is not present
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: nunoplopes at sapo dot pt
  Target Milestone: ---

struct expr {
  std::strong_ordering operator<=>(const expr &rhs) const;
  //bool operator==(const expr &rhs) const;
};

int f() {
  return std::is_eq(std::optional<expr>() <=> std::optional<expr>());
  //return std::optional<expr>() == std::optional<expr>();
}


Function f() is compiled to 0 like that. But if you uncomment the
expr::operator== line, it returns 1.

https://gcc.godbolt.org/z/fv85eP

[Bug libstdc++/98842] optional's spaceship operations generates wrong code when operator== is not present

[gcc@nicholas-schwab.de]

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

gcc@nicholas-schwab.de changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |gcc@nicholas-schwab.de

--- Comment #1 from gcc@nicholas-schwab.de ---
Three problem lies in the concept three_way_comparable_with. It uses
__detail::__weakly_eq_cmp_with that requires u == t. Due to paper P1185
operator== will not lookup operator<=>. Therefore having only operator<=>
defined does not suffice to fulfill __detail::__weakly_eq_cmp. Hence the first
of these two templates is SFINAE'D out and the second is taken. 

   template<typename _Tp, three_way_comparable_with<_Tp> _Up>
    constexpr compare_three_way_result_t<_Tp, _Up>
    operator<=>(const optional<_Tp>& __x, const optional<_Up>& __y)

  template<typename _Tp, typename _Up>
    constexpr compare_three_way_result_t<_Tp, _Up>
    operator<=>(const optional<_Tp>& __x, const _Up& __v) 

The second will however compare false whenever the left hand side is empty. 

The standard requires that three_way_comparable_with also requires
weakly_equality_comparable. Hence the implementation of this concept is
correct. However this might be a defect in the standard. 

Note that the standard also wants three_way_comparable_with<_Tp> for the second
template (http://eel.is/c++draft/optional#comp.with.t-25). So the second
template above is non-confirming. But it seems to me that a comparison between
two optionals then is always ambiguous, so this should be a defect.

[Bug libstdc++/98842] optional's spaceship operations generates wrong code when operator== is not present

[gcc@nicholas-schwab.de]

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

--- Comment #2 from gcc@nicholas-schwab.de ---
Created attachment 50063
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50063&action=edit
Patch operator<=>

The operator<=>(optional<_Tp>, _Up) is currently underconstrained. cf
http://eel.is/c++draft/optional.comp.with.t#25 . This patch fixes this by
adding the constraint three_way_comparable_with<_Tp> for _Up. Then in the
program that started this bug report 

std::optional<expr>{} <=> std::optional<expr>{} 

will not compile if operator==(expr, expr) is not declared.

[Bug libstdc++/98842] optional's spaceship operations generates wrong code when operator== is not present

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2021-01-26

[Bug libstdc++/98842] optional's spaceship operations generates wrong code when operator== is not present

[cvs-commit at gcc dot gnu.org]

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

--- Comment #3 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:adec14811714e22a6c1f7f0199adc05370f0d8b0

commit r12-1260-gadec14811714e22a6c1f7f0199adc05370f0d8b0
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Mon Jun 7 13:02:15 2021 +0100

    libstdc++: Constrain three-way comparison for std::optional [PR 98842]

    The operator<=>(const optional<T>&, const U&) operator is supposed to be
    constrained with three_way_comparable_with<U, T> so that it can only be
    used when T and U are weakly-equality-comparable and also three-way
    comparable.

    Adding that constrain completely breaks std::optional comparisons,
    because it causes constraint recursion. To avoid that, an additional
    check that U is not a specialization of std::optional is needed. That
    appears to be a defect in the standard and should be reported to LWG.

    Signed-off-by: Jonathan Wakely <jwakely@redhat.com>

    libstdc++-v3/ChangeLog:

            PR libstdc++/98842
            * include/std/optional (operator<=>(const optional<T>& const U&)):
            Add missing constraint and add workaround for template
            recursion.
            * testsuite/20_util/optional/relops/three_way.cc: Check that
            type without equality comparison cannot be compared when wrapped
            in std::optional.

[Bug libstdc++/98842] optional's spaceship operations generates wrong code when operator== is not present

[cvs-commit at gcc dot gnu.org]

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

--- Comment #4 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Jonathan Wakely
<redi@gcc.gnu.org>:

https://gcc.gnu.org/g:4f11586945fffd0ff808c16f2f341f9e85d83749

commit r11-8559-g4f11586945fffd0ff808c16f2f341f9e85d83749
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Mon Jun 7 13:02:15 2021 +0100

    libstdc++: Constrain three-way comparison for std::optional [PR 98842]

    The operator<=>(const optional<T>&, const U&) operator is supposed to be
    constrained with three_way_comparable_with<U, T> so that it can only be
    used when T and U are weakly-equality-comparable and also three-way
    comparable.

    Adding that constrain completely breaks std::optional comparisons,
    because it causes constraint recursion. To avoid that, an additional
    check that U is not a specialization of std::optional is needed. That
    appears to be a defect in the standard and should be reported to LWG.

    Signed-off-by: Jonathan Wakely <jwakely@redhat.com>

    libstdc++-v3/ChangeLog:

            PR libstdc++/98842
            * include/std/optional (operator<=>(const optional<T>& const U&)):
            Add missing constraint and add workaround for template
            recursion.
            * testsuite/20_util/optional/relops/three_way.cc: Check that
            type without equality comparison cannot be compared when wrapped
            in std::optional.

    (cherry picked from commit adec14811714e22a6c1f7f0199adc05370f0d8b0)

[Bug libstdc++/98842] optional's spaceship operations generates wrong code when operator== is not present

[cvs-commit at gcc dot gnu.org]

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

--- Comment #5 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Jonathan Wakely
<redi@gcc.gnu.org>:

https://gcc.gnu.org/g:3a415b6a93765f29bffd0582a001bf03c4b93f3c

commit r10-9996-g3a415b6a93765f29bffd0582a001bf03c4b93f3c
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Mon Jun 7 13:02:15 2021 +0100

    libstdc++: Constrain three-way comparison for std::optional [PR 98842]

    The operator<=>(const optional<T>&, const U&) operator is supposed to be
    constrained with three_way_comparable_with<U, T> so that it can only be
    used when T and U are weakly-equality-comparable and also three-way
    comparable.

    Adding that constrain completely breaks std::optional comparisons,
    because it causes constraint recursion. To avoid that, an additional
    check that U is not a specialization of std::optional is needed. That
    appears to be a defect in the standard and should be reported to LWG.

    Signed-off-by: Jonathan Wakely <jwakely@redhat.com>

    libstdc++-v3/ChangeLog:

            PR libstdc++/98842
            * include/std/optional (operator<=>(const optional<T>& const U&)):
            Add missing constraint and add workaround for template
            recursion.
            * testsuite/20_util/optional/relops/three_way.cc: Check that
            type without equality comparison cannot be compared when wrapped
            in std::optional.

    (cherry picked from commit adec14811714e22a6c1f7f0199adc05370f0d8b0)

[Bug libstdc++/98842] optional's spaceship operations generates wrong code when operator== is not present

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED
   Target Milestone|---                         |10.4

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Fixed for 10.4 and 11.2

[Bug libstdc++/98842] optional's spaceship operations generates wrong code when operator== is not present

[redi at gcc dot gnu.org]

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

--- Comment #7 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to GCC Commits from comment #3)
>     Adding that constrain completely breaks std::optional comparisons,
>     because it causes constraint recursion. To avoid that, an additional
>     check that U is not a specialization of std::optional is needed. That
>     appears to be a defect in the standard and should be reported to LWG.

For the record, that is https://cplusplus.github.io/LWG/issue3566