Skip to content

Commit cab4048

Browse files
algorithms: fix operator!= inconsistency in prefetching_iterator
operator== checked both idx_ and base_, but operator!= only checked idx_. This meant two iterators with equal idx_ but different base_ pointers would have both operator== and operator!= return false simultaneously -- a logical impossibility for a well-formed iterator. All loop termination code using 'it != end' relied on operator!=, while equality checks used operator==, making the two operators silently inconsistent for iterators constructed from different prefetcher contexts over the same-sized ranges. Fix: delegate operator!= to operator== via !(*this == rhs), making the two operators proper logical negations as required by the C++ EqualityComparable concept. Remove both FIXME comments that noted the uncertainty. Also add a regression test (prefetching_iterator.cpp) that directly exercises the pre-fix contradiction in test_different_base_consistency. Signed-off-by: arpittkhandelwal <arpitkhandelwal810@gmail.com>
1 parent 93253e0 commit cab4048

3 files changed

Lines changed: 119 additions & 3 deletions

File tree

libs/core/algorithms/include/hpx/parallel/util/prefetching.hpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -159,16 +159,14 @@ namespace hpx::parallel::util {
159159
return tmp;
160160
}
161161

162-
// FIXME: should other members be compared too?
163162
bool operator==(prefetching_iterator const& rhs) const
164163
{
165164
return idx_ == rhs.idx_ && base_ == rhs.base_;
166165
}
167166

168-
// FIXME: should the base iterators be compared too?
169167
bool operator!=(prefetching_iterator const& rhs) const
170168
{
171-
return idx_ != rhs.idx_;
169+
return !(*this == rhs);
172170
}
173171
bool operator>(prefetching_iterator const& rhs) const
174172
{

libs/core/algorithms/tests/unit/algorithms/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ set(tests
5555
foreach
5656
foreach_executors
5757
foreach_prefetching
58+
prefetching_iterator
5859
foreach_scheduler
5960
foreachn
6061
foreachn_exception
Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
// Copyright (c) 2026 Arpit Khandelwal
2+
//
3+
// SPDX-License-Identifier: BSL-1.0
4+
// Distributed under the Boost Software License, Version 1.0. (See accompanying
5+
// file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt)
6+
7+
// Regression test for the inconsistency between operator== and operator!= in
8+
// hpx::parallel::util::detail::prefetching_iterator.
9+
//
10+
// Before the fix, operator== checked both idx_ and base_ while operator!=
11+
// only checked idx_. This meant two iterators with equal idx_ but different
12+
// base_ would have both == and != return false -- a logical contradiction.
13+
14+
#include <hpx/init.hpp>
15+
#include <hpx/modules/testing.hpp>
16+
#include <hpx/parallel/util/prefetching.hpp>
17+
18+
#include <cstddef>
19+
#include <numeric>
20+
#include <vector>
21+
22+
///////////////////////////////////////////////////////////////////////////////
23+
void test_same_context_consistency()
24+
{
25+
std::size_t const prefetch_distance_factor = 2;
26+
std::vector<double> c(128, 1.0);
27+
std::vector<std::size_t> range(128);
28+
std::iota(range.begin(), range.end(), 0);
29+
30+
auto ctx = hpx::parallel::util::make_prefetcher_context(
31+
range.begin(), range.end(), prefetch_distance_factor, c);
32+
33+
auto it_end = ctx.end();
34+
for (auto it = ctx.begin(); it != it_end; ++it)
35+
{
36+
// operator!= and operator== must be logical negations at every step
37+
HPX_TEST(!(it == it_end));
38+
39+
auto it2 = it;
40+
HPX_TEST(it2 == it);
41+
HPX_TEST(!(it2 != it));
42+
}
43+
44+
auto it = ctx.end();
45+
HPX_TEST(it == it_end);
46+
HPX_TEST(!(it != it_end));
47+
}
48+
49+
///////////////////////////////////////////////////////////////////////////////
50+
// Directly verifies the pre-fix bug: two iterators with equal idx_ but
51+
// different base_ must satisfy (a != b) == !(a == b).
52+
// Before the fix, both == and != returned false simultaneously for this case.
53+
void test_different_base_consistency()
54+
{
55+
std::size_t const prefetch_distance_factor = 2;
56+
57+
std::vector<double> c1(64, 1.0);
58+
std::vector<std::size_t> range1(64);
59+
std::iota(range1.begin(), range1.end(), 0);
60+
61+
std::vector<double> c2(64, 2.0);
62+
std::vector<std::size_t> range2(64);
63+
std::iota(range2.begin(), range2.end(), 0);
64+
65+
auto ctx1 = hpx::parallel::util::make_prefetcher_context(range1.data(),
66+
range1.data() + range1.size(), prefetch_distance_factor, c1);
67+
auto ctx2 = hpx::parallel::util::make_prefetcher_context(range2.data(),
68+
range2.data() + range2.size(), prefetch_distance_factor, c2);
69+
70+
auto it1 = ctx1.begin();
71+
auto it2 = ctx2.begin();
72+
73+
// idx_ is 0 for both; base_ points into different underlying sequences/contexts.
74+
HPX_TEST(!(it1 == it2));
75+
// Before the fix: operator!= only checked idx_, returning false here.
76+
HPX_TEST(it1 != it2);
77+
// The fundamental invariant: (a != b) == !(a == b)
78+
HPX_TEST((it1 != it2) == !(it1 == it2));
79+
}
80+
81+
///////////////////////////////////////////////////////////////////////////////
82+
void test_reflexive_symmetric()
83+
{
84+
std::size_t const prefetch_distance_factor = 2;
85+
std::vector<double> c(32, 1.0);
86+
std::vector<std::size_t> range(32);
87+
std::iota(range.begin(), range.end(), 0);
88+
89+
auto ctx = hpx::parallel::util::make_prefetcher_context(
90+
range.begin(), range.end(), prefetch_distance_factor, c);
91+
92+
auto it = ctx.begin();
93+
auto it2 = it;
94+
95+
HPX_TEST(it == it);
96+
HPX_TEST(!(it != it));
97+
HPX_TEST(it == it2);
98+
HPX_TEST(it2 == it);
99+
HPX_TEST(!(it != it2));
100+
HPX_TEST(!(it2 != it));
101+
}
102+
103+
///////////////////////////////////////////////////////////////////////////////
104+
int hpx_main()
105+
{
106+
test_same_context_consistency();
107+
test_different_base_consistency();
108+
test_reflexive_symmetric();
109+
110+
return hpx::finalize();
111+
}
112+
113+
int main(int argc, char* argv[])
114+
{
115+
HPX_TEST_EQ(hpx::init(argc, argv), 0);
116+
return hpx::util::report_errors();
117+
}

0 commit comments

Comments
 (0)