This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Implement `operator<=> for `thread::id`
ClosedPublic

Authored by avogelsgesang on Aug 7 2022, 9:47 AM.

Details

Summary

The new operator<=> is mapped onto the existing functions
__libcpp_thread_id_equal and __libcpp_thread_id_less. Introducing a
new __libcpp_thread_id_compare_three_way might lead to more efficient
code. Given that we can still introduce __libcpp_thread_id_compare_three_way
later, for this commit I opted to not break ABI. If requested, I will
add __libcpp_thread_id_compare_three_way in a follow-up commit.

Implements part of P1614R2 "The Mothership has Landed"

Diff Detail

Event Timeline

avogelsgesang created this revision.Aug 7 2022, 9:47 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 7 2022, 9:47 AM
avogelsgesang requested review of this revision.Aug 7 2022, 9:47 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 7 2022, 9:48 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript

update review link

actually check the results in the test case

avogelsgesang planned changes to this revision.Aug 7 2022, 5:38 PM

need to fix CI

correct synopsis

fix clang-tidy

Mordante added inline comments.Aug 8 2022, 8:47 AM
libcxx/include/__threading_support
614

Note when implementing this according to P1614 we don't need to expose this function.

626

I read your reasoning about why you did it, but I think we should fix it. There's already a bug report regarding this issue.
https://github.com/llvm/llvm-project/issues/56187

626

For new code please use _LIBCPP_HIDE_FROM_ABI instead, both do the same but this name is the new name.

629

Please use clang-format on the new code.

avogelsgesang added inline comments.Aug 8 2022, 9:07 AM
libcxx/include/__threading_support
626

Do you want me to turn the friends into non-friends as part of this commit or should I use a separate commit for this?

Mordante added inline comments.Aug 8 2022, 10:09 AM
libcxx/include/__threading_support
626

I prefer a separate commit for the unfriending, but I like to do the spaceship operator the proper way.
Feel free to assign the bug to yourself I haven't started it; I mainly self-assigned since I intended to work on the spaceship operators. But I have enough other things on my todo list.

avogelsgesang planned changes to this revision.Aug 8 2022, 10:23 AM

I first need to fix https://github.com/llvm/llvm-project/issues/56187 and will come back to this patch afterwards

libcxx/include/__threading_support
626

ok, I will fix this bug in a separate commit, then, and come back to this change here after fixing it

rebase on D131430 and implement operator<=> as free function

avogelsgesang edited the summary of this revision. (Show Details)Aug 8 2022, 2:02 PM
avogelsgesang marked 5 inline comments as done.Aug 8 2022, 2:03 PM

one more _LIBCPP_HIDE_FROM_ABI

Mordante accepted this revision.Aug 13 2022, 4:57 AM

LGTM!

This revision is now accepted and ready to land.Aug 13 2022, 4:57 AM
This revision was automatically updated to reflect the committed changes.
libcxx/test/std/thread/thread.threads/thread.thread.class/thread.thread.id/eq.pass.cpp