Skip to content

Fix a selection of const related issues with ranget. #3833

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

Merged
merged 6 commits into from
Jan 19, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 13 additions & 37 deletions src/util/range.h
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,8 @@ class filter_iteratort
public:
using difference_type = typename iteratort::difference_type;
using value_type = typename iteratort::value_type;
using pointer = const value_type *;
using reference = const value_type &;
using pointer = typename iteratort::pointer;
using reference = typename iteratort::reference;
using iterator_category = std::forward_iterator_tag;

bool operator==(const filter_iteratort &other) const
Expand Down Expand Up @@ -143,22 +143,12 @@ class filter_iteratort
return tmp;
}

value_type &operator*()
reference operator*() const
{
return *underlying;
}

value_type *operator->()
{
return &(*underlying);
}

const value_type &operator*() const
{
return *underlying;
}

const value_type *operator->() const
pointer operator->() const
{
return &(*underlying);
}
Expand Down Expand Up @@ -208,8 +198,8 @@ struct concat_iteratort
public:
using difference_type = typename first_iteratort::difference_type;
using value_type = typename first_iteratort::value_type;
using pointer = const value_type *;
using reference = const value_type &;
using pointer = typename first_iteratort::pointer;
using reference = typename first_iteratort::reference;
using iterator_category = std::forward_iterator_tag;

static_assert(
Expand Down Expand Up @@ -245,28 +235,14 @@ struct concat_iteratort
return tmp;
}

value_type &operator*()
reference operator*() const
{
if(first_begin == first_end)
return *second_begin;
return *first_begin;
}

value_type *operator->()
{
if(first_begin == first_end)
return &(*second_begin);
return &(*first_begin);
}

const value_type &operator*() const
{
if(first_begin == first_end)
return *second_begin;
return *first_begin;
}

