Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[libc++][ranges] LWG4001: iota_view should provide empty #79687

Merged
2 changes: 1 addition & 1 deletion libcxx/docs/Status/Cxx2cIssues.csv
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
"`3974 <https://wg21.link/LWG3974>`__","``mdspan::operator[]`` should not copy ``OtherIndexTypes``","Kona November 2023","","",""
"`3987 <https://wg21.link/LWG3987>`__","Including ``<flat_foo>`` doesn't provide ``std::begin``/``end``","Kona November 2023","","","|flat_containers|"
"`3990 <https://wg21.link/LWG3990>`__","Program-defined specializations of ``std::tuple`` and ``std::variant`` can't be properly supported","Kona November 2023","","",""
"`4001 <https://wg21.link/LWG4001>`__","``iota_view`` should provide ``empty``","Kona November 2023","","","|ranges|"
"`4001 <https://wg21.link/LWG4001>`__","``iota_view`` should provide ``empty``","Kona November 2023","|Complete|","19.0","|ranges|"
"","","","","",""
"`3767 <https://wg21.link/LWG3767>`__","``codecvt<charN_t, char8_t, mbstate_t>`` incorrectly added to locale","Tokyo March 2024","","",""
"`3919 <https://wg21.link/LWG3919>`__","``enumerate_view`` may invoke UB for sized common non-forward underlying ranges","Tokyo March 2024","","","|ranges|"
Expand Down
2 changes: 2 additions & 0 deletions libcxx/include/__ranges/iota_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,8 @@ class iota_view : public view_interface<iota_view<_Start, _BoundSentinel>> {
return __iterator{__bound_sentinel_};
}

_LIBCPP_HIDE_FROM_ABI constexpr bool empty() const { return __value_ == __bound_sentinel_; }

_LIBCPP_HIDE_FROM_ABI constexpr auto size() const
requires(same_as<_Start, _BoundSentinel> && __advanceable<_Start>) ||
(integral<_Start> && integral<_BoundSentinel>) || sized_sentinel_for<_BoundSentinel, _Start>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
//===----------------------------------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

// UNSUPPORTED: c++03, c++11, c++14, c++17

// constexpr bool empty() const;

#include <cassert>
#include <concepts>
#include <ranges>
#include <utility>
#include <vector>

#include "types.h"

template <typename R>
concept HasEmpty = requires(R r) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
concept HasEmpty = requires(R r) {
concept HasEmpty = requires(R const r) {

Otherwise you don't test anywhere that the method is marked as const.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

std::ranges::empty(r);
{ r.empty() } -> std::same_as<bool>;
};

H-G-Hristov marked this conversation as resolved.
Show resolved Hide resolved
constexpr void test_empty_iota_sfinae() {
std::vector<int> ev;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: what do ev and iv stand for?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Normally I prefer long descriptive names but the libc++ tests use short names, so I try to be consistent:
ev -> empty vector vs v -> non-empty vector.
iv -> iota_view

I can change these.


// Both parameters are non-const
{
auto iv = std::views::iota(std::ranges::begin(ev), std::ranges::end(ev));

static_assert(HasEmpty<decltype(iv)>);
}
// Left parameter is const
{
auto iv = std::views::iota(std::ranges::begin(std::as_const(ev)), std::ranges::end(ev));

static_assert(HasEmpty<decltype(iv)>);
}
// Right parameter is const
{
auto iv = std::views::iota(std::ranges::begin(ev), std::ranges::end(std::as_const(ev)));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: does using as_const really matter to test iv.empty()?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought it was good to be detailed. Should I remove those?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed


static_assert(HasEmpty<decltype(iv)>);
}
// Both parameters are const
{
auto iv = std::views::iota(std::ranges::begin(std::as_const(ev)), std::ranges::end(std::as_const(ev)));

static_assert(HasEmpty<decltype(iv)>);
}
}

constexpr void test_nonempty_iota_sfinae() {
// Default ctr
{
std::ranges::iota_view<Int42<DefaultTo42>> iv;

static_assert(HasEmpty<decltype(iv)>);
}
// Value pass
{
std::ranges::iota_view<SomeInt> iv(SomeInt(94));

static_assert(HasEmpty<decltype(iv)>);
}

{
std::vector<char> v;
auto it = std::back_inserter(v);
auto iv = std::views::iota(it);

static_assert(HasEmpty<decltype(iv)>);
}
{
std::vector<char> v{'b', 'a', 'b', 'a', 'z', 'm', 't'};
auto it = std::back_inserter(v);
auto iv = std::views::iota(it);

static_assert(HasEmpty<decltype(iv)>);
}
}

constexpr void test_empty_iota() {
std::vector<int> ev;

// Both parameters are non-const
{
auto iv = std::views::iota(std::ranges::begin(ev), std::ranges::end(ev));

assert(iv.empty());
}
// Left parameter is const
{
auto iv = std::views::iota(std::ranges::begin(std::as_const(ev)), std::ranges::end(ev));

assert(iv.empty());
}
// Right parameter is const
{
auto iv = std::views::iota(std::ranges::begin(ev), std::ranges::end(std::as_const(ev)));

assert(iv.empty());
}
// Both parameters are const
{
auto iv = std::views::iota(std::ranges::begin(std::as_const(ev)), std::ranges::end(std::as_const(ev)));

assert(iv.empty());
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really understand the purpose of these tests. We're interested in checking that empty() can be called on a const iota_view, not that we can call it on a mutable iota_view constructed from const iterators?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed

}

constexpr void test_nonempty_iota() {
// Default ctr
{
std::ranges::iota_view<Int42<DefaultTo42>> iv;

assert(!iv.empty());
}
// Value pass
{
std::ranges::iota_view<SomeInt> iv(SomeInt(94));

assert(!iv.empty());
}

{
std::vector<char> v;
auto it = std::back_inserter(v);
auto iv = std::views::iota(it);

assert(!iv.empty());
}
{
std::vector<char> v{'b', 'a', 'b', 'a', 'z', 'm', 't'};
auto it = std::back_inserter(v);
auto iv = std::views::iota(it);

assert(!iv.empty());
}
}

constexpr bool test() {
test_empty_iota();
test_nonempty_iota();

return true;
}

int main(int, char**) {
test();
static_assert(test());

return 0;
}
Loading