Page MenuHomePhabricator

[libcxx] adds std::compare_three_way
Needs ReviewPublic

Authored by cjdb on May 31 2020, 2:23 PM.

Details

Reviewers
EricWF
ldionne
mclow.lists
BRevzin
jfb
Group Reviewers
Restricted Project
Summary

[libcxx] adds std::compare_three_way

  • Adds std::compare_three_way without concept-checking
  • Adds test for std::compare_three_way

Diff Detail

Event Timeline

cjdb created this revision.May 31 2020, 2:23 PM
miscco added a subscriber: miscco.May 31 2020, 11:28 PM
miscco added inline comments.
libcxx/include/functional
515

Usually this is spellec as #ifndef _LIBCPP_HAS_NO_SPACESHIP_OPERATOR

833

I had to read it twice o could we improve that comment to:
// TODO(cjdb): Needs concept three_way_comparable_with

cjdb updated this revision to Diff 267641.Jun 1 2020, 9:00 AM
cjdb marked an inline comment as done.
cjdb added inline comments.
libcxx/include/functional
515

Ack, I am following convention I've seen elsewhere. lmk if this is a hard sell and I can change it.

cjdb added a reviewer: Restricted Project.Jun 22 2020, 9:16 AM

Ping.

My 2 cents.

libcxx/include/functional
151

Shouldn't you keep using is_transparent = unspecified; in synopsis?

516

Shouldn't we avoid conditional includes within libc++?

libcxx/test/std/utilities/function.objects/comparisons/compare_three_way.pass.cpp
13

Nit: less? why not compare_three_way?

64

Sorry for being repetitive in my reviews :). Could you make the run-time and compile-time tests equivalent?
I imagine that this might need to get rid of reinterpret_cast from do_pointer_comparison_test when making it constexpr.

curdeius added inline comments.Jun 23 2020, 12:41 AM
libcxx/include/functional
834

I think you can use noexcept instead of _NOEXCEPT_ as it's always available in C++20.