Skip to content

"Unspecified" parameter for the standard comparison category types' comparison operators might be overconstrained #96237

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dangelog opened this issue Jun 20, 2024 · 1 comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.

Comments

@dangelog
Copy link

dangelog commented Jun 20, 2024

Hi,

Consider https://gcc.godbolt.org/z/ndrvzE8j1

#include <compare>

struct P {
    operator std::partial_ordering() const;
};

int main() {
    std::partial_ordering::equivalent == P{};
}

Clang + libc++ rejects this with:

<source>:8:39: error: use of overloaded operator '==' is ambiguous (with operand types 'const partial_ordering' and 'P')
    8 |     std::partial_ordering::equivalent == P{};
      |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^  ~~~
/opt/compiler-explorer/clang-trunk-20240620/bin/../include/c++/v1/__compare/ordering.h:62:47: note: candidate function
   62 |   _LIBCPP_HIDE_FROM_ABI friend constexpr bool operator==(partial_ordering, partial_ordering) noexcept = default;
      |                                               ^
/opt/compiler-explorer/clang-trunk-20240620/bin/../include/c++/v1/__compare/ordering.h:64:47: note: candidate function
   64 |   _LIBCPP_HIDE_FROM_ABI friend constexpr bool operator==(partial_ordering __v, _CmpUnspecifiedParam) noexcept {
      |                                               ^
1 error generated.
Compiler returned: 1

It's not entirely clear why this should be rejected, instead of using the friend constexpr bool operator==(partial_ordering v, partial_ordering w) noexcept = default; operator described by https://eel.is/c++draft/cmp .

https://eel.is/c++draft/cmp#categories.pre-3 is quite ambiguous about whether the code above should be accepted, or if it falls under the "In this context, the behavior of a program that supplies an argument other than a literal 0 is undefined" https://eel.is/c++draft/cmp#categories.pre-3.sentence-4 sentence.

I am not sure what "in this context" means: does it mean that passing P{} is UB because it's not a literal 0? That sounds like a vexing interpretation: the very same overload is selected by e.g. std::partial_ordering::equivalent == std::strong_ordering::equivalent after converting strong_ordering, and I don't see a provision that would allow this.

Ultimately it boils down to the SFINAE on _CmpUnspecifiedParam's constructor here: https://github.com/llvm/llvm-project/blob/main/libcxx/include/__compare/ordering.h#L39 , which looks "poisoned". It excludes the standard comparison types, but not types convertible to those. It may also break user-defined comparisons between standard category types and user-defined types (not convertible to anything), like here: https://gcc.godbolt.org/z/3aPedMWoe

Related:


For some more background: in Qt we have defined our own Qt::*_ordering types because Qt only requires C++17. Basically, we've backported the standard comparison types. We want them to be interoperable with the standard ones, so they define constructors/conversion operators towards the standard types. Which leads to the problem shown by the testcase above (P is Qt::partial_ordering), tracked by https://bugreports.qt.io/browse/QTBUG-126541 .

@EugeneZelenko EugeneZelenko added libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. clang:frontend Language frontend issues, e.g. anything involving "Sema" and removed new issue labels Jun 20, 2024
@llvmbot
Copy link
Member

llvmbot commented Jun 20, 2024

@llvm/issue-subscribers-clang-frontend

Author: Giuseppe D'Angelo (dangelog)

Hi,

Consider https://gcc.godbolt.org/z/ndrvzE8j1

#include &lt;compare&gt;

struct P {
    operator std::partial_ordering() const;
};

int main() {
    std::partial_ordering::equivalent == P{};
}

Clang + libc++ rejects this with:

&lt;source&gt;:8:39: error: use of overloaded operator '==' is ambiguous (with operand types 'const partial_ordering' and 'P')
    8 |     std::partial_ordering::equivalent == P{};
      |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^  ~~~
/opt/compiler-explorer/clang-trunk-20240620/bin/../include/c++/v1/__compare/ordering.h:62:47: note: candidate function
   62 |   _LIBCPP_HIDE_FROM_ABI friend constexpr bool operator==(partial_ordering, partial_ordering) noexcept = default;
      |                                               ^
/opt/compiler-explorer/clang-trunk-20240620/bin/../include/c++/v1/__compare/ordering.h:64:47: note: candidate function
   64 |   _LIBCPP_HIDE_FROM_ABI friend constexpr bool operator==(partial_ordering __v, _CmpUnspecifiedParam) noexcept {
      |                                               ^
1 error generated.
Compiler returned: 1

It's not entirely clear why this should be rejected, instead of using the friend constexpr bool operator==(partial_ordering v, partial_ordering w) noexcept = default; operator described by https://eel.is/c++draft/cmp .

https://eel.is/c++draft/cmp#categories.pre-3 is quite ambiguous about whether the code above should be accepted, or if it falls under the "In this context, the behavior of a program that supplies an argument other than a literal 0 is undefined" https://eel.is/c++draft/cmp#categories.pre-3.sentence-4 sentence.

I am not sure what "in this context" means: does it mean that passing P{} is UB because it's not a literal 0? That sounds like a vexing interpretation: the very same overload is selected by e.g. std::partial_ordering::equivalent == std::strong_ordering::equivalent after converting strong_ordering, and I don't see a provision that would allow this.

Ultimately it boils down to the SFINAE on _CmpUnspecifiedParam's constructor here: https://github.com/llvm/llvm-project/blob/main/libcxx/include/__compare/ordering.h#L39 , which looks "poisoned". It excludes the standard comparison types, but not types convertible to those. It may also break user-defined comparisons between standard category types and user-defined types (not convertible to anything), like here: https://gcc.godbolt.org/z/3aPedMWoe

Related:


For some more background: in Qt we have defined our own Qt::*_ordering types because Qt only requires C++17. Basically, we've backported the standard comparison types. We want them to be interoperable with the standard ones, so they define constructors/conversion operators towards the standard types. Which leads to the problem shown by the testcase above (P is Qt::partial_ordering), tracked by https://bugreports.qt.io/browse/QTBUG-126541 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

No branches or pull requests

3 participants