[libcxx-commits] [PATCH] D107671: [libcxx][ranges] Add `ranges::join_view`.

[Zoe Carver via Phabricator via libcxx-commits]

zoecarver marked 13 inline comments as done.
zoecarver added inline comments.


================
Comment at: libcxx/include/__ranges/join_view.h:80
+    : public view_interface<join_view<_View>>
+    , public __inner_cache<range_reference_t<_View>> {
+  private:
----------------
ldionne wrote:
> Instead, I would do:
> 
> ```
> static constexpr bool _UseCache = !is_reference_v<_InnerRange>;
> using _Cache = _If<_UseCache, __non_propagating_cache<remove_cv_t<_InnerRange>>, __empty_cache>;
> [[no_unique_address]] _Cache __cache_;
> ```
> 
> That way, you get rid of `__inner_cache` altogether.
I have removed this cache altogether now but if I need to add it back I'll do it like this. 


================
Comment at: libcxx/include/__ranges/join_view.h:106-116
+        for (; __outer_ != ranges::end(__parent_->__base_); ++__outer_) {
+          auto&& __inner = _CONSTEXPR_TERNARY(__ref_is_glvalue,
+            *__outer_,
+            __parent_->__inner_.__emplace_deref(__outer_));
+          __inner_ = ranges::begin(__inner);
+          if (*__inner_ != ranges::end(__inner))
+            return;
----------------
ldionne wrote:
> IMO the spec is easier to understand as-is, and it doesn't require introducing a `_CONSTEXPR_TERNARY` macro that is only used in two places.
> 
> I don't mind if you use a IIFE or not.
No longer have two paths here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107671