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

[Louis Dionne via Phabricator via libcxx-commits]

ldionne marked an inline comment as done.
ldionne 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 {
----------------
Quuxplusone wrote:
> ldionne wrote:
> > Mordante wrote:
> > > Quuxplusone wrote:
> > > > Mordante wrote:
> > > > > http://eel.is/c++draft/range.nonprop.cache#1.1 `non-propagating-cache<T> constrains its type parameter T with is_­object_­v<T>.`
> > > > This is undetectable, and therefore it's cheaper (in terms of compile time and source code simplicity) not to bother with it.
> > > Ok. (It just makes me wonder why it's specified in the Standard.)
> > I added it. It is detectable with SFINAE.
> @ldionne: can you clarify what you mean? I believe it's undetectable in the sense that libc++'s users are not even allowed to //name// `__non_propagating_cache`, and libc++'s own code never attempts to SFINAE on it; therefore the SFINAE doesn't matter. Do you know any place where libc++ is trying to make a decision based on the well-formedness of `__non_propagating_cache<T>`?
Oh. No, indeed, you're right from that perspective. For a second, I put libc++ itself as the user and thought "well, libc++ can technically tell whether it's constrained or not".

In that case, I don't feel strongly about it, but I would rather keep the constraint and invoke the consistency-with-the-spec argument. I originally removed the constraint because I didn't realize it was part of the spec.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102121