Skip to content

<compare>: fix constructibility of ordering types, prevent structured bindings #5911

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.

Sign up for GitHub

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
vmichal wants to merge 8 commits into
base: main
Choose a base branch
from
Open

<compare>: fix constructibility of ordering types, prevent structured bindings #5911

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ebbcf3b
Add tests.
vmichal Nov 26, 2025
b03a19d
Make {strong, partial, weak}_ordering not default constructible and c…
vmichal Nov 26, 2025
3eafafb
Say constexpr explicit rather than the other way around.
vmichal Nov 26, 2025
83d2f42
Mention the github issue in code.
vmichal Nov 26, 2025
86ffa8d
Fix incorrect formatting.
vmichal Nov 26, 2025
2ebf75a
Remove heder include circular dependency.
vmichal Nov 26, 2025
9dcdd2b
Restore xxx_ordering eligibility to passing in registers under MSVC c…
vmichal Nov 26, 2025
bb8654b
Add TRANSITION, ABI comment.
vmichal Nov 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 49 additions & 17 deletions stl/inc/compare
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,14 @@ enum class _Compare_eq : _Compare_t { equal = 0, equivalent = equal };
enum class _Compare_ord : _Compare_t { less = -1, greater = 1 };
enum class _Compare_ncmp : _Compare_t { unordered = -128 };

// TRANSITION, ABI, this is used for allowing comparison category types to be passing and returning in registers
// in MSVC ABI while making them non-default-constructible.
// It is intentional to use a specific tag type to avoid impact on overload resolution.
struct _Secret_ordering_construction_tag {};

_EXPORT_STD struct partial_ordering {
constexpr partial_ordering(same_as<_Secret_ordering_construction_tag> auto) = delete;

static const partial_ordering less;
static const partial_ordering equivalent;
static const partial_ordering greater;
Expand Down Expand Up @@ -102,24 +109,28 @@ _EXPORT_STD struct partial_ordering {
// The stored value is either less (0xff), equivalent (0x00), greater (0x01), or unordered (0x80).
// Subtracting from 0 produces either 0x01, 0x00, 0xff, or 0x80. Note that the effect is to
// exchange less for greater (and vice versa), while leaving equivalent and unordered unchanged.
return {static_cast<_Compare_t>(0 - static_cast<unsigned int>(_Val._Value))};
return _STD _Bit_cast<partial_ordering>(static_cast<_Compare_t>(0 - static_cast<unsigned int>(_Val._Value)));
}

_Compare_t _Value;
};

inline constexpr partial_ordering partial_ordering::less{static_cast<_Compare_t>(_Compare_ord::less)};
inline constexpr partial_ordering partial_ordering::equivalent{static_cast<_Compare_t>(_Compare_eq::equivalent)};
inline constexpr partial_ordering partial_ordering::greater{static_cast<_Compare_t>(_Compare_ord::greater)};
inline constexpr partial_ordering partial_ordering::unordered{static_cast<_Compare_t>(_Compare_ncmp::unordered)};
inline constexpr partial_ordering partial_ordering::less{_STD _Bit_cast<partial_ordering>(_Compare_ord::less)};
inline constexpr partial_ordering partial_ordering::equivalent{
_STD _Bit_cast<partial_ordering>(_Compare_eq::equivalent)};
inline constexpr partial_ordering partial_ordering::greater{_STD _Bit_cast<partial_ordering>(_Compare_ord::greater)};
inline constexpr partial_ordering partial_ordering::unordered{
_STD _Bit_cast<partial_ordering>(_Compare_ncmp::unordered)};

