This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Implement `operator<=>` for `unique_ptr`
ClosedPublic

Authored by avogelsgesang on Jul 31 2022, 4:12 AM.

Details

Summary

Implements part of:

  • P1614R2 The Mothership has Landed

Fixes LWG3426

Diff Detail

Event Timeline

avogelsgesang created this revision.Jul 31 2022, 4:12 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 31 2022, 4:12 AM
avogelsgesang requested review of this revision.Jul 31 2022, 4:12 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 31 2022, 4:12 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript

Fix tests on older C++ versions

avogelsgesang edited the summary of this revision. (Show Details)Jul 31 2022, 3:27 PM
avogelsgesang planned changes to this revision.Aug 1 2022, 2:48 PM

need to update based on comments in https://reviews.llvm.org/D130852

Update based on feedback from D130852

avogelsgesang edited the summary of this revision. (Show Details)Aug 2 2022, 12:22 PM

Thanks! Looks quite good overall, but a few remarks.

libcxx/include/__memory/unique_ptr.h
601
711
libcxx/include/memory
509

Interesting these aren't removed, including in the WP.

libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.special/cmp.pass.cpp
72

Can you also test with the other two comparison categories? Not 100% sure it's even possible.

libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.special/cmp_nullptr.pass.cpp
22

Nice catch! It seems we've added _NOEXCEPT in the code. @ldionne should we remove these noexcepts in a separate commit?

55
avogelsgesang marked 3 inline comments as done.

address review comments

libcxx/include/memory
509

I agree. The revision history of P1614R2 states

The comparisons between unique_ptr<T, D> and nullptr were originally removed and replaced with a <=>, but this was reverted.

But I don't know where to find the history on why that change was made

libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.special/cmp.pass.cpp
72

I don't understand.

unique_ptr <=> unique_ptr is specified to return std::strong_ordering. If I added, e.g.,

AssertOrderReturn<std::weak_ordering, std::unique_ptr<int>>();

this test would fail because the result of unique_ptr <=> unique_ptr is strong_ordering, not weak_ordering

Mordante accepted this revision.Aug 6 2022, 6:04 AM

LGTM, thanks!
I'll commit this on your behalf.

libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.special/cmp.pass.cpp
72

After I wrote the question I wondered whether it would be possible at all. The odd part is that std::shared_ptr uses strong_ordering, but unique_ptr deduces the ordering. But it seems it indeed should always be strong_ordering.

This revision is now accepted and ready to land.Aug 6 2022, 6:04 AM
This revision was automatically updated to reflect the committed changes.
libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.special/eq.pass.cpp