const value_type *operator->() const
pointer operator->() const
{
if(first_begin == first_end)
return &(*second_begin);
Expand Down Expand Up @@ -311,14 +287,14 @@ template <typename iteratort>
struct ranget final
{
public:
using value_typet = typename iteratort::value_type;
using value_type = typename iteratort::value_type;

ranget(iteratort begin, iteratort end) : begin_value(begin), end_value(end)
{
}

ranget<filter_iteratort<iteratort>>
filter(std::function<bool(const value_typet &)> f)
filter(std::function<bool(const value_type &)> f)
{
auto shared_f = std::make_shared<decltype(f)>(std::move(f));
filter_iteratort<iteratort> filter_begin(shared_f, begin(), end());
Expand All @@ -335,9 +311,9 @@ struct ranget final
template <typename functiont>
auto map(functiont &&f) -> ranget<map_iteratort<
iteratort,
typename std::result_of<functiont(value_typet)>::type>>
typename std::result_of<functiont(value_type)>::type>>
{
using outputt = typename std::result_of<functiont(value_typet)>::type;
using outputt = typename std::result_of<functiont(value_type)>::type;
auto shared_f = std::make_shared<
std::function<outputt(const typename iteratort::value_type &)>>(
std::forward<functiont>(f));
Expand Down Expand Up @@ -365,7 +341,7 @@ struct ranget final
return begin_value == end_value;
}

iteratort begin()
iteratort begin() const
{
return begin_value;
}
Expand Down
97 changes: 91 additions & 6 deletions unit/util/range.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,14 @@ Author: Romain Brenguier, [email protected]
#include <testing-utils/catch.hpp>
#include <util/range.h>

/// Trivial example template function requiring a container to have a
/// `value_type`.
template <typename containert>
typename containert::value_type front(containert container)
{
return *container.begin();
}

SCENARIO("range tests", "[core][util][range]")
{
GIVEN("A vector with three strings")
Expand All @@ -21,15 +29,15 @@ SCENARIO("range tests", "[core][util][range]")
list.emplace_back("acdef");
THEN("Use range-for to compute the total length")
{
auto range = make_range(list);
const auto range = make_range(list);
std::size_t total_length = 0;
for(const auto &s : range)
total_length += s.length();
REQUIRE(total_length == 12);
}
THEN("Use map to compute individual lengths")
{
auto length_range =
const auto length_range =
make_range(list).map([](const std::string &s) { return s.length(); });
auto it = length_range.begin();
REQUIRE(*it == 3);
Expand All @@ -42,16 +50,27 @@ SCENARIO("range tests", "[core][util][range]")
}
THEN("Filter using lengths")
{
auto filtered_range = make_range(list).filter(
const auto filtered_range = make_range(list).filter(
[&](const std::string &s) { return s.length() == 4; });
auto it = filtered_range.begin();
REQUIRE(*it == "cdef");
++it;
REQUIRE(it == filtered_range.end());
}
THEN(
"A const instance of a `filter_iteratort` can mutate the input "
"collection.")
{
const auto it =
make_range(list)
.filter([&](const std::string &s) { return s.length() == 3; })
.begin();
*it += "x";
REQUIRE(*list.begin() == "abcx");
}
THEN("Filter, map and use range-for on the same list")
{
auto range =
const auto range =
make_range(list)
.filter([&](const std::string &s) -> bool { return s[0] == 'a'; })
.map([&](const std::string &s) { return s.length(); });
Expand All @@ -64,6 +83,72 @@ SCENARIO("range tests", "[core][util][range]")
REQUIRE(total == 8);
}
}
GIVEN("A const vector of ints")
{
const std::vector<int> input{1, 2, 3, 4};
THEN("Filter the vector using range.")
{
const auto odds_range =
make_range(input).filter([](const int number) { return number % 2; });
const std::vector<int> odds{odds_range.begin(), odds_range.end()};
const std::vector<int> expected_odds{1, 3};
REQUIRE(odds == expected_odds);
}
THEN(
"The unit testing template function requiring `value_type` works with "
"`std::vector`.")
{
REQUIRE(front(input) == 1);
}
THEN(
"A range can be used with a template function expecting a container "
"which has a `value_type`.")
{
REQUIRE(front(make_range(input)) == 1);
}
THEN("Map over the vector using range.")
{
const auto plus_one_range =
make_range(input).map([](const int number) { return number + 1; });
const std::vector<int> plus_one_collection{plus_one_range.begin(),
plus_one_range.end()};
const std::vector<int> expected_output{2, 3, 4, 5};
REQUIRE(plus_one_collection == expected_output);
};
}
GIVEN("Two const vectors of ints")
{
const std::vector<int> input1{1, 2};
const std::vector<int> input2{3, 4};
THEN("Concat the vectors using range.")
{
const auto range = make_range(input1).concat(make_range(input2));
const std::vector<int> output{range.begin(), range.end()};
const std::vector<int> expected{1, 2, 3, 4};
REQUIRE(output == expected);
};
}
GIVEN("Two non-const vectors of ints.")
{
std::vector<int> input1{1, 2};
std::vector<int> input2{3, 4};
THEN(
"Const instances of `concat_iteratort` should enable the input "
"collections to be mutated.")
{
const auto concat_range = make_range(input1).concat(make_range(input2));
int x = 5;
for(auto it = concat_range.begin(); it != concat_range.end(); ++it, ++x)
{
const auto const_it = it;
*const_it = x;
}
std::vector<int> expected_result1{5, 6};
std::vector<int> expected_result2{7, 8};
REQUIRE(input1 == expected_result1);
REQUIRE(input2 == expected_result2);
}
}
}

class move_onlyt
Expand Down Expand Up @@ -98,13 +183,13 @@ SCENARIO(
input.emplace_back(i);
THEN("Values from a range of made from the vector can be moved.")
{
auto input_range = make_range(input);
const auto input_range = make_range(input);
move_onlyt destination{std::move(*input_range.begin())};
REQUIRE(destination.value == 1);
}
THEN("A range of made from the vector can be filtered.")
{
auto odds_filter = make_range(input).filter(is_odd);
const auto odds_filter = make_range(input).filter(is_odd);
const std::size_t total =
std::distance(odds_filter.begin(), odds_filter.end());
REQUIRE(total == 5);
Expand Down