_EXPORT_STD struct weak_ordering {
constexpr weak_ordering(same_as<_Secret_ordering_construction_tag> auto) = delete;

static const weak_ordering less;
static const weak_ordering equivalent;
static const weak_ordering greater;

constexpr operator partial_ordering() const noexcept {
return {static_cast<_Compare_t>(_Value)};
return _STD _Bit_cast<partial_ordering>(_Value);
}

_NODISCARD friend constexpr bool operator==(const weak_ordering _Val, _Literal_zero) noexcept {
Expand Down Expand Up @@ -165,28 +176,30 @@ _EXPORT_STD struct weak_ordering {
}

_NODISCARD friend constexpr weak_ordering operator<=>(_Literal_zero, const weak_ordering _Val) noexcept {
return {static_cast<_Compare_t>(-_Val._Value)};
return _STD _Bit_cast<weak_ordering>(static_cast<_Compare_t>(-_Val._Value));
}

_Compare_t _Value;
};

inline constexpr weak_ordering weak_ordering::less{static_cast<_Compare_t>(_Compare_ord::less)};
inline constexpr weak_ordering weak_ordering::equivalent{static_cast<_Compare_t>(_Compare_eq::equivalent)};
inline constexpr weak_ordering weak_ordering::greater{static_cast<_Compare_t>(_Compare_ord::greater)};
inline constexpr weak_ordering weak_ordering::less{_STD _Bit_cast<weak_ordering>(_Compare_ord::less)};
inline constexpr weak_ordering weak_ordering::equivalent{_STD _Bit_cast<weak_ordering>(_Compare_eq::equivalent)};
inline constexpr weak_ordering weak_ordering::greater{_STD _Bit_cast<weak_ordering>(_Compare_ord::greater)};

_EXPORT_STD struct strong_ordering {
constexpr strong_ordering(same_as<_Secret_ordering_construction_tag> auto) = delete;

static const strong_ordering less;
static const strong_ordering equal;
static const strong_ordering equivalent;
static const strong_ordering greater;

constexpr operator partial_ordering() const noexcept {
return {static_cast<_Compare_t>(_Value)};
return _STD _Bit_cast<partial_ordering>(_Value);
}

constexpr operator weak_ordering() const noexcept {
return {static_cast<_Compare_t>(_Value)};
return _STD _Bit_cast<weak_ordering>(_Value);
}

_NODISCARD friend constexpr bool operator==(const strong_ordering _Val, _Literal_zero) noexcept {
Expand Down Expand Up @@ -232,16 +245,35 @@ _EXPORT_STD struct strong_ordering {
}

_NODISCARD friend constexpr strong_ordering operator<=>(_Literal_zero, const strong_ordering _Val) noexcept {
return {static_cast<_Compare_t>(-_Val._Value)};
return _STD _Bit_cast<strong_ordering>(static_cast<_Compare_t>(-_Val._Value));
}

_Compare_t _Value;
};

inline constexpr strong_ordering strong_ordering::less{static_cast<_Compare_t>(_Compare_ord::less)};
inline constexpr strong_ordering strong_ordering::equal{static_cast<_Compare_t>(_Compare_eq::equal)};
inline constexpr strong_ordering strong_ordering::equivalent{static_cast<_Compare_t>(_Compare_eq::equivalent)};
inline constexpr strong_ordering strong_ordering::greater{static_cast<_Compare_t>(_Compare_ord::greater)};
inline constexpr strong_ordering strong_ordering::less{_STD _Bit_cast<strong_ordering>(_Compare_ord::less)};
inline constexpr strong_ordering strong_ordering::equal{_STD _Bit_cast<strong_ordering>(_Compare_eq::equal)};
inline constexpr strong_ordering strong_ordering::equivalent{_STD _Bit_cast<strong_ordering>(_Compare_eq::equivalent)};
inline constexpr strong_ordering strong_ordering::greater{_STD _Bit_cast<strong_ordering>(_Compare_ord::greater)};

template <typename>
struct tuple_size;

// Prevent structured binding to objects of xxx_ordering, see GH-5689
template <>
struct tuple_size<partial_ordering> {
void value();
};
Copy link
Contributor

@frederick-vs-ja frederick-vs-ja Nov 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem conforming. I think tuple_size<partial_ordering> etc. are supposed to be incomplete types.

Also, accidentally allowing structured binding doesn't seem to be a bug even if a strict resolution of LWG-4306 is accepted.


template <>
struct tuple_size<weak_ordering> {
void value();
};

template <>
struct tuple_size<strong_ordering> {
void value();
};

_EXPORT_STD _NODISCARD constexpr bool is_eq(const partial_ordering _Comp) noexcept {
return _Comp == 0;
Expand Down
17 changes: 17 additions & 0 deletions tests/std/tests/P0768R1_spaceship_operator/test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,19 @@ void test_algorithm() {
assert((test_algorithm2<Ty2, Ty1>()));
}

// Test added to handle GH-5689
template <typename T>
void test_not_constructible() {
static_assert(!std::is_default_constructible_v<T>);
static_assert(!std::is_constructible_v<T, int>);

// How to test this does not compile?
//{
// auto [n] = 0 <=> 0;
// (void) n;
//}
}

int main() {
static_assert(test_orderings());
test_orderings();
Expand All @@ -230,4 +243,8 @@ int main() {
test_algorithm<int, char>();
test_algorithm<int, unsigned char>();
test_algorithm<char, unsigned char>();

test_not_constructible<std::strong_ordering>();
test_not_constructible<std::weak_ordering>();
test_not_constructible<std::partial_ordering>();
}