[libcxx-commits] [PATCH] D122806: [libc++] add zip_view and views::zip for C++23

[Hui via Phabricator via libcxx-commits]

huixie90 added inline comments.


================
Comment at: libcxx/include/__ranges/zip_view.h:40
+#endif
+
+_LIBCPP_BEGIN_NAMESPACE_STD
----------------
Mordante wrote:
> Since you include `__algorithm/min.h` you need to guard against macros using `min`
> ```
> _LIBCPP_PUSH_MACROS
> #include <__undef_macros>
> ```
> and at the end of the file
> `_LIBCPP_POP_MACROS`
> You can look at the `__algorithm/min.h` as an example.
I did notice this because the macro unit test failed and i worked around it by adding additional parentheses `(std::min)(arg…)` . The additional parenthesis prevent macro from expanding.  Is this considered bad practice? 


================
Comment at: libcxx/include/__ranges/zip_view.h:365
+        (three_way_comparable<iterator_t<__maybe_const<_Const, _Views>>> && ...) {
+        // clang-format off
+        return __x.__current_ <=> __y.__current_;
----------------
Mordante wrote:
> What happens when you remove this?
> Is there a clang-format bug report?
A white space is added between the spaceship  `<= >`. I think my clang format version is quite recent. Not sure if this is known issue


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122806