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

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added inline comments.


================
Comment at: libcxx/include/__ranges/non_propagating_cache.h:42
+  // may refer to internal details of the source view.
+  template<class _Tp>
+  class __non_propagating_cache {
----------------
http://eel.is/c++draft/range.nonprop.cache#1.1 `non-propagating-cache<T> constrains its type parameter T with is_­object_­v<T>.`


================
Comment at: libcxx/include/__ranges/non_propagating_cache.h:43
+  template<class _Tp>
+  class __non_propagating_cache {
+    optional<_Tp> __value_ = nullopt;
----------------
There's no `_LIBCPP_TEMPLATE_VIS`.


================
Comment at: libcxx/include/__ranges/non_propagating_cache.h:87
+    constexpr void __set(_Tp&& __value) { __value_.emplace(_VSTD::move(__value)); }
+  };
+
----------------
I miss http://eel.is/c++draft/range.nonprop.cache#1.6
```
non-propagating-cache<T> has an additional member function template specified as follows:

template<class I>
  constexpr T& emplace-deref(const I& i);    // exposition only

```
Is that intentionally?


================
Comment at: libcxx/test/libcxx/ranges/range.nonprop.cache/assign.copy.pass.cpp:50
+  {
+    Cache a; a.__set(T{3});
+    Cache b;
----------------
This tests an implementation detail. Shouldn't this be guarded for usage with the libc++ library?
Other tests also use implementation details, please grep for `__`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102121