[libcxx-commits] [PATCH] D102020: [libcxx][ranges] Add class ref_view.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added a comment.
This revision now requires changes to proceed.

Basically LGTM, requesting changes just so it shows up right in the queue since there are a few pending comments to apply.



================
Comment at: libcxx/include/__ranges/ref_view.h:40-41
+
+    static void __test(_Range&);
+    static void __test(_Range&&) = delete;
+
----------------
Those can be made private, and I would perhaps rename it to something like `__binds_to_lvalue_ref` or something like that.

Do you have a better understanding of what this function is used for than I do? IIUC, it's only used to check that `__tp` in the constructor below would bind to a lvalue, but not to a rvalue. I'm kind of wondering why the Standard is using this obtuse wording -- there must be a good reason?


================
Comment at: libcxx/include/__ranges/ref_view.h:44
+    template<class _Tp>
+      requires (!same_as<_Tp, ref_view>) &&
+        convertible_to<_Tp, _Range&> && requires { __test(declval<_Tp>()); }
----------------
I guess you want to use `__not_same_as` here. Not a big fan of that helper concept, but oh well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102020