[libcxx-commits] [PATCH] D150081: [libc++] Make a few standard types trivially equality comparable

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added a comment.
This revision now requires changes to proceed.

I think this LGTM w/ comments applied, but I'd like to see it one last time w/ green CI and comments applied before we ship this.



================
Comment at: libcxx/include/__exception/exception_ptr.h:1
 //===----------------------------------------------------------------------===//
 //
----------------
`libcxx/test/std/language.support/support.exception/propagation/exception_ptr.pass.cpp` is very very light, it doesn't even test for `!=` -- can you add a test for that?


================
Comment at: libcxx/include/__exception/exception_ptr.h:39
 
+#if _LIBCPP_STD_VER < 20
   friend _LIBCPP_HIDE_FROM_ABI bool operator==(const exception_ptr& __x, const exception_ptr& __y) _NOEXCEPT {
----------------
Please add a comment explaining that this allows making the class trivially equality comparable.


================
Comment at: libcxx/include/__expected/unexpected.h:111
 
+  bool operator==(const unexpected&) const requires is_object_v<_Err> = default;
+
----------------
Please add a comment like:

```
// conforming extension: this is not observable beyond SFINAE but it allows making `unexpected` trivially comparable
```


================
Comment at: libcxx/include/array:262
+public:
+    bool operator==(const array&) const = default;
+#endif
----------------
Same thing, please add a comment throughout whenever it's a public API. For example not in `__wrap_iter`.


================
Comment at: libcxx/include/forward_list:395
+#else
+public:
+    bool operator==(const __forward_list_iterator&) const = default;
----------------
No need for `public:`, we're already in `public`. Please check throughout.


================
Comment at: libcxx/include/tuple:1316
+public:
+    bool operator==(const tuple&) const requires (__all<is_object_v<_Tp>...>::value) = default;
+#endif
----------------
Can you please ensure that we have tests for comparing tuples that contain reference types?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150081