[libcxx-commits] [PATCH] D102121: [libcxx][ranges] adds _`non-propagating-cache`_

Christopher Di Bella via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Mon May 10 09:23:35 PDT 2021

cjdb added inline comments.

Comment at: libcxx/include/__ranges/non_propagating_cache.h:34
+namespace ranges {
+  template<class _Tp, bool __enable_cache>
+  requires is_object_v<_Tp>
zoecarver wrote:
> Could we constrain this further? 
Why should we constrain this further?

Comment at: libcxx/include/__ranges/non_propagating_cache.h:38
+    using optional<_Tp>::optional;
+    inline static constexpr bool cache_enabled = true;
zoecarver wrote:
> This name should be mangled.

Comment at: libcxx/test/libcxx/ranges/range.nonprop.cache/cache_disabled.compile.pass.cpp:20
+static_assert(std::is_empty_v<std::ranges::__non_propagating_cache<std::array<int, 10>, false> >);
+static_assert(std::is_empty_v<std::ranges::__non_propagating_cache<std::array<int, 10>::iterator, false> >);
zoecarver wrote:
> This isn't so much a request for a changes as a question: most of the time do we want to store a whole range or just an iterator. At least for drop_view, I feel like we'd only need an iterator.
> In light of that, maybe test this with more than one type (it doesn't have to be a range or an iterator, in fact). 
Wholly, or just here?

  rG LLVM Github Monorepo



More information about the libcxx-commits mailing list