Page MenuHomePhabricator

[libcxx] removes `weak_equality` and `strong_equality` from <compare>
ClosedPublic

Authored by cjdb on Apr 11 2021, 10:27 PM.

Details

Summary

weak_equality and strong_equality were removed before being
standardised, and need to be removed.

Also adjusts common_comparison_category since its test needed
adjusting due to the equality deletions.

Diff Detail

Event Timeline

cjdb requested review of this revision.Apr 11 2021, 10:27 PM
cjdb created this revision.
Herald added a project: Restricted Project. ยท View Herald TranscriptApr 11 2021, 10:27 PM
Herald added a reviewer: Restricted Project. ยท View Herald Transcript
Herald added a subscriber: libcxx-commits. ยท View Herald Transcript

Please compare to D74186. (@BRevzin @EricWF maybe D74186 should have landed by now?)
If this diff is byte-for-byte identical to D74186, then ship it! ๐Ÿ˜›

cjdb updated this revision to Diff 342198.Sat, May 1, 9:00 PM

rebases to activate CI

Quuxplusone requested changes to this revision.Sun, May 2, 3:06 PM

I downloaded the raw diffs for D74186 and D100283, applied them in separate git branches, and then did git diff barry chris to find the places they differed.
The diff is here, if you want to check my work and see if I missed anything: https://pastebin.com/XcAS19dU

libcxx/include/compare
25โ€“26

Instead of deleting these lines, change weak_equality to partial_ordering.
I think this was probably a cut-and-paste error in the original? Anyway, is_eq(partial_ordering) still exists and should be synopsized here.

423

Please leave this comment line here.

518โ€“519

Remove _WeakEq and _StrongEq.

544โ€“545

Remove _WeakEq and _StrongEq.

559

You still need the (_Cat == _None) case here: if any of the elements are noncomparable then the result of this function must be void(), not strong_ordering::equivalent.
(At first I was worried that we had no tests for this; but I see below we do have tests. Don't remove those tests.)
https://eel.is/c++draft/cmp.common#2

libcxx/test/std/language.support/cmp/cmp.common/common_comparison_category.pass.cpp
43โ€“51

You cannot remove these tests. They need to keep passing in C++20.
https://eel.is/c++draft/cmp.common#2

This revision now requires changes to proceed.Sun, May 2, 3:06 PM
cjdb updated this revision to Diff 342291.Sun, May 2, 4:05 PM
cjdb marked 5 inline comments as done.

applies @Quuxplusone's feedback

cjdb updated this revision to Diff 342292.Sun, May 2, 4:07 PM
cjdb marked an inline comment as done.

removes zero-width code points

cjdb updated this revision to Diff 342293.Sun, May 2, 4:07 PM

removes another 0WCP

cjdb added a comment.Sun, May 2, 4:07 PM

I downloaded the raw diffs for D74186 and D100283, applied them in separate git branches, and then did git diff barry chris to find the places they differed.
The diff is here, if you want to check my work and see if I missed anything: https://pastebin.com/XcAS19dU

Thanks! I assumed D74186 would take priority (but needed this to be open since D74186 is old and downstream patches depend on this one).

ldionne accepted this revision.Wed, May 5, 10:36 AM
cjdb updated this revision to Diff 344112.Mon, May 10, 10:33 AM

rebases to activate CI

This revision was not accepted when it landed; it landed in state Needs Review.Mon, May 10, 1:46 PM
This revision was automatically updated to reflect the committed changes.