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

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
Comment at: libcxx/include/__ranges/zip_view.h:142-144
+    _LIBCPP_HIDE_FROM_ABI
+    constexpr explicit zip_view(_Views... __views)
+        : __views_(std::move(__views)...) {}
----------------
huixie90 wrote:
> philnik wrote:
> > huixie90 wrote:
> > > philnik wrote:
> > > > The `constexpr`s should be on the same line as `_LIBCPP_HIDE_FROM_ABI`.
> > > I took a look at several other views (transform, single, subrange, take, join). they all have `_LIBCPP_HIDE_FROM_ABI` on a separate line.
> > > 
> > > A side question. why don't we put `_LIBCPP_HIDE_FROM_ABI` put on the class declaration so we can avoid putting `_LIBCPP_HIDE_FROM_ABI` on all member functions
> > I meant that it should be `_LIBCPP_HIDE_FROM_ABI constexpr`. Although I'm not actually sure that we use this style consistently. We probably don't use any style consistently.
> > As to why we put `_LIBCPP_HIDE_FROM_ABI` on every declaration I think that not all possible attributes in `_LIBCPP_HIDE_FROM_ABI` can be put on classes. I think the possible attributes are `__exclude_from_explicit_instantiation__`, `internal_linkage`, `always_inline`, `__visibility__("hidden")` and combinations.
> hmm, I take the example from other views (transform, single, subrange, take, join), they have the style like this
> ```cpp
> _LIBCPP_HIDE_FROM_ABI
> constexpr auto begin() const {
>    ...
> }
> ```
> 
> I did the same and after applying clang-format, it stays the same
> 
> are you suggesting to make it look like
> 
> ```cpp
> _LIBCPP_HIDE_FROM_ABI constexpr 
> auto begin() const {
>    ...
> }
> ```
> If I do this, after clang-format, it becomes
> 
> ```cpp
> _LIBCPP_HIDE_FROM_ABI constexpr auto begin() const {
>    ...
> }
> ```
> 
> . do you have example files that I can look at the styles?  I found the style from other views (which is the same as the style here) a bit clang-format friendly. 
That's the style I use in the ranges algorithms (i.e. `__algorithm/ranges_min.h`). If it's easier for you to use
```
_LIBCPP_HIDE_FROM_ABI
constexpr foo() {}
```
you can use that style. I didn't know we use your current style in other views. I hope we will get a usable `.clang-format` soon. That would make so many things easier.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122806