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.

Sign up for GitHub

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

Jump to bottom

Fix ranges::find_end return for bidi-common case #2270

Merged
merged 2 commits into from Nov 13, 2021
Merged

Fix ranges::find_end return for bidi-common case #2270

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4364f73
Fix ranges::find_end return for bidi-common case
CaseyCarter Oct 11, 2021
ebdbb45
Miscco's comments
CaseyCarter Oct 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion stl/inc/algorithm
View file
Open in desktop
Expand Up @@ -2363,7 +2363,7 @@ namespace ranges {
auto _Next2 = _Last2;
for (;;) { // test if [_First2, _Last2) is a suffix of [_First1, _Candidate)
if (_First2 == _Next2) { // match found
return {_STD move(_First1), _STD move(_Last1)};
return {_STD move(_Next1), _STD move(_Candidate)};
}

if (_First1 == _Next1) {
Expand Down
170 changes: 82 additions & 88 deletions tests/std/tests/P0896R4_ranges_alg_find_end/test.cpp
View file
Open in desktop
Expand Up @@ -11,104 +11,98 @@

#include <range_algorithm_support.hpp>

constexpr void smoke_test() {
using ranges::find_end, ranges::iterator_t, ranges::subrange, std::array, std::same_as;
using P = std::pair<int, int>;
using namespace std;

// Validate dangling story
STATIC_ASSERT(same_as<decltype(find_end(borrowed<false>{}, array<int, 42>{})), ranges::dangling>);
STATIC_ASSERT(same_as<decltype(find_end(borrowed<true>{}, array<int, 42>{})), subrange<int*>>);
// Validate dangling story
STATIC_ASSERT(same_as<decltype(ranges::find_end(borrowed<false>{}, array<int, 42>{})), ranges::dangling>);
CaseyCarter marked this conversation as resolved.
Show resolved Hide resolved
STATIC_ASSERT(same_as<decltype(ranges::find_end(borrowed<true>{}, array<int, 42>{})), ranges::subrange<int*>>);

const array pairs = {P{0, 42}, P{1, 42}, P{0, 42}, P{1, 42}, P{0, 42}, P{1, 42}, P{0, 42}};
struct instantiator {
static constexpr pair<int, int> pairs[] = {{0, 42}, {1, 42}, {0, 42}, {1, 42}, {0, 42}};
static constexpr auto pred = [](const int x, const int y) { return x == y + 1; };

static constexpr int good_needle[] = {-1, 0};
static constexpr int bad_needle[] = {0, 0};

template <ranges::forward_range Fwd1, ranges::forward_range Fwd2>
static constexpr void test() {
using ranges::find_end, ranges::begin, ranges::end, ranges::iterator_t, ranges::next, ranges::subrange;

{
// Validate range overload [found case]
Fwd1 haystack{pairs};
Fwd2 needle{good_needle};
const same_as<subrange<iterator_t<Fwd1>>> auto result = find_end(haystack, needle, pred, get_first);
STATIC_ASSERT(
CanMemberSize<subrange<iterator_t<Fwd1>>> == sized_sentinel_for<iterator_t<Fwd1>, iterator_t<Fwd1>>);
if constexpr (CanMemberSize<subrange<iterator_t<Fwd1>>>) {
assert(result.size() == 2);
}
assert(result.begin() == next(begin(haystack), 2));
assert(result.end() == next(begin(haystack), 4));
}

const auto pred = [](const int x, const int y) { return x == y + 1; };
{
// Validate range overload [not found case]
Fwd1 haystack{pairs};
Fwd2 needle{bad_needle};
STATIC_ASSERT(
CanMemberSize<subrange<iterator_t<Fwd1>>> == sized_sentinel_for<iterator_t<Fwd1>, iterator_t<Fwd1>>);
const same_as<subrange<iterator_t<Fwd1>>> auto result = find_end(haystack, needle, pred, get_first);
assert(result.empty());
assert(result.begin() == end(haystack));
assert(result.end() == end(haystack));
}

const array good_needle = {-1, 0};
{
// Validate range overload [found case]
const auto result = find_end(pairs, good_needle, pred, get_first);
STATIC_ASSERT(same_as<decltype(result), const subrange<iterator_t<decltype(pairs)>>>);
assert(result.size() == 2);
assert(result.begin() == pairs.begin() + 4);
assert(result.end() == pairs.begin() + 6);
}
{
// Validate iterator + sentinel overload [found case]
const auto result =
find_end(pairs.begin(), pairs.end(), good_needle.begin(), good_needle.end(), pred, get_first);
STATIC_ASSERT(same_as<decltype(result), const subrange<iterator_t<decltype(pairs)>>>);
assert(result.size() == 2);
assert(result.begin() == pairs.begin() + 4);
assert(result.end() == pairs.begin() + 6);
}
{
// Validate iterator + sentinel overload [found case]
Fwd1 haystack{pairs};
Fwd2 needle{good_needle};
const same_as<subrange<iterator_t<Fwd1>>> auto result =
find_end(begin(haystack), end(haystack), begin(needle), end(needle), pred, get_first);
STATIC_ASSERT(
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick, is there a reason to shout STATIC_ASSERT, at least in ranges code we should be able to be a bit less shouty

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes there is: I refuse to manually map from line number to failure condition when I can make MSVC do it for me by capitalizing static_assert.

CanMemberSize<subrange<iterator_t<Fwd1>>> == sized_sentinel_for<iterator_t<Fwd1>, iterator_t<Fwd1>>);
if constexpr (CanMemberSize<subrange<iterator_t<Fwd1>>>) {
assert(result.size() == 2);
}
assert(result.begin() == next(begin(haystack), 2));
assert(result.end() == next(begin(haystack), 4));
}

const array bad_needle = {0, 0};
{
// Validate range overload [not found case]
const auto result = find_end(pairs, bad_needle, pred, get_first);
STATIC_ASSERT(same_as<decltype(result), const subrange<iterator_t<decltype(pairs)>>>);
assert(result.empty());
assert(result.begin() == pairs.end());
assert(result.end() == pairs.end());
}
{
// Validate range overload [not found case]
const auto result = find_end(pairs.begin(), pairs.end(), bad_needle.begin(), bad_needle.end(), pred, get_first);
STATIC_ASSERT(same_as<decltype(result), const subrange<iterator_t<decltype(pairs)>>>);
assert(result.empty());
assert(result.begin() == pairs.end());
assert(result.end() == pairs.end());
{
// Validate range overload [not found case]
Fwd1 haystack{pairs};
Fwd2 needle{bad_needle};
const same_as<subrange<iterator_t<Fwd1>>> auto result =
find_end(begin(haystack), end(haystack), begin(needle), end(needle), pred, get_first);
assert(result.empty());
assert(result.begin() == end(haystack));
assert(result.end() == end(haystack));
}
}

{
// Validate the memcmp optimization
const int haystack[] = {1, 2, 3, 1, 2, 3, 1, 2, 3};
const int needle[] = {1, 2, 3};
const auto result = find_end(haystack, std::span<const int>{needle});
STATIC_ASSERT(same_as<decltype(result), const subrange<const int*>>);
assert(result.begin() == haystack + 6);
assert(result.end() == haystack + 9);
template <ranges::forward_range Fwd1, ranges::forward_range Fwd2>
static void call() {
test<Fwd1, Fwd2>();
STATIC_ASSERT((test<Fwd1, Fwd2>(), true));
}
}
};

int main() {
STATIC_ASSERT((smoke_test(), true));
smoke_test();
constexpr bool memcmp_test() {
// Validate the memcmp optimization
const int haystack[] = {1, 2, 3, 1, 2, 3, 1, 2, 3};
const int needle[] = {2, 3, 1};
const same_as<ranges::subrange<const int*>> auto result = ranges::find_end(haystack, span<const int>{needle});
assert(result.begin() == haystack + 4);
assert(result.end() == haystack + 7);
return true;
}

int main() {
#ifndef _PREFAST_ // TRANSITION, GH-1030
CaseyCarter marked this conversation as resolved.
Show resolved Hide resolved
struct instantiator {
template <class Fwd1, class Fwd2>
static void call() {
if constexpr (!is_permissive) { // These fail to compile in C1XX's permissive mode due to VSO-566808
using ranges::iterator_t;

Fwd1 fwd1{std::span<const int, 0>{}};
Fwd2 fwd2{std::span<const int, 0>{}};

(void) ranges::find_end(fwd1, fwd2);
(void) ranges::find_end(ranges::begin(fwd1), ranges::end(fwd1), ranges::begin(fwd2), ranges::end(fwd2));

BinaryPredicateFor<iterator_t<Fwd1>, iterator_t<Fwd2>> pred{};
(void) ranges::find_end(fwd1, fwd2, pred);
(void) ranges::find_end(
ranges::begin(fwd1), ranges::end(fwd1), ranges::begin(fwd2), ranges::end(fwd2), pred);
test_fwd_fwd<instantiator, const pair<int, int>, const int>();
#endif // TRANSITION

HalfProjectedBinaryPredicateFor<iterator_t<Fwd2>> halfpred{};
ProjectionFor<iterator_t<Fwd1>> halfproj{};
(void) ranges::find_end(fwd1, fwd2, halfpred, halfproj);
(void) ranges::find_end(
ranges::begin(fwd1), ranges::end(fwd1), ranges::begin(fwd2), ranges::end(fwd2), halfpred, halfproj);

ProjectedBinaryPredicate<0, 1> projpred{};
ProjectionFor<iterator_t<Fwd1>, 0> proj1{};
ProjectionFor<iterator_t<Fwd2>, 1> proj2{};
(void) ranges::find_end(fwd1, fwd2, projpred, proj1, proj2);
(void) ranges::find_end(
ranges::begin(fwd1), ranges::end(fwd1), ranges::begin(fwd2), ranges::end(fwd2), projpred, proj1, proj2);
}
}
};

template void test_fwd_fwd<instantiator, const int, const int>();
#endif // TRANSITION, GH-1030
STATIC_ASSERT(memcmp_test());
memcmp_test();
}