algorithms: fix operator!= inconsistency in prefetching_iterator#7294
Merged
hkaiser merged 3 commits intoMay 29, 2026
Merged
Conversation
Up to standards ✅🟢 Issues
|
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a regression test and fixes prefetching_iterator comparison semantics to eliminate an inconsistency between operator== and operator!=.
Changes:
- Fix
prefetching_iterator::operator!=to be the logical negation ofoperator==. - Add a new unit test covering equality/inequality consistency (including the previously failing scenario).
- Register the new unit test target in the algorithms unit test CMake list.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| libs/core/algorithms/tests/unit/algorithms/prefetching_iterator.cpp | New regression/unit tests for prefetching_iterator equality/inequality invariants. |
| libs/core/algorithms/tests/unit/algorithms/CMakeLists.txt | Adds the new prefetching_iterator test to the unit test set. |
| libs/core/algorithms/include/hpx/parallel/util/prefetching.hpp | Makes operator!= consistent by implementing it as !(*this == rhs). |
arpittkhandelwal
force-pushed
the
fix/prefetching-iterator-comparison
branch
from
08eef18 to
cab4048
Compare
hkaiser
reviewed
May 24, 2026
Collaborator
Performance test reportHPX PerformanceComparison
Info
Comparison
Info
Comparison
Info
Explanation of Symbols
|
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>
hkaiser
force-pushed
the
fix/prefetching-iterator-comparison
branch
from
cab4048 to
20b43fb
Compare
- flyby: move test to regressions directory Signed-off-by: Hartmut Kaiser <hartmut.kaiser@gmail.com>
hkaiser
force-pushed
the
fix/prefetching-iterator-comparison
branch
from
20b43fb to
44ba229
Compare
Collaborator
Performance test reportHPX PerformanceComparison
Info
Comparison
Info
Comparison
Info
Explanation of Symbols
|
Signed-off-by: Hartmut Kaiser <hartmut.kaiser@gmail.com>
Collaborator
Performance test reportHPX PerformanceComparison
Info
Comparison
Info
Comparison
Info
Explanation of Symbols
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fixes an inconsistency in
hpx::parallel::util::detail::prefetching_iteratorwhereoperator==compared bothidx_andbase_, butoperator!=only comparedidx_.As a result, two iterators with equal
idx_but differentbase_pointers would have both==and!=returnfalsesimultaneously, which violates the fundamental semantic requirements of C++ iterator comparison. Any range-based or manual loop termination check ofit != endrelied onoperator!=, while other equality checks usedoperator==, making the two operators silently inconsistent for iterators constructed from different prefetcher contexts over the same-sized ranges.Fix
operator!=to delegate tooperator==via!(*this == rhs).// FIXMEcomments regarding member and base iterator comparisons.libs/core/algorithms/tests/unit/algorithms/prefetching_iterator.cppthat directly tests this behavior (includingtest_different_base_consistencywhich fails prior to the fix).