Skip to content

Commit 54bfd34

Browse files
committed
SG9 feedback: default ctor + moved from init an empty view
1 parent a08e3ee commit 54bfd34

File tree

3 files changed

+229
-8
lines changed

3 files changed

+229
-8
lines changed

impl/any_view/any_view.hpp

Lines changed: 108 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -472,10 +472,72 @@ class any_view {
472472

473473
using sentinel = any_sentinel;
474474

475+
struct empty_iterator {
476+
static consteval any_iterator_vtable get_vtable() {
477+
any_iterator_vtable t;
478+
479+
t.deref_ = [](const iterator_storage &) -> Ref {
480+
assert(false && "Dereferencing empty iterator");
481+
std::unreachable();
482+
};
483+
t.increment_ = [](iterator_storage &) {
484+
assert(false && "Incrementing empty iterator");
485+
};
486+
t.iter_move_ = [](const iterator_storage &) -> RValueRef {
487+
assert(false && "iter_moving empty iterator");
488+
std::unreachable();
489+
};
490+
491+
if constexpr (Traversal >= any_view_options::forward) {
492+
t.equal_ = [](const iterator_storage &, const iterator_storage &) {
493+
return true;
494+
};
495+
}
496+
497+
if constexpr (Traversal >= any_view_options::bidirectional) {
498+
t.decrement_ = [](iterator_storage &) {
499+
assert(false && "Decrementing empty iterator");
500+
};
501+
}
502+
503+
if constexpr (Traversal >= any_view_options::random_access) {
504+
t.advance_ = [](iterator_storage &, Diff) {
505+
assert(false && "Advancing empty iterator");
506+
};
507+
t.distance_to_ = [](const iterator_storage &,
508+
const iterator_storage &) -> Diff {
509+
assert(false && "Distance to empty iterator");
510+
std::unreachable();
511+
};
512+
}
513+
514+
if constexpr (Traversal == any_view_options::contiguous) {
515+
t.arrow_ = [](const iterator_storage &) -> std::add_pointer_t<Ref> {
516+
assert(false && "Arrow operator on empty iterator");
517+
std::unreachable();
518+
};
519+
}
520+
521+
return t;
522+
}
523+
524+
static constexpr any_iterator_vtable vtable = get_vtable();
525+
};
526+
527+
struct empty_sentinel {
528+
static consteval any_sentinel_vtable get_vtable() {
529+
any_sentinel_vtable t;
530+
t.equal_ = [](const iterator &, const any_sentinel &) { return true; };
531+
return t;
532+
}
533+
534+
static constexpr any_sentinel_vtable vtable = get_vtable();
535+
};
536+
475537
using view_storage =
476538
detail::storage<4 * sizeof(void *), sizeof(void *), is_view_copyable>;
477539
struct sized_vtable {
478-
std::size_t (*size_)(const view_storage &);
540+
std::__make_unsigned_t<Diff> (*size_)(const view_storage &);
479541
};
480542
struct any_view_vtable
481543
: maybe_t<sized_vtable,
@@ -514,9 +576,31 @@ class any_view {
514576
}
515577

516578
template <class View>
517-
static constexpr std::size_t size(const view_storage &v) {
518-
return std::ranges::size(*(v.template get_ptr<View>()));
579+
static constexpr std::__make_unsigned_t<Diff> size(const view_storage &v) {
580+
return std::__make_unsigned_t<Diff>(
581+
std::ranges::size(*(v.template get_ptr<View>())));
582+
}
583+
};
584+
585+
struct empty_view_ {
586+
static consteval any_view_vtable get_vtable() {
587+
any_view_vtable t;
588+
t.begin_ = [](view_storage &) -> iterator {
589+
return any_iterator(&empty_iterator::vtable, empty_iterator{});
590+
};
591+
t.end_ = [](view_storage &) -> sentinel {
592+
return any_sentinel(&empty_sentinel::vtable, empty_sentinel{});
593+
};
594+
if constexpr ((Opts & any_view_options::sized) !=
595+
any_view_options::none) {
596+
t.size_ = [](const view_storage &) -> std::__make_unsigned_t<Diff> {
597+
return 0;
598+
};
599+
}
600+
return t;
519601
}
602+
603+
static constexpr any_view_vtable vtable = get_vtable();
520604
};
521605

522606
template <class View>
@@ -545,8 +629,8 @@ class any_view {
545629
return false;
546630
}
547631

548-
if constexpr (!std::convertible_to<std::ranges::range_rvalue_reference_t<View>,
549-
RValueRef>) {
632+
if constexpr (!std::convertible_to<
633+
std::ranges::range_rvalue_reference_t<View>, RValueRef>) {
550634
return false;
551635
}
552636

@@ -569,6 +653,8 @@ class any_view {
569653
}
570654
}
571655

656+
constexpr any_view() : view_vtable_(&empty_view_::vtable), view_() {}
657+
572658
template <class Range>
573659
requires(!std::same_as<remove_cvref_t<Range>, any_view> &&
574660
std::ranges::viewable_range<Range> &&
@@ -582,25 +668,39 @@ class any_view {
582668
requires is_view_copyable
583669
= default;
584670

585-
constexpr any_view(any_view &&) = default;
671+
constexpr any_view(any_view &&other) noexcept
672+
: view_vtable_(other.view_vtable_), view_(std::move(other.view_)) {
673+
other.view_vtable_ = &empty_view_::vtable;
674+
}
586675

587676
constexpr any_view &operator=(const any_view &)
588677
requires is_view_copyable
589678
= default;
590679

591-
constexpr any_view &operator=(any_view &&) = default;
680+
constexpr any_view &operator=(any_view &&other) noexcept {
681+
if (this != &other) {
682+
any_view(std::move(other)).swap(*this);
683+
}
684+
return *this;
685+
}
592686

593687
constexpr ~any_view() = default;
594688

595689
constexpr iterator begin() { return (*(view_vtable_->begin_))(view_); }
596690
constexpr sentinel end() { return (*(view_vtable_->end_))(view_); }
597691

598-
constexpr std::size_t size() const
692+
constexpr std::__make_unsigned_t<Diff> size() const
599693
requires((Opts & any_view_options::sized) != any_view_options::none)
600694
{
601695
return (*(view_vtable_->size_))(view_);
602696
}
603697

698+
constexpr void swap(any_view &other) noexcept {
699+
view_.swap(other.view_);
700+
std::swap(view_vtable_, other.view_vtable_);
701+
}
702+
constexpr friend void swap(any_view &x, any_view &y) noexcept { x.swap(y); }
703+
604704
private:
605705
template <class Iter>
606706
static constexpr any_iterator_vtable iter_vtable =
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
#include <algorithm>
2+
#include <array>
3+
#include <cassert>
4+
#include <catch2/catch_test_macros.hpp>
5+
#include <cstddef>
6+
#include <ranges>
7+
8+
#include "../helper.hpp"
9+
#include "any_view.hpp"
10+
11+
#define TEST_POINT(x) TEST_CASE(x, "[default]")
12+
13+
namespace {
14+
15+
using AnyView =
16+
std::ranges::any_view<int, std::ranges::any_view_options::forward>;
17+
18+
[[maybe_unused]] constexpr void test_default() {
19+
std::array arr{1, 2, 3, 4, 5};
20+
21+
AnyView view1;
22+
23+
assert(std::ranges::empty(view1));
24+
for (const auto& elem : view1) {
25+
assert(false);
26+
(void)elem;
27+
}
28+
29+
view1 = arr;
30+
assert(*view1.begin() == 1);
31+
}
32+
33+
34+
constexpr bool test() {
35+
test_default();
36+
return true;
37+
}
38+
39+
TEST_POINT("default") {
40+
test();
41+
static_assert(test());
42+
}
43+
44+
} // namespace
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
#include <algorithm>
2+
#include <array>
3+
#include <cassert>
4+
#include <catch2/catch_test_macros.hpp>
5+
#include <cstddef>
6+
#include <ranges>
7+
8+
#include "../helper.hpp"
9+
#include "any_view.hpp"
10+
11+
#define TEST_POINT(x) TEST_CASE(x, "[moved_from]")
12+
13+
namespace {
14+
15+
using AnyView =
16+
std::ranges::any_view<int, std::ranges::any_view_options::forward>;
17+
18+
[[maybe_unused]] constexpr void small_buffer() {
19+
std::array arr{1, 2, 3, 4, 5};
20+
21+
AnyView view1(arr);
22+
AnyView view2(std::move(view1));
23+
24+
assert(*view2.begin() == 1);
25+
26+
assert(std::ranges::empty(view1));
27+
for (const auto& elem : view1) {
28+
assert(false);
29+
(void)elem;
30+
}
31+
32+
view1 = arr;
33+
assert(*view1.begin() == 1);
34+
}
35+
36+
struct BigView : std::ranges::view_base {
37+
int* data;
38+
std::size_t size;
39+
40+
std::array<char, 1024> stuff{};
41+
42+
constexpr BigView(int* d, std::size_t s) : data(d), size(s) {}
43+
44+
constexpr auto begin() const { return data; }
45+
constexpr auto end() const { return data + size; }
46+
};
47+
48+
constexpr void heap() {
49+
std::array arr{1, 2, 3, 4, 5};
50+
BigView big_view(arr.data(), arr.size());
51+
AnyView view1(big_view);
52+
AnyView view2(std::move(view1));
53+
54+
assert(*view2.begin() == 1);
55+
56+
assert(std::ranges::empty(view1));
57+
for (const auto& elem : view1) {
58+
assert(false);
59+
(void)elem;
60+
}
61+
62+
view1 = arr;
63+
assert(*view1.begin() == 1);
64+
}
65+
66+
constexpr bool test() {
67+
small_buffer();
68+
heap();
69+
return true;
70+
}
71+
72+
TEST_POINT("moved_from") {
73+
test();
74+
static_assert(test());
75+
}
76+
77+
} // namespace

0 commit comments

Comments
 (0)