[libcxx-commits] [PATCH] D101737: [libcxx] Implement view.interface.

Louis Dionne via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Mon May 31 14:46:06 PDT 2021

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

Generally LGTM, with some comments!

Comment at: libcxx/include/CMakeLists.txt:43-45
+  __ranges/view_interface.h
Should be sorted!

Comment at: libcxx/include/__ranges/view_interface.h:38
+class view_interface : public view_base {
+  [[nodiscard]] constexpr _Derived& __derived() noexcept {
+    return static_cast<_Derived&>(*this);
Can drop `nodiscard`.

Comment at: libcxx/include/__ranges/view_interface.h:55
+  template<class _D2 = _Derived>
+  [[nodiscard]] constexpr bool empty() const
Why is that necessary? Is that the workaround for `gcc -fconcepts` you're discussing in the comments below?

Comment at: libcxx/include/__ranges/view_interface.h:149
+    _LIBCPP_ASSERT(!empty(), "");
+    return *(--ranges::end(__derived()));
+  }
cjdb wrote:
> zoecarver wrote:
> > Review note: this is used until `ranges::prev` is implemented.
> Thanks for highlighting this, was about to comment.
This can be changed now (and also in the `noexcept(...)` above).

Comment at: libcxx/test/std/ranges/range.utility/view.interface/view.interface.pass.cpp:12
+// UNSUPPORTED: gcc-10
+// XFAIL: msvc && clang
I don't think this is necessary anymore.

Comment at: libcxx/test/std/ranges/range.utility/view.interface/view.interface.pass.cpp:184
+// TODO: why is the child member picked?
+//   assert(ranges::data(dataNull) == nullptr);
Can you elaborate on what this TODO means?

  rG LLVM Github Monorepo



More information about the libcxx-commits mailing list