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

<algorithm>: The bidirectional common range version of ranges::find_end returns the wrong value #2268

Closed
hewillk opened this issue Oct 11, 2021 · 2 comments · Fixed by #2270
Labels
bug Something isn't working fixed Something works now, yay!

Comments

@hewillk
Copy link
Contributor

hewillk commented Oct 11, 2021

STL/stl/inc/algorithm

Lines 2361 to 2372 in 8833270

for (auto _Candidate = _Last1;; --_Candidate) { // try a match at _Candidate
auto _Next1 = _Candidate;
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)};
}
if (_First1 == _Next1) {
// [_First1, _Candidate) is shorter than [_First2, _Last2); remaining candidates nonviable
return {_Last1, _Last1};
}

IIUC, the first return statement should return {std::move(_Next1), std::move(_Candidate)};.

Test case:

#include <ranges>
#include <algorithm>

constexpr auto find = [] {
  auto r = std::views::iota(0, 2)
         | std::views::filter([](int) { return true; });
  auto s = std::ranges::find_end(r, std::views::single(1));
  return s.front() == 1;
}();

static_assert(find);

https://godbolt.org/z/1dzq8Pe1K

@fsb4000
Copy link
Contributor

fsb4000 commented Oct 11, 2021

I tried "possible implementation" from https://en.cppreference.com/w/cpp/algorithm/ranges/find_end
it gives the answer as gcc. So yeah, I think you're right.

@CaseyCarter CaseyCarter added the bug Something isn't working label Oct 11, 2021
@CaseyCarter
Copy link
Member

Yes, this is completely bogus and lacking test coverage. We went light on some test coverage for range algorithms that were virtually transcriptions of the std flavor with the expectation that the guts were known good and the interface bits were likely to be correct if they compiled. That policy obviously failed here, and we should have full test coverage of ranges::find_end.

CaseyCarter added a commit to CaseyCarter/STL that referenced this issue Oct 12, 2021
@StephanTLavavej StephanTLavavej added the fixed Something works now, yay! label Nov 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fixed Something works now, yay!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants