[libcxx-commits] [PATCH] D128281: [libc++] fix views::all hard error on lvalue move only views instead of SFINAE

[Hui via Phabricator via libcxx-commits]

huixie90 created this revision.
Herald added a project: All.
huixie90 requested review of this revision.
Herald added a project: libc++.
Herald added a subscriber: libcxx-commits.
Herald added a reviewer: libc++.

For an lvalue reference to a move only view x, views::all(x) gives hard error because the expression inside noexcept is not well formed and it is not SFINAE friendly.

Given a move only view type `V`, and a concept

  template <class R>
  concept can_all = requires {
      std::views::all(std::declval<R>());
  };

The expression `can_all<V&>` returns
libstdc++: false
msvc stl : false
libc++   : error: static_cast from 'V' to 'typename decay<decltype((std::forward<V &>(__t)))>::type' (aka 'V') uses deleted function

  noexcept(noexcept(_LIBCPP_AUTO_CAST(std::forward<_Tp>(__t))))

The standard spec has its own problem, the spec says it is expression equivalent to `decay-copy(E)` but the spec of `decay-copy` does not have any constraint, which means the expression `decay-copy(declval<V&>())` is well-formed and the concept `can_all<V&>` should return true and should error when instantiating the function body of decay-copy. This is clearly wrong behaviour in the spec and we will probably create an LWG issue. But the libc++'s behaviour is clearly not correct. The `noexcept` is an "extension" in libc++ which is not in the spec, but the expression inside `noexpect` triggers hard error, which is not right.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128281

Files:
  libcxx/include/__ranges/all.h
  libcxx/test/std/ranges/range.adaptors/range.all/all.pass.cpp


Index: libcxx/test/std/ranges/range.adaptors/range.all/all.pass.cpp
===================================================================
--- libcxx/test/std/ranges/range.adaptors/range.all/all.pass.cpp
+++ libcxx/test/std/ranges/range.adaptors/range.all/all.pass.cpp
@@ -49,6 +49,17 @@
 static_assert(std::ranges::view<CopyableView<true>>);
 static_assert(std::ranges::view<CopyableView<false>>);
 
+struct MoveOnlyView : std::ranges::view_base{
+  MoveOnlyView() = default;
+  MoveOnlyView(const MoveOnlyView&) = delete;
+  MoveOnlyView& operator=(const MoveOnlyView&) = delete;
+  MoveOnlyView(MoveOnlyView&&) = default;
+  MoveOnlyView& operator=(MoveOnlyView&&) = default;
+
+  int* begin() const;
+  int* end() const;
+};
+
 struct Range {
   int start_;
   constexpr explicit Range(int start) noexcept : start_(start) {}
@@ -139,6 +150,7 @@
   {
     static_assert(!std::is_invocable_v<decltype(std::views::all)>);
     static_assert(!std::is_invocable_v<decltype(std::views::all), RandomAccessRange, RandomAccessRange>);
+    static_assert(!std::is_invocable_v<decltype(std::views::all), MoveOnlyView&>);
   }
 
   // Test that std::views::all is a range adaptor
Index: libcxx/include/__ranges/all.h
===================================================================
--- libcxx/include/__ranges/all.h
+++ libcxx/include/__ranges/all.h
@@ -39,6 +39,7 @@
     [[nodiscard]] _LIBCPP_HIDE_FROM_ABI
     constexpr auto operator()(_Tp&& __t) const
       noexcept(noexcept(_LIBCPP_AUTO_CAST(std::forward<_Tp>(__t))))
+      -> decltype(_LIBCPP_AUTO_CAST(std::forward<_Tp>(__t)))
     {
       return _LIBCPP_AUTO_CAST(std::forward<_Tp>(__t));
     }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D128281.438692.patch
Type: text/x-patch
Size: 1651 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20220621/3927a542/attachment-0001.bin>