Skip to content

Conversation

@statementreply
Copy link
Contributor

Works towards #62.

@statementreply statementreply requested a review from a team as a code owner January 26, 2021 14:08
Comment on lines 218 to +223
friend bool operator==(thread::id _Left, thread::id _Right) noexcept;
#if _HAS_CXX20
friend strong_ordering operator<=>(thread::id _Left, thread::id _Right) noexcept;
#else // ^^^ _HAS_CXX20 / !_HAS_CXX20 vvv
friend bool operator<(thread::id _Left, thread::id _Right) noexcept;
#endif // !_HAS_CXX20
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to default the operator== for C++20 and make them hidden friends?

Copy link
Contributor

Choose a reason for hiding this comment

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

They can't be hidden friends; the difference is observable. Arguably operator== also cannot be defaulted because doing so would make it constexpr and we're forbidden to make things constexpr where the Standard doesn't depict them as such.

@CaseyCarter CaseyCarter added cxx20 C++20 feature spaceship C++20 operator <=> labels Jan 26, 2021

#if _HAS_CXX20
_NODISCARD inline strong_ordering operator<=>(thread::id _Left, thread::id _Right) noexcept {
return _Left._Id <=> _Right._Id;
Copy link
Contributor

@CaseyCarter CaseyCarter Jan 26, 2021

Choose a reason for hiding this comment

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

Fun fact: the Standard doesn't require the total ordering over thread ids that this function observes to be consistent with operator==, so we could have the two functions order them differently ;)

(I've submitted an LWG issue to fix this.)

std::thread::id id1_equal;
std::thread::id id2 = std::this_thread::get_id();

spaceship_test<std::strong_ordering>(id1, id1_equal, id2);
Copy link
Member

Choose a reason for hiding this comment

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

I believe there's an implementation assumption here - nothing in the Standard appears to require that std::thread::id{} occurs first in the unspecified total ordering. (It's a safe assumption for us, since we use 0 and store unsigned int.) I'll push a comment.

@StephanTLavavej StephanTLavavej merged commit 091958d into microsoft:feature/spaceship Jan 28, 2021
@StephanTLavavej
Copy link
Member

Thanks for implementing and testing this Clause! 🛸 🧵

@statementreply statementreply deleted the spaceship-clause-32 branch April 17, 2021 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cxx20 C++20 feature spaceship C++20 operator <=>

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants