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

[Hui via Phabricator via libcxx-commits]

huixie90 marked an inline comment as done.
huixie90 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)...) {}
----------------
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. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122806