From 20869c5ba0692974a3ec76b935360aaa67d0218b Mon Sep 17 00:00:00 2001 From: Hui Xie Date: Wed, 22 Jun 2022 09:24:09 +0100 Subject: [PATCH] [libc++] fix views::all hard error on lvalue move only views instead of SFINAE 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 concept can_all = requires { std::views::all(std::declval()); }; ``` The expression `can_all` returns libstdc++: false msvc stl : false libc++ : error: static_cast from 'V' to 'typename decay(__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())` is well-formed and the concept `can_all` 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. Reviewed By: #libc, ldionne, var-const Differential Revision: https://reviews.llvm.org/D128281 --- libcxx/include/__ranges/all.h | 1 + .../ranges/range.adaptors/range.all/all.pass.cpp | 16 ++++++++++++++++ 2 files changed, 17 insertions(+) diff --git a/libcxx/include/__ranges/all.h b/libcxx/include/__ranges/all.h index 5f9bf7e277ed7..181477419c850 100644 --- a/libcxx/include/__ranges/all.h +++ b/libcxx/include/__ranges/all.h @@ -39,6 +39,7 @@ namespace __all { [[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)); } diff --git a/libcxx/test/std/ranges/range.adaptors/range.all/all.pass.cpp b/libcxx/test/std/ranges/range.adaptors/range.all/all.pass.cpp index 0e3d5abb11edc..4d4d1ebc0b226 100644 --- a/libcxx/test/std/ranges/range.adaptors/range.all/all.pass.cpp +++ b/libcxx/test/std/ranges/range.adaptors/range.all/all.pass.cpp @@ -49,6 +49,17 @@ struct CopyableView : std::ranges::view_base { static_assert(std::ranges::view>); static_assert(std::ranges::view>); +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,11 @@ constexpr bool test() { { static_assert(!std::is_invocable_v); static_assert(!std::is_invocable_v); + + // `views::all(v)` is expression equivalent to `decay-copy(v)` if the decayed type + // of `v` models `view`. If `v` is an lvalue-reference to a move-only view, the + // expression should be ill-formed because `v` is not copyable + static_assert(!std::is_invocable_v); } // Test that std::views::all is a range adaptor