Skip to content

Commit

Permalink
[libc++] fix views::all hard error on lvalue move only views instead …
Browse files Browse the repository at this point in the history
…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 <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.

Reviewed By: #libc, ldionne, var-const

Differential Revision: https://reviews.llvm.org/D128281
  • Loading branch information
huixie90 committed Jun 22, 2022
1 parent 1f88d80 commit 20869c5
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 0 deletions.
1 change: 1 addition & 0 deletions libcxx/include/__ranges/all.h
Expand Up @@ -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));
}
Expand Down
16 changes: 16 additions & 0 deletions libcxx/test/std/ranges/range.adaptors/range.all/all.pass.cpp
Expand Up @@ -49,6 +49,17 @@ struct CopyableView : std::ranges::view_base {
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) {}
Expand Down Expand Up @@ -139,6 +150,11 @@ constexpr bool test() {
{
static_assert(!std::is_invocable_v<decltype(std::views::all)>);
static_assert(!std::is_invocable_v<decltype(std::views::all), RandomAccessRange, RandomAccessRange>);

// `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<decltype(std::views::all), MoveOnlyView&>);
}

// Test that std::views::all is a range adaptor
Expand Down

0 comments on commit 20869c5

Please sign in to comment.