Skip to content

Add support for containers of move only typed values to range. #3586

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 7 commits into from
Dec 18, 2018
118 changes: 118 additions & 0 deletions unit/util/range.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,3 +65,121 @@ SCENARIO("range tests", "[core][util][range]")
}
}
}

class move_onlyt
{
public:
move_onlyt(move_onlyt &&) = default;
move_onlyt &operator=(move_onlyt &&) = default;
move_onlyt(const move_onlyt &) = delete;
move_onlyt &operator=(const move_onlyt &) = delete;

explicit move_onlyt(int value) : value{value} {};
int value = 0;
};

bool is_odd(const move_onlyt &move_only)
{
return move_only.value % 2 != 0;
}

const auto add = [](int left) {
return [=](const move_onlyt &right) { return left + right.value; };
};

SCENARIO(
"Range tests, with collections of move only typed values.",
"[core][util][range]")
{
GIVEN("A vector of move only typed values.")
{
std::vector<move_onlyt> input;
for(int i = 1; i <= 10; ++i)
input.emplace_back(i);
THEN("Values from a range of made from the vector can be moved.")
{
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);
std::size_t total = 0;
for(const auto &move_only : odds_filter)
total += 1;
REQUIRE(total == 5);
auto iterator = odds_filter.begin();
REQUIRE((iterator++)->value == 1);
REQUIRE((iterator++)->value == 3);
REQUIRE((iterator++)->value == 5);
REQUIRE((iterator++)->value == 7);
REQUIRE((iterator++)->value == 9);
}
THEN("Values from a filtered range made from the vector can be moved.")
{
std::vector<move_onlyt> odds;
for(move_onlyt &odd : make_range(input).filter(is_odd))
odds.emplace_back(std::move(odd));

REQUIRE(odds.size() == 5);
REQUIRE(odds[0].value == 1);
REQUIRE(odds[1].value == 3);
REQUIRE(odds[2].value == 5);
REQUIRE(odds[3].value == 7);
REQUIRE(odds[4].value == 9);
}
THEN("Map can be applied to a range of move only typed values.")
{
std::vector<int> results;
for(int result : make_range(input).map(add(1)))
results.push_back(result);

const std::vector<int> expected_results{2, 3, 4, 5, 6, 7, 8, 9, 10, 11};
REQUIRE(results == expected_results);
}
}
GIVEN("Two vectors containing move only types values.")
{
std::vector<move_onlyt> input1;
for(int i = 1; i <= 3; ++i)
input1.emplace_back(i);
std::vector<move_onlyt> input2;
for(int i = 7; i <= 9; ++i)
input2.emplace_back(i);

THEN("Values from concatenated ranges made from the vector can be moved.")
{
std::vector<move_onlyt> both_inputs;
for(move_onlyt &input : make_range(input1).concat(make_range(input2)))
both_inputs.emplace_back(std::move(input));

REQUIRE(both_inputs.size() == 6);
REQUIRE(both_inputs[0].value == 1);
REQUIRE(both_inputs[1].value == 2);
REQUIRE(both_inputs[2].value == 3);
REQUIRE(both_inputs[3].value == 7);
REQUIRE(both_inputs[4].value == 8);
REQUIRE(both_inputs[5].value == 9);
}
}
GIVEN("A const vector of ints.")
{
const std::vector<int> input{1, 2, 3, 4, 5};

THEN("The vector can be mapped into a range of move-only types")
Copy link
Contributor

Choose a reason for hiding this comment

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

If it works - you should add a test that maps from a move only type to a regular type, and one that moves the same type through the map

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The test which "maps from a move only type to a regular type" already exists. As we discussed, you missed it when reviewing. The f passed into map can't move from its parameter, because the parameter has to be a const reference. I have expanded the doxygen to document the extent of the support for move-only types and its limitations.

{
std::vector<move_onlyt> results;
const auto make_move_only = [](int i) { return move_onlyt{i}; };
for(auto &incremented : make_range(input).map(make_move_only))
results.emplace_back(std::move(incremented));

REQUIRE(results.size() == 5);
REQUIRE(results[0].value == 1);
REQUIRE(results[1].value == 2);
REQUIRE(results[2].value == 3);
REQUIRE(results[3].value == 4);
REQUIRE(results[4].value == 5);
}
}
}