[libcxx-commits] [PATCH] D103581: [libc++][compare]Implement compare_three_way_result[_t]

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/test/std/language.support/cmp/cmp.result/compare_three_way_result.pass.cpp:30
+struct OtherOperand {
+    friend std::strong_ordering operator<=>( const OtherOperand&, const OtherOperand& ) = delete;
+};
----------------
```
friend std::strong_ordering operator<=>(const OtherOperand&, const OtherOperand&) = delete;
```
but also consider whether it would break anything to make this simply
```
friend std::strong_ordering operator<=>(OtherOperand, OtherOperand) = delete;
```
because empty structs are typically passed by value. (Or even omit it entirely, although I assume that //would// break something you're trying to test here, is that right?)


================
Comment at: libcxx/test/std/language.support/cmp/cmp.result/compare_three_way_result.pass.cpp:46
+    static_assert(std::is_same_v<std::compare_three_way_result_t<OperandType, OperandType>, SameOperandsOrdering>);
+    static_assert(std::is_same_v<std::compare_three_way_result_t<std::add_lvalue_reference_t<OperandType>, std::add_lvalue_reference_t<OperandType>>,
+                                SameOperandsOrdering>);
----------------
Mordante wrote:
> Please make these line fit in our maximum column width of 120 characters.
Throughout the file, make these substitutions:
`SameOperandsOrdering` -> `SameOrder`
`DifferentOperandsOrdering` -> `DifferentOrder`
`OperandType` -> `OT`
`do_compare_three_way_result_positive_test` -> `test`
`do_compare_three_way_result_negative_test` -> `negative_test`
Remove redundant usages of type traits such as `add_lvalue_reference`.
Use the `ASSERT_SAME_TYPE` test helper.
So that makes this line
```
ASSERT_SAME_TYPE(std::compare_three_way_result_t<OT&, OT&>, SameOrder>);
```
which comfortably fits within our informally enforced column limit.
Please make these changes throughout.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103581/new/

https://reviews.llvm.org/D